From d1a985053141114aa1781b1c985d09697fddb23d Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 9 Aug 2026 15:06:08 +0200 Subject: [PATCH] Read the OSC 1717 records a diff renderer emits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A diff renderer that restructures the diff — into columns, or with the +/- markers replaced by colour — leaves us no way to tell which line of which file a rendered row came from, which is what acting on the row requires. The OSC 1717 protocol has the renderer say so directly: it prefixes each line it renders with a record naming the file and the line's position in the old and new versions of it. Attach each record to the cells it precedes, so that a row's records survive wrapping and the columns of a side-by-side rendering, and hand them to readers together with the row's text: the two have to describe the same buffer, and a re-render can rebuild it between two reads. Co-authored-by: Claude Opus 5 (1M context) --- pkg/gocui/escape.go | 23 +++++- pkg/gocui/view.go | 54 ++++++++++--- pkg/gocui/view_test.go | 76 ++++++++++++++++++- .../controllers/helpers/diff_line_helper.go | 4 +- .../controllers/helpers/diff_line_parser.go | 11 +++ 5 files changed, 148 insertions(+), 20 deletions(-) diff --git a/pkg/gocui/escape.go b/pkg/gocui/escape.go index d6b341153..b5e84674c 100644 --- a/pkg/gocui/escape.go +++ b/pkg/gocui/escape.go @@ -23,6 +23,10 @@ type escapeInterpreter struct { // the digits of the OSC number seen so far, while we don't yet know which // OSC this is oscNumber strings.Builder + // the payload of an OSC 1717 sequence, in which a diff renderer states + // which line of which file it is about to render; accumulated like + // hyperlink, and attached to the cells that follow it + metadata strings.Builder // ConPTY emits cursor-positioning escapes (CUP) to skip over blank // rows rather than emitting LFs for them. To convert those into row @@ -88,6 +92,7 @@ const ( stateOSC stateOSCParams stateOSCHyperlink + stateOSCMetadata stateOSCEndEscape stateOSCSkipUnknown @@ -437,10 +442,14 @@ func (ei *escapeInterpreter) parseOne(ch []byte) (isEscape bool, err error) { ei.oscNumber.WriteByte(ch[0]) return true, nil case characterEquals(ch, ';'): - if ei.oscNumber.String() == "8" { + switch ei.oscNumber.String() { + case "8": ei.hyperlink.Reset() ei.state = stateOSCParams - } else { + case "1717": + ei.metadata.Reset() + ei.state = stateOSCMetadata + default: ei.state = stateOSCSkipUnknown } ei.oscNumber.Reset() @@ -477,6 +486,16 @@ func (ei *escapeInterpreter) parseOne(ch []byte) (isEscape bool, err error) { ei.hyperlink.Write(ch) } return true, nil + case stateOSCMetadata: + switch { + case characterEquals(ch, 0x07): + ei.state = stateNone + case characterEquals(ch, 0x1b): + ei.state = stateOSCEndEscape + default: + ei.metadata.Write(ch) + } + return true, nil case stateOSCEndEscape: ei.state = stateNone return true, nil diff --git a/pkg/gocui/view.go b/pkg/gocui/view.go index 2961447f6..991a62502 100644 --- a/pkg/gocui/view.go +++ b/pkg/gocui/view.go @@ -727,6 +727,9 @@ type cell struct { width int // number of terminal cells occupied by chr (always 1 or 2) bgColor, fgColor Attribute hyperlink string + // the OSC 1717 payload in effect when the cell was written, i.e. what the + // diff renderer said about the diff line this cell is part of + metadata string } type cells []cell @@ -1131,6 +1134,10 @@ func (b *viewBuffer) write(v *View, p []byte) { if b.wy >= len(b.lines) { b.lines = append(b.lines, lineType{}) } + // An OSC 1717 record describes the line it precedes and is never + // closed, so it stops applying at the line's end; a renderer emits a + // fresh one for each line it has something to say about. + b.ei.metadata.Reset() } if b.pendingNewline { @@ -1340,6 +1347,7 @@ func (b *viewBuffer) parseInput(v *View, ch []byte, width int, x int, _ int) (bo fgColor: b.ei.curFgColor, bgColor: b.ei.curBgColor, hyperlink: b.ei.hyperlink.String(), + metadata: b.ei.metadata.String(), chr: string(ch), width: width, } @@ -1918,22 +1926,46 @@ func (v *View) BufferLines() []string { return lines } -// LinesAsWritten returns the lines of the view's internal buffer as their writer -// wrote them, escape sequences left out, where BufferLines returns them as the -// cells spell them. The two differ where the cells can't spell what was written: -// a tab, which they hold as the spaces it fills, and a carriage return, which -// they hold as the overwrite it caused. A reader that parses the content rather -// than showing it wants this form. git, for one, terminates a path containing a -// space with a tab in a diff header, and a parser of the diff has to see the tab. -func (v *View) LinesAsWritten() []string { +// DiffLineContent holds what one line of a rendered diff offers to a reader trying +// to recover which line of which file it came from: the line's text, which can be +// parsed as a unified diff when the rendering preserves one, and the OSC 1717 +// records a diff renderer attached to it, which state the answer outright. +type DiffLineContent struct { + // The line's text as its writer wrote it, escape sequences left out, where + // BufferLines gives the text as the cells spell it. The two differ where the + // cells can't spell what was written: a tab, which they hold as the spaces it + // fills, and a carriage return, which they hold as the overwrite it caused. + // git terminates a path containing a space with a tab in a diff header, and a + // parser of the diff has to see the tab. + Text string + // The distinct OSC 1717 payloads carried by the line's cells, in + // left-to-right order. A single-column rendering tags every cell of a line + // with the same payload, so there is one; a side-by-side rendering tags + // each side separately, so a line showing a deletion beside the addition + // that replaces it carries both. + Metadata []string +} + +// DiffLineContents returns the per-line material a diff-line reader works from +// (see DiffLineContent), indexed by unwrapped buffer line. Text and records are +// snapshotted in a single locked pass, so they stay consistent with each other +// and with the buffer they came from even while a re-render rebuilds it. +func (v *View) DiffLineContents() []DiffLineContent { v.writeMutex.Lock() defer v.writeMutex.Unlock() - lines := make([]string, len(v.buf.lines)) + contents := make([]DiffLineContent, len(v.buf.lines)) for i := range v.buf.lines { - lines[i] = v.buf.lines[i].textAsWritten() + line := &v.buf.lines[i] + var metadata []string + for _, c := range line.cells { + if c.metadata != "" && !slices.Contains(metadata, c.metadata) { + metadata = append(metadata, c.metadata) + } + } + contents[i] = DiffLineContent{Text: line.textAsWritten(), Metadata: metadata} } - return lines + return contents } // BufferLineForViewLine maps a view line index (which counts wrapped lines) to diff --git a/pkg/gocui/view_test.go b/pkg/gocui/view_test.go index 57d18abde..31e0e7c05 100644 --- a/pkg/gocui/view_test.go +++ b/pkg/gocui/view_test.go @@ -124,7 +124,12 @@ func TestOverwriteLinesAfterContentEndingInANewline(t *testing.T) { assert.Equal(t, []string{"x", "b"}, v.BufferLines()) } -func TestLinesAsWritten(t *testing.T) { +// diffLineTexts returns the Text of each of the given contents. +func diffLineTexts(contents []DiffLineContent) []string { + return lo.Map(contents, func(c DiffLineContent, _ int) string { return c.Text }) +} + +func TestDiffLineContentsTextIsTheTextAsWritten(t *testing.T) { tests := []struct { name string stringsToWrite []string @@ -184,12 +189,12 @@ func TestLinesAsWritten(t *testing.T) { v.writeString(s) } assert.Equal(t, test.expectedShown, v.BufferLines()) - assert.Equal(t, test.expectedAsWritten, v.LinesAsWritten()) + assert.Equal(t, test.expectedAsWritten, diffLineTexts(v.DiffLineContents())) }) } } -func TestLinesAsWrittenOfAnOverwrittenLine(t *testing.T) { +func TestDiffLineContentsTextOfAnOverwrittenLine(t *testing.T) { v := NewView("name", 0, 0, 20, 10, OutputNormal) v.writeString("a\tb\nc\td") @@ -198,7 +203,7 @@ func TestLinesAsWrittenOfAnOverwrittenLine(t *testing.T) { v.OverwriteLines(0, "xy") assert.Equal(t, []string{"xy", "c d"}, v.BufferLines()) - assert.Equal(t, []string{"xy", "c\td"}, v.LinesAsWritten()) + assert.Equal(t, []string{"xy", "c\td"}, diffLineTexts(v.DiffLineContents())) } func TestUpdatedCursorAndOrigin(t *testing.T) { @@ -246,6 +251,69 @@ func TestAutoRenderingHyperlinks(t *testing.T) { assert.Equal(t, "https://example.com", v.buf.lines[0].cells[0].hyperlink) } +// osc1717 wraps an OSC 1717 payload in the sequence a diff renderer emits it in: +// the ESC ] introducer with the OSC number, and ESC \ as the terminator. +func osc1717(payload string) string { + return "\x1b]1717;" + payload + "\x1b\\" +} + +func TestDiffLineContents(t *testing.T) { + v := NewView("name", 0, 0, 80, 10, OutputNormal) + + // A diff renderer prefixes each line it renders with a record naming the + // file and the line's position in it: version;type;new-line;old-line;file. + v.writeString(strings.Join([]string{ + osc1717("1;c;1;;foo.txt") + "line1", + osc1717("1;d;2;2;foo.txt") + "old2", + osc1717("1;a;2;;foo.txt") + "new2", + "@@ a hunk header, which carries no record @@", + }, "\n")) + + assert.Equal(t, []DiffLineContent{ + {Text: "line1", Metadata: []string{"1;c;1;;foo.txt"}}, + {Text: "old2", Metadata: []string{"1;d;2;2;foo.txt"}}, + {Text: "new2", Metadata: []string{"1;a;2;;foo.txt"}}, + // The record of the line before doesn't bleed onto this one. + {Text: "@@ a hunk header, which carries no record @@"}, + }, v.DiffLineContents()) +} + +func TestDiffLineContentsWithSideBySideRecords(t *testing.T) { + v := NewView("name", 0, 0, 80, 10, OutputNormal) + + // A side-by-side renderer puts two diff lines on one rendered line, and so + // emits a record before each half. + v.writeString(strings.Join([]string{ + osc1717("1;c;1;;foo.txt") + "context " + osc1717("1;c;1;;foo.txt") + "context", + osc1717("1;d;2;2;foo.txt") + "old2 " + osc1717("1;a;2;;foo.txt") + "new2", + }, "\n")) + + assert.Equal(t, []DiffLineContent{ + // The two halves of a context line are the same diff line, stated twice. + {Text: "context context", Metadata: []string{"1;c;1;;foo.txt"}}, + {Text: "old2 new2", Metadata: []string{"1;d;2;2;foo.txt", "1;a;2;;foo.txt"}}, + }, v.DiffLineContents()) +} + +func TestDiffLineContentsOfWrappedLine(t *testing.T) { + v := NewView("name", 0, 0, 10, 10, OutputNormal) // InnerWidth is 9 + v.Wrap = true + + // A line that gocui wraps is still one buffer line, so its record covers + // every view line it is displayed on. + v.writeString(osc1717("1;a;1;;foo.txt") + "a line too long to fit") + + assert.Equal(t, []DiffLineContent{ + {Text: "a line too long to fit", Metadata: []string{"1;a;1;;foo.txt"}}, + }, v.DiffLineContents()) + assert.Equal(t, 3, v.ViewLinesHeight()) + for viewLine := range 3 { + bufferLine, ok := v.BufferLineForViewLine(viewLine) + assert.True(t, ok) + assert.Equal(t, 0, bufferLine) + } +} + // An async re-render builds into an off-screen buffer and swaps it in once it // has enough to paint, so readers keep seeing the previous render — coherent and // consistent — until the new content appears in one step. See View.offscreen. diff --git a/pkg/gui/controllers/helpers/diff_line_helper.go b/pkg/gui/controllers/helpers/diff_line_helper.go index de4a60960..4402d124d 100644 --- a/pkg/gui/controllers/helpers/diff_line_helper.go +++ b/pkg/gui/controllers/helpers/diff_line_helper.go @@ -33,9 +33,7 @@ func (self *DiffLineHelper) GetDiffLineInfo(view *gocui.View, viewLineIdx int) ( return types.DiffLineInfo{}, false } - // The lines as written, not as shown: git ends the path field of a diff header - // with a tab when the path contains a space, and the view shows a tab as spaces. - parsed, ok := parseDiffLineFromBuffer(view.LinesAsWritten(), bufferLineIdx) + parsed, ok := parseDiffLineFromBuffer(diffLineTexts(view.DiffLineContents()), bufferLineIdx) if !ok { return types.DiffLineInfo{}, false } diff --git a/pkg/gui/controllers/helpers/diff_line_parser.go b/pkg/gui/controllers/helpers/diff_line_parser.go index d58633f7e..109fd758e 100644 --- a/pkg/gui/controllers/helpers/diff_line_parser.go +++ b/pkg/gui/controllers/helpers/diff_line_parser.go @@ -6,6 +6,7 @@ import ( "strings" "github.com/jesseduffield/lazygit/pkg/commands/patch" + "github.com/jesseduffield/lazygit/pkg/gocui" "github.com/jesseduffield/lazygit/pkg/gui/types" ) @@ -92,6 +93,16 @@ func parseAllDiffLinesFromBuffer(bufferLines []string) []bufferLineParse { return result } +// diffLineTexts extracts the text of each rendered row — the material the buffer +// parser works on. +func diffLineTexts(contents []gocui.DiffLineContent) []string { + texts := make([]string, len(contents)) + for i, content := range contents { + texts[i] = content.Text + } + return texts +} + // fileSectionBounds returns the half-open range [start, end) of the file section // containing targetIdx: the nearest line starting a section at or above it, up to // where that section ends. start is -1 when targetIdx is above the first file