charmbracelet / glamour

Stylesheet-based markdown rendering for your CLI apps 💇🏻‍♀️
MIT License
2.5k stars 184 forks source link

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

Open rwinkhart opened 3 months ago

rwinkhart commented 3 months ago

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

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 commented 2 months ago

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 commented 2 months ago

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||