From 91aef76471cd99e1b020b2739291b12d3015c55a Mon Sep 17 00:00:00 2001 From: Brian McGee Date: Sat, 23 Nov 2024 14:54:13 +0000 Subject: [PATCH] fix: non-ascii paths in git walker `git ls-files` was providing some `"..."` quoted paths when it encountered emojis in the file/dir name. Running them through `strconv.Unquote` fixes things. Fixes #485 Signed-off-by: Brian McGee --- cmd/root_test.go | 190 +++++++++--------- .../README.md" | 1 + walk/filesystem_test.go | 5 +- walk/git.go | 23 ++- walk/git_test.go | 4 +- 5 files changed, 123 insertions(+), 100 deletions(-) create mode 100644 "test/examples/emoji \360\237\225\260\357\270\217/README.md" diff --git a/cmd/root_test.go b/cmd/root_test.go index 78f32b51..85beb701 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -227,7 +227,7 @@ func TestSpecifyingFormatters(t *testing.T) { withNoError(t), withModtimeBump(tempDir, time.Second), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, + stats.Traversed: 33, stats.Matched: 3, stats.Formatted: 3, stats.Changed: 3, @@ -241,7 +241,7 @@ func TestSpecifyingFormatters(t *testing.T) { withModtimeBump(tempDir, time.Second), withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, + stats.Traversed: 33, stats.Matched: 2, stats.Formatted: 2, stats.Changed: 2, @@ -253,7 +253,7 @@ func TestSpecifyingFormatters(t *testing.T) { withModtimeBump(tempDir, time.Second), withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, + stats.Traversed: 33, stats.Matched: 2, stats.Formatted: 2, stats.Changed: 2, @@ -265,7 +265,7 @@ func TestSpecifyingFormatters(t *testing.T) { withModtimeBump(tempDir, time.Second), withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, + stats.Traversed: 33, stats.Matched: 1, stats.Formatted: 1, stats.Changed: 1, @@ -288,7 +288,7 @@ func TestSpecifyingFormatters(t *testing.T) { withNoError(t), withModtimeBump(tempDir, time.Second), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, + stats.Traversed: 33, stats.Matched: 2, stats.Formatted: 2, stats.Changed: 2, @@ -325,9 +325,9 @@ func TestIncludesAndExcludes(t *testing.T) { withConfig(configPath, cfg), withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, - stats.Matched: 32, - stats.Formatted: 32, + stats.Traversed: 33, + stats.Matched: 33, + stats.Formatted: 33, stats.Changed: 0, }), ) @@ -340,9 +340,9 @@ func TestIncludesAndExcludes(t *testing.T) { withConfig(configPath, cfg), withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, - stats.Matched: 31, - stats.Formatted: 31, + stats.Traversed: 33, + stats.Matched: 32, + stats.Formatted: 32, stats.Changed: 0, }), ) @@ -355,9 +355,9 @@ func TestIncludesAndExcludes(t *testing.T) { withConfig(configPath, cfg), withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, - stats.Matched: 25, - stats.Formatted: 25, + stats.Traversed: 33, + stats.Matched: 26, + stats.Formatted: 26, stats.Changed: 0, }), ) @@ -372,9 +372,9 @@ func TestIncludesAndExcludes(t *testing.T) { withConfig(configPath, cfg), withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, - stats.Matched: 23, - stats.Formatted: 23, + stats.Traversed: 33, + stats.Matched: 24, + stats.Formatted: 24, stats.Changed: 0, }), ) @@ -387,9 +387,9 @@ func TestIncludesAndExcludes(t *testing.T) { withConfig(configPath, cfg), withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, - stats.Matched: 22, - stats.Formatted: 22, + stats.Traversed: 33, + stats.Matched: 23, + stats.Formatted: 23, stats.Changed: 0, }), ) @@ -404,7 +404,7 @@ func TestIncludesAndExcludes(t *testing.T) { withConfig(configPath, cfg), withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, + stats.Traversed: 33, stats.Matched: 1, stats.Formatted: 1, stats.Changed: 0, @@ -419,7 +419,7 @@ func TestIncludesAndExcludes(t *testing.T) { withConfig(configPath, cfg), withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, + stats.Traversed: 33, stats.Matched: 2, stats.Formatted: 2, stats.Changed: 0, @@ -461,9 +461,9 @@ func TestConfigFile(t *testing.T) { withArgs("--config-file", configPath, "--tree-root", tempDir), withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, - stats.Matched: 32, - stats.Formatted: 32, + stats.Traversed: 33, + stats.Matched: 33, + stats.Formatted: 33, stats.Changed: 0, }), ) @@ -492,8 +492,8 @@ func TestConfigFile(t *testing.T) { }), withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, - stats.Matched: 32, + stats.Traversed: 33, + stats.Matched: 33, stats.Formatted: 0, stats.Changed: 0, }), @@ -541,10 +541,10 @@ func TestCache(t *testing.T) { treefmt(t, withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, - stats.Matched: 32, - stats.Formatted: 32, - stats.Changed: 32, + stats.Traversed: 33, + stats.Matched: 33, + stats.Formatted: 33, + stats.Changed: 33, }), ) @@ -552,8 +552,8 @@ func TestCache(t *testing.T) { treefmt(t, withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, - stats.Matched: 32, + stats.Traversed: 33, + stats.Matched: 33, stats.Formatted: 0, stats.Changed: 0, }), @@ -564,10 +564,10 @@ func TestCache(t *testing.T) { withArgs("-c"), withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, - stats.Matched: 32, - stats.Formatted: 32, - stats.Changed: 32, + stats.Traversed: 33, + stats.Matched: 33, + stats.Formatted: 33, + stats.Changed: 33, }), ) @@ -575,8 +575,8 @@ func TestCache(t *testing.T) { treefmt(t, withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, - stats.Matched: 32, + stats.Traversed: 33, + stats.Matched: 33, stats.Formatted: 0, stats.Changed: 0, }), @@ -587,10 +587,10 @@ func TestCache(t *testing.T) { withNoError(t), withModtimeBump(tempDir, time.Second), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, - stats.Matched: 32, - stats.Formatted: 32, - stats.Changed: 32, + stats.Traversed: 33, + stats.Matched: 33, + stats.Formatted: 33, + stats.Changed: 33, }), ) @@ -599,10 +599,10 @@ func TestCache(t *testing.T) { withArgs("--no-cache"), withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, - stats.Matched: 32, - stats.Formatted: 32, - stats.Changed: 32, + stats.Traversed: 33, + stats.Matched: 33, + stats.Formatted: 33, + stats.Changed: 33, }), ) @@ -628,7 +628,7 @@ func TestCache(t *testing.T) { as.ErrorIs(err, format.ErrFormattingFailures) }), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, + stats.Traversed: 33, stats.Matched: 6, stats.Formatted: 0, stats.Changed: 0, @@ -641,7 +641,7 @@ func TestCache(t *testing.T) { as.ErrorIs(err, format.ErrFormattingFailures) }), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, + stats.Traversed: 33, stats.Matched: 6, stats.Formatted: 0, stats.Changed: 0, @@ -661,7 +661,7 @@ func TestCache(t *testing.T) { treefmt(t, withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, + stats.Traversed: 33, stats.Matched: 6, stats.Formatted: 6, stats.Changed: 6, @@ -712,7 +712,7 @@ func TestChangeWorkingDirectory(t *testing.T) { withConfig(configPath, cfg), withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, + stats.Traversed: 33, }), ) }) @@ -748,7 +748,7 @@ func TestChangeWorkingDirectory(t *testing.T) { withConfig(configPath, cfg), withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, + stats.Traversed: 33, }), ) }) @@ -800,7 +800,7 @@ func TestFailOnChange(t *testing.T) { as.ErrorIs(err, formatCmd.ErrFailOnChange) }), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, + stats.Traversed: 33, stats.Matched: 2, stats.Formatted: 2, stats.Changed: 2, @@ -813,7 +813,7 @@ func TestFailOnChange(t *testing.T) { withArgs("--fail-on-change"), withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, + stats.Traversed: 33, stats.Matched: 2, stats.Formatted: 0, stats.Changed: 0, @@ -861,7 +861,7 @@ func TestFailOnChange(t *testing.T) { as.ErrorIs(err, formatCmd.ErrFailOnChange) }), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, + stats.Traversed: 33, stats.Matched: 7, stats.Formatted: 7, stats.Changed: 7, @@ -874,7 +874,7 @@ func TestFailOnChange(t *testing.T) { withArgs("--fail-on-change"), withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, + stats.Traversed: 33, stats.Matched: 7, stats.Formatted: 0, stats.Changed: 0, @@ -912,7 +912,7 @@ func TestCacheBusting(t *testing.T) { withConfig(configPath, cfg), withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, + stats.Traversed: 33, stats.Matched: 8, stats.Formatted: 8, stats.Changed: 6, @@ -926,7 +926,7 @@ func TestCacheBusting(t *testing.T) { withConfig(configPath, cfg), withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, + stats.Traversed: 33, stats.Matched: 8, stats.Formatted: 6, stats.Changed: 6, @@ -937,7 +937,7 @@ func TestCacheBusting(t *testing.T) { withConfig(configPath, cfg), withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, + stats.Traversed: 33, stats.Matched: 8, stats.Formatted: 0, stats.Changed: 0, @@ -951,7 +951,7 @@ func TestCacheBusting(t *testing.T) { withConfig(configPath, cfg), withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, + stats.Traversed: 33, stats.Matched: 8, stats.Formatted: 6, stats.Changed: 0, // echo doesn't affect the files so no changes expected @@ -962,7 +962,7 @@ func TestCacheBusting(t *testing.T) { withConfig(configPath, cfg), withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, + stats.Traversed: 33, stats.Matched: 8, stats.Formatted: 0, stats.Changed: 0, @@ -977,7 +977,7 @@ func TestCacheBusting(t *testing.T) { withConfig(configPath, cfg), withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, + stats.Traversed: 33, stats.Matched: 6, stats.Formatted: 0, stats.Changed: 0, @@ -992,7 +992,7 @@ func TestCacheBusting(t *testing.T) { withConfig(configPath, cfg), withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, + stats.Traversed: 33, stats.Matched: 5, stats.Formatted: 0, stats.Changed: 0, @@ -1039,7 +1039,7 @@ func TestCacheBusting(t *testing.T) { withConfig(configPath, cfg), withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 33, + stats.Traversed: 34, stats.Matched: 3, stats.Formatted: 3, stats.Changed: 1, @@ -1054,7 +1054,7 @@ func TestCacheBusting(t *testing.T) { withConfig(configPath, cfg), withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 33, + stats.Traversed: 34, stats.Matched: 3, stats.Formatted: 1, stats.Changed: 1, @@ -1065,7 +1065,7 @@ func TestCacheBusting(t *testing.T) { withConfig(configPath, cfg), withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 33, + stats.Traversed: 34, stats.Matched: 3, stats.Formatted: 0, stats.Changed: 0, @@ -1085,7 +1085,7 @@ func TestCacheBusting(t *testing.T) { withConfig(configPath, cfg), withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 33, + stats.Traversed: 34, stats.Matched: 3, stats.Formatted: 1, stats.Changed: 1, @@ -1096,7 +1096,7 @@ func TestCacheBusting(t *testing.T) { withConfig(configPath, cfg), withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 33, + stats.Traversed: 34, stats.Matched: 3, stats.Formatted: 0, stats.Changed: 0, @@ -1125,7 +1125,7 @@ func TestCacheBusting(t *testing.T) { withConfig(configPath, cfg), withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, + stats.Traversed: 33, stats.Matched: 2, stats.Formatted: 2, stats.Changed: 2, @@ -1137,7 +1137,7 @@ func TestCacheBusting(t *testing.T) { withConfig(configPath, cfg), withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, + stats.Traversed: 33, stats.Matched: 2, stats.Formatted: 0, stats.Changed: 0, @@ -1156,7 +1156,7 @@ func TestCacheBusting(t *testing.T) { withConfig(configPath, cfg), withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, + stats.Traversed: 33, stats.Matched: 3, stats.Formatted: 1, stats.Changed: 1, @@ -1175,7 +1175,7 @@ func TestCacheBusting(t *testing.T) { withConfig(configPath, cfg), withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, + stats.Traversed: 33, stats.Matched: 3, stats.Formatted: 2, stats.Changed: 2, @@ -1187,7 +1187,7 @@ func TestCacheBusting(t *testing.T) { withConfig(configPath, cfg), withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, + stats.Traversed: 33, stats.Matched: 3, stats.Formatted: 0, stats.Changed: 0, @@ -1203,7 +1203,7 @@ func TestCacheBusting(t *testing.T) { withConfig(configPath, cfg), withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, + stats.Traversed: 33, stats.Matched: 3, stats.Formatted: 2, stats.Changed: 2, @@ -1215,7 +1215,7 @@ func TestCacheBusting(t *testing.T) { withConfig(configPath, cfg), withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, + stats.Traversed: 33, stats.Matched: 3, stats.Formatted: 0, stats.Changed: 0, @@ -1230,7 +1230,7 @@ func TestCacheBusting(t *testing.T) { withConfig(configPath, cfg), withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, + stats.Traversed: 33, stats.Matched: 3, stats.Formatted: 2, stats.Changed: 2, @@ -1242,7 +1242,7 @@ func TestCacheBusting(t *testing.T) { withConfig(configPath, cfg), withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, + stats.Traversed: 33, stats.Matched: 3, stats.Formatted: 0, stats.Changed: 0, @@ -1258,7 +1258,7 @@ func TestCacheBusting(t *testing.T) { withConfig(configPath, cfg), withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, + stats.Traversed: 33, stats.Matched: 2, stats.Formatted: 0, stats.Changed: 0, @@ -1308,9 +1308,9 @@ func TestGit(t *testing.T) { withConfig(configPath, cfg), withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, - stats.Matched: 32, - stats.Formatted: 32, + stats.Traversed: 33, + stats.Matched: 33, + stats.Formatted: 33, stats.Changed: 0, }), ) @@ -1325,8 +1325,8 @@ func TestGit(t *testing.T) { withConfig(configPath, cfg), withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 29, - stats.Matched: 29, + stats.Traversed: 30, + stats.Matched: 30, stats.Formatted: 0, stats.Changed: 0, }), @@ -1336,17 +1336,17 @@ func TestGit(t *testing.T) { as.NoError(os.Remove(filepath.Join(tempDir, "nixpkgs.toml"))) // walk with filesystem instead of with git - // the .git folder contains 49 additional files - // when added to the 31 we started with (32 minus nixpkgs.toml which we removed from the filesystem), we should - // traverse 80 files. + // the .git folder contains 50 additional files + // when added to the 32 we started with (34 minus nixpkgs.toml which we removed from the filesystem), we should + // traverse 82 files. treefmt(t, withArgs("--walk", "filesystem"), withConfig(configPath, cfg), withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 80, - stats.Matched: 80, - stats.Formatted: 49, // the echo formatter should only be applied to the new files + stats.Traversed: 82, + stats.Matched: 82, + stats.Formatted: 51, // the echo formatter should only be applied to the new files stats.Changed: 0, }), ) @@ -1485,9 +1485,9 @@ func TestPathsArg(t *testing.T) { treefmt(t, withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, - stats.Matched: 32, - stats.Formatted: 32, + stats.Traversed: 33, + stats.Matched: 33, + stats.Formatted: 33, stats.Changed: 0, }), ) @@ -1760,9 +1760,9 @@ func TestRunInSubdir(t *testing.T) { treefmt(t, withNoError(t), withStats(t, map[stats.Type]int{ - stats.Traversed: 32, - stats.Matched: 32, - stats.Formatted: 32, + stats.Traversed: 33, + stats.Matched: 33, + stats.Formatted: 33, stats.Changed: 0, }), ) diff --git "a/test/examples/emoji \360\237\225\260\357\270\217/README.md" "b/test/examples/emoji \360\237\225\260\357\270\217/README.md" new file mode 100644 index 00000000..e9168dbb --- /dev/null +++ "b/test/examples/emoji \360\237\225\260\357\270\217/README.md" @@ -0,0 +1 @@ +# Hello World \ No newline at end of file diff --git a/walk/filesystem_test.go b/walk/filesystem_test.go index 4a684ad7..8c5b74c6 100644 --- a/walk/filesystem_test.go +++ b/walk/filesystem_test.go @@ -17,6 +17,7 @@ import ( var examplesPaths = []string{ "elm/elm.json", "elm/src/Main.elm", + "emoji 🕰️/README.md", "go/go.mod", "go/main.go", "haskell/CHANGELOG.md", @@ -78,8 +79,8 @@ func TestFilesystemReader(t *testing.T) { } } - as.Equal(32, count) - as.Equal(32, statz.Value(stats.Traversed)) + as.Equal(33, count) + as.Equal(33, statz.Value(stats.Traversed)) as.Equal(0, statz.Value(stats.Matched)) as.Equal(0, statz.Value(stats.Formatted)) as.Equal(0, statz.Value(stats.Changed)) diff --git a/walk/git.go b/walk/git.go index 6cf480e7..8283700d 100644 --- a/walk/git.go +++ b/walk/git.go @@ -8,6 +8,7 @@ import ( "os" "os/exec" "path/filepath" + "strconv" "strings" "github.com/charmbracelet/log" @@ -50,6 +51,21 @@ func (g *GitReader) Read(ctx context.Context, files []*File) (n int, err error) g.scanner = bufio.NewScanner(r) } + nextLine := func() (string, error) { + line := g.scanner.Text() + + if len(line) == 0 || line[0] != '"' { + return line, nil + } + + unquoted, err := strconv.Unquote(line) + if err != nil { + return "", fmt.Errorf("failed to unquote line %s: %w", line, err) + } + + return unquoted, nil + } + LOOP: for n < len(files) { @@ -66,7 +82,12 @@ LOOP: default: // read the next file if g.scanner.Scan() { - path := filepath.Join(g.root, g.path, g.scanner.Text()) + entry, err := nextLine() + if err != nil { + return n, err + } + + path := filepath.Join(g.root, g.path, entry) g.log.Debugf("processing file: %s", path) diff --git a/walk/git_test.go b/walk/git_test.go index 83103a0f..acef85b0 100644 --- a/walk/git_test.go +++ b/walk/git_test.go @@ -62,8 +62,8 @@ func TestGitReader(t *testing.T) { } } - as.Equal(32, count) - as.Equal(32, statz.Value(stats.Traversed)) + as.Equal(33, count) + as.Equal(33, statz.Value(stats.Traversed)) as.Equal(0, statz.Value(stats.Matched)) as.Equal(0, statz.Value(stats.Formatted)) as.Equal(0, statz.Value(stats.Changed))