From 808b6af8026823e5681d1e49dc44e96d55ff0bb5 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 9 Aug 2026 15:02:57 +0200 Subject: [PATCH 1/7] Recognize OSC numbers with more than one digit The OSC parser dispatched on a single character, so only the single-digit OSC 8 could ever be recognized; the diff-line metadata protocol we are about to read uses OSC 1717. Co-authored-by: Claude Opus 5 (1M context) --- pkg/gocui/escape.go | 54 +++++++++++++++++++++++++--------------- pkg/gocui/escape_test.go | 1 + 2 files changed, 35 insertions(+), 20 deletions(-) diff --git a/pkg/gocui/escape.go b/pkg/gocui/escape.go index 7f3de9e6e..d6b341153 100644 --- a/pkg/gocui/escape.go +++ b/pkg/gocui/escape.go @@ -20,6 +20,10 @@ type escapeInterpreter struct { instruction instruction hyperlink strings.Builder + // the digits of the OSC number seen so far, while we don't yet know which + // OSC this is + oscNumber strings.Builder + // ConPTY emits cursor-positioning escapes (CUP) to skip over blank // rows rather than emitting LFs for them. To convert those into row // advances the view can act on, we track where in the pseudo-terminal @@ -82,7 +86,6 @@ const ( stateParams stateCSIDiscard stateOSC - stateOSCWaitForParams stateOSCParams stateOSCHyperlink stateOSCEndEscape @@ -427,27 +430,38 @@ func (ei *escapeInterpreter) parseOne(ch []byte) (isEscape bool, err error) { } return true, nil case stateOSC: - if characterEquals(ch, '8') { - ei.state = stateOSCWaitForParams - ei.hyperlink.Reset() + // Accumulate the OSC number until the ';' that terminates it, then + // dispatch on the whole number rather than on a single digit. + switch { + case len(ch) == 1 && ch[0] >= '0' && ch[0] <= '9': + ei.oscNumber.WriteByte(ch[0]) + return true, nil + case characterEquals(ch, ';'): + if ei.oscNumber.String() == "8" { + ei.hyperlink.Reset() + ei.state = stateOSCParams + } else { + ei.state = stateOSCSkipUnknown + } + ei.oscNumber.Reset() + return true, nil + default: + // Not an OSC we understand — it has no number, or a character + // follows the number where the ';' should be. Rather than + // erroring, which would reset state mid-OSC and leak the rest of + // the sequence into the view as literal text, skip to its + // terminator, which this character may already be. + ei.oscNumber.Reset() + switch { + case characterEquals(ch, 0x07): + ei.state = stateNone + case characterEquals(ch, 0x1b): + ei.state = stateOSCEndEscape + default: + ei.state = stateOSCSkipUnknown + } return true, nil } - - ei.state = stateOSCSkipUnknown - return true, nil - case stateOSCWaitForParams: - if !characterEquals(ch, ';') { - // Malformed OSC 8 (expected ';' after '8'). Rather than - // erroring — which would reset state mid-OSC and cause the - // rest of the sequence to leak as literal text — treat the - // whole OSC as one we don't understand and skip to its - // terminator. - ei.state = stateOSCSkipUnknown - return true, nil - } - - ei.state = stateOSCParams - return true, nil case stateOSCParams: if characterEquals(ch, ';') { ei.state = stateOSCHyperlink diff --git a/pkg/gocui/escape_test.go b/pkg/gocui/escape_test.go index 39ccbe908..0efef2a77 100644 --- a/pkg/gocui/escape_test.go +++ b/pkg/gocui/escape_test.go @@ -167,6 +167,7 @@ func TestParseOneIgnoresUnknownSequences(t *testing.T) { "\x1b[0 q", // intermediate byte after a param "\x1b[1;;m", // malformed SGR: empty middle param "\x1b]8bogus\x07", // OSC 8 missing ';' + "\x1b]1337;File=inline=1\x07", // OSC with a number we don't implement "\x1b[" + strings.Repeat("0", 300) + "m", // single param overflows length cap "\x1b[" + strings.Repeat("1;", 25) + "1m", // too many params } From a7f08990f8674f22e3d8acc81d3bc1620abeb99e Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 9 Aug 2026 15:06:08 +0200 Subject: [PATCH 2/7] 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 From 82684f1fa6705aad5f855d6634e679895df626ad Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 9 Aug 2026 15:07:26 +0200 Subject: [PATCH 3/7] Swallow a diff renderer's protocol handshake MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before the diff, a conforming renderer emits one OSC 1717 record that carries only the version — its way of announcing that it speaks the protocol at all, without a host having to inspect what it renders. It describes no line, so keeping it would give the first line of the diff a record that says nothing about it. Co-authored-by: Claude Opus 5 (1M context) --- pkg/gocui/escape.go | 14 ++++++++++++++ pkg/gocui/view_test.go | 17 +++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/pkg/gocui/escape.go b/pkg/gocui/escape.go index b5e84674c..0226a59fb 100644 --- a/pkg/gocui/escape.go +++ b/pkg/gocui/escape.go @@ -489,8 +489,10 @@ func (ei *escapeInterpreter) parseOne(ch []byte) (isEscape bool, err error) { case stateOSCMetadata: switch { case characterEquals(ch, 0x07): + ei.dropMetadataIfHandshake() ei.state = stateNone case characterEquals(ch, 0x1b): + ei.dropMetadataIfHandshake() ei.state = stateOSCEndEscape default: ei.metadata.Write(ch) @@ -511,6 +513,18 @@ func (ei *escapeInterpreter) parseOne(ch []byte) (isEscape bool, err error) { return false, nil } +// 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 +// find that out by asking rather than by inspecting a rendering. It says nothing +// about any line, so it must not attach to the line that follows it; a per-line +// record always has fields, and is kept. +func (ei *escapeInterpreter) dropMetadataIfHandshake() { + if !strings.Contains(ei.metadata.String(), ";") { + ei.metadata.Reset() + } +} + func (ei *escapeInterpreter) outputCSI() error { n := len(ei.csiParam) for i := 0; i < n; { diff --git a/pkg/gocui/view_test.go b/pkg/gocui/view_test.go index 31e0e7c05..c74ba97a2 100644 --- a/pkg/gocui/view_test.go +++ b/pkg/gocui/view_test.go @@ -314,6 +314,23 @@ func TestDiffLineContentsOfWrappedLine(t *testing.T) { } } +func TestDiffLineContentsSwallowsHandshake(t *testing.T) { + v := NewView("name", 0, 0, 80, 10, OutputNormal) + + // A diff renderer announces itself with a version-only record before the + // diff. It must leave no trace: no visible bytes, no line of its own, and + // above all no record on the line that follows it. + v.writeString(osc1717("1") + strings.Join([]string{ + "diff --git a/foo.txt b/foo.txt", + osc1717("1;a;1;;foo.txt") + "added", + }, "\n")) + + assert.Equal(t, []DiffLineContent{ + {Text: "diff --git a/foo.txt b/foo.txt"}, + {Text: "added", Metadata: []string{"1;a;1;;foo.txt"}}, + }, v.DiffLineContents()) +} + // 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. From 03347edd723efb6c4bb2e0ea14252f5102e56531 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 9 Aug 2026 15:10:26 +0200 Subject: [PATCH 4/7] 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) From 7563dc0f52ebf7fbc39dd41d937fc1cf6792f82d Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 9 Aug 2026 15:20:29 +0200 Subject: [PATCH 5/7] Rename parsedDiffLine.RelPath to Path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A diff line's identity is about to become recoverable from a second source, a diff renderer's own records, and a renderer states the path however it likes — absolute paths included. The field can't promise repo-relative any more. Co-authored-by: Claude Opus 5 (1M context) --- .../controllers/helpers/diff_line_helper.go | 2 +- .../controllers/helpers/diff_line_parser.go | 8 ++--- .../helpers/diff_line_parser_test.go | 34 +++++++++---------- 3 files changed, 22 insertions(+), 22 deletions(-) diff --git a/pkg/gui/controllers/helpers/diff_line_helper.go b/pkg/gui/controllers/helpers/diff_line_helper.go index 4402d124d..77f70611e 100644 --- a/pkg/gui/controllers/helpers/diff_line_helper.go +++ b/pkg/gui/controllers/helpers/diff_line_helper.go @@ -45,7 +45,7 @@ func (self *DiffLineHelper) GetDiffLineInfo(view *gocui.View, viewLineIdx int) ( // absolute-path identity consumers work with. func (self *DiffLineHelper) diffLineInfoFromParsed(parsed parsedDiffLine) types.DiffLineInfo { return types.DiffLineInfo{ - Path: filepath.Join(self.c.Git().RepoPaths.WorktreePath(), parsed.RelPath), + Path: filepath.Join(self.c.Git().RepoPaths.WorktreePath(), parsed.Path), Type: parsed.Type, NewLine: parsed.NewLine, OldLine: parsed.OldLine, diff --git a/pkg/gui/controllers/helpers/diff_line_parser.go b/pkg/gui/controllers/helpers/diff_line_parser.go index 109fd758e..c9033960d 100644 --- a/pkg/gui/controllers/helpers/diff_line_parser.go +++ b/pkg/gui/controllers/helpers/diff_line_parser.go @@ -32,10 +32,10 @@ var submoduleSectionPattern = regexp.MustCompile( `^Submodule (.+) (?:contains (?:untracked|modified) content|[0-9a-f]+\.{2,3}[0-9a-f]+(?: \(.*\))?:?)$`) // parsedDiffLine is what the parser recovers about a row of a rendered diff. -// RelPath is the path as the diff header spells it, i.e. relative to the repo -// root; the caller turns it into the absolute path of types.DiffLineInfo. +// Path is the path as the diff header spells it, i.e. relative to the repo root; +// the caller turns it into the absolute path of types.DiffLineInfo. type parsedDiffLine struct { - RelPath string + Path string Type types.DiffLineType NewLine int OldLine int @@ -215,7 +215,7 @@ func parseFileSection(fileLines []string, endsTheBuffer bool) []bufferLineParse break } parsed := parsedDiffLine{ - RelPath: relPath, + Path: relPath, Type: diffLineTypeForKind(patchLines[i].Kind), NewLine: p.LineNumberOfLine(i), } diff --git a/pkg/gui/controllers/helpers/diff_line_parser_test.go b/pkg/gui/controllers/helpers/diff_line_parser_test.go index ae620d23c..11f078abd 100644 --- a/pkg/gui/controllers/helpers/diff_line_parser_test.go +++ b/pkg/gui/controllers/helpers/diff_line_parser_test.go @@ -40,16 +40,16 @@ func TestParseDiffLineFromBuffer(t *testing.T) { expected parsedDiffLine expectOk bool }{ - {"file header", 0, parsedDiffLine{RelPath: "file1.go", Type: types.DiffLineFileHeader, NewLine: 1}, true}, - {"hunk header", 4, parsedDiffLine{RelPath: "file1.go", Type: types.DiffLineHunkHeader, NewLine: 1}, true}, - {"context line", 5, parsedDiffLine{RelPath: "file1.go", Type: types.DiffLineContext, NewLine: 1}, true}, + {"file header", 0, parsedDiffLine{Path: "file1.go", Type: types.DiffLineFileHeader, NewLine: 1}, true}, + {"hunk header", 4, parsedDiffLine{Path: "file1.go", Type: types.DiffLineHunkHeader, NewLine: 1}, true}, + {"context line", 5, parsedDiffLine{Path: "file1.go", Type: types.DiffLineContext, NewLine: 1}, true}, // The two deletions share new-file line 2 but have distinct old-file lines. - {"first deletion", 6, parsedDiffLine{RelPath: "file1.go", Type: types.DiffLineDeleted, NewLine: 2, OldLine: 2}, true}, - {"second deletion", 7, parsedDiffLine{RelPath: "file1.go", Type: types.DiffLineDeleted, NewLine: 2, OldLine: 3}, true}, + {"first deletion", 6, parsedDiffLine{Path: "file1.go", Type: types.DiffLineDeleted, NewLine: 2, OldLine: 2}, true}, + {"second deletion", 7, parsedDiffLine{Path: "file1.go", Type: types.DiffLineDeleted, NewLine: 2, OldLine: 3}, true}, // The second file: its path comes from the second "diff --git" section, // and its additions get distinct new-file line numbers. - {"first addition", 15, parsedDiffLine{RelPath: "dir/file2.go", Type: types.DiffLineAdded, NewLine: 10}, true}, - {"second addition", 16, parsedDiffLine{RelPath: "dir/file2.go", Type: types.DiffLineAdded, NewLine: 11}, true}, + {"first addition", 15, parsedDiffLine{Path: "dir/file2.go", Type: types.DiffLineAdded, NewLine: 10}, true}, + {"second addition", 16, parsedDiffLine{Path: "dir/file2.go", Type: types.DiffLineAdded, NewLine: 11}, true}, {"out of range", 999, parsedDiffLine{}, false}, } @@ -75,7 +75,7 @@ rename to new.go`, "\n") result, ok := parseDiffLineFromBuffer(pureRename, 2) assert.True(t, ok) - assert.Equal(t, parsedDiffLine{RelPath: "new.go", Type: types.DiffLineFileHeader, NewLine: 1}, result) + assert.Equal(t, parsedDiffLine{Path: "new.go", Type: types.DiffLineFileHeader, NewLine: 1}, result) renameWithModification := strings.Split(`diff --git a/old.go b/new.go similarity index 62% @@ -91,7 +91,7 @@ index 1111111..2222222 100644 result, ok = parseDiffLineFromBuffer(renameWithModification, 10) assert.True(t, ok) - assert.Equal(t, parsedDiffLine{RelPath: "new.go", Type: types.DiffLineAdded, NewLine: 2}, result) + assert.Equal(t, parsedDiffLine{Path: "new.go", Type: types.DiffLineAdded, NewLine: 2}, result) } func TestParseDiffLineFromBufferDeletedFile(t *testing.T) { @@ -107,7 +107,7 @@ index 1111111..0000000 result, ok := parseDiffLineFromBuffer(deletedFile, 7) assert.True(t, ok) - assert.Equal(t, parsedDiffLine{RelPath: "gone.go", Type: types.DiffLineDeleted, NewLine: 0, OldLine: 2}, result) + assert.Equal(t, parsedDiffLine{Path: "gone.go", Type: types.DiffLineDeleted, NewLine: 0, OldLine: 2}, result) } func TestParseDiffLineFromBufferSubmodule(t *testing.T) { @@ -129,13 +129,13 @@ Submodule modules/xyz a32f27c..2d9f921: result, ok := parseDiffLineFromBuffer(withSubmodule, targetIdx) assert.True(t, ok) assert.Equal(t, - parsedDiffLine{RelPath: "modules/xyz", Type: types.DiffLineFileHeader, NewLine: 1}, + parsedDiffLine{Path: "modules/xyz", Type: types.DiffLineFileHeader, NewLine: 1}, result, "line %d", targetIdx) } result, ok := parseDiffLineFromBuffer(withSubmodule, 7) assert.True(t, ok) - assert.Equal(t, parsedDiffLine{RelPath: "file.txt", Type: types.DiffLineAdded, NewLine: 3}, result) + assert.Equal(t, parsedDiffLine{Path: "file.txt", Type: types.DiffLineAdded, NewLine: 3}, result) } func TestParseDiffLineFromBufferSubmoduleInARendering(t *testing.T) { @@ -153,8 +153,8 @@ products/a.txt two`, "\n") all := parseAllDiffLinesFromBuffer(rendered) - assert.Equal(t, "modules/xyz", all[0].parsed.RelPath) - assert.Equal(t, "modules/xyz", all[1].parsed.RelPath) + assert.Equal(t, "modules/xyz", all[0].parsed.Path) + assert.Equal(t, "modules/xyz", all[1].parsed.Path) for i := 2; i < len(rendered); i++ { assert.False(t, all[i].ok, "line %d: %q", i, rendered[i]) } @@ -230,7 +230,7 @@ func TestParseDiffLineFromBufferReadInPart(t *testing.T) { cutShort := lines[:len(lines)-1] result, ok := parseDiffLineFromBuffer(cutShort, 15) assert.True(t, ok) - assert.Equal(t, parsedDiffLine{RelPath: "dir/file2.go", Type: types.DiffLineAdded, NewLine: 10}, result) + assert.Equal(t, parsedDiffLine{Path: "dir/file2.go", Type: types.DiffLineAdded, NewLine: 10}, result) // Only the section the buffer breaks off in is read that way. One that another // section follows is all there, so a hunk short of what its header declares means @@ -287,13 +287,13 @@ func TestParseDiffLineFromBufferQuotedPath(t *testing.T) { result, ok := parseDiffLineFromBuffer(renamed, 10) assert.True(t, ok) - assert.Equal(t, parsedDiffLine{RelPath: "café new.go", Type: types.DiffLineAdded, NewLine: 2}, result) + assert.Equal(t, parsedDiffLine{Path: "café new.go", Type: types.DiffLineAdded, NewLine: 2}, result) // The same rename without a content change has no +++/--- lines, so the path // comes from the "diff --git" line, where both paths are quoted. result, ok = parseDiffLineFromBuffer(renamed[:4], 2) assert.True(t, ok) - assert.Equal(t, parsedDiffLine{RelPath: "café new.go", Type: types.DiffLineFileHeader, NewLine: 1}, result) + assert.Equal(t, parsedDiffLine{Path: "café new.go", Type: types.DiffLineFileHeader, NewLine: 1}, result) } func TestParseAllDiffLinesFromBuffer(t *testing.T) { From 2770b2e5a2801db7023e8efdb56b666654b622b2 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 9 Aug 2026 15:21:48 +0200 Subject: [PATCH 6/7] Take a diff renderer at its word about a diff line Where a renderer states which line of which file it is rendering, take that as the line's identity. The renderer knows, and for a rendering that no longer looks like a diff nothing else does. Reading the rendered text stays the way for the renderings that keep a diff's structure, and for the renderers that say nothing at all. Which of the two is used is settled for the rendering as a whole, not row by row. A rendering with records is not parsed at all, not even for the rows the renderer says nothing about. Its text is no unified diff, and a row of it can read like one. Under delta, a commit that adds a test whose input is a diff shows the test's "diff --git a/img.png b/img.png" line as a row of its own; parsed, that row opens a file section that runs to the end of the rendering, and every row delta puts between hunks and files comes out as a header of a file called img.png. So the untagged rows of such a rendering stay unresolved, as the protocol has it (spec section 6.4). Header records are accepted too. This lets a renderer point at a file with no content lines, such as a pure rename, a mode change or a binary file: for those files the header is the only row in the diff. Co-authored-by: Claude Opus 5 (1M context) Co-Authored-By: Claude Fable 5.1 --- .../controllers/helpers/diff_line_helper.go | 49 +++++++-- .../controllers/helpers/diff_line_parser.go | 83 +++++++++++++++ .../helpers/diff_line_parser_test.go | 100 ++++++++++++++++++ 3 files changed, 222 insertions(+), 10 deletions(-) diff --git a/pkg/gui/controllers/helpers/diff_line_helper.go b/pkg/gui/controllers/helpers/diff_line_helper.go index 77f70611e..be96e4912 100644 --- a/pkg/gui/controllers/helpers/diff_line_helper.go +++ b/pkg/gui/controllers/helpers/diff_line_helper.go @@ -18,10 +18,16 @@ func NewDiffLineHelper(c *HelperCommon) *DiffLineHelper { // GetDiffLineInfo recovers the identity — file, kind, and old/new line number — // of the diff row at the given (wrapped) view line of the given view. It is the // seam every consumer of a diff row goes through, so that how we recover that -// identity can change without them noticing: today the only way is to parse the -// view's contents as a unified diff, which works for the renderings that keep a -// diff's structure (no renderer, `git diff --color`, a renderer that only -// colorizes) and fails for the ones that restructure it. +// identity can change without them noticing. +// +// There are two ways, and which one is used is settled for the rendering as a +// whole (see renderingStatesDiffLines). A diff renderer that speaks the OSC 1717 +// protocol states the identity of each line it renders. That is the only way to +// recover it from a rendering that doesn't look like a diff any more (columns, or +// +/- markers replaced by colour), and a row such a renderer says nothing about +// has no identity. Otherwise we parse the view's contents as a unified diff; this +// works for the renderings that keep a diff's structure (no renderer, `git diff +// --color`, a renderer that only colorizes) and fails for the rest. // // ok is false when the row's identity can't be recovered, in which case the // caller must not act on the line at all. @@ -33,19 +39,42 @@ func (self *DiffLineHelper) GetDiffLineInfo(view *gocui.View, viewLineIdx int) ( return types.DiffLineInfo{}, false } - parsed, ok := parseDiffLineFromBuffer(diffLineTexts(view.DiffLineContents()), bufferLineIdx) + contents := view.DiffLineContents() + if bufferLineIdx >= len(contents) { + return types.DiffLineInfo{}, false + } + + if renderingStatesDiffLines(contents) { + // A row can carry more than one record, when the rendering puts two diff + // lines on it; the first one is the row's identity, and the leftmost record + // is the one a reader would call the row's own. + if metadata := contents[bufferLineIdx].Metadata; len(metadata) > 0 { + if parsed, ok := parseDiffLineMetadata(metadata[0]); ok { + return self.diffLineInfo(parsed), true + } + } + return types.DiffLineInfo{}, false + } + + parsed, ok := parseDiffLineFromBuffer(diffLineTexts(contents), bufferLineIdx) if !ok { return types.DiffLineInfo{}, false } - return self.diffLineInfoFromParsed(parsed), true + return self.diffLineInfo(parsed), true } -// diffLineInfoFromParsed turns the parser's repo-relative result into the -// absolute-path identity consumers work with. -func (self *DiffLineHelper) diffLineInfoFromParsed(parsed parsedDiffLine) types.DiffLineInfo { +// diffLineInfo turns a parser's result into the absolute-path identity consumers +// work with. The path arrives repo-relative from the diff header, but a renderer +// states it however it likes, absolute paths included. +func (self *DiffLineHelper) diffLineInfo(parsed parsedDiffLine) types.DiffLineInfo { + path := parsed.Path + if !filepath.IsAbs(path) { + path = filepath.Join(self.c.Git().RepoPaths.WorktreePath(), path) + } + return types.DiffLineInfo{ - Path: filepath.Join(self.c.Git().RepoPaths.WorktreePath(), parsed.Path), + Path: path, Type: parsed.Type, NewLine: parsed.NewLine, OldLine: parsed.OldLine, diff --git a/pkg/gui/controllers/helpers/diff_line_parser.go b/pkg/gui/controllers/helpers/diff_line_parser.go index c9033960d..dd7270af2 100644 --- a/pkg/gui/controllers/helpers/diff_line_parser.go +++ b/pkg/gui/controllers/helpers/diff_line_parser.go @@ -2,6 +2,7 @@ package helpers import ( "regexp" + "slices" "strconv" "strings" @@ -103,6 +104,27 @@ func diffLineTexts(contents []gocui.DiffLineContent) []string { return texts } +// renderingStatesDiffLines reports whether the renderer stated, for at least one row +// of the rendering, which diff line it shows. The version-only record a renderer +// announces the protocol with names no line, and doesn't count. +// +// The answer settles how the whole rendering is read. A renderer that states its +// lines lays the diff out as it likes, so its text is no unified diff and must not be +// parsed as one, not even for the rows it says nothing about. Such a row can read +// like a diff header when the file being diffed is itself a diff; the parser would +// take that for the start of a file section and place every untagged row below it +// in a file the diff doesn't have. So the rows a renderer leaves untagged (dividers, +// padding) have no identity, as the protocol has it. A rendering without any record +// is a diff that describes itself, and is parsed as one. +func renderingStatesDiffLines(contents []gocui.DiffLineContent) bool { + return slices.ContainsFunc(contents, func(content gocui.DiffLineContent) bool { + return slices.ContainsFunc(content.Metadata, func(record string) bool { + _, ok := parseDiffLineMetadata(record) + return ok + }) + }) +} + // 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 @@ -313,6 +335,67 @@ func stripDiffPathPrefix(path string) string { return path } +// parseDiffLineMetadata parses the payload of an OSC 1717 record, in which a +// diff renderer states which line of which file it is rendering. The v1 payload +// is positional and ';'-delimited: +// +// version;type;new-line;old-line;file +// +// The file comes last so that it may itself contain a ';'. The old-file line is +// empty unless the line is a deletion, the only kind that needs it, and the +// new-file line is empty on a file header, the one kind that has no line. +// +// ok is false for a payload of an unknown version or shape, so that the caller +// can fall back to reading the rendered text. +func parseDiffLineMetadata(payload string) (parsedDiffLine, bool) { + fields := strings.SplitN(payload, ";", 5) + if len(fields) < 5 || fields[0] != "1" { + return parsedDiffLine{}, false + } + + lineType, ok := diffLineTypeFromMetadata(fields[1]) + if !ok { + return parsedDiffLine{}, false + } + + newLine := 0 + if fields[2] != "" { + var err error + if newLine, err = strconv.Atoi(fields[2]); err != nil { + return parsedDiffLine{}, false + } + } else if lineType != types.DiffLineFileHeader { + return parsedDiffLine{}, false + } + + oldLine := 0 + if fields[3] != "" { + var err error + if oldLine, err = strconv.Atoi(fields[3]); err != nil { + return parsedDiffLine{}, false + } + } + + return parsedDiffLine{Path: fields[4], Type: lineType, NewLine: newLine, OldLine: oldLine}, true +} + +func diffLineTypeFromMetadata(typeField string) (types.DiffLineType, bool) { + switch typeField { + case "c": + return types.DiffLineContext, true + case "a": + return types.DiffLineAdded, true + case "d": + return types.DiffLineDeleted, true + case "f": + return types.DiffLineFileHeader, true + case "h": + return types.DiffLineHunkHeader, true + default: + return types.DiffLineOther, false + } +} + // pathFromDiffGitLine extracts the new-file path from a "diff --git a/X b/X" // line, where the two paths are separated by a space and either may be quoted. // A path containing " b/" (or ` "b/`) would defeat this, but the +++/--- lines diff --git a/pkg/gui/controllers/helpers/diff_line_parser_test.go b/pkg/gui/controllers/helpers/diff_line_parser_test.go index 11f078abd..f320a6ef5 100644 --- a/pkg/gui/controllers/helpers/diff_line_parser_test.go +++ b/pkg/gui/controllers/helpers/diff_line_parser_test.go @@ -5,6 +5,7 @@ import ( "strings" "testing" + "github.com/jesseduffield/lazygit/pkg/gocui" "github.com/jesseduffield/lazygit/pkg/gui/types" "github.com/stretchr/testify/assert" ) @@ -320,3 +321,102 @@ func TestParseAllDiffLinesFromBuffer(t *testing.T) { } assert.True(t, all[6].ok) } + +func TestParseDiffLineMetadata(t *testing.T) { + scenarios := []struct { + name string + payload string + expected parsedDiffLine + expectOk bool + }{ + {"context", "1;c;1;;foo.txt", parsedDiffLine{Path: "foo.txt", Type: types.DiffLineContext, NewLine: 1}, true}, + {"added", "1;a;3;;foo.txt", parsedDiffLine{Path: "foo.txt", Type: types.DiffLineAdded, NewLine: 3}, true}, + // A deletion carries both numbers; two consecutive deletions share the + // new-file line and differ only in the old-file one. + {"first deletion", "1;d;2;2;foo.txt", parsedDiffLine{Path: "foo.txt", Type: types.DiffLineDeleted, NewLine: 2, OldLine: 2}, true}, + {"second deletion", "1;d;2;3;foo.txt", parsedDiffLine{Path: "foo.txt", Type: types.DiffLineDeleted, NewLine: 2, OldLine: 3}, true}, + // A whole-file deletion has new-file position 0 and the old path. + {"deleted file", "1;d;0;1;gone.txt", parsedDiffLine{Path: "gone.txt", Type: types.DiffLineDeleted, NewLine: 0, OldLine: 1}, true}, + // The path is the last field, so a ';' within it survives. + {"path with semicolon", "1;c;5;;weird;name.txt", parsedDiffLine{Path: "weird;name.txt", Type: types.DiffLineContext, NewLine: 5}, true}, + // A renderer may state the path absolutely; the parser keeps it verbatim + // and leaves resolving it to the caller. + {"absolute path", "1;a;7;;/abs/foo.txt", parsedDiffLine{Path: "/abs/foo.txt", Type: types.DiffLineAdded, NewLine: 7}, true}, + // A file header has no line number; a hunk header carries the new-file + // line of the hunk's first line (0 for a whole-file deletion, mirroring + // `@@ -1,N +0,0 @@`). + {"file header", "1;f;;;foo.txt", parsedDiffLine{Path: "foo.txt", Type: types.DiffLineFileHeader}, true}, + {"hunk header", "1;h;10;;foo.txt", parsedDiffLine{Path: "foo.txt", Type: types.DiffLineHunkHeader, NewLine: 10}, true}, + {"hunk header of a deleted file", "1;h;0;;gone.txt", parsedDiffLine{Path: "gone.txt", Type: types.DiffLineHunkHeader, NewLine: 0}, true}, + // A file header's line number is always empty, but a renderer that fills + // it in anyway is taken at its word rather than rejected. + {"file header with a line number", "1;f;10;;foo.txt", parsedDiffLine{Path: "foo.txt", Type: types.DiffLineFileHeader, NewLine: 10}, true}, + + {"unknown version", "2;c;1;;foo.txt", parsedDiffLine{}, false}, + {"unknown type", "1;x;1;;foo.txt", parsedDiffLine{}, false}, + {"too few fields", "1;c;1", parsedDiffLine{}, false}, + {"non-numeric new-line", "1;c;x;;foo.txt", parsedDiffLine{}, false}, + {"non-numeric old-line", "1;d;2;y;foo.txt", parsedDiffLine{}, false}, + // Only a file header may omit the new-file line; on any other kind the + // record is malformed, and rejecting it falls the row back to the diff + // text rather than acting on a line number we don't have. + {"empty new-line on a content line", "1;c;;;foo.txt", parsedDiffLine{}, false}, + {"empty new-line on a hunk header", "1;h;;;foo.txt", parsedDiffLine{}, false}, + } + + for _, s := range scenarios { + t.Run(s.name, func(t *testing.T) { + result, ok := parseDiffLineMetadata(s.payload) + assert.Equal(t, s.expectOk, ok) + if s.expectOk { + assert.Equal(t, s.expected, result) + } + }) + } +} + +func TestRenderingStatesDiffLines(t *testing.T) { + row := func(text string, records ...string) gocui.DiffLineContent { + return gocui.DiffLineContent{Text: text, Metadata: records} + } + + scenarios := []struct { + name string + contents []gocui.DiffLineContent + expected bool + }{ + { + name: "a rendering without records is read as a diff", + contents: []gocui.DiffLineContent{row("diff --git a/foo.txt b/foo.txt"), row("+one")}, + expected: false, + }, + { + name: "a record on any row makes the records the source", + contents: []gocui.DiffLineContent{row("foo.txt"), row("one", "1;c;1;;foo.txt"), row("")}, + expected: true, + }, + { + // A renderer announces the protocol with a record that names no line; a + // rendering with nothing but that one says nothing about its rows. + name: "the version-only handshake record doesn't count", + contents: []gocui.DiffLineContent{row("foo.txt", "1"), row("one")}, + expected: false, + }, + { + name: "records of a version we don't understand don't count", + contents: []gocui.DiffLineContent{row("one", "2;c;1;;foo.txt")}, + expected: false, + }, + { + name: "an empty rendering states nothing", + contents: nil, + expected: false, + }, + } + + for _, s := range scenarios { + t.Run(s.name, func(t *testing.T) { + assert.Equal(t, s.expected, renderingStatesDiffLines(s.contents)) + }) + } +} From 3af5e7940ce7c86061a24d69473f21b8ff479cf5 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 9 Aug 2026 15:24:29 +0200 Subject: [PATCH 7/7] Ask diff renderers for OSC 1717 metadata MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A renderer that speaks the protocol emits nothing unless it is asked to, so that its output stays plain wherever it is used outside lazygit. The variable names the versions we understand. git is one of the renderers we ask. It has no pager to spawn, and so no terminal to spawn one in, but it still renders the diff itself — for the word-diff formats, whose markup nothing else could resolve — so the request has to be made before we decide a pty isn't needed. Co-authored-by: Claude Opus 5 (1M context) --- pkg/gui/main_view_render.go | 10 ++++ .../tests/diff/diff_renderer_metadata.go | 50 +++++++++++++++++++ pkg/integration/tests/test_list.go | 1 + 3 files changed, 61 insertions(+) create mode 100644 pkg/integration/tests/diff/diff_renderer_metadata.go diff --git a/pkg/gui/main_view_render.go b/pkg/gui/main_view_render.go index fdaf73798..7b9b75c23 100644 --- a/pkg/gui/main_view_render.go +++ b/pkg/gui/main_view_render.go @@ -36,6 +36,16 @@ type renderSpec struct { // user has configured. The renderer lays its rendering out to the width of the // view, which only the layout settles, so the task is created after it. func (gui *Gui) newRenderTask(view *gocui.View, cmd *exec.Cmd, prefix string) error { + // Ask whatever renders the diff to state, in an OSC 1717 record per line, + // which line of which file it is rendering. This lets us act on the line the + // user is pointing at even when the rendering no longer looks like a diff. + // The variable names the protocol versions we understand, and a renderer + // that doesn't understand it ignores it, so we can set it always. It has to + // be set before the plain path below, since on that path git renders the + // diff itself, and git speaks the protocol too, for its word-diff formats, + // whose markup we could not otherwise resolve. + cmd.Env = append(cmd.Env, "OSC1717=V1") + if gui.stateAccessor.GetDiffRendererConfigManager().GetDiffRendererType() == config.DiffRendererType_RawGit { // If we're not using a custom diff renderer, then we don't need to use a pty return gui.newCmdTask(view, cmd, prefix) diff --git a/pkg/integration/tests/diff/diff_renderer_metadata.go b/pkg/integration/tests/diff/diff_renderer_metadata.go new file mode 100644 index 000000000..ff211717d --- /dev/null +++ b/pkg/integration/tests/diff/diff_renderer_metadata.go @@ -0,0 +1,50 @@ +package diff + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var DiffRendererMetadata = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "A diff renderer is told that we understand the OSC 1717 metadata protocol, and the records it emits don't show up in the rendered diff", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(cfg *config.AppConfig) { + // A fake conforming renderer: it announces the protocol with a + // version-only record, reports the protocol versions it was offered, and + // then passes the diff through with a per-line record in front of every + // line. + cfg.GetUserConfig().Git.DiffRenderers = []config.DiffRendererConfig{ + {Command: `printf '\033]1717;1\007'; ` + + `printf 'OFFERED:%s\n' "$OSC1717"; ` + + `while IFS= read -r line; do printf '\033]1717;1;c;1;;file1\007%s\n' "$line"; done`}, + } + }, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("file1", "one\ntwo\n") + shell.Commit("one") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("one").IsSelected(), + ) + + t.Views().Main(). + // The renderer was offered the protocol version we understand. + Content(Contains("OFFERED:V1")). + // Its records are escape sequences, so none of them reaches the + // screen; the diff reads exactly as the renderer wrote it. + ContainsLines( + Equals("diff --git a/file1 b/file1"), + Contains("new file mode"), + Contains("index "), + Equals("--- /dev/null"), + Equals("+++ b/file1"), + Equals("@@ -0,0 +1,2 @@"), + Equals("+one"), + Equals("+two"), + ) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 67caf3bbb..d699d8cb6 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -228,6 +228,7 @@ var tests = []*components.IntegrationTest{ diff.DiffAndApplyPatch, diff.DiffCommits, diff.DiffNonStickyRange, + diff.DiffRendererMetadata, diff.IgnoreWhitespace, diff.RenameSimilarityThresholdChange, diff.RenderThroughAPipe,