From 243ede84d729acc4f8978887ac0f906f6890db1c Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 7 Oct 2026 13:57:25 +0200 Subject: [PATCH] Fix staging lines in the main view when diff.mnemonicPrefix is set If diff.mnemonicPrefix is set, pressing space on a line in the focused main view does nothing. The command log shows "Apply patch", but the line is not staged. Other commands that read a path out of the working tree's diff go wrong as well, such as jumping to a file. Building a custom patch from a commit's diff still works, because git uses mnemonic prefixes only when one side of the diff is the index or the working tree. With this config, the paths in the working tree's diff start with i/ and w/ instead of a/ and b/. We only strip a/ and b/, so the path we read out of the diff header is not a file of the files panel, and we skip the line without a message. Users who set the config want to see these prefixes, so keep them. Strip the first component of the path instead, whatever it is. `git apply` does the same by default, and ParseableDiffPrefixes makes sure there is such a component. A section without +++ and --- lines needs more than that, because we find the new path on its "diff --git" line by searching for " b/". Take the path from the "rename to" or "copy to" line instead where there is one. Otherwise the two paths on the "diff --git" line are the same. Split the line where the two halves match after stripping their prefixes, like `git apply` does. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../git_commands/git_command_builder.go | 3 + .../controllers/helpers/diff_line_parser.go | 64 ++++++++++++------ .../helpers/diff_line_parser_test.go | 66 +++++++++++++++++++ ...stage_diff_lines_with_mnemonic_prefixes.go | 3 - 4 files changed, 112 insertions(+), 24 deletions(-) diff --git a/pkg/commands/git_commands/git_command_builder.go b/pkg/commands/git_commands/git_command_builder.go index cc01784a3..fbc204f7a 100644 --- a/pkg/commands/git_commands/git_command_builder.go +++ b/pkg/commands/git_commands/git_command_builder.go @@ -151,6 +151,9 @@ func (self *GitCommandBuilder) NoLineEndingConversion() *GitCommandBuilder { // // For this we override three configs of the user. diff.noprefix leaves the // prefix out, and diff.srcPrefix and diff.dstPrefix can set it to any string. +// We leave diff.mnemonicPrefix alone. The prefixes it chooses, such as i/ and +// w/, are a single path component like a/ and b/, so we can strip them too, and +// the diffs we show keep the prefixes the user asked for. func (self *GitCommandBuilder) ParseableDiffPrefixes() *GitCommandBuilder { return self. Config("diff.noprefix=false"). diff --git a/pkg/gui/controllers/helpers/diff_line_parser.go b/pkg/gui/controllers/helpers/diff_line_parser.go index 73206a4ee..39d99c9ec 100644 --- a/pkg/gui/controllers/helpers/diff_line_parser.go +++ b/pkg/gui/controllers/helpers/diff_line_parser.go @@ -308,15 +308,17 @@ func diffLineTypeForKind(kind patch.PatchLineKind) types.DiffLineType { // pathFromDiffHeader extracts the new-file path of a single diff section. A // submodule's section states its path in the line it opens with. For a file of the -// repo the path comes from the "+++ b/" line, falling back to "--- a/" -// when the new path is /dev/null (a deleted file), and to the "diff --git" line when -// there are no such lines at all (a pure rename, which has no hunks). +// repo the path comes from the "+++ " line, falling back to the "--- " line when the +// new path is /dev/null (a deleted file). A section without these lines has no hunks: +// it is a rename or copy without a content change, a change of the file mode, or a +// binary file. The path then comes from the "rename to" or "copy to" line, or failing +// that from the "diff --git" line. func pathFromDiffHeader(fileLines []string) string { if path := submodulePath(fileLines[0]); path != "" { return path } - var oldPath, newPath string + var oldPath, newPath, copiedOrRenamedPath string for _, line := range fileLines { if strings.HasPrefix(line, "@@") { break // past the header @@ -326,6 +328,10 @@ func pathFromDiffHeader(fileLines []string) string { newPath = pathFromDiffHeaderField(strings.TrimPrefix(line, "+++ ")) case strings.HasPrefix(line, "--- "): oldPath = pathFromDiffHeaderField(strings.TrimPrefix(line, "--- ")) + case strings.HasPrefix(line, "rename to "): + copiedOrRenamedPath, _ = unquoteDiffPath(strings.TrimPrefix(line, "rename to ")) + case strings.HasPrefix(line, "copy to "): + copiedOrRenamedPath, _ = unquoteDiffPath(strings.TrimPrefix(line, "copy to ")) } } @@ -335,6 +341,9 @@ func pathFromDiffHeader(fileLines []string) string { if oldPath != "" && oldPath != "/dev/null" { return oldPath } + if copiedOrRenamedPath != "" { + return copiedOrRenamedPath + } return pathFromDiffGitLine(fileLines[0]) } @@ -342,13 +351,16 @@ func pathFromDiffHeader(fileLines []string) string { // after "--- " or "+++ ", or one of the two paths on the "diff --git" line — // into the repo-relative path it names. // -// Returns "" for a quoted field we can't decode: better to resolve nothing than -// to point a consumer at a path that doesn't exist. +// Returns "" for a field we can't decode: better to resolve nothing than to point +// a consumer at a path that doesn't exist. func pathFromDiffHeaderField(field string) string { path, ok := unquoteDiffPath(field) if !ok { return "" } + if path == "/dev/null" { + return path + } return stripDiffPathPrefix(path) } @@ -374,14 +386,20 @@ func unquoteDiffPath(field string) (path string, ok bool) { return field, true } -// stripDiffPathPrefix removes the a/ or b/ prefix git puts on the paths in a -// diff header. We ask git for these prefixes explicitly (diff.noprefix=false), -// so they are always there. +// stripDiffPathPrefix removes the prefix git puts in front of the paths in a diff +// header. By default this is a/ or b/, but with diff.mnemonicPrefix git uses a +// letter that names what is compared, such as i/ for the index and w/ for the +// working tree. Either way the prefix is the first component of the path, and we +// ask git to always put one there (ParseableDiffPrefixes). `git apply` strips the +// same component by default. +// +// Returns "" for a path without a slash, which has no prefix to strip. func stripDiffPathPrefix(path string) string { - if strings.HasPrefix(path, "a/") || strings.HasPrefix(path, "b/") { - return path[2:] + _, rest, found := strings.Cut(path, "/") + if !found { + return "" } - return path + return rest } // parseDiffLineMetadata parses the payload of an OSC 1717 record, in which a @@ -445,17 +463,21 @@ func diffLineTypeFromMetadata(typeField string) (types.DiffLineType, bool) { } } -// 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 -// are unambiguous and we only get here when they are absent. +// pathFromDiffGitLine extracts the path from a "diff --git" line of a file whose +// path is the same on both sides. The two paths are separated by a space, but a +// path may contain spaces too. So, like `git apply`, we split the line at the space +// where the two halves name the same path once their prefixes are stripped. Either +// half may be quoted. func pathFromDiffGitLine(line string) string { rest := strings.TrimPrefix(line, diffFilePrefix) - if idx := strings.LastIndex(rest, ` "b/`); idx != -1 { - return pathFromDiffHeaderField(rest[idx+1:]) - } - if idx := strings.LastIndex(rest, " b/"); idx != -1 { - return pathFromDiffHeaderField(rest[idx+1:]) + for i, c := range rest { + if c != ' ' { + continue + } + path := pathFromDiffHeaderField(rest[i+1:]) + if path != "" && path == pathFromDiffHeaderField(rest[:i]) { + return path + } } return "" } diff --git a/pkg/gui/controllers/helpers/diff_line_parser_test.go b/pkg/gui/controllers/helpers/diff_line_parser_test.go index 5c5c41297..f08618b74 100644 --- a/pkg/gui/controllers/helpers/diff_line_parser_test.go +++ b/pkg/gui/controllers/helpers/diff_line_parser_test.go @@ -250,6 +250,11 @@ func TestPathFromDiffHeaderField(t *testing.T) { {"new side", "b/file.go", "file.go"}, {"old side", "a/file.go", "file.go"}, {"a missing file", "/dev/null", "/dev/null"}, + {"file in a directory", "b/dir/file.go", "dir/file.go"}, + // diff.mnemonicPrefix makes git name the two sides by what they are. + {"index side", "i/file.go", "file.go"}, + {"working tree side", "w/file.go", "file.go"}, + {"no prefix", "file.go", ""}, // git terminates the field with a tab when the path has a space in it. {"path with a space", "b/with space.go\t", "with space.go"}, // With core.quotePath enabled (the default) every non-ASCII byte is @@ -260,6 +265,7 @@ func TestPathFromDiffHeaderField(t *testing.T) { {"path with a backslash", `"b/back\\slash.go"`, `back\slash.go`}, {"path with a tab", `"b/tab\there.go"`, "tab\there.go"}, {"undecodable", `"b/unterminated`, ""}, + {"quoted with a mnemonic prefix", `"w/caf\303\251.go"`, "café.go"}, } for _, s := range scenarios { @@ -269,6 +275,66 @@ func TestPathFromDiffHeaderField(t *testing.T) { } } +func TestPathFromDiffGitLine(t *testing.T) { + scenarios := []struct { + name string + line string + expected string + }{ + {"default prefixes", "diff --git a/file.go b/file.go", "file.go"}, + {"mnemonic prefixes", "diff --git i/file.go w/file.go", "file.go"}, + {"path with a space", "diff --git i/my file.go w/my file.go", "my file.go"}, + {"path that looks like it holds the other path", "diff --git i/x w/y w/x w/y", "x w/y"}, + {"quoted paths", `diff --git "i/caf\303\251.go" "w/caf\303\251.go"`, "café.go"}, + // A rename; its path comes from the "rename to" line. + {"different paths", "diff --git c/old.go i/new.go", ""}, + } + + for _, s := range scenarios { + t.Run(s.name, func(t *testing.T) { + assert.Equal(t, s.expected, pathFromDiffGitLine(s.line)) + }) + } +} + +func TestParseDiffLineFromBufferMnemonicPrefixes(t *testing.T) { + // With diff.mnemonicPrefix, git names the two sides of the working tree's diff + // i/ and w/, and those of the staged diff c/ and i/. + modified := strings.Split(`diff --git i/file.go w/file.go +index 1111111..2222222 100644 +--- i/file.go ++++ w/file.go +@@ -1,2 +1,2 @@ + apple +-grape ++kiwi`, "\n") + + result, ok := parseDiffLineFromBuffer(modified, 7) + assert.True(t, ok) + assert.Equal(t, parsedDiffLine{Path: "file.go", Type: types.DiffLineAdded, NewLine: 2}, result) + + // A change of the file mode has no +++/--- lines, so the path comes from the + // "diff --git" line. + modeChange := strings.Split(`diff --git i/run.sh w/run.sh +old mode 100644 +new mode 100755`, "\n") + + result, ok = parseDiffLineFromBuffer(modeChange, 2) + assert.True(t, ok) + assert.Equal(t, parsedDiffLine{Path: "run.sh", Type: types.DiffLineFileHeader, NewLine: 1}, result) + + // Neither does a copy without a content change, and the two paths on its + // "diff --git" line differ, so the path comes from the "copy to" line. + copied := strings.Split(`diff --git c/run.sh i/copy of run.sh +similarity index 100% +copy from run.sh +copy to copy of run.sh`, "\n") + + result, ok = parseDiffLineFromBuffer(copied, 2) + assert.True(t, ok) + assert.Equal(t, parsedDiffLine{Path: "copy of run.sh", Type: types.DiffLineFileHeader, NewLine: 1}, result) +} + func TestParseDiffLineFromBufferQuotedPath(t *testing.T) { // A rename of a file whose name needs quoting, with a content change: the // path is quoted on the "diff --git" line and on both of the +++/--- lines. diff --git a/pkg/integration/tests/main_view/stage_diff_lines_with_mnemonic_prefixes.go b/pkg/integration/tests/main_view/stage_diff_lines_with_mnemonic_prefixes.go index c74cbe614..25675b7f8 100644 --- a/pkg/integration/tests/main_view/stage_diff_lines_with_mnemonic_prefixes.go +++ b/pkg/integration/tests/main_view/stage_diff_lines_with_mnemonic_prefixes.go @@ -42,10 +42,7 @@ var StageDiffLinesWithMnemonicPrefixes = NewIntegrationTest(NewIntegrationTestAr PressPrimaryAction() t.Views().Files().Lines( - /* EXPECTED: Contains("MM file1"), - ACTUAL: */ - Contains(" M file1"), ) }, })