From a2533c8f5bb48b42b9c05fd960e5297adb4f0565 Mon Sep 17 00:00:00 2001 From: Piotr Kalinowski Date: Tue, 2 Aug 2022 20:49:15 +0200 Subject: [PATCH] fix(git): better cross-platform path resolution and tests The simple filepath.IsAbs() is not enough on Windows, where the relative path may start with a separator and should then be taken relative to the volume name. So, introduce resolveGitPath() helper function that will do the right thing on both Windows and other systems. As a bonus, it returns paths converted to slashes for ease of use with the rest of the git segment: - Git for Windows uses only slashes. - Slashes do work as a path separator on Windows anyway. - Some tests in git segment still use (and rightfully so) slashes. Add tests for resolveGitPath() on both types of systems and fix TestEnableInWorktree using a system-dependent testing constant as a root path. --- src/segments/git.go | 8 ++--- src/segments/git_test.go | 62 +++++++++++++++----------------- src/segments/git_unix.go | 13 +++++++ src/segments/git_unix_test.go | 36 +++++++++++++++++++ src/segments/git_windows.go | 22 ++++++++++++ src/segments/git_windows_test.go | 42 ++++++++++++++++++++++ 6 files changed, 144 insertions(+), 39 deletions(-) create mode 100644 src/segments/git_unix.go create mode 100644 src/segments/git_unix_test.go create mode 100644 src/segments/git_windows.go create mode 100644 src/segments/git_windows_test.go diff --git a/src/segments/git.go b/src/segments/git.go index 604b41fe..3076a753 100644 --- a/src/segments/git.go +++ b/src/segments/git.go @@ -196,18 +196,14 @@ func (g *Git) hasWorktree(gitdir *environment.FileInfo) bool { // we need the parent folder to detect where the real .git folder is ind = strings.LastIndex(g.workingFolder, "/.git/modules") if ind > -1 { - if !filepath.IsAbs(g.workingFolder) { - g.rootFolder = filepath.Join(gitdir.ParentFolder, g.workingFolder) - } else { - g.rootFolder = g.workingFolder - } + g.rootFolder = resolveGitPath(gitdir.ParentFolder, g.workingFolder) // this might be both a worktree and a submodule, where the path would look like // this: path/.git/modules/module/path/worktrees/location. We cannot distinguish // between worktree and a module path containing the word 'worktree,' however. ind = strings.LastIndex(g.rootFolder, "/worktrees/") if ind > -1 && g.env.HasFilesInDir(g.rootFolder, "gitdir") { gitDir := filepath.Join(g.rootFolder, "gitdir") - realGitFolder := filepath.Clean(g.env.FileContent(gitDir)) + realGitFolder := g.env.FileContent(gitDir) g.realFolder = strings.TrimSuffix(realGitFolder, ".git\n") g.rootFolder = g.rootFolder[:ind] g.workingFolder = g.rootFolder diff --git a/src/segments/git_test.go b/src/segments/git_test.go index 4595d81d..025ffb6f 100644 --- a/src/segments/git_test.go +++ b/src/segments/git_test.go @@ -57,6 +57,11 @@ func TestEnabledInWorkingDirectory(t *testing.T) { assert.Equal(t, fileInfo.Path, g.workingFolder) } +func TestResolveEmptyGitPath(t *testing.T) { + base := "base" + assert.Equal(t, base, resolveGitPath(base, "")) +} + func TestEnabledInWorktree(t *testing.T) { cases := []struct { Case string @@ -67,63 +72,59 @@ func TestEnabledInWorktree(t *testing.T) { ExpectedRealFolder string ExpectedWorkingFolder string ExpectedRootFolder string - WindowsPaths bool }{ { Case: "worktree", ExpectedEnabled: true, - WorkingFolder: "/dev/.git/worktrees/folder_worktree", + WorkingFolder: TestRootPath + "dev/.git/worktrees/folder_worktree", WorkingFolderAddon: "gitdir", - WorkingFolderContent: "/dev/worktree.git\n", - ExpectedWorkingFolder: "/dev/.git/worktrees/folder_worktree", - ExpectedRealFolder: "/dev/worktree", - ExpectedRootFolder: "/dev/.git", + WorkingFolderContent: TestRootPath + "dev/worktree.git\n", + ExpectedWorkingFolder: TestRootPath + "dev/.git/worktrees/folder_worktree", + ExpectedRealFolder: TestRootPath + "dev/worktree", + ExpectedRootFolder: TestRootPath + "dev/.git", }, { Case: "submodule", ExpectedEnabled: true, WorkingFolder: "./.git/modules/submodule", - ExpectedWorkingFolder: "/dev/.git/modules/submodule", - ExpectedRealFolder: "/dev/.git/modules/submodule", - ExpectedRootFolder: "/dev/.git/modules/submodule", - WindowsPaths: true, + ExpectedWorkingFolder: TestRootPath + "dev/.git/modules/submodule", + ExpectedRealFolder: TestRootPath + "dev/.git/modules/submodule", + ExpectedRootFolder: TestRootPath + "dev/.git/modules/submodule", }, { Case: "submodule with root working folder", ExpectedEnabled: true, - WorkingFolder: "/repo/.git/modules/submodule", - ExpectedWorkingFolder: "/repo/.git/modules/submodule", - ExpectedRealFolder: "/repo/.git/modules/submodule", - ExpectedRootFolder: "/repo/.git/modules/submodule", - WindowsPaths: true, + WorkingFolder: TestRootPath + "repo/.git/modules/submodule", + ExpectedWorkingFolder: TestRootPath + "repo/.git/modules/submodule", + ExpectedRealFolder: TestRootPath + "repo/.git/modules/submodule", + ExpectedRootFolder: TestRootPath + "repo/.git/modules/submodule", }, { Case: "submodule with worktrees", ExpectedEnabled: true, - WorkingFolder: "/dev/.git/modules/module/path/worktrees/location", + WorkingFolder: TestRootPath + "dev/.git/modules/module/path/worktrees/location", WorkingFolderAddon: "gitdir", - WorkingFolderContent: "/dev/worktree.git\n", - ExpectedWorkingFolder: "/dev/.git/modules/module/path", - ExpectedRealFolder: "/dev/worktree", - ExpectedRootFolder: "/dev/.git/modules/module/path", - WindowsPaths: true, + WorkingFolderContent: TestRootPath + "dev/worktree.git\n", + ExpectedWorkingFolder: TestRootPath + "dev/.git/modules/module/path", + ExpectedRealFolder: TestRootPath + "dev/worktree", + ExpectedRootFolder: TestRootPath + "dev/.git/modules/module/path", }, { Case: "separate git dir", ExpectedEnabled: true, - WorkingFolder: "/dev/separate/.git/posh", - ExpectedWorkingFolder: "/dev/", - ExpectedRealFolder: "/dev/", - ExpectedRootFolder: "/dev/separate/.git/posh", + WorkingFolder: TestRootPath + "dev/separate/.git/posh", + ExpectedWorkingFolder: TestRootPath + "dev/", + ExpectedRealFolder: TestRootPath + "dev/", + ExpectedRootFolder: TestRootPath + "dev/separate/.git/posh", }, } fileInfo := &environment.FileInfo{ - Path: "/dev/.git", - ParentFolder: "/dev", + Path: TestRootPath + "dev/.git", + ParentFolder: TestRootPath + "dev", } for _, tc := range cases { env := new(mock.MockedEnvironment) - env.On("FileContent", "/dev/.git").Return(fmt.Sprintf("gitdir: %s", tc.WorkingFolder)) + env.On("FileContent", TestRootPath+"dev/.git").Return(fmt.Sprintf("gitdir: %s", tc.WorkingFolder)) env.On("FileContent", filepath.Join(tc.WorkingFolder, tc.WorkingFolderAddon)).Return(tc.WorkingFolderContent) env.On("HasFilesInDir", tc.WorkingFolder, tc.WorkingFolderAddon).Return(true) env.On("HasFilesInDir", tc.WorkingFolder, "HEAD").Return(true) @@ -135,11 +136,6 @@ func TestEnabledInWorktree(t *testing.T) { }, } assert.Equal(t, tc.ExpectedEnabled, g.hasWorktree(fileInfo), tc.Case) - if tc.WindowsPaths { - tc.ExpectedRealFolder = filepath.Clean(tc.ExpectedRealFolder) - tc.ExpectedRootFolder = filepath.Clean(tc.ExpectedRootFolder) - tc.ExpectedWorkingFolder = filepath.Clean(tc.ExpectedWorkingFolder) - } assert.Equal(t, tc.ExpectedWorkingFolder, g.workingFolder, tc.Case) assert.Equal(t, tc.ExpectedRealFolder, g.realFolder, tc.Case) assert.Equal(t, tc.ExpectedRootFolder, g.rootFolder, tc.Case) diff --git a/src/segments/git_unix.go b/src/segments/git_unix.go new file mode 100644 index 00000000..6c1ab7c2 --- /dev/null +++ b/src/segments/git_unix.go @@ -0,0 +1,13 @@ +//go:build !windows + +package segments + +import "path/filepath" + +// resolveGitPath resolves path relative to base. +func resolveGitPath(base, path string) string { + if filepath.IsAbs(path) { + return path + } + return filepath.Join(base, path) +} diff --git a/src/segments/git_unix_test.go b/src/segments/git_unix_test.go new file mode 100644 index 00000000..05290c33 --- /dev/null +++ b/src/segments/git_unix_test.go @@ -0,0 +1,36 @@ +//go:build !windows + +package segments + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +const TestRootPath = "/" + +func TestResolveGitPath(t *testing.T) { + cases := []struct { + Case string + Base string + Path string + Expected string + }{ + { + Case: "relative path", + Base: "dir/", + Path: "sub", + Expected: "dir/sub", + }, + { + Case: "absolute path", + Base: "/base", + Path: "/absolute/path", + Expected: "/absolute/path", + }, + } + for _, tc := range cases { + assert.Equal(t, tc.Expected, resolveGitPath(tc.Base, tc.Path), tc.Case) + } +} diff --git a/src/segments/git_windows.go b/src/segments/git_windows.go new file mode 100644 index 00000000..424a5a51 --- /dev/null +++ b/src/segments/git_windows.go @@ -0,0 +1,22 @@ +package segments + +import "path/filepath" + +// resolveGitPath resolves path relative to base. +func resolveGitPath(base, path string) string { + if len(path) == 0 { + return base + } + if filepath.IsAbs(path) { + return path + } + // Note that git on Windows uses slashes exclusively. And it's okay + // because Windows actually accepts both directory separators. More + // importantly, however, parts of the git segment depend on those + // slashes. + if path[0] == '/' { + // path is a disk-relative path. + return filepath.VolumeName(base) + path + } + return filepath.ToSlash(filepath.Join(base, path)) +} diff --git a/src/segments/git_windows_test.go b/src/segments/git_windows_test.go new file mode 100644 index 00000000..436fdcd3 --- /dev/null +++ b/src/segments/git_windows_test.go @@ -0,0 +1,42 @@ +//go:build windows + +package segments + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +const TestRootPath = "C:/" + +func TestResolveGitPath(t *testing.T) { + cases := []struct { + Case string + Base string + Path string + Expected string + }{ + { + Case: "relative path", + Base: "dir\\", + Path: "sub", + Expected: "dir/sub", + }, + { + Case: "absolute path", + Base: "C:\\base", + Path: "C:/absolute/path", + Expected: "C:/absolute/path", + }, + { + Case: "disk-relative path", + Base: "C:\\base", + Path: "/absolute/path", + Expected: "C:/absolute/path", + }, + } + for _, tc := range cases { + assert.Equal(t, tc.Expected, resolveGitPath(tc.Base, tc.Path), tc.Case) + } +}