From dd01639f57812955b1e8aec56c0a94314d17a39d Mon Sep 17 00:00:00 2001 From: Cristian Betivu Date: Wed, 16 Aug 2023 20:57:02 +0300 Subject: [PATCH 1/7] Improve error message --- pkg/commands/git_commands/repo_paths.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/commands/git_commands/repo_paths.go b/pkg/commands/git_commands/repo_paths.go index 1fdc016d8..912202ca2 100644 --- a/pkg/commands/git_commands/repo_paths.go +++ b/pkg/commands/git_commands/repo_paths.go @@ -223,7 +223,7 @@ func getCurrentRepoGitDirPath( // If this error causes issues, we could relax the constraint and just always // return the path - return "", "", errors.Errorf("could not find git dir for %s: path is not under `worktrees` or `modules` directories", currentPath) + return "", "", errors.Errorf("could not find git dir for %s: path '%s' is not under `worktrees` or `modules` directories.", currentPath, worktreeGitPath) } // takes a path containing a symlink and returns the true path From d7b1deb465e77b8839584690b75c077d7bdd38fe Mon Sep 17 00:00:00 2001 From: Cristian Betivu Date: Wed, 16 Aug 2023 20:55:54 +0300 Subject: [PATCH 2/7] Clean path --- pkg/commands/git_commands/repo_paths.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/commands/git_commands/repo_paths.go b/pkg/commands/git_commands/repo_paths.go index 912202ca2..a124fd18a 100644 --- a/pkg/commands/git_commands/repo_paths.go +++ b/pkg/commands/git_commands/repo_paths.go @@ -208,7 +208,7 @@ func getCurrentRepoGitDirPath( } // confirm whether the next directory up is the worktrees directory - parent := path.Dir(worktreeGitPath) + parent := path.Dir(path.Clean((worktreeGitPath))) if path.Base(parent) == "worktrees" { gitDirPath := path.Dir(parent) return gitDirPath, path.Dir(gitDirPath), nil From a2b2336173f05a119338f850377668c4579169a1 Mon Sep 17 00:00:00 2001 From: Cristian Betivu Date: Wed, 16 Aug 2023 21:09:15 +0300 Subject: [PATCH 3/7] Stylistic changes --- pkg/commands/git_commands/repo_paths.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/commands/git_commands/repo_paths.go b/pkg/commands/git_commands/repo_paths.go index a124fd18a..8bae26258 100644 --- a/pkg/commands/git_commands/repo_paths.go +++ b/pkg/commands/git_commands/repo_paths.go @@ -208,7 +208,7 @@ func getCurrentRepoGitDirPath( } // confirm whether the next directory up is the worktrees directory - parent := path.Dir(path.Clean((worktreeGitPath))) + parent := path.Dir(path.Clean(worktreeGitPath)) if path.Base(parent) == "worktrees" { gitDirPath := path.Dir(parent) return gitDirPath, path.Dir(gitDirPath), nil @@ -223,7 +223,7 @@ func getCurrentRepoGitDirPath( // If this error causes issues, we could relax the constraint and just always // return the path - return "", "", errors.Errorf("could not find git dir for %s: path '%s' is not under `worktrees` or `modules` directories.", currentPath, worktreeGitPath) + return "", "", errors.Errorf("could not find git dir for %s: the path '%s' is not under `worktrees` or `modules` directories.", currentPath, worktreeGitPath) } // takes a path containing a symlink and returns the true path From ee308a49946633c0ba6d96e38aec99e0f88eac90 Mon Sep 17 00:00:00 2001 From: Cristian Betivu Date: Wed, 16 Aug 2023 21:13:28 +0300 Subject: [PATCH 4/7] Clean before convertion? --- pkg/commands/git_commands/repo_paths.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/commands/git_commands/repo_paths.go b/pkg/commands/git_commands/repo_paths.go index 8bae26258..e80b19a1c 100644 --- a/pkg/commands/git_commands/repo_paths.go +++ b/pkg/commands/git_commands/repo_paths.go @@ -158,6 +158,7 @@ func linkedWorktreeGitDirPath(fs afero.Fs, worktreePath string) (string, error) gitDir := strings.TrimPrefix(gitDirLine[0], "gitdir: ") + gitDir = filepath.Clean(gitDir) // For windows support gitDir = filepath.ToSlash(gitDir) @@ -208,7 +209,7 @@ func getCurrentRepoGitDirPath( } // confirm whether the next directory up is the worktrees directory - parent := path.Dir(path.Clean(worktreeGitPath)) + parent := path.Dir(worktreeGitPath) if path.Base(parent) == "worktrees" { gitDirPath := path.Dir(parent) return gitDirPath, path.Dir(gitDirPath), nil From 03694f7502a014ce56e6fc694c2cd044b5662395 Mon Sep 17 00:00:00 2001 From: Cristian Betivu Date: Sat, 19 Aug 2023 19:10:25 +0300 Subject: [PATCH 5/7] Fix arg order to asserts --- pkg/commands/git_commands/worktree_loader_test.go | 2 +- pkg/utils/formatting_test.go | 4 ++-- pkg/utils/rebase_todo_test.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/commands/git_commands/worktree_loader_test.go b/pkg/commands/git_commands/worktree_loader_test.go index 7f805b1b9..12b308074 100644 --- a/pkg/commands/git_commands/worktree_loader_test.go +++ b/pkg/commands/git_commands/worktree_loader_test.go @@ -186,7 +186,7 @@ branch refs/heads/mybranch-worktree assert.EqualError(t, errors.New(s.expectedErr), err.Error()) } else { assert.NoError(t, err) - assert.EqualValues(t, worktrees, s.expectedWorktrees) + assert.EqualValues(t, s.expectedWorktrees, worktrees) } }) } diff --git a/pkg/utils/formatting_test.go b/pkg/utils/formatting_test.go index 2de282311..b1777911b 100644 --- a/pkg/utils/formatting_test.go +++ b/pkg/utils/formatting_test.go @@ -79,7 +79,7 @@ func TestGetPadWidths(t *testing.T) { for _, test := range tests { output := getPadWidths(test.input) - assert.EqualValues(t, output, test.expected) + assert.EqualValues(t, test.expected, output) } } @@ -217,6 +217,6 @@ func TestRenderDisplayStrings(t *testing.T) { for _, test := range tests { output := RenderDisplayStrings(test.input, test.columnAlignments) - assert.EqualValues(t, output, test.expected) + assert.EqualValues(t, test.expected, output) } } diff --git a/pkg/utils/rebase_todo_test.go b/pkg/utils/rebase_todo_test.go index a9bcb574b..40f44a3cb 100644 --- a/pkg/utils/rebase_todo_test.go +++ b/pkg/utils/rebase_todo_test.go @@ -326,7 +326,7 @@ func TestRebaseCommands_moveFixupCommitDown(t *testing.T) { assert.EqualError(t, actualErr, scenario.expectedErr.Error()) } - assert.EqualValues(t, actualTodos, scenario.expectedTodos) + assert.EqualValues(t, scenario.expectedTodos, actualTodos) }) } } From 382ecb6bfebeb9805dca95d86341137a08ba98f8 Mon Sep 17 00:00:00 2001 From: Cristian Betivu Date: Sat, 19 Aug 2023 19:10:35 +0300 Subject: [PATCH 6/7] Add unit test --- pkg/commands/git_commands/repo_paths_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/pkg/commands/git_commands/repo_paths_test.go b/pkg/commands/git_commands/repo_paths_test.go index 6533b733d..1bee52ec5 100644 --- a/pkg/commands/git_commands/repo_paths_test.go +++ b/pkg/commands/git_commands/repo_paths_test.go @@ -55,6 +55,24 @@ func TestGetRepoPathsAux(t *testing.T) { }, Err: nil, }, + { + Name: "worktree with trailing separator in path", + BeforeFunc: func(fs afero.Fs) { + // setup for linked worktree + _ = fs.MkdirAll("/path/to/repo/.git/worktrees/worktree1", 0o755) + _ = afero.WriteFile(fs, "/path/to/repo/worktree1/.git", []byte("gitdir: /path/to/repo/.git/worktrees/worktree1/"), 0o644) + }, + Path: "/path/to/repo/worktree1", + Expected: &RepoPaths{ + currentPath: "/path/to/repo/worktree1", + worktreePath: "/path/to/repo/worktree1", + worktreeGitDirPath: "/path/to/repo/.git/worktrees/worktree1", + repoPath: "/path/to/repo", + repoGitDirPath: "/path/to/repo/.git", + repoName: "repo", + }, + Err: nil, + }, { Name: "worktree .git file missing gitdir directive", BeforeFunc: func(fs afero.Fs) { From 7a4a0c85c433fd29185c05d0021cd0db87284f23 Mon Sep 17 00:00:00 2001 From: Cristian Betivu Date: Sat, 19 Aug 2023 19:12:36 +0300 Subject: [PATCH 7/7] Fix test --- pkg/commands/git_commands/repo_paths.go | 2 +- pkg/commands/git_commands/repo_paths_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/commands/git_commands/repo_paths.go b/pkg/commands/git_commands/repo_paths.go index e80b19a1c..13cda86a4 100644 --- a/pkg/commands/git_commands/repo_paths.go +++ b/pkg/commands/git_commands/repo_paths.go @@ -224,7 +224,7 @@ func getCurrentRepoGitDirPath( // If this error causes issues, we could relax the constraint and just always // return the path - return "", "", errors.Errorf("could not find git dir for %s: the path '%s' is not under `worktrees` or `modules` directories.", currentPath, worktreeGitPath) + return "", "", errors.Errorf("could not find git dir for %s: the path '%s' is not under `worktrees` or `modules` directories", currentPath, worktreeGitPath) } // takes a path containing a symlink and returns the true path diff --git a/pkg/commands/git_commands/repo_paths_test.go b/pkg/commands/git_commands/repo_paths_test.go index 1bee52ec5..5e6275522 100644 --- a/pkg/commands/git_commands/repo_paths_test.go +++ b/pkg/commands/git_commands/repo_paths_test.go @@ -135,7 +135,7 @@ func TestGetRepoPathsAux(t *testing.T) { }, Path: "/path/to/repo/my/submodule1", Expected: nil, - Err: errors.New("failed to get repo git dir path: could not find git dir for /path/to/repo/my/submodule1: path is not under `worktrees` or `modules` directories"), + Err: errors.New("failed to get repo git dir path: could not find git dir for /path/to/repo/my/submodule1: the path '/random/submodule1' is not under `worktrees` or `modules` directories"), }, }