diff --git a/README.md b/README.md index 7361144..24c6084 100644 --- a/README.md +++ b/README.md @@ -166,11 +166,12 @@ wtp add feature/remote-only # If branch exists in multiple remotes, shows helpful error: # Error: branch 'feature/shared' exists in multiple remotes: origin, upstream -# Please specify the remote explicitly (e.g., --track origin/feature/shared) +# Create a local branch for the remote you want, then run wtp add again wtp add feature/shared -# Explicitly specify which remote to track -wtp add -b feature/shared upstream/feature/shared +# Example manual disambiguation: +git branch --track feature/shared upstream/feature/shared +wtp add feature/shared ``` ### Management Commands @@ -409,7 +410,12 @@ wtp provides clear error messages: Error: branch 'nonexistent' not found in local or remote branches # Multiple remotes have same branch -Error: branch 'feature' exists in multiple remotes: origin, upstream. Please specify remote explicitly +Error: branch 'feature' exists in multiple remotes: origin, upstream + +Solution: Create a local tracking branch for the remote you want without checking it out, then run wtp add again. + • git branch --track feature origin/feature + • git branch --track feature upstream/feature + • wtp add feature # Worktree already exists Error: failed to create worktree: exit status 128 diff --git a/cmd/wtp/add.go b/cmd/wtp/add.go index cb8cef7..f84fab7 100644 --- a/cmd/wtp/add.go +++ b/cmd/wtp/add.go @@ -23,14 +23,15 @@ import ( wtpio "github.com/satococoa/wtp/v2/internal/io" ) +const addUsageText = "wtp add [--quiet]\n" + + " wtp add -b [] [--quiet]" + // NewAddCommand creates the add command definition func NewAddCommand() *cli.Command { return &cli.Command{ - Name: "add", - Usage: "Create a new worktree", - UsageText: "wtp add \n" + - " wtp add -b []\n" + - " wtp add -b --quiet", + Name: "add", + Usage: "Create a new worktree", + UsageText: addUsageText, Description: "Creates a new worktree for the specified branch. If the branch doesn't exist locally " + "but exists on a remote, it will be automatically tracked.\n\n" + "Examples:\n" + @@ -190,7 +191,6 @@ func buildWorktreeCommand( Branch: cmd.String("branch"), } - // Use resolved track if provided if resolvedTrack != "" { opts.Track = resolvedTrack } @@ -199,11 +199,11 @@ func buildWorktreeCommand( // Handle different argument patterns based on flags if resolvedTrack != "" { - // When using resolved tracking, the commitish is the remote branch + // When tracking, the commitish is the remote branch reference. commitish = resolvedTrack // If there's an argument, it's the local branch name (not used as commitish) if cmd.Args().Len() > 0 && opts.Branch == "" { - // The first argument is the branch name when using resolved tracking without -b + // The first argument is the local branch name when using tracking without -b. opts.Branch = cmd.Args().Get(0) } } else if cmd.Args().Len() > 0 { @@ -249,10 +249,10 @@ func analyzeGitWorktreeError(workTreePath, branchName string, gitError error, gi } } - if isMultipleBranchesError(errorOutput) { - return &MultipleBranchesError{ - BranchName: branchName, - GitError: gitError, + if isAmbiguousReferenceError(errorOutput) { + return &AmbiguousReferenceError{ + Reference: branchName, + GitError: gitError, } } @@ -307,8 +307,10 @@ func isPathAlreadyExistsError(errorOutput string) bool { return strings.Contains(errorOutput, "already exists") } -func isMultipleBranchesError(errorOutput string) bool { - return strings.Contains(errorOutput, "more than one remote") || strings.Contains(errorOutput, "ambiguous") +func isAmbiguousReferenceError(errorOutput string) bool { + return strings.Contains(errorOutput, "matched multiple branches") || + strings.Contains(errorOutput, "ambiguous argument") || + strings.Contains(errorOutput, "is ambiguous") } func isInvalidPathError(errorOutput, workTreePath, gitOutput string) bool { @@ -333,7 +335,6 @@ func (e *WorktreeAlreadyExistsError) Error() string { The branch '%s' is already checked out in another worktree. Solutions: - • Use '--force' flag to allow multiple checkouts • Choose a different branch • Remove the existing worktree first @@ -371,27 +372,29 @@ func (e *PathAlreadyExistsError) Error() string { The target directory already exists and is not empty. Solutions: - • Use --force flag to overwrite existing directory • Remove the existing directory • Use a different branch name Original error: %v`, e.Path, e.GitError) } -// MultipleBranchesError reports that a branch name resolves to multiple remotes and needs disambiguation. -type MultipleBranchesError struct { - BranchName string - GitError error +// AmbiguousReferenceError indicates that git could not uniquely resolve the requested ref. +type AmbiguousReferenceError struct { + Reference string + GitError error } -func (e *MultipleBranchesError) Error() string { - return fmt.Sprintf(`branch '%s' exists in multiple remotes +func (e *AmbiguousReferenceError) Error() string { + return fmt.Sprintf(`reference '%s' is ambiguous + +The git command matched more than one branch or revision for '%s'. -Use the --track flag to specify which remote to use: - • wtp add --track origin/%s %s - • wtp add --track upstream/%s %s +Suggestions: + • Use a specific local branch name + • Use an exact commit SHA + • If you need a remote branch, create a local tracking branch first with 'git branch --track %s /%s' -Original error: %v`, e.BranchName, e.BranchName, e.BranchName, e.BranchName, e.BranchName, e.GitError) +Original error: %v`, e.Reference, e.Reference, e.Reference, e.Reference, e.GitError) } func executePostCreateHooks(w io.Writer, cfg *config.Config, repoPath, workTreePath string) error { @@ -467,7 +470,7 @@ func executePostCreateCommand( func validateAddInput(cmd *cli.Command) error { if cmd.Args().Len() == 0 && cmd.String("branch") == "" { - return errors.BranchNameRequired("wtp add | -b []") + return errors.BranchNameRequired(addUsageText) } return nil @@ -674,13 +677,6 @@ func resolveBranchTracking( // Check if branch exists locally or in remotes resolvedBranch, isRemote, err := repo.ResolveBranch(branchName) if err != nil { - // Check if it's a multiple branches error - if strings.Contains(err.Error(), "exists in multiple remotes") { - return "", &MultipleBranchesError{ - BranchName: branchName, - GitError: err, - } - } return "", err } diff --git a/cmd/wtp/add_test.go b/cmd/wtp/add_test.go index f7d15dc..b21ac99 100644 --- a/cmd/wtp/add_test.go +++ b/cmd/wtp/add_test.go @@ -23,11 +23,12 @@ func TestNewAddCommand(t *testing.T) { assert.NotNil(t, cmd) assert.Equal(t, "add", cmd.Name) assert.Equal(t, "Create a new worktree", cmd.Usage) + assert.Equal(t, addUsageText, cmd.UsageText) assert.NotEmpty(t, cmd.Description) assert.NotNil(t, cmd.Action) assert.NotNil(t, cmd.ShellComplete) - // Check simplified flags exist + // Check supported flags exist flagNames := []string{"branch", "exec", "quiet"} for _, name := range flagNames { found := false @@ -59,7 +60,6 @@ func TestWorkTreeAlreadyExistsError(t *testing.T) { // Then: should contain branch name, solutions, and original error assert.Contains(t, message, "feature/awesome") assert.Contains(t, message, "already checked out in another worktree") - assert.Contains(t, message, "--force") assert.Contains(t, message, "Choose a different branch") assert.Contains(t, message, "Remove the existing worktree") assert.Contains(t, message, "branch already checked out") @@ -133,7 +133,6 @@ func TestPathAlreadyExistsError(t *testing.T) { // Then: should contain path, solutions, and original error assert.Contains(t, message, "/existing/path") assert.Contains(t, message, "already exists and is not empty") - assert.Contains(t, message, "--force flag") assert.Contains(t, message, "Remove the existing directory") assert.Contains(t, message, "directory not empty") }) @@ -154,43 +153,6 @@ func TestPathAlreadyExistsError(t *testing.T) { }) } -func TestMultipleBranchesError(t *testing.T) { - t.Run("should format error message with branch name and track suggestions", func(t *testing.T) { - // Given: a MultipleBranchesError with branch name - originalErr := &MockGitError{msg: "multiple remotes found"} - err := &MultipleBranchesError{ - BranchName: "feature/shared", - GitError: originalErr, - } - - // When: getting error message - message := err.Error() - - // Then: should contain branch name, track suggestions, and original error - assert.Contains(t, message, "feature/shared") - assert.Contains(t, message, "exists in multiple remotes") - assert.Contains(t, message, "--track origin/feature/shared") - assert.Contains(t, message, "--track upstream/feature/shared") - assert.Contains(t, message, "multiple remotes found") - }) - - t.Run("should handle special characters in branch name", func(t *testing.T) { - // Given: error with special characters in branch name - err := &MultipleBranchesError{ - BranchName: "feature/fix-bugs-#123", - GitError: &MockGitError{msg: "test error"}, - } - - // When: getting error message - message := err.Error() - - // Then: should properly format all instances of branch name - assert.Contains(t, message, "feature/fix-bugs-#123") - assert.Contains(t, message, "--track origin/feature/fix-bugs-#123") - assert.Contains(t, message, "--track upstream/feature/fix-bugs-#123") - }) -} - // Mock error for testing type MockGitError struct { msg string @@ -276,6 +238,7 @@ func TestValidateAddInput(t *testing.T) { if tt.wantErr { assert.Error(t, err) assert.Contains(t, err.Error(), "branch name is required") + assert.Contains(t, err.Error(), addUsageText) } else { assert.NoError(t, err) } @@ -315,7 +278,7 @@ func TestResolveWorktreePath(t *testing.T) { cfg := &config.Config{ Defaults: config.Defaults{BaseDir: tt.baseDir}, } - cmd := createTestCLICommand(tt.flags, []string{tt.branchName}) + cmd := createTestCLICommand(t, tt.flags, []string{tt.branchName}) path, branch := resolveWorktreePath(cfg, "/test/repo", tt.branchName, cmd) assert.Equal(t, tt.expectedPath, path) @@ -364,7 +327,7 @@ func TestAddCommand_CommandConstruction(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - cmd := createTestCLICommand(tt.flags, tt.args) + cmd := createTestCLICommand(t, tt.flags, tt.args) var buf bytes.Buffer mockExec := &mockCommandExecutor{} @@ -407,7 +370,7 @@ func TestAddCommand_SuccessMessage(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - cmd := createTestCLICommand(map[string]any{"branch": tt.branchName}, []string{tt.branchName}) + cmd := createTestCLICommand(t, map[string]any{"branch": tt.branchName}, []string{tt.branchName}) var buf bytes.Buffer mockExec := &mockCommandExecutor{} @@ -425,7 +388,7 @@ func TestAddCommand_SuccessMessage(t *testing.T) { func TestAddCommand_QuietModeOutput(t *testing.T) { t.Run("success should print only path to stdout", func(t *testing.T) { - cmd := createTestCLICommand(map[string]any{ + cmd := createTestCLICommand(t, map[string]any{ "branch": "feature/quiet", "quiet": true, }, []string{}) @@ -444,7 +407,7 @@ func TestAddCommand_QuietModeOutput(t *testing.T) { }) t.Run("hook failure should keep path on stdout and warnings on stderr", func(t *testing.T) { - cmd := createTestCLICommand(map[string]any{ + cmd := createTestCLICommand(t, map[string]any{ "branch": "feature/hook-fail", "quiet": true, }, []string{}) @@ -468,7 +431,7 @@ func TestAddCommand_QuietModeOutput(t *testing.T) { }) t.Run("exec output should go to stderr and path to stdout", func(t *testing.T) { - cmd := createTestCLICommand(map[string]any{ + cmd := createTestCLICommand(t, map[string]any{ "branch": "feature/exec", "quiet": true, "exec": "echo hi", @@ -497,7 +460,7 @@ func TestAddCommand_QuietModeOutput(t *testing.T) { }) t.Run("worktree creation failure should not print path", func(t *testing.T) { - cmd := createTestCLICommand(map[string]any{ + cmd := createTestCLICommand(t, map[string]any{ "branch": "feature/fail", "quiet": true, }, []string{}) @@ -534,7 +497,7 @@ func TestAddCommand_ValidationErrors(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - cmd := createTestCLICommand(tt.flags, tt.args) + cmd := createTestCLICommand(t, tt.flags, tt.args) err := validateAddInput(cmd) assert.Error(t, err) assert.Contains(t, err.Error(), tt.expectedError) @@ -545,7 +508,7 @@ func TestAddCommand_ValidationErrors(t *testing.T) { func TestAddCommand_ExecutionError(t *testing.T) { mockExec := &mockCommandExecutor{shouldFail: true} var buf bytes.Buffer - cmd := createTestCLICommand(map[string]any{"branch": "feature/auth"}, []string{"feature/auth"}) + cmd := createTestCLICommand(t, map[string]any{"branch": "feature/auth"}, []string{"feature/auth"}) cfg := &config.Config{ Defaults: config.Defaults{BaseDir: "/test/worktrees"}, } @@ -557,7 +520,7 @@ func TestAddCommand_ExecutionError(t *testing.T) { } func TestAddCommand_ExecFailureKeepsCreationContext(t *testing.T) { - cmd := createTestCLICommand(map[string]any{ + cmd := createTestCLICommand(t, map[string]any{ "branch": "feature/auth", "exec": "false", }, []string{}) @@ -608,7 +571,7 @@ func TestAddCommand_InternationalCharacters(t *testing.T) { t.Run(tt.name, func(t *testing.T) { mockExec := &mockCommandExecutor{} var buf bytes.Buffer - cmd := createTestCLICommand(map[string]any{"branch": tt.branchName}, []string{tt.branchName}) + cmd := createTestCLICommand(t, map[string]any{"branch": tt.branchName}, []string{tt.branchName}) cfg := &config.Config{ Defaults: config.Defaults{BaseDir: "/test/worktrees"}, } @@ -625,46 +588,12 @@ func TestAddCommand_InternationalCharacters(t *testing.T) { // ===== Helper Functions ===== -func createTestCLICommand(flags map[string]any, args []string) *cli.Command { - app := &cli.Command{ - Name: "test", - Commands: []*cli.Command{ - { - Name: "add", - Flags: []cli.Flag{ - &cli.BoolFlag{Name: "force"}, - &cli.BoolFlag{Name: "detach"}, - &cli.StringFlag{Name: "branch"}, - &cli.StringFlag{Name: "track"}, - &cli.StringFlag{Name: "exec"}, - &cli.BoolFlag{Name: "quiet"}, - &cli.BoolFlag{Name: "cd"}, - &cli.BoolFlag{Name: "no-cd"}, - }, - Action: func(_ context.Context, _ *cli.Command) error { - return nil - }, - }, - }, - } - - cmdArgs := []string{"test", "add"} - for key, value := range flags { - switch v := value.(type) { - case bool: - if v { - cmdArgs = append(cmdArgs, "--"+key) - } - case string: - cmdArgs = append(cmdArgs, "--"+key, v) - } - } - cmdArgs = append(cmdArgs, args...) - - ctx := context.Background() - _ = app.Run(ctx, cmdArgs) - - return app.Commands[0] +func createTestCLICommand(t *testing.T, flags map[string]any, args []string) *cli.Command { + return createTestSubcommand(t, "add", []cli.Flag{ + &cli.StringFlag{Name: "branch"}, + &cli.StringFlag{Name: "exec"}, + &cli.BoolFlag{Name: "quiet"}, + }, flags, args) } // ===== Integration Tests ===== @@ -674,7 +603,7 @@ func TestAddCommand_SimplifiedInterface(t *testing.T) { // Given: existing branch in repository mockExec := &mockCommandExecutor{} var buf bytes.Buffer - cmd := createTestCLICommand(map[string]any{}, []string{"main"}) + cmd := createTestCLICommand(t, map[string]any{}, []string{"main"}) cfg := &config.Config{ Defaults: config.Defaults{BaseDir: "/test/worktrees"}, } @@ -693,7 +622,7 @@ func TestAddCommand_SimplifiedInterface(t *testing.T) { // Given: new branch name mockExec := &mockCommandExecutor{} var buf bytes.Buffer - cmd := createTestCLICommand(map[string]any{"branch": "feature/new"}, []string{}) + cmd := createTestCLICommand(t, map[string]any{"branch": "feature/new"}, []string{}) cfg := &config.Config{ Defaults: config.Defaults{BaseDir: "/test/worktrees"}, } @@ -713,7 +642,7 @@ func TestAddCommand_SimplifiedInterface(t *testing.T) { // Given: new branch name and commit mockExec := &mockCommandExecutor{} var buf bytes.Buffer - cmd := createTestCLICommand(map[string]any{"branch": "hotfix/urgent"}, []string{"main"}) + cmd := createTestCLICommand(t, map[string]any{"branch": "hotfix/urgent"}, []string{"main"}) cfg := &config.Config{ Defaults: config.Defaults{BaseDir: "/test/worktrees"}, } @@ -731,7 +660,7 @@ func TestAddCommand_SimplifiedInterface(t *testing.T) { t.Run("should error with no arguments and no -b flag", func(t *testing.T) { // Given: no arguments and no -b flag - cmd := createTestCLICommand(map[string]any{}, []string{}) + cmd := createTestCLICommand(t, map[string]any{}, []string{}) // When: validating input err := validateAddInput(cmd) @@ -745,17 +674,17 @@ func TestAddCommand_SimplifiedInterface(t *testing.T) { // Test that validation works for the simplified interface // Valid case: existing branch - cmd1 := createTestCLICommand(map[string]any{}, []string{"main"}) + cmd1 := createTestCLICommand(t, map[string]any{}, []string{"main"}) err1 := validateAddInput(cmd1) assert.NoError(t, err1) // Valid case: new branch with -b - cmd2 := createTestCLICommand(map[string]any{"branch": "new-feature"}, []string{}) + cmd2 := createTestCLICommand(t, map[string]any{"branch": "new-feature"}, []string{}) err2 := validateAddInput(cmd2) assert.NoError(t, err2) // Invalid case: no args and no -b - cmd3 := createTestCLICommand(map[string]any{}, []string{}) + cmd3 := createTestCLICommand(t, map[string]any{}, []string{}) err3 := validateAddInput(cmd3) assert.Error(t, err3) assert.Contains(t, err3.Error(), "branch name is required") @@ -771,14 +700,9 @@ func TestAddCommand_Integration(t *testing.T) { { Name: "add", Flags: []cli.Flag{ - &cli.StringFlag{Name: "path"}, - &cli.BoolFlag{Name: "force"}, - &cli.BoolFlag{Name: "detach"}, &cli.StringFlag{Name: "branch", Aliases: []string{"b"}}, - &cli.StringFlag{Name: "track", Aliases: []string{"t"}}, &cli.StringFlag{Name: "exec"}, - &cli.BoolFlag{Name: "cd"}, - &cli.BoolFlag{Name: "no-cd"}, + &cli.BoolFlag{Name: "quiet", Aliases: []string{"q"}}, }, Action: addCommand, }, @@ -802,14 +726,9 @@ func TestAddCommand_Integration(t *testing.T) { { Name: "add", Flags: []cli.Flag{ - &cli.StringFlag{Name: "path"}, - &cli.BoolFlag{Name: "force"}, - &cli.BoolFlag{Name: "detach"}, &cli.StringFlag{Name: "branch", Aliases: []string{"b"}}, - &cli.StringFlag{Name: "track", Aliases: []string{"t"}}, &cli.StringFlag{Name: "exec"}, - &cli.BoolFlag{Name: "cd"}, - &cli.BoolFlag{Name: "no-cd"}, + &cli.BoolFlag{Name: "quiet", Aliases: []string{"q"}}, }, Action: addCommand, }, @@ -1111,8 +1030,8 @@ func TestAnalyzeGitWorktreeError(t *testing.T) { workTreePath: "/path/to/worktree", branchName: "ambiguous-branch", gitOutput: "fatal: 'ambiguous-branch' matched multiple branches", - expectedError: "", - expectedType: &MultipleBranchesError{}, + expectedError: "reference 'ambiguous-branch' is ambiguous", + expectedType: &AmbiguousReferenceError{}, }, { name: "invalid path error", diff --git a/cmd/wtp/remove_test.go b/cmd/wtp/remove_test.go index 26f1e2c..585b33e 100644 --- a/cmd/wtp/remove_test.go +++ b/cmd/wtp/remove_test.go @@ -173,7 +173,7 @@ func TestRemoveCommand_CommandConstruction(t *testing.T) { }, { name: "remove with branch deletion", - flags: map[string]any{"branch": true}, + flags: map[string]any{"with-branch": true}, worktreeName: "feature-branch", mockWorktreeList: "worktree /path/to/main\nHEAD abc123\nbranch refs/heads/main\n\n" + "worktree /path/to/worktrees/feature-branch\nHEAD def456\nbranch refs/heads/feature-branch\n\n", @@ -213,11 +213,11 @@ func TestRemoveCommand_CommandConstruction(t *testing.T) { }, } - cmd := createRemoveTestCLICommand(tt.flags, []string{tt.worktreeName}) + cmd := createRemoveTestCLICommand(t, tt.flags, []string{tt.worktreeName}) var buf bytes.Buffer forceFlag := tt.flags["force"] == true - branchFlag := tt.flags["branch"] == true + branchFlag := tt.flags["with-branch"] == true err := removeCommandWithCommandExecutor( cmd, &buf, mockExec, "/test/repo", tt.worktreeName, forceFlag, branchFlag, false, ) @@ -279,10 +279,10 @@ func TestRemoveCommand_SuccessMessage(t *testing.T) { flags := map[string]any{} if tt.branchFlag { - flags["branch"] = true + flags["with-branch"] = true } - cmd := createRemoveTestCLICommand(flags, []string{tt.worktreeName}) + cmd := createRemoveTestCLICommand(t, flags, []string{tt.worktreeName}) var buf bytes.Buffer branchFlag := tt.branchFlag @@ -360,7 +360,7 @@ func TestRemoveCommand_WorktreeNotFound(t *testing.T) { }, } - cmd := createRemoveTestCLICommand(map[string]any{}, []string{"nonexistent"}) + cmd := createRemoveTestCLICommand(t, map[string]any{}, []string{"nonexistent"}) var buf bytes.Buffer err := removeCommandWithCommandExecutor(cmd, &buf, mockExec, "/test/repo", "nonexistent", false, false, false) @@ -380,7 +380,7 @@ func TestRemoveCommand_WorktreeNotFound_ShowsConsistentNames(t *testing.T) { }, } - cmd := createRemoveTestCLICommand(map[string]any{}, []string{"nonexistent"}) + cmd := createRemoveTestCLICommand(t, map[string]any{}, []string{"nonexistent"}) var buf bytes.Buffer err := removeCommandWithCommandExecutor(cmd, &buf, mockExec, "/repo", "nonexistent", false, false, false) @@ -424,7 +424,7 @@ func TestRemoveCommand_FailsWhenRemovingCurrentWorktree(t *testing.T) { }, } - cmd := createRemoveTestCLICommand(map[string]any{}, []string{"feature/foo"}) + cmd := createRemoveTestCLICommand(t, map[string]any{}, []string{"feature/foo"}) var buf bytes.Buffer err := removeCommandWithCommandExecutor(cmd, &buf, mockExec, tt.cwd, "feature/foo", false, false, false) @@ -454,7 +454,7 @@ func TestRemoveCommand_ExecutionError(t *testing.T) { errorMsg: "git command failed", } - cmd := createRemoveTestCLICommand(map[string]any{}, []string{"feature-branch"}) + cmd := createRemoveTestCLICommand(t, map[string]any{}, []string{"feature-branch"}) var buf bytes.Buffer err := removeCommandWithCommandExecutor(cmd, &buf, mockExec, "/test/repo", "feature-branch", false, false, false) @@ -527,7 +527,7 @@ func TestRemoveCommand_DirtyWorktree(t *testing.T) { flags["force"] = true } - cmd := createRemoveTestCLICommand(flags, []string{"dirty-feature"}) + cmd := createRemoveTestCLICommand(t, flags, []string{"dirty-feature"}) var buf bytes.Buffer err := removeCommandWithCommandExecutor( @@ -617,13 +617,13 @@ func TestRemoveCommand_BranchRemovalWithUnmergedCommits(t *testing.T) { } flags := map[string]any{ - "branch": true, + "with-branch": true, } if tt.forceBranchFlag { flags["force-branch"] = true } - cmd := createRemoveTestCLICommand(flags, []string{"feature-unmerged"}) + cmd := createRemoveTestCLICommand(t, flags, []string{"feature-unmerged"}) var buf bytes.Buffer err := removeCommandWithCommandExecutor( @@ -692,7 +692,7 @@ func TestRemoveCommand_InternationalCharacters(t *testing.T) { // Extract the basename from the path for matching worktreeName := filepath.Base(tt.worktreePath) - cmd := createRemoveTestCLICommand(map[string]any{}, []string{worktreeName}) + cmd := createRemoveTestCLICommand(t, map[string]any{}, []string{worktreeName}) var buf bytes.Buffer err := removeCommandWithCommandExecutor(cmd, &buf, mockExec, "/test/repo", worktreeName, false, false, false) @@ -721,7 +721,7 @@ func TestRemoveCommand_PathWithSpaces(t *testing.T) { }, } - cmd := createRemoveTestCLICommand(map[string]any{}, []string{"feature branch"}) + cmd := createRemoveTestCLICommand(t, map[string]any{}, []string{"feature branch"}) var buf bytes.Buffer err := removeCommandWithCommandExecutor(cmd, &buf, mockExec, "/path/to/main", "feature branch", false, false, false) @@ -777,7 +777,7 @@ branch refs/heads/test-feature }, } - cmd := createRemoveTestCLICommand(map[string]any{}, []string{tt.input}) + cmd := createRemoveTestCLICommand(t, map[string]any{}, []string{tt.input}) var buf bytes.Buffer err := removeCommandWithCommandExecutor(cmd, &buf, mockExec, "/test/repo", tt.input, false, false, false) @@ -792,41 +792,12 @@ branch refs/heads/test-feature // ===== Helper Functions ===== -func createRemoveTestCLICommand(flags map[string]any, args []string) *cli.Command { - app := &cli.Command{ - Name: "test", - Commands: []*cli.Command{ - { - Name: "remove", - Flags: []cli.Flag{ - &cli.BoolFlag{Name: "force"}, - &cli.BoolFlag{Name: "branch"}, - &cli.BoolFlag{Name: "force-branch"}, - }, - Action: func(_ context.Context, _ *cli.Command) error { - return nil - }, - }, - }, - } - - cmdArgs := []string{"test", "remove"} - for key, value := range flags { - switch v := value.(type) { - case bool: - if v { - cmdArgs = append(cmdArgs, "--"+key) - } - case string: - cmdArgs = append(cmdArgs, "--"+key, v) - } - } - cmdArgs = append(cmdArgs, args...) - - ctx := context.Background() - _ = app.Run(ctx, cmdArgs) - - return app.Commands[0] +func createRemoveTestCLICommand(t *testing.T, flags map[string]any, args []string) *cli.Command { + return createTestSubcommand(t, "remove", []cli.Flag{ + &cli.BoolFlag{Name: "force"}, + &cli.BoolFlag{Name: "with-branch"}, + &cli.BoolFlag{Name: "force-branch"}, + }, flags, args) } // ===== Mock Implementations ===== diff --git a/cmd/wtp/testhelpers_test.go b/cmd/wtp/testhelpers_test.go index bfd106e..123f73b 100644 --- a/cmd/wtp/testhelpers_test.go +++ b/cmd/wtp/testhelpers_test.go @@ -2,12 +2,14 @@ package main import ( "bytes" + "context" "io" "os" "path/filepath" "testing" "github.com/stretchr/testify/assert" + "github.com/urfave/cli/v3" "github.com/satococoa/wtp/v2/internal/config" ) @@ -66,3 +68,45 @@ func RunNameFromPathTests( assert.Equal(t, "../../../../completely/different/path", name) }) } + +func createTestSubcommand( + t *testing.T, + commandName string, + flags []cli.Flag, + flagValues map[string]any, + args []string, +) *cli.Command { + t.Helper() + + app := &cli.Command{ + Name: "test", + Commands: []*cli.Command{ + { + Name: commandName, + Flags: flags, + Action: func(_ context.Context, _ *cli.Command) error { + return nil + }, + }, + }, + } + + cmdArgs := []string{"test", commandName} + for key, value := range flagValues { + switch v := value.(type) { + case bool: + if v { + cmdArgs = append(cmdArgs, "--"+key) + } + case string: + cmdArgs = append(cmdArgs, "--"+key, v) + } + } + cmdArgs = append(cmdArgs, args...) + + if err := app.Run(context.Background(), cmdArgs); err != nil { + t.Fatalf("app.Run failed: %v", err) + } + + return app.Commands[0] +} diff --git a/internal/errors/errors.go b/internal/errors/errors.go index ce8b453..971ae7b 100644 --- a/internal/errors/errors.go +++ b/internal/errors/errors.go @@ -4,6 +4,7 @@ package errors import ( "errors" "fmt" + "sort" "strings" ) @@ -43,7 +44,7 @@ Usage: %s Examples: • wtp add feature/auth • wtp add -b new-feature - • wtp add --track origin/main main`, commandExample) + • wtp add -b new-feature --quiet`, commandExample) return errors.New(msg) } @@ -102,7 +103,9 @@ func WorktreeCreationFailed(path, branch string, gitError error) error { msg += ` Cause: Branch is already checked out in another worktree -Solution: Use '--force' flag to allow multiple checkouts, or choose a different branch` +Solutions: + • Choose a different branch + • Remove the existing worktree first` } else if strings.Contains(gitErrorStr, "not a valid object name") { msg += ` @@ -229,8 +232,7 @@ func ConfigAlreadyExists(configPath string) error { Options: • Edit the existing file manually - • Delete it and run 'wtp init' again - • Use 'wtp init --force' to overwrite (if that flag exists)`, configPath) + • Delete it and run 'wtp init' again`, configPath) return errors.New(msg) } @@ -300,16 +302,24 @@ Suggestions: return errors.New(msg) } -// MultipleBranchesFound reports that a branch name matches multiple remotes and needs a track specifier. +// MultipleBranchesFound reports that a branch name matches multiple remotes and needs manual disambiguation. func MultipleBranchesFound(branchName string, remotes []string) error { - msg := fmt.Sprintf("branch '%s' exists in multiple remotes: %s", branchName, strings.Join(remotes, ", ")) - msg += fmt.Sprintf(` + sortedRemotes := append([]string(nil), remotes...) + sort.Strings(sortedRemotes) -Solution: Specify the remote explicitly: - • wtp add --track %s/%s %s`, remotes[0], branchName, branchName) + msg := fmt.Sprintf("branch '%s' exists in multiple remotes", branchName) + if len(sortedRemotes) > 0 { + msg += fmt.Sprintf(": %s", strings.Join(sortedRemotes, ", ")) + } - if len(remotes) > 1 { - msg += fmt.Sprintf("\n • wtp add --track %s/%s %s", remotes[1], branchName, branchName) + msg += "\n\nSolution: Create a local tracking branch for the remote you want " + + "without checking it out, then run wtp add again." + if len(sortedRemotes) > 0 { + msg += "\n\nExamples (choose one remote):" + for _, remote := range sortedRemotes { + msg += fmt.Sprintf("\n • git branch --track %s %s/%s", branchName, remote, branchName) + } + msg += fmt.Sprintf("\n • wtp add %s", branchName) } return errors.New(msg) diff --git a/internal/errors/errors_test.go b/internal/errors/errors_test.go index 3ff871e..7126cf3 100644 --- a/internal/errors/errors_test.go +++ b/internal/errors/errors_test.go @@ -76,6 +76,7 @@ func TestBranchNameRequired(t *testing.T) { assert.Contains(t, err.Error(), "branch name is required") assert.Contains(t, err.Error(), commandExample) assert.Contains(t, err.Error(), "wtp add feature/auth") + assert.Contains(t, err.Error(), "wtp add -b new-feature --quiet") assert.Contains(t, err.Error(), "Examples:") } @@ -159,7 +160,8 @@ func TestWorktreeCreationFailed(t *testing.T) { expected: []string{ "failed to create worktree at '/path/to/worktree' for branch 'feature/auth'", "already checked out", - "--force", + "Choose a different branch", + "Remove the existing worktree first", "Original error:", }, }, @@ -306,6 +308,8 @@ func TestConfigAlreadyExists(t *testing.T) { assert.Contains(t, err.Error(), "configuration file already exists") assert.Contains(t, err.Error(), path) assert.Contains(t, err.Error(), "Options:") + assert.Contains(t, err.Error(), "Delete it and run 'wtp init' again") + assert.NotContains(t, err.Error(), "wtp init --force") } func TestDirectoryAccessFailed(t *testing.T) { @@ -389,14 +393,17 @@ func TestBranchNotFound(t *testing.T) { func TestMultipleBranchesFound(t *testing.T) { branchName := "feature" - remotes := []string{"origin", "upstream"} + remotes := []string{"upstream", "origin"} err := MultipleBranchesFound(branchName, remotes) assert.Error(t, err) - assert.Contains(t, err.Error(), "branch 'feature' exists in multiple remotes") + assert.Contains(t, err.Error(), "branch 'feature' exists in multiple remotes: origin, upstream") assert.Contains(t, err.Error(), "origin") assert.Contains(t, err.Error(), "upstream") - assert.Contains(t, err.Error(), "Specify the remote explicitly") + assert.Contains(t, err.Error(), "Create a local tracking branch for the remote you want without checking it out") + assert.Contains(t, err.Error(), "git branch --track feature origin/feature") + assert.Contains(t, err.Error(), "git branch --track feature upstream/feature") + assert.Contains(t, err.Error(), "wtp add feature") } func TestHookExecutionFailed(t *testing.T) {