From 958cc9502675893aba0b6fd5c1d00f7347ead9d5 Mon Sep 17 00:00:00 2001 From: Frederic BIDON Date: Fri, 31 Jul 2026 20:44:58 +0200 Subject: [PATCH] fix(genspec-tui): drop the YAML position workarounds, upgrade core to v0.0.3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The spec pane lays the lexer's tokens out by position to colour them. Two workarounds stood between it and the YAML lexer, both now fixed upstream in go-openapi/core v0.0.3. A YAML block collection has no "{" / "[" / "}" / "]" character, so the lexer had nothing to point its container delimiters at. It reported the opener at the first entry's separator — a line BELOW the key it precedes — and the closer at line 0 column 0, which is not a position at all. The accumulator therefore had to sort each line's runs by column and drop the positionless ones. Both are gone: delimiters now take the span of what they enclose, so the stream is emitted in non-decreasing position order and every token carries a real position. Non-decreasing, not strictly increasing. A block delimiter has no characters of its own, so it shares its column with the token that does own the text there: the opener with the first token inside, the closer with the last. Appended naively that makes a zero-width run followed by one painting its neighbour as punctuation — `- b` and `ok: true` would render their value as a delimiter. addSpan now collapses same-column runs, and the token with characters wins. Neither first-wins nor last-wins would do, because the opener precedes its neighbour while the closer follows it. Verified over the 283 golden specs in both renders, ~97k tokens: no positionless token, no backwards step, and every same-position pair involves a delimiter — two content tokens never collide. The pointer and $ref indexes built from those documents are byte-identical before and after the upgrade, so only the spans moved. Visible effect: YAML separator colouring becomes uniform. It had been accidental — the borrowed positions punctuation-coloured the ":" of a mapping's first entry and the "-" of a sequence's first item, and nothing else. The YAML lexer elides separator tokens by design, so ":" and "-" now take the preceding run's class everywhere. The spec pane's JSON render is unaffected: its lexer emits real delimiter tokens at real columns and never collided. Also brings the `go` directive back to 1.25.0 across the workspace. It had been raised to a PATCH release because core/json declared one; core no longer does, and go-openapi does not require patch versions. The job C note in the toolchain-independence workflow named the value explicitly and is reworded to stop naming a patch at all — it had already gone stale once for this reason. Note for other core consumers: `go get` raises the `go` directive from the PRE-upgrade graph, so upgrading away from core/json v0.0.2 re-applies the 1.25.8 floor one last time. It has to be set back by hand afterwards. Signed-off-by: Frederic BIDON --- .github/workflows/toolchain-independence.yml | 9 +- cmd/genspec-tui/go.mod | 6 +- cmd/genspec-tui/go.sum | 8 +- .../internal/ux/index/highlight.go | 39 +++++---- .../internal/ux/index/highlight_test.go | 85 ++++++++++--------- cmd/genspec-tui/internal/ux/index/refindex.go | 2 +- docs/examples/go.mod | 2 +- fixtures/go.mod | 2 +- go.mod | 2 +- go.work | 4 +- 10 files changed, 81 insertions(+), 78 deletions(-) diff --git a/.github/workflows/toolchain-independence.yml b/.github/workflows/toolchain-independence.yml index 67474c4d..faa988e1 100644 --- a/.github/workflows/toolchain-independence.yml +++ b/.github/workflows/toolchain-independence.yml @@ -78,11 +78,10 @@ jobs: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.0 # fixtures source (FixturesDir resolves the build-time checkout path, identical across jobs) # oldstable is a DIFFERENT minor than job A's stable, so its GOROOT lives at a different path. # - # It must satisfy the fixtures module's `go` directive (currently go 1.25.8, aligned with the - # rest of the workspace because go-openapi/core/json declares go 1.25.8). The floor moved by a - # PATCH, not a minor, so oldstable still satisfies it: oldstable resolves to the latest patch - # of the previous minor, and that is >= any released patch of it. Holds going forward, since - # oldstable only ever increases. + # It must satisfy the fixtures module's `go` directive (currently go 1.25.0, the same across + # the whole workspace). oldstable resolves to the latest patch of the previous minor, so it + # satisfies any directive naming that minor or an earlier one — which holds as long as we keep + # naming a MINOR there and never a patch. - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 with: go-version: oldstable diff --git a/cmd/genspec-tui/go.mod b/cmd/genspec-tui/go.mod index c2cd20ce..5f357791 100644 --- a/cmd/genspec-tui/go.mod +++ b/cmd/genspec-tui/go.mod @@ -1,6 +1,6 @@ module github.com/go-openapi/codescan/cmd/genspec-tui -go 1.25.8 +go 1.25.0 require ( github.com/charmbracelet/bubbles v1.0.0 @@ -8,8 +8,8 @@ require ( github.com/charmbracelet/lipgloss v1.1.0 github.com/fsnotify/fsnotify v1.10.1 github.com/go-openapi/codescan v0.34.0 - github.com/go-openapi/core/json v0.0.2 - github.com/go-openapi/core/json/lexers/yaml-lexer v0.0.2 + github.com/go-openapi/core/json v0.0.3 + github.com/go-openapi/core/json/lexers/yaml-lexer v0.0.3 github.com/go-openapi/testify/v2 v2.6.0 github.com/muesli/termenv v0.16.0 go.yaml.in/yaml/v3 v3.0.5 diff --git a/cmd/genspec-tui/go.sum b/cmd/genspec-tui/go.sum index 39b8e154..bc1dd5cb 100644 --- a/cmd/genspec-tui/go.sum +++ b/cmd/genspec-tui/go.sum @@ -28,10 +28,10 @@ github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f h1:Y/CXytFA4m6 github.com/erikgeiser/coninput v0.0.0-20211004153227-1c3628e74d0f/go.mod h1:vw97MGsxSvLiUE2X8qFplwetxpGLQrlU1Q9AUEIzCaM= github.com/fsnotify/fsnotify v1.10.1 h1:b0/UzAf9yR5rhf3RPm9gf3ehBPpf0oZKIjtpKrx59Ho= github.com/fsnotify/fsnotify v1.10.1/go.mod h1:TLheqan6HD6GBK6PrDWyDPBaEV8LspOxvPSjC+bVfgo= -github.com/go-openapi/core/json v0.0.2 h1:RACr1Kjs6U8Rzpu0JLnJ2tw5TZ86+5ROXFPNQg1L9I0= -github.com/go-openapi/core/json v0.0.2/go.mod h1:vEcP/Wkw1ImzIAmGt7lmY+dJ8Ilf0TtvV8vPe8HtVA4= -github.com/go-openapi/core/json/lexers/yaml-lexer v0.0.2 h1:aJC6mspwBIPzxJoduTagX416RvVRMZL6yygngyITHoM= -github.com/go-openapi/core/json/lexers/yaml-lexer v0.0.2/go.mod h1:p4x5CYKYZecVZLy22fOMap8EGW5Rkb4yPxpIzWuCYr4= +github.com/go-openapi/core/json v0.0.3 h1:L4YuBIsLVtn5x52u27z+xRdJQkD0BaverQFNvxl7h6E= +github.com/go-openapi/core/json v0.0.3/go.mod h1:nQl4bCBPXPOLlpjfSUcT2hcRKUBVuz+xLHx92aH524w= +github.com/go-openapi/core/json/lexers/yaml-lexer v0.0.3 h1:SJ8eF4ebyJiiBMTcqWYve8mrFmwqxcyhJVuIJtefUq4= +github.com/go-openapi/core/json/lexers/yaml-lexer v0.0.3/go.mod h1:m+JesPi99fRt2yNDlLm+fXhqW3HmA/leqv0uXj0FO+Y= github.com/go-openapi/jsonpointer v1.0.0 h1:kR9tHqY0CtZaOPVFm622dPVNhrvYpwr4uCxgL3h1H8s= github.com/go-openapi/jsonpointer v1.0.0/go.mod h1:Z3rw7dWu1p9IgitXCFamSlA5lmDiklEB6vkaxcNZW5Y= github.com/go-openapi/jsonreference v1.0.0 h1:jlmTr6torcd1YgDQvSfNmRtKzYDO4FGBkrAdlAVWnpY= diff --git a/cmd/genspec-tui/internal/ux/index/highlight.go b/cmd/genspec-tui/internal/ux/index/highlight.go index 51362c09..d270f8f0 100644 --- a/cmd/genspec-tui/internal/ux/index/highlight.go +++ b/cmd/genspec-tui/internal/ux/index/highlight.go @@ -4,8 +4,6 @@ package index import ( - "sort" - "github.com/go-openapi/core/json/lexers/token" "github.com/go-openapi/codescan/cmd/genspec-tui/internal/ux/theme" @@ -78,26 +76,27 @@ func syntaxKind(k token.Kind) theme.SyntaxKind { } // addSpan records one token's run. line is 0-based; col is the lexer's 1-based -// column. Tokens with no position (the EOF delimiters the YAML lexer reports at -// line 0) are dropped rather than attributed to the first line. +// column. Both lexers emit in non-decreasing position order, so runs arrive in +// column order and no sort is needed. +// +// They do not arrive in strictly increasing order, though. A YAML block +// collection has no "{" / "[" / "}" / "]" character for its delimiters to point +// at, so they take the span of what they enclose: the opener reports the first +// token inside, the closer the last. Either way a delimiter shares its column +// with the token that owns the text there, and the two would otherwise make a +// zero-width run followed by one painting its neighbour as punctuation. The +// token with characters of its own wins the column. func (a *indexAccum) addSpan(line, col int, kind token.Kind) { - if line < 0 || col < 1 { - return - } - a.spans[line] = append(a.spans[line], theme.Span{Col: col, Kind: syntaxKind(kind)}) -} + runs := a.spans[line] + syntax := syntaxKind(kind) -// finishSpans orders each line's runs by column. -// -// The sort is required, not defensive: the YAML lexer currently reports a -// mapping's value-delimiter BEFORE the key on the same line, so the stream is -// not in source order. Fred plans to improve that lexer's column reporting -// upstream; when it lands in source order this sort becomes a no-op and can go. -func (a *indexAccum) finishSpans() *HighlightIndex { - for line := range a.spans { - runs := a.spans[line] - sort.Slice(runs, func(i, j int) bool { return runs[i].Col < runs[j].Col }) + if n := len(runs); n > 0 && runs[n-1].Col == col { + if syntax != theme.SyntaxPunct { + runs[n-1].Kind = syntax + } + + return } - return &HighlightIndex{byLine: a.spans} + a.spans[line] = append(runs, theme.Span{Col: col, Kind: syntax}) } diff --git a/cmd/genspec-tui/internal/ux/index/highlight_test.go b/cmd/genspec-tui/internal/ux/index/highlight_test.go index 869d9253..b7fad10c 100644 --- a/cmd/genspec-tui/internal/ux/index/highlight_test.go +++ b/cmd/genspec-tui/internal/ux/index/highlight_test.go @@ -50,59 +50,64 @@ func TestHighlight_ClassifiesJSONTokens(t *testing.T) { assert.Contains(t, kindsOn(idx, 6), theme.SyntaxString) } -// Spans record where a run starts, so columns must be 1-based and ascending — -// the renderer takes each run to the next one's column. -func TestHighlight_SpansAreOrderedByColumn(t *testing.T) { - idx := BuildJSONIndex([]byte(hlJSON)).Highlight - - for line := range 8 { - spans := idx.Spans(line) - for i, sp := range spans { - assert.Positive(t, sp.Col, "line %d span %d: columns are 1-based", line, i) - if i > 0 { - assert.Greater(t, sp.Col, spans[i-1].Col, - "line %d: spans must ascend, or runs would overlap", line) - } - } - } -} - -// The YAML lexer currently reports a mapping's value-delimiter BEFORE the key on -// the same line, so the accumulator sorts. Without that, the renderer would take -// the first run from column 12 back to column 1 and paint the line wrong. -func TestHighlight_YAMLSpansAreSortedDespiteEmissionOrder(t *testing.T) { - const hlYAML = `definitions: +// A YAML block collection's delimiters have no character of their own, so they +// report the span of what they enclose — the opener the first token inside, the +// closer the last. Both therefore share a column with a token that does own the +// text, and the accumulator must let that token keep the run: a delimiter run +// starting where a value starts would paint the value as punctuation. +const hlYAML = `definitions: User: count: 3 ok: true +tags: + - a + - b ` + +func TestHighlight_YAMLBlockDelimitersDoNotStealTheirNeighboursColumn(t *testing.T) { idx := BuildYAMLIndex([]byte(hlYAML)).Highlight require.Positive(t, idx.Len()) - for line := range 4 { - spans := idx.Spans(line) - for i := 1; i < len(spans); i++ { - assert.Greater(t, spans[i].Col, spans[i-1].Col, "line %d", line) - } - } - - // Line 0 is `definitions:` — the key must come first despite being emitted - // after its delimiter. + // The opening delimiters of `definitions:` — the root mapping's, emitted + // BEFORE the key it shares column 1 with. first := idx.Spans(0) require.NotEmpty(t, first) - assert.Equal(t, theme.SyntaxKey, first[0].Kind) + assert.Equal(t, theme.SyntaxKey, first[0].Kind, "the key owns column 1, not the mapping opener") assert.Equal(t, 1, first[0].Col) -} -// The YAML lexer reports its trailing EOF delimiters at line 0 / column 0. -// Attributing those to the first line would paint a run that is not there. -func TestHighlight_DropsPositionlessTokens(t *testing.T) { - idx := BuildYAMLIndex([]byte("a: 1\n")).Highlight + // ` ok: true` closes User and definitions, so two closing delimiters land + // on `true`'s own column, after it. + assert.Equal(t, + []theme.SyntaxKind{theme.SyntaxKey, theme.SyntaxKeyword}, + kindsOn(idx, 3), "`true` keeps its class through the closers") - for _, sp := range idx.Spans(0) { - assert.Positive(t, sp.Col) + // Same at the end of a block sequence: ` - b` closes the sequence and the + // document. + assert.Equal(t, + []theme.SyntaxKind{theme.SyntaxString}, + kindsOn(idx, 6), "`b` keeps its class through the closers") +} + +// Spans record where a run STARTS, so a line can never hold two runs at the same +// column, whichever lexer produced them — the renderer would emit one of them as +// a zero-width run and paint the other over its neighbour's text. +func TestHighlight_SpansStartAtDistinctColumns(t *testing.T) { + for name, idx := range map[string]*HighlightIndex{ + "json": BuildJSONIndex([]byte(hlJSON)).Highlight, + "yaml": BuildYAMLIndex([]byte(hlYAML)).Highlight, + } { + t.Run(name, func(t *testing.T) { + for line, spans := range idx.All() { + assert.GreaterOrEqual(t, line, 0, "nothing is filed under a negative line") + for i, sp := range spans { + assert.Positive(t, sp.Col, "line %d span %d: columns are 1-based", line, i) + if i > 0 { + assert.Greater(t, sp.Col, spans[i-1].Col, "line %d", line) + } + } + } + }) } - assert.Empty(t, idx.Spans(-1), "nothing is filed under a negative line") } func TestHighlight_NilAndEmpty(t *testing.T) { diff --git a/cmd/genspec-tui/internal/ux/index/refindex.go b/cmd/genspec-tui/internal/ux/index/refindex.go index 4487180d..34de7de8 100644 --- a/cmd/genspec-tui/internal/ux/index/refindex.go +++ b/cmd/genspec-tui/internal/ux/index/refindex.go @@ -218,6 +218,6 @@ func (a *indexAccum) finish() Indexes { return Indexes{ Spec: NewSpecIndex(a.line2ptr, a.ptr2line), Refs: &RefIndex{byTarget: a.byTarget, byLine: a.byLine, total: a.total}, - Highlight: a.finishSpans(), + Highlight: &HighlightIndex{byLine: a.spans}, } } diff --git a/docs/examples/go.mod b/docs/examples/go.mod index 7e91eb8f..945c4482 100644 --- a/docs/examples/go.mod +++ b/docs/examples/go.mod @@ -4,7 +4,7 @@ // codescan consumers. module github.com/go-openapi/codescan/docs/examples -go 1.25.8 +go 1.25.0 require ( github.com/go-openapi/codescan v0.0.0 diff --git a/fixtures/go.mod b/fixtures/go.mod index 38ac93f7..99df9cd0 100644 --- a/fixtures/go.mod +++ b/fixtures/go.mod @@ -1,6 +1,6 @@ module github.com/go-openapi/codescan/fixtures -go 1.25.8 +go 1.25.0 require ( github.com/go-openapi/runtime v0.29.3 diff --git a/go.mod b/go.mod index 91c32305..f064ae2e 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/go-openapi/codescan -go 1.25.8 +go 1.25.0 toolchain go1.26.1 diff --git a/go.work b/go.work index ba644503..976a6a2d 100644 --- a/go.work +++ b/go.work @@ -1,4 +1,4 @@ -go 1.25.8 +go 1.25.0 // Workspace for the codescan monorepo: the main library module (.) and the // genspec-tui front-end module (./cmd/genspec-tui), kept in separate go.mod @@ -9,7 +9,7 @@ go 1.25.8 // own. `go install .../cmd/genspec-tui@latest` ignores this file, so the TUI // module's own go.mod carries the real `require` on the library. // -// Keep the `go` directive above in step with the modules (all 1.25.8): raising +// Keep the `go` directive above in step with the modules (all 1.25.0): raising // it past them imposes a toolchain floor the code does not need, and would take // the oldstable CI job out. use (