diff --git a/pkg/gui/controllers/helpers/diff_line_parser.go b/pkg/gui/controllers/helpers/diff_line_parser.go index c9abcf5c2..d58633f7e 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" + "strconv" "strings" "github.com/jesseduffield/lazygit/pkg/commands/patch" @@ -249,9 +250,9 @@ func pathFromDiffHeader(fileLines []string) string { } switch { case strings.HasPrefix(line, "+++ "): - newPath = stripDiffPathPrefix(strings.TrimPrefix(line, "+++ ")) + newPath = pathFromDiffHeaderField(strings.TrimPrefix(line, "+++ ")) case strings.HasPrefix(line, "--- "): - oldPath = stripDiffPathPrefix(strings.TrimPrefix(line, "--- ")) + oldPath = pathFromDiffHeaderField(strings.TrimPrefix(line, "--- ")) } } @@ -264,6 +265,33 @@ func pathFromDiffHeader(fileLines []string) string { return pathFromDiffGitLine(fileLines[0]) } +// pathFromDiffHeaderField decodes one path field of a diff header — the part +// after "--- " or "+++ ", or one of the two paths on the "diff --git" line — +// into the repo-relative path it names. +// +// git spells such a field in three ways: plain; terminated by a tab, when the +// path contains a space; or C-quoted as a whole, when the path contains +// characters git won't print raw — which, with core.quotePath enabled (the +// default), includes every non-ASCII byte, so `café` arrives as +// `"b/caf\303\251"`. The quoting is Go's string syntax, octal escapes included, +// so strconv decodes it for us. +// +// 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. +func pathFromDiffHeaderField(field string) string { + field = strings.TrimSuffix(field, "\t") + + if strings.HasPrefix(field, `"`) { + unquoted, err := strconv.Unquote(field) + if err != nil { + return "" + } + field = unquoted + } + + return stripDiffPathPrefix(field) +} + // 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. @@ -275,12 +303,16 @@ func stripDiffPathPrefix(path string) string { } // pathFromDiffGitLine extracts the new-file path from a "diff --git a/X b/X" -// line. A path containing " b/" would defeat this, but the +++/--- lines are -// unambiguous and we only get here when they are absent. +// 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. 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 rest[idx+len(" b/"):] + return pathFromDiffHeaderField(rest[idx+1:]) } return "" } diff --git a/pkg/gui/controllers/helpers/diff_line_parser_test.go b/pkg/gui/controllers/helpers/diff_line_parser_test.go index 8e8bec91a..ae620d23c 100644 --- a/pkg/gui/controllers/helpers/diff_line_parser_test.go +++ b/pkg/gui/controllers/helpers/diff_line_parser_test.go @@ -240,6 +240,62 @@ func TestParseDiffLineFromBufferReadInPart(t *testing.T) { assert.False(t, ok) } +func TestPathFromDiffHeaderField(t *testing.T) { + scenarios := []struct { + name string + field string + expected string + }{ + {"new side", "b/file.go", "file.go"}, + {"old side", "a/file.go", "file.go"}, + {"a missing file", "/dev/null", "/dev/null"}, + // 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 + // escaped, and the field is quoted as a whole, prefix included. + {"non-ASCII path", `"b/caf\303\251.go"`, "café.go"}, + {"non-ASCII path with a space", "\"b/caf\\303\\251 x.go\"\t", "café x.go"}, + {"path with a double quote", `"b/we\"ird.go"`, `we"ird.go`}, + {"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`, ""}, + } + + for _, s := range scenarios { + t.Run(s.name, func(t *testing.T) { + assert.Equal(t, s.expected, pathFromDiffHeaderField(s.field)) + }) + } +} + +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. + renamed := []string{ + `diff --git "a/caf\303\251 old.go" "b/caf\303\251 new.go"`, + "similarity index 62%", + `rename from "caf\303\251 old.go"`, + `rename to "caf\303\251 new.go"`, + "index 1111111..2222222 100644", + "--- \"a/caf\\303\\251 old.go\"\t", + "+++ \"b/caf\\303\\251 new.go\"\t", + "@@ -1,2 +1,2 @@", + " apple", + "-grape", + "+kiwi", + } + + result, ok := parseDiffLineFromBuffer(renamed, 10) + assert.True(t, ok) + assert.Equal(t, parsedDiffLine{RelPath: "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) +} + func TestParseAllDiffLinesFromBuffer(t *testing.T) { // Some decoration above the diff, which belongs to no file section: a commit // message and a diffstat, as `git show` renders them.