Wrap adds a new line to strings containing ANSI codes

[mikelorant]

When using the Wrap function as part of the ANSI package, strings containing ANSI codes that have a printable width equal to the wrap value have a new line appended.

Following code demonstrates this issue:

package main

import (
    "fmt"

    "github.com/charmbracelet/lipgloss"
    "github.com/charmbracelet/x/exp/term/ansi"
)

func main() {
    str := "1"
    red := lipgloss.Color("9")

    style1 := lipgloss.NewStyle().
        Foreground(red).
        Render(str)

    style2 := lipgloss.NewStyle().
        Width(1).
        Render(style1)

    fmt.Println("Lipgloss")
    fmt.Printf("Before Width: '%v'\n", style1)
    fmt.Printf("After Width : '%v'\n", style2)
    fmt.Println()

    fmt.Println("ANSI")
    fmt.Printf("ASCII Width : '%v'\n", ansi.Wrap(str, 1, ""))
    fmt.Printf("ANSI Width  : '%v'\n", ansi.Wrap(style1, 1, ""))
}

This is a causing a regression in Lip Gloss when using the style Width function to force a string to a specific width.

As the only test function that calls Wrap is TestSmartWrap, this test case can be added to check for this problem.

{
	name:     "exact",
	input:    "\x1b[91mfoo\x1b[0",
	expected: "\x1b[91mfoo\x1b[0",
	width:    3,
},

This test case can be made to pass changing the following line:

x/exp/term/ansi/wrap.go

Line 355 in 62a6965

} else if curWidth+wordLen >= limit {

To:

} else if curWidth+wordLen > limit {

However this causes other tests to fail, so there is some other edge cases to consider.

@aymanbagabas Let me know if there is anything else I need to provide to debug this issue further.

[mikelorant]

Figured this one out but have no idea how to fix it.

By the time we get to the 3rd and last printed character, we have moved i to a value of 7. However, the final position i will get to is 10.

The logic that matters:

if i < len(b)-1 {
        addNewline()
}

The remaining 3 characters are <ESC>[0. These 3 characters reset the ANSI sequence. So nothing actually happens, but it is too late, we have already added the new line.

How do we detect there are no more printable characters before we add the new line?

1. Delay this new line UNTIL we know another character needs to be printed.
2. Strip lines that have a new line with nothing on them. We don't want to return foo\n\n.

[meowgorithm]

We really appreciate you looking into this, @mikelorant. If this is causing a regression in Lip Gloss (currently master) perhaps we should revert charmbracelet/lipgloss#268 until this is sorted.

[mikelorant]

Might be better off to delay promoting the master branch of Lip Gloss to a release. This gives @aymanbagabas some time to investigate this without rolling back his work. I think if we can come up with an approach on how to handle this edge case that the implementation won't be too difficult.

[meowgorithm]

Cool, makes sense to me. In general, moving over to term is the next thing we expect to do to with Lip Gloss.

[mikelorant]

Be aware that this really needs to be done in parallel with Bubbles as it is so dependent on Lip Gloss. I've done some auditing and there are even parts of Bubbles that assume all strings can be handled as runes. These needed to brought into using the new methods provided by term.

[aymanbagabas]

@mikelorant thanks for reporting this. PR #59 includes a fix for this along with handling more edge cases. Let me know if you find any other bugs.

[mikelorant]

@aymanbagabas I was still trying to come up with ideas on how to tackle it, and you've got a fix! Amazing work. 👏

[mikelorant]

Making a note that this is confirmed fixed.