From 84ddc5bacf30fc7294e1995134c2ff420d633efb Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 9 Aug 2026 15:10:26 +0200 Subject: [PATCH] Keep the OSC 1717 records that cover no cell MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wherever two diff lines end up on one rendered line, a renderer emits their records back to back: the deletion and the addition of a modification collapsed into a single column, or a banner announcing a file and its first hunk at once. A changed line that is empty is rendered as its record alone. Attaching a record only to the cells it precedes loses all of these — the last record of a run wins, and an empty changed line becomes a line we can say nothing about at all. Give such a record a cell of its own instead. It renders nothing, so the diff looks the same, but the line keeps every record it was given. Co-authored-by: Claude Opus 5 (1M context) --- pkg/gocui/escape.go | 32 +++++++++++++++++++++++++++++++- pkg/gocui/view.go | 35 ++++++++++++++++++++++++++++++++--- pkg/gocui/view_test.go | 21 +++++++++++++++++++++ 3 files changed, 84 insertions(+), 4 deletions(-) diff --git a/pkg/gocui/escape.go b/pkg/gocui/escape.go index 0226a59fb..16b8f1cc7 100644 --- a/pkg/gocui/escape.go +++ b/pkg/gocui/escape.go @@ -27,6 +27,17 @@ type escapeInterpreter struct { // which line of which file it is about to render; accumulated like // hyperlink, and attached to the cells that follow it metadata strings.Builder + // whether the payload currently in metadata has reached a cell, so that one + // that never does can be recognized and kept as an orphan + metadataConsumed bool + // OSC 1717 payloads that no cell took, because the next record followed + // with nothing rendered in between. A renderer emits records back to back + // wherever two diff lines share a rendered line — the deletion and the + // addition of a modification collapsed into one column, or a banner + // announcing a file and its first hunk at once. The write loop gives these + // cells of their own, so that a line keeps every record it was given rather + // than only the last. + orphanedMetadata []string // ConPTY emits cursor-positioning escapes (CUP) to skip over blank // rows rather than emitting LFs for them. To convert those into row @@ -447,7 +458,7 @@ func (ei *escapeInterpreter) parseOne(ch []byte) (isEscape bool, err error) { ei.hyperlink.Reset() ei.state = stateOSCParams case "1717": - ei.metadata.Reset() + ei.orphanUnconsumedMetadata() ei.state = stateOSCMetadata default: ei.state = stateOSCSkipUnknown @@ -513,6 +524,25 @@ func (ei *escapeInterpreter) parseOne(ch []byte) (isEscape bool, err error) { return false, nil } +// orphanUnconsumedMetadata clears the metadata accumulator for a new OSC 1717 +// record, keeping the payload it held as an orphan if no cell took it (see +// orphanedMetadata). +func (ei *escapeInterpreter) orphanUnconsumedMetadata() { + if ei.metadata.Len() > 0 && !ei.metadataConsumed { + ei.orphanedMetadata = append(ei.orphanedMetadata, ei.metadata.String()) + } + ei.metadata.Reset() + ei.metadataConsumed = false +} + +// takeOrphanedMetadata hands the accumulated orphaned payloads to the caller and +// clears the list. +func (ei *escapeInterpreter) takeOrphanedMetadata() []string { + result := ei.orphanedMetadata + ei.orphanedMetadata = nil + return result +} + // dropMetadataIfHandshake discards a just-completed OSC 1717 payload that // carries nothing beyond the version. A diff renderer emits such a record ahead // of everything else to announce that it speaks the protocol, so that a host can diff --git a/pkg/gocui/view.go b/pkg/gocui/view.go index 991a62502..42a087dce 100644 --- a/pkg/gocui/view.go +++ b/pkg/gocui/view.go @@ -1127,6 +1127,19 @@ func (b *viewBuffer) write(v *View, p []byte) { finishLine := func() { b.autoRenderHyperlinksInCurrentLine(v) + // A record that reached the line's end without covering a cell still + // belongs to the line: an orphan (see escapeInterpreter.orphanedMetadata), + // or the record of a changed line that is empty, which a renderer emits + // with nothing but the newline after it. Give each a cell of its own, so + // that the line is still recognizable as the diff line it renders rather + // than as nothing at all. + for _, payload := range b.ei.takeOrphanedMetadata() { + b.writeCells([]cell{{metadata: payload}}) + } + if b.ei.metadata.Len() > 0 && !b.ei.metadataConsumed { + b.writeCells([]cell{{metadata: b.ei.metadata.String()}}) + b.ei.metadataConsumed = true + } } advanceToNextLine := func() { @@ -1286,6 +1299,15 @@ func (b *viewBuffer) parseInput(v *View, ch []byte, width int, x int, _ int) (bo truncateLine := false isEscape, err := b.ei.parseOne(ch) + + // A record that the next one superseded before any cell took it still + // belongs to this line (see escapeInterpreter.orphanedMetadata); give each + // a cell of its own, in the order they were emitted, ahead of whatever this + // character produces. + for _, payload := range b.ei.takeOrphanedMetadata() { + cells = append(cells, cell{metadata: payload}) + } + if err != nil { characters := b.ei.characters() for _, chr := range characters { @@ -1314,7 +1336,7 @@ func (b *viewBuffer) parseInput(v *View, ch []byte, width int, x int, _ int) (bo fg: b.ei.curFgColor, bg: b.ei.curBgColor, } - return truncateLine, []cell{} + return truncateLine, cells } else if cf, ok := b.ei.instruction.(cursorForward); ok { // emit `n` space cells under the parser-tracked SGR — used // to materialize ConPTY's compressed runs of spaces (which @@ -1325,8 +1347,12 @@ func (b *viewBuffer) parseInput(v *View, ch []byte, width int, x int, _ int) (bo width = 1 b.noteAsWritten(bytes.Repeat(ch, repeatCount)) } else if isEscape { - // do not output anything - return truncateLine, nil + // the escape itself outputs nothing, but any cells carrying an + // orphaned record still need writing + if len(cells) == 0 { + return truncateLine, nil + } + return truncateLine, cells } else if characterEquals(ch, '\t') { // The cells hold a tab as the spaces it fills; the text as written // keeps the tab itself. @@ -1351,6 +1377,9 @@ func (b *viewBuffer) parseInput(v *View, ch []byte, width int, x int, _ int) (bo chr: string(ch), width: width, } + if c.metadata != "" { + b.ei.metadataConsumed = true + } for range repeatCount { cells = append(cells, c) } diff --git a/pkg/gocui/view_test.go b/pkg/gocui/view_test.go index c74ba97a2..e3ac6c49e 100644 --- a/pkg/gocui/view_test.go +++ b/pkg/gocui/view_test.go @@ -314,6 +314,27 @@ func TestDiffLineContentsOfWrappedLine(t *testing.T) { } } +func TestDiffLineContentsWithRecordsCoveringNoCell(t *testing.T) { + v := NewView("name", 0, 0, 80, 10, OutputNormal) + + v.writeString(strings.Join([]string{ + // A banner announcing a file and its first hunk at once carries both + // records back to back. + osc1717("1;f;;;foo.txt") + osc1717("1;h;5;;foo.txt") + "foo.txt --- Go", + // So does a modification whose deletion and addition are collapsed into + // a single rendered line. + osc1717("1;d;5;5;foo.txt") + osc1717("1;a;5;;foo.txt") + "595 new content", + // A changed line that is empty is rendered as its record and nothing else. + osc1717("1;a;6;;foo.txt"), + }, "\n") + "\n") + + assert.Equal(t, []DiffLineContent{ + {Text: "foo.txt --- Go", Metadata: []string{"1;f;;;foo.txt", "1;h;5;;foo.txt"}}, + {Text: "595 new content", Metadata: []string{"1;d;5;5;foo.txt", "1;a;5;;foo.txt"}}, + {Text: "", Metadata: []string{"1;a;6;;foo.txt"}}, + }, v.DiffLineContents()) +} + func TestDiffLineContentsSwallowsHandshake(t *testing.T) { v := NewView("name", 0, 0, 80, 10, OutputNormal)