Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extra new lines between list items when word wrap is set to terminal width #331

Open
rwinkhart opened this issue Aug 6, 2024 · 2 comments · May be fixed by #334
Open

Extra new lines between list items when word wrap is set to terminal width #331

rwinkhart opened this issue Aug 6, 2024 · 2 comments · May be fixed by #334

Comments

@rwinkhart
Copy link

Describe the bug
New with v0.8.0! When using a custom renderer with word wrap set to the full terminal width using glamour.WithWordWrap(), extra blank lines are printed between list items.

Setup

  • OS: Reproduced on Artix Linux and Windows
  • Shell: Reproduced on ZSH, Bash, and Powershell 7
  • Terminal Emulator: Reproduced on Konsole and Windows Terminal
  • Terminal Multiplexer: N/A

To Reproduce
Steps to reproduce the behavior:

  1. Get the terminal width (columns)
  2. Create a custom Glamour renderer using the terminal width to set glamour.WithWordWrap()
  3. Try to render ordered or unordered lists

Source Code
I wrote a demo program that creates a renderer and renders a couple lists. If using Glamour v0.7.0, they render correctly. If using Glamour v0.8.0, they appear broken and have extra new lines between them.

package main

import (
	"fmt"
	"os"

	"github.com/charmbracelet/glamour"
	"golang.org/x/term"
)

func renderString(test string) {
	width, _, _ := term.GetSize(int(os.Stdout.Fd()))
	r, _ := glamour.NewTermRenderer(glamour.WithAutoStyle(), glamour.WithWordWrap(width))
	markdownNotesString, _ := r.Render(test)

	fmt.Print(markdownNotesString)
}

func main() {
	renderString("# Unordered List\n- Item 1\n    - Item 2\n    - Item 3\n        - Item 4\n        - Item 5")
	renderString("# Ordered Lists\n1. Item 1\n2. Item 2\n3. Item 3\n    - Sub item\n        - Sub sub item\n    - Another sub item\n4. Item 5")
}

Screenshots

v0.8.0

image

v0.7.0

image

Additional Context
Subtracting from the full width only very inconsistently solves the issue. Depending on the document and total terminal width, a different value needs to be subtracted from the terminal width to mitigate the bug. Sometimes I can fix it by using width-2, other times I need width-5 or other.

@maxmoehl
Copy link

maxmoehl commented Aug 29, 2024

I've done some debugging to understand what is happening using this sample:

* Bullet 1
* Bullet 2
* A bullet which is way to long to fit in a single line, so it has to be
  wrapped in the source as well as in the rendered output.

Output (run using md, strip ansi codes, show width, you must use the GNU version of sed and awk):

$ go run . -w 50 ./test.md | sed 's/\x1b\[[0-9;]*m//g' | awk '{ print "|" length($0) "|" $0 "|" }'
|0||
|52|                                                    |
|52|  • Bullet 1                                        |
|52|  • Bullet 2                                        |
|52|  • A bullet which is way to long to fit in a single|
|52|  line, so it has to be                             |
|52|  wrapped in the source as well as in the rendered  |
|52|  output.                                           |

Wrapping of the text is done in github.com/charmbracelet/x/ansi.Wordwrap, the function is called here with • Bullet 1 and a limit of 50, however, this is already the maximum width I'm allowing. Since the bullet is prepended with some space (it's later rendered as • Bullet 1 the available width is actually 48 (or maybe even just 46 accounting for both margins). So it seems like it forgets to account for the margin which is added.

There is also this weird effect that the newline gets preserved within the bullet which seems odd and the continuation of the bullet should be indented but this is the same with <0.8.0.

The issue seems to be introduced with 5f5965e, reverting it locally and re-running my test from above yields this result:

$ go run . -w 50 ./test.md | sed 's/\x1b\[[0-9;]*m//g' | awk '{ print "|" length($0) "|" $0 "|" }'
|0||
|48|                                                |
|48|  • Bullet 1                                    |
|48|  • Bullet 2                                    |
|48|  • A bullet which is way to long to fit in a   |
|48|  single line, so it has to be                  |
|48|  wrapped in the source as well as in the       |
|48|  rendered output.                              |
|0||

I think it could be this line, changing the width calculation back to +/- fixes the issue:

// Width returns the available rendering width.
func (s BlockStack) Width(ctx RenderContext) uint {
	if s.Indent()+s.Margin() > uint(ctx.options.WordWrap) {
		return 0
	}
	return uint(ctx.options.WordWrap) - s.Indent() - s.Margin()
}

Because the indent is 0 multiplying it with the margin is also 0, I also don't think it was ever intended to be used that way as both produce the absolute margin / indent from what I can see.

Result (with 0.8.0 + my fix):

$ go run . -w 50 ./test.md | sed 's/\x1b\[[0-9;]*m//g' | awk '{ print "|" length($0) "|" $0 "|" }'
|0||
|50|                                                  |
|50|  • Bullet 1                                      |
|50|  • Bullet 2                                      |
|50|  • A bullet which is way to long to fit in a     |
|50|  single line, so it has to be                    |
|50|  wrapped in the source as well as in the rendered|
|50|  output.                                         |
|0||

As you can see the margin calculation is still off, it should probably be 2*s.Margin() but then something else breaks (see example above with the commit reverted) and it will not use the full width again.

I've opened #343.

@maxmoehl
Copy link

maxmoehl commented Aug 30, 2024

I just noticed that we do need the *2. The resulting output is only 48 characters wide because the margin is not printed as whitespace on the right side although everything else is filled with spaces. See this:

$ go run . -w 50 ./test.md | sed 's/\x1b\[[0-9;]*m//g' | awk '{ print "|" length($0) "|" $0 "|" }'
|0||
|48|                                                |
|48|  • Bullet 1                                    |
|48|  • Bullet 2                                    |
|48|    • Sub-bullet                                |
|48|  • A bullet which is way to long to fit in a   |
|48|  single line, so it has to be                  |
|48|  wrapped in the source as well as in the       |
|48|  rendered output.                              |
|48|  • This line just fits in the space that is avi|
|0||

@maxmoehl maxmoehl linked a pull request Aug 30, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants