charmbracelet / charmbracelet/bubbles

viewport: SetHighlights misplaces ranges on ANSI-carrying content

Open
#1,049 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
8.9k
Forks
457
Avg merge
1d 18h
Merged PRs (30d)
5

Description

**Describe the bug**

`viewport.SetHighlights` misplaces highlight ranges when the content carries ANSI escapes: the range lands shifted to the right by exactly the number of escape bytes that precede the match on that line, and it disappears entirely when that shift runs past the end of the line.

`SetHighlights` documents its input as byte offsets against the content it was given ("matches are measured in bytes, e.g. what `regex.FindAllStringIndex` would return", "matches were made against the given content"). For styled content — which `View()` output normally is — that contract does not hold.

**Setup**

- OS: Linux (Ubuntu 24.04)
- Go: 1.26, `charm.land/bubbles/v2 v2.2.1` (the newest published version)
- Shell / terminal emulator / multiplexer: not terminal-dependent — the reproducer below is a plain program, no TTY involved

**To Reproduce**

1. Put the program under **Source Code** in an empty module (it needs only `charm.land/bubbles/v2` and `charm.land/lipgloss/v2`).
2. `go run .`
3. Compare the two lines: they differ only by a 10-byte escape prefix (`\x1b[31mX\x1b[0m`), and both pass the correct raw-content offsets from `strings.Index`.

Observed:

```
match 24..30 -> "padding padding padding \x1b[7m\x1b[m\x1b[7mneedle\x1b[m\x1b[7m\x1b[m padding padding \n ..."

match 35..41 -> "\x1b[31mX\x1b[0m padding padding padding needle pa\x1b[7m\x1b[m\x1b[31m\x1b[0m\x1b[7mdding \x1b[m\x1b[31m\x1b[0m\x1b[7m\x1b[m\x1b[31m\x1b[0mpadding \n ..."
```

The first line highlights `needle`. The second highlights `dding ` — the range moved 10 columns to the right, which is the escape-byte count. Two more shapes of the same defect:

| content | match bytes | expected | actual |
|---|---|---|---|
| `"aaa\nneed\nle here\n"` (no escapes) | 4..11 | `need` + `le` | `need` + `le` ✅ |
| `"a\x1b[32mb\x1b[0mc\nneed\nle here\n"` | 13..20 | `need` + `le` | nothing highlighted |
| `"a\x1b[31mRED\x1b[0m and the needle here\n"` | 22..28 | `needle` | nothing highlighted |

The last three rows matter for triage: the two failing ones differ from the passing one **only** by escapes, so this is not the multi-line branch — it is a byte-space mismatch.

**Source Code**

```go
package main

import (
"fmt"
"strings"

"charm.land/bubbles/v2/viewport"
"charm.land/lipgloss/v2"
)

func main() {
for _, content := range []string{
"padding padding padding needle padding padding\n",
"\x1b[31mX\x1b[0m padding padding padding needle padding padding\n",
} {
i := strings.Index(content, "needle")
vp := viewport.New()
vp.HighlightStyle = lipgloss.NewStyle().Reverse(true)
vp.SelectedHighlightStyle = lipgloss.NewStyle().Reverse(true)
vp.SetWidth(60)
vp.SetHeight(3)
vp.SetContent(content)
vp.SetHighlights([][]int{{i, i + len("needle")}})
fmt.Printf("match %d..%d -> %q\n\n", i, i+len("needle"), vp.View())
}
}
```

**Expected behavior**

The ranges land on the matches the caller computed against the given content, regardless of escapes, and a multi-line match highlights each line it covers.

**Screenshots**

Not applicable — the reproducer is unit-level and needs no TTY.

**Additional context**

Root cause is in `viewport/highlight.go`: `parseMatches` walks graphemes of the **stripped** content but keeps its byte cursor in the **raw** content's coordinate space.

- `highlight.go:34` — `gr := uniseg.NewGraphemes(ansi.Strip(content))`
- `highlight.go:55` and `:84` — `bytePos += len(gr.Str())` accumulates *stripped* lengths
- `highlight.go:50` and `:70` — `if content[bytePos] == '\n'` indexes the *raw* string with that cursor (the newline bookkeeping that maps grapheme positions to line columns)
- so `for byteStart > bytePos` / `for byteEnd > bytePos` compare a stripped-space cursor against raw byte offsets

Escape bytes are never counted, so each raw offset is consumed as if it were a stripped offset: the walk overshoots by the escape bytes before the match on that line, and the newline detection reads an arbitrary byte once any escape precedes it.

Why the existing test misses it: `viewport/viewport_test.go:411` `TestMatchesToHighlights` is the only highlight test, and its content literal at `:412` is plain `"hello\nworld\n..."` — every sub-test (first, multiple, span lines, ends with newline, empty lines, wide characters) runs over ANSI-free text, so the raw/stripped divergence is never exercised.

Still present on `main`: `viewport/highlight.go` is byte-identical at `v2.2.1` and `main` (`blob 7bb5025f32bdd8892972de15c41127381b8dca4d`, 3136 bytes), and the `viewport/` directory has no commits since the `v2.2.1` tag.

Fix sketch: keep the cursor in raw-content byte space — iterate the raw content while skipping sequences (`ansi.DecodeSequence`) but still counting their bytes — or build a raw-to-stripped offset map once and translate `byteStart`/`byteEnd` before the grapheme walk. The newline check then reads real bytes again.

Impact, for context: any app that highlights search hits inside styled output hits this today (we hit it in a TUI that marks search matches in ANSI-styled results and had to compute our own offsets); an ANSI-free test case with escapes added to the existing table would pin it.

Contributor guide

Open the contributing guide

Research direction

Start with viewport/highlight.go, especially parseMatches, then read viewport/viewport_test.go around TestMatchesToHighlights at line 411. Run the existing viewport tests and add an ANSI-styled content case using raw byte offsets. Done means highlights remain on the requested matches with escapes, including multiline matches, while the existing cases still pass.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
cli
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
82/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.