From e6bfb2c7204efe8c5ecdabb9838c36af64dd25b5 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 7 Jun 2026 17:44:16 +0200 Subject: [PATCH] Use pager-emitted OSC diff metadata as the highest-fidelity line-info backend Add mechanism #2 to GetDiffLineInfo: when a patched pager annotated each diff line with OSC 456 metadata, read it back as the first backend, ahead of the buffer parser (#1) and the lazygit-edit hyperlink. It is strictly higher fidelity -- it carries the side explicitly, so it serves the renderings #1 cannot parse (delta's default mode, --line-numbers, diff-so-fancy) and conveys deletions, which the hyperlink can't. The host advertises the protocol versions it understands by setting EMIT_OSC456_METADATA on the pager subprocess; a pager that doesn't understand it ignores the variable, so this is safe to set unconditionally. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../controllers/helpers/diff_line_parser.go | 53 +++++++++++++++++-- .../helpers/diff_line_parser_test.go | 41 ++++++++++++++ pkg/gui/controllers/helpers/staging_helper.go | 48 ++++++++++++++--- pkg/gui/pty.go | 7 +++ 4 files changed, 139 insertions(+), 10 deletions(-) diff --git a/pkg/gui/controllers/helpers/diff_line_parser.go b/pkg/gui/controllers/helpers/diff_line_parser.go index c2af555c9..7695edf0d 100644 --- a/pkg/gui/controllers/helpers/diff_line_parser.go +++ b/pkg/gui/controllers/helpers/diff_line_parser.go @@ -1,6 +1,7 @@ package helpers import ( + "strconv" "strings" "github.com/jesseduffield/lazygit/pkg/commands/patch" @@ -11,9 +12,11 @@ import ( // unified diff. const diffFilePrefix = "diff --git " -// parsedDiffLine is what parseDiffLineFromBuffer recovers about a rendered diff -// row. RelPath is repo-relative (as the diff header spells it); the caller turns -// it into the absolute path of types.DiffLineInfo. +// parsedDiffLine is what a backend recovers about a rendered diff row. RelPath +// is the path as the source spells it — repo-relative from the diff header for +// the buffer parser (#1), or whatever the pager emitted (possibly absolute) for +// the OSC metadata (#2); the caller turns it into the absolute path of +// types.DiffLineInfo. type parsedDiffLine struct { RelPath string Type types.DiffLineType @@ -152,3 +155,47 @@ func pathFromDiffGitLine(line string) string { } return "" } + +// parseDiffLineMetadata parses mechanism #2's OSC 456 payload (v1): +// version;type;new-line;old-line;file — positional and ';'-delimited, with the +// file last (so it may itself contain ';') and old-line empty unless the line is +// a deletion. See diff-line-metadata-notes.md §9.2. ok is false for a payload of +// an unknown version or shape, so the caller can fall back to another backend. +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, err := strconv.Atoi(fields[2]) + if err != nil { + return parsedDiffLine{}, false + } + + oldLine := 0 + if fields[3] != "" { + if oldLine, err = strconv.Atoi(fields[3]); err != nil { + return parsedDiffLine{}, false + } + } + + return parsedDiffLine{RelPath: 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 + default: + return types.DiffLineOther, false + } +} diff --git a/pkg/gui/controllers/helpers/diff_line_parser_test.go b/pkg/gui/controllers/helpers/diff_line_parser_test.go index b59b3ead7..d749a94fd 100644 --- a/pkg/gui/controllers/helpers/diff_line_parser_test.go +++ b/pkg/gui/controllers/helpers/diff_line_parser_test.go @@ -90,3 +90,44 @@ index 1111111..2222222 100644 _, ok := parseDiffLineFromBuffer(mangled, 6) assert.False(t, ok) } + +func TestParseDiffLineMetadata(t *testing.T) { + scenarios := []struct { + name string + payload string + expected parsedDiffLine + expectOk bool + }{ + // These payloads are exactly what the patched delta emits (verified + // against the real binary; see diff-line-metadata-notes.md §9). + {"context", "1;c;1;;foo.txt", parsedDiffLine{RelPath: "foo.txt", Type: types.DiffLineContext, NewLine: 1}, true}, + {"added", "1;a;3;;foo.txt", parsedDiffLine{RelPath: "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 line. + {"first deletion", "1;d;2;2;foo.txt", parsedDiffLine{RelPath: "foo.txt", Type: types.DiffLineDeleted, NewLine: 2, OldLine: 2}, true}, + {"second deletion", "1;d;2;3;foo.txt", parsedDiffLine{RelPath: "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{RelPath: "gone.txt", Type: types.DiffLineDeleted, NewLine: 0, OldLine: 1}, true}, + // The path is the last field, so a ';' within it is preserved. + {"path with semicolon", "1;c;5;;weird;name.txt", parsedDiffLine{RelPath: "weird;name.txt", Type: types.DiffLineContext, NewLine: 5}, true}, + // A pager may emit an absolute path; the parser keeps it verbatim (the + // caller decides whether to join the worktree path). + {"absolute path", "1;a;7;;/abs/foo.txt", parsedDiffLine{RelPath: "/abs/foo.txt", Type: types.DiffLineAdded, NewLine: 7}, 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}, + } + + 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) + } + }) + } +} diff --git a/pkg/gui/controllers/helpers/staging_helper.go b/pkg/gui/controllers/helpers/staging_helper.go index 9be563056..5b31dcc8d 100644 --- a/pkg/gui/controllers/helpers/staging_helper.go +++ b/pkg/gui/controllers/helpers/staging_helper.go @@ -146,25 +146,59 @@ func (self *StagingHelper) mainStagingFocused() bool { // patch explorer consumers go through to act on the line the user is pointing // at, and the strategy behind it is swappable (see diff-line-metadata-notes.md). // -// Today it first parses the decolorized view buffer (mechanism #1), which serves -// the structure-preserving renderings (no pager, git diff --color, -// delta --color-only, diff-so-fancy --patch). When that fails — e.g. a pager -// restructured the diff, as delta's default mode does — it falls back to delta's -// lazygit-edit:// hyperlinks. The hyperlink can't convey the side, so its result -// is reported as a non-deletion content line. A future backend reading #2's -// per-cell OSC metadata would slot in ahead of these, behind the same shape. +// It tries three backends in order of fidelity. First, mechanism #2: per-line +// OSC metadata emitted by a patched pager (delta), which carries the side +// directly and so serves the renderings #1 can't parse — delta's default mode, +// --line-numbers, diff-so-fancy. Failing that, mechanism #1: parsing the +// decolorized view buffer, which serves the structure-preserving renderings (no +// pager, git diff --color, delta --color-only, diff-so-fancy --patch). Failing +// that, delta's lazygit-edit:// hyperlinks; the hyperlink can't convey the side, +// so its result is reported as a non-deletion content line. func (self *StagingHelper) GetDiffLineInfo(windowName string, viewLineIdx int) (types.DiffLineInfo, bool) { v, _ := self.c.GocuiGui().View(self.windowHelper.GetViewNameForWindow(windowName)) if v == nil { return types.DiffLineInfo{}, false } + if info, ok := self.diffLineInfoFromMetadata(v, viewLineIdx); ok { + return info, true + } if info, ok := self.diffLineInfoFromBuffer(v, viewLineIdx); ok { return info, true } return self.diffLineInfoFromHyperlink(v, viewLineIdx) } +// diffLineInfoFromMetadata reads mechanism #2's per-line OSC metadata. The +// payload is positional and ';'-delimited — version;type;new-line;old-line;file +// — with the file last so it may itself contain ';', and old-line empty unless +// the line is a deletion. See diff-line-metadata-notes.md §9.2. +func (self *StagingHelper) diffLineInfoFromMetadata(v *gocui.View, viewLineIdx int) (types.DiffLineInfo, bool) { + payload, ok := v.DiffLineMetadataInLine(viewLineIdx) + if !ok { + return types.DiffLineInfo{}, false + } + + parsed, ok := parseDiffLineMetadata(payload) + if !ok { + return types.DiffLineInfo{}, false + } + + // The pager may emit an absolute or a repo-relative path (whichever is + // convenient for it); normalize to the absolute path the consumers expect. + path := parsed.RelPath + if !filepath.IsAbs(path) { + path = filepath.Join(self.c.Git().RepoPaths.WorktreePath(), path) + } + + return types.DiffLineInfo{ + Path: path, + Type: parsed.Type, + NewLine: parsed.NewLine, + OldLine: parsed.OldLine, + }, true +} + func (self *StagingHelper) diffLineInfoFromBuffer(v *gocui.View, viewLineIdx int) (types.DiffLineInfo, bool) { bufferLineIdx, ok := v.BufferLineForViewLine(viewLineIdx) if !ok { diff --git a/pkg/gui/pty.go b/pkg/gui/pty.go index 87cf270dd..f6b471eb7 100644 --- a/pkg/gui/pty.go +++ b/pkg/gui/pty.go @@ -101,6 +101,13 @@ func (gui *Gui) newPtyTask(view *gocui.View, cmd *exec.Cmd, prefix string) error cmd.Env = append(cmd.Env, "GIT_PAGER="+pager) + // Advertise to a metadata-aware pager (e.g. a patched delta) the diff-line + // metadata protocol versions we understand, so it annotates each line with + // an OSC sequence we can read back (see diff-line-metadata-notes.md). A + // pager that doesn't understand it ignores the variable, so this is safe to + // set unconditionally. + cmd.Env = append(cmd.Env, "EMIT_OSC456_METADATA=V1") + manager := gui.getManager(view) // Size the pty from the view's dimensions here, on the UI thread; the