From c105e3925249a304caae090128cd38f705192329 Mon Sep 17 00:00:00 2001 From: Umputun Date: Wed, 19 Aug 2026 22:59:58 -0500 Subject: [PATCH 1/3] fix(highlight): raise regexp2 backtracking cap so long tokens keep their color regexp2 v2.7.1 added a default MaxBacktrackingStackSize of 100000 that did not exist in v2.2.1. Chroma compiles every lexer rule with a bare regexp2.Compile and its matchRules drops results whose match returned an error, so a rule that trips the cap degrades to a non-match with nothing logged. A Go string literal over roughly 17k characters was repainted as chroma.Error, and revdiff could not see it because highlightFile only checks the error Tokenise itself returns. Raise the package default from New(), which is the only route into chroma's deferred rule compilation. The cap stays finite rather than going back to unbounded: revdiff opens arbitrary repos and 2.7.1 added the bound for a reason. Multi-megabyte tokens still exceed it, and the comment says so. regexp2 moves from indirect to direct in go.mod as a result. --- app/highlight/highlight.go | 17 +++++++++++++++++ app/highlight/highlight_test.go | 22 ++++++++++++++++++++++ go.mod | 2 +- 3 files changed, 40 insertions(+), 1 deletion(-) diff --git a/app/highlight/highlight.go b/app/highlight/highlight.go index b231f5e4..a2c5bec1 100644 --- a/app/highlight/highlight.go +++ b/app/highlight/highlight.go @@ -4,14 +4,30 @@ import ( "fmt" "log" "strings" + "sync" "github.com/alecthomas/chroma/v2" "github.com/alecthomas/chroma/v2/lexers" "github.com/alecthomas/chroma/v2/styles" + "github.com/dlclark/regexp2/v2" "github.com/umputun/revdiff/app/diff" ) +// maxBacktrackingStack raises regexp2's default 100k-slot cap enough for long single-line tokens, +// such as the 40k-character Go string that exposed the regression. Chroma compiles lexer rules +// with a bare regexp2.Compile, and matchRules ignores match errors and tries later rules. Affected +// text can therefore lose its intended color or fall back to chroma.Error. This finite memory +// budget does not restore regexp2's former unbounded behavior, and multi-megabyte tokens can still +// exceed it. +const maxBacktrackingStack = 10_000_000 + +// chroma defers rule compilation until a lexer is first used, and a Highlighter is the only way +// into that path, so raising the package default here reaches every lexer revdiff builds. +var raiseBacktrackingCap = sync.OnceFunc(func() { + regexp2.DefaultOptimizationOptions.MaxBacktrackingStackSize = maxBacktrackingStack +}) + // chromaFallbackStyle is the name of the Chroma style that doubles as styles.Fallback. // styles.Get returns Fallback for unknown names, but "swapoff" is a real built-in style // whose registry entry IS the Fallback sentinel, so we must special-case it. @@ -26,6 +42,7 @@ type Highlighter struct { // New creates a Highlighter with the given Chroma style name and enabled state. // if styleName is empty, defaults to "monokai". Logs a warning if the style name is unknown. func New(styleName string, enabled bool) *Highlighter { + raiseBacktrackingCap() if styleName == "" { styleName = "monokai" } diff --git a/app/highlight/highlight_test.go b/app/highlight/highlight_test.go index 03cbc5f0..098b5b52 100644 --- a/app/highlight/highlight_test.go +++ b/app/highlight/highlight_test.go @@ -1,6 +1,7 @@ package highlight import ( + "fmt" "strings" "testing" @@ -206,3 +207,24 @@ func TestSetStyle_unknownStyle(t *testing.T) { assert.False(t, ok) assert.Equal(t, "monokai", h.StyleName(), "style should not change on failure") } + +func TestHighlighter_LongQuotedStringUsesStringColor(t *testing.T) { + // pins the regexp2 v2.7.1 regression that repainted a 40k-character Go quoted string as an error + h := New("monokai", true) + + render := func(n int) string { + content := `x := "` + strings.Repeat("a", n) + `"` + got := h.HighlightLines("main.go", []diff.DiffLine{{NewNum: 1, Content: content, ChangeType: diff.ChangeContext}}) + require.Len(t, got, 1) + return got[0] + } + + fg := func(tt chroma.TokenType) string { + c := styles.Get("monokai").Get(tt).Colour //nolint:misspell // chroma API uses British spelling + return fmt.Sprintf("\033[38;2;%d;%d;%dm", c.Red(), c.Green(), c.Blue()) + } + + long := render(40000) + assert.Contains(t, long, fg(chroma.LiteralString), "long literal must keep the string color") + assert.NotContains(t, long, fg(chroma.Error), "long literal must not be painted as an error token") +} diff --git a/go.mod b/go.mod index 8d69c89e..83a2e3a3 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,7 @@ require ( github.com/charmbracelet/bubbletea v1.3.10 github.com/charmbracelet/lipgloss v1.1.0 github.com/charmbracelet/x/ansi v0.11.8 + github.com/dlclark/regexp2/v2 v2.7.1 github.com/jessevdk/go-flags v1.6.1 github.com/mattn/go-runewidth v0.0.28 github.com/muesli/termenv v0.16.0 @@ -22,7 +23,6 @@ require ( github.com/charmbracelet/x/term v0.2.2 // indirect github.com/clipperhouse/displaywidth v0.11.0 // indirect github.com/clipperhouse/uax29/v2 v2.7.0 // indirect - github.com/dlclark/regexp2/v2 v2.7.1 // indirect github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f // indirect github.com/lucasb-eyer/go-colorful v1.4.1 // indirect github.com/mattn/go-isatty v0.0.24 // indirect From 01aa6e39881b95fc7a8e5e6c9b32c865fd4be2aa Mon Sep 17 00:00:00 2001 From: Umputun Date: Wed, 19 Aug 2026 23:00:03 -0500 Subject: [PATCH 2/3] docs(backlog): refresh the race timeout measurements re-measured during the PR #327 review: the app package now runs 81.5s and 87.5s on master against 68s before, so headroom against the 100s budget is down from roughly 32s to roughly 13s. Still later rather than yes, but the next addition to the launcher matrix is what flips it. --- docs/backlog/race-timeout-budget-too-tight.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/backlog/race-timeout-budget-too-tight.md b/docs/backlog/race-timeout-budget-too-tight.md index dee7198d..eef10386 100644 --- a/docs/backlog/race-timeout-budget-too-tight.md +++ b/docs/backlog/race-timeout-budget-too-tight.md @@ -15,5 +15,11 @@ review worktree, passed in 192s standalone under that load, and was green at 68s Nothing about the PR was involved. CI is green and so is an idle local run, which is why this is `later` rather than `yes`. +Re-measured during the PR #327 review (2026-08-19), and the margin has shrunk: the `app` package now runs +81.5s and 87.5s on master and 79.9s on that PR's branch, against 68s before, with +`TestShellLaunchersPreserveAnnotationExitCode` alone at ~77s. Headroom against the 100s budget is down from +roughly 32s to roughly 13s, so an idle run is no longer comfortably clear of it. Still `later`, but the next +addition to the launcher matrix is what turns this into `yes`. + Fix is a choice, not a one-liner: raise the timeout, or split the launcher matrix into its own target with its own budget so the ordinary race run stays fast and the slow matrix is allowed to be slow. From db0c3b5f9b3dae7455a030ced9bd9a5849c8af15 Mon Sep 17 00:00:00 2001 From: Umputun Date: Thu, 20 Aug 2026 01:23:39 -0500 Subject: [PATCH 3/3] fix(highlight): lower the backtracking cap to 1m and correct its comment 10m covered roughly 1.7M characters, far past anything revdiff needs, and the cost was not free. growTrack doubles until it reaches the cap, so the steps before ErrBacktrackingStackLimit scale with it, while chroma sets a separate 250ms MatchTimeout on every rule and discards that error through the same err == nil gate. A cap that large turns a sub-millisecond abort into a stall of up to a quarter second on the bubbletea event loop, which themeselect hits on every keypress that changes the chroma style. The chroma Go quoted-string rule needs 240045 slots for the 40k test input, so 1m keeps about four times headroom and still fixes the regression. Comment corrections that go with it: slots are ints, so 1m is about 8MB on 64-bit; the cap bounds runtrack only, since runstack grows through doubleIntSlice with no limit check, so the two together reach roughly twice the nominal budget; and the 250ms timeout is a second independent ceiling that raising this constant cannot help. New's godoc now names the process-wide change to regexp2.DefaultOptimizationOptions. --- app/highlight/highlight.go | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/app/highlight/highlight.go b/app/highlight/highlight.go index a2c5bec1..9b9e5024 100644 --- a/app/highlight/highlight.go +++ b/app/highlight/highlight.go @@ -14,13 +14,18 @@ import ( "github.com/umputun/revdiff/app/diff" ) -// maxBacktrackingStack raises regexp2's default 100k-slot cap enough for long single-line tokens, -// such as the 40k-character Go string that exposed the regression. Chroma compiles lexer rules -// with a bare regexp2.Compile, and matchRules ignores match errors and tries later rules. Affected -// text can therefore lose its intended color or fall back to chroma.Error. This finite memory -// budget does not restore regexp2's former unbounded behavior, and multi-megabyte tokens can still -// exceed it. -const maxBacktrackingStack = 10_000_000 +// maxBacktrackingStack raises regexp2's default 100k-slot cap for long single-line tokens. The +// 40k-character Go string that exposed the regression needs 240,045 slots, leaving about four +// times headroom. Each slot is an int, so one million slots allow about 8 MB for runtrack on +// 64-bit targets. regexp2 caps only runtrack; runstack grows through doubleIntSlice with no limit +// check, so the two stacks together can use roughly twice the nominal runtrack budget. +// +// Chroma compiles lexer rules with a bare regexp2.Compile and sets a separate 250ms MatchTimeout on +// each rule. matchRules ignores either error and tries later rules, so affected text can lose its +// intended color or fall back to chroma.Error. Raising this cap cannot help once the timeout is the +// binding ceiling. This finite budget does not restore regexp2's former unbounded behavior, and +// larger tokens can still exceed either ceiling. +const maxBacktrackingStack = 1_000_000 // chroma defers rule compilation until a lexer is first used, and a Highlighter is the only way // into that path, so raising the package default here reaches every lexer revdiff builds. @@ -40,7 +45,9 @@ type Highlighter struct { } // New creates a Highlighter with the given Chroma style name and enabled state. -// if styleName is empty, defaults to "monokai". Logs a warning if the style name is unknown. +// If styleName is empty, defaults to "monokai". Logs a warning if the style name is unknown. +// It also changes regexp2.DefaultOptimizationOptions.MaxBacktrackingStackSize process-wide before +// Chroma compiles lexer rules. func New(styleName string, enabled bool) *Highlighter { raiseBacktrackingCap() if styleName == "" {