Simplify add command interface for better usability#9
Conversation
This change dramatically simplifies the `wtp add` command interface by removing complex features and flags that added cognitive overhead without significant value for most users. ## Removed Features - `--detach` flag and detached HEAD support - `--track` flag (auto-tracking of remote branches is now automatic) - `--force` flag (users can use git worktree directly for edge cases) - `--cd`/`--no-cd` flags (shell integration handles directory switching) - `wtp add <worktree-name> <commit-ish>` pattern (ambiguous with branch names) ## New Simplified Interface - `wtp add <existing-branch>` - Create worktree from existing local/remote branch - `wtp add -b <new-branch>` - Create new branch and worktree from HEAD - `wtp add -b <new-branch> <commit>` - Create new branch from specific commit ## Benefits - Reduced complexity: Only 1 flag instead of 6 - Consistent with git-checkout pattern (`-b` for new branches) - Eliminates ambiguous argument parsing - Maintains all common workflows while removing edge cases - Better error messages and user guidance ## Implementation Details - Removed 100+ lines of complex argument parsing logic - Simplified test suite by removing tests for deleted features - Updated documentation and help text - Enhanced success messages with friendly emojis and guidance - Maintained backward compatibility for core use cases This change follows the principle that software should be easy to use correctly and hard to use incorrectly. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughThe Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant Repo
User->>CLI: wtp add [-b branch] [commitish]
CLI->>CLI: Validate input (branch or -b)
CLI->>Repo: Resolve branch/tracking (if needed)
Repo-->>CLI: Branch/tracking info
CLI->>Repo: Create worktree at path
Repo-->>CLI: Worktree created
CLI->>User: Display success message with switch instructions
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15–20 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧬 Code Graph Analysis (1)cmd/wtp/add.go (5)
🔇 Additional comments (9)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR simplifies the wtp add command interface by removing complex features that added cognitive overhead without significant value. The changes focus on eliminating flags like --detach, --track, --force, and --cd while maintaining core worktree creation functionality through a streamlined interface using only the -b flag for new branch creation.
- Removes 6 command flags and reduces interface to 3 simple patterns: existing branch, new branch, and new branch from commit
- Updates all tests to reflect the simplified interface and removes tests for deleted functionality
- Enhances success messages with friendly emojis and improved user guidance
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/e2e/worktree_test.go | Updates tests to remove detached HEAD and force checkout functionality |
| test/e2e/remote_test.go | Removes --track flag tests and updates remote branch handling tests |
| test/e2e/framework/assertions.go | Updates worktree creation assertion to support new friendly success message format |
| test/e2e/error_test.go | Updates error message expectations to reflect removal of --force and --track flags |
| cmd/wtp/add_test.go | Extensive test updates removing complex flag tests and adding simplified interface tests |
| cmd/wtp/add.go | Core implementation changes removing complex flag handling and simplifying command building |
| README.md | Updates documentation examples to reflect simplified interface |
| CLAUDE.md | Documents the simplification changes and design decisions |
Comments suppressed due to low confidence (1)
cmd/wtp/add_test.go:512
- The createTestCLICommand function still includes flags that were removed from the simplified interface (force, detach, track, cd, no-cd). These should be removed to match the actual simplified command structure.
}
|
|
||
| import ( | ||
| "strings" | ||
| "testing" |
There was a problem hiding this comment.
The removed import "strings" is still needed by the code on line 109 and other locations that use strings.Contains(). This will cause compilation errors.
| "testing" | |
| "testing" | |
| "strings" |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
cmd/wtp/add_test.go (1)
481-486: Remove obsolete flags from test helper.The test helper includes flags (
force,detach,track,cd,no-cd) that were removed according to the PR objectives. These should be removed to maintain consistency with the actual command implementation.Flags: []cli.Flag{ - &cli.BoolFlag{Name: "force"}, - &cli.BoolFlag{Name: "detach"}, &cli.StringFlag{Name: "branch"}, - &cli.StringFlag{Name: "track"}, - &cli.BoolFlag{Name: "cd"}, - &cli.BoolFlag{Name: "no-cd"}, },
🧹 Nitpick comments (3)
cmd/wtp/add.go (2)
118-151: Consider simplifying the argument handling logic.While the logic is correct, the nested conditions make it harder to follow. Consider extracting the commitish resolution logic into a helper function.
func buildWorktreeCommand( cmd *cli.Command, workTreePath, _, resolvedTrack string, ) command.Command { opts := command.GitWorktreeAddOptions{ Branch: cmd.String("branch"), } // Use resolved track if provided if resolvedTrack != "" { opts.Track = resolvedTrack } - var commitish string - - // Handle different argument patterns based on flags - if resolvedTrack != "" { - // When using resolved tracking, the commitish is the remote branch - 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 - opts.Branch = cmd.Args().Get(0) - } - } else if cmd.Args().Len() > 0 { - // Normal case: first argument is the branch/commitish - commitish = cmd.Args().Get(0) - // If branch creation with -b, second arg (if any) is the commitish - if opts.Branch != "" && cmd.Args().Len() > 1 { - commitish = cmd.Args().Get(1) - } - } + commitish := resolveCommitish(cmd, opts, resolvedTrack) return command.GitWorktreeAdd(workTreePath, commitish, opts) } + +func resolveCommitish(cmd *cli.Command, opts command.GitWorktreeAddOptions, resolvedTrack string) string { + if resolvedTrack != "" { + // When using resolved tracking, set branch name from args if not using -b + if cmd.Args().Len() > 0 && opts.Branch == "" { + opts.Branch = cmd.Args().Get(0) + } + return resolvedTrack + } + + if cmd.Args().Len() == 0 { + return "" + } + + // Normal case: first argument is the branch/commitish + if opts.Branch != "" && cmd.Args().Len() > 1 { + // With -b flag, second argument is the commitish + return cmd.Args().Get(1) + } + return cmd.Args().Get(0) +}
370-383: Consider logging errors for debugging.While returning
falseon error is safe, logging the error would help with debugging path resolution issues.func isMainWorktree(workTreePath, mainRepoPath string) bool { absWorkTreePath, err := filepath.Abs(workTreePath) if err != nil { + // Log error for debugging, but continue with safe default + // fmt.Fprintf(os.Stderr, "Warning: failed to get absolute path for %s: %v\n", workTreePath, err) return false } absMainRepoPath, err := filepath.Abs(mainRepoPath) if err != nil { + // Log error for debugging, but continue with safe default + // fmt.Fprintf(os.Stderr, "Warning: failed to get absolute path for %s: %v\n", mainRepoPath, err) return false } return absWorkTreePath == absMainRepoPath }cmd/wtp/add_test.go (1)
516-607: Comprehensive test coverage for simplified interface.The test suite effectively covers all three usage patterns and validation scenarios. The acknowledgment of expected failures in non-git environments is appropriate.
Consider adding a helper to skip git-dependent tests when not in a repository:
func skipIfNotInGitRepo(t *testing.T) { _, err := git.NewRepository(".") if err != nil { t.Skip("Skipping test - not in a git repository") } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
CLAUDE.md(2 hunks)README.md(2 hunks)cmd/wtp/add.go(8 hunks)cmd/wtp/add_test.go(6 hunks)test/e2e/error_test.go(2 hunks)test/e2e/framework/assertions.go(1 hunks)test/e2e/remote_test.go(2 hunks)test/e2e/worktree_test.go(1 hunks)
🔇 Additional comments (17)
test/e2e/worktree_test.go (2)
60-62: LGTM! Clear documentation of removed functionality.The comment clearly explains why the DetachedHead test was removed, aligning with the interface simplification that eliminated the
--detachflag.
63-74: Excellent addition - tests the new error behavior without --force flag.This test case properly verifies that attempting to add the same branch twice now fails with an appropriate error message, since the
--forceflag was removed as part of the interface simplification. The test logic is sound and the error assertion is appropriate.test/e2e/framework/assertions.go (1)
12-17: Well-implemented assertion update with backward compatibility.The updated assertion properly supports the new emoji-enhanced success message format while maintaining backward compatibility with existing message formats. This aligns with the PR's goal of improving user experience with friendly emojis.
README.md (2)
134-134: Documentation properly updated for simplified interface.The example correctly shows the new syntax using
-bflag for creating a new branch from a remote branch, replacing the removed--trackflag. This aligns perfectly with the interface simplification goals.
148-148: Consistent documentation update.Another example properly updated to use the new
-bflag pattern instead of the deprecated--trackflag, maintaining consistency with the simplified interface.test/e2e/error_test.go (2)
68-74: Appropriate update for simplified interface error handling.The test correctly removes expectations for the deprecated
--forceflag while still verifying that helpful error messages are provided when worktrees already exist. This aligns with the interface simplification that removed the--forceflag.
106-113: Proper error message expectations for multiple remotes.The updated test appropriately removes references to the deprecated
--trackflag while still ensuring helpful guidance is provided for handling ambiguous remote branch scenarios. The assertions now focus on the new-bflag pattern and general guidance.test/e2e/remote_test.go (2)
46-51: Excellent update to demonstrate new explicit remote tracking syntax.The test properly shows how to achieve explicit remote tracking using the new simplified interface with
-b <branch> <remote/branch>instead of the deprecated--trackflag. This maintains the same functionality with clearer syntax.
148-174: Well-refactored test suite for simplified interface.The function rename and test updates properly reflect the new simplified interface focus. The new test cases effectively demonstrate creating branches from remotes and commits using the
-bflag pattern, maintaining comprehensive coverage while removing dependencies on deprecated flags.CLAUDE.md (1)
12-12: Inconsistent documentation about--detachflag.Line 12 states that the
--detachflag functionality was "completely removed", but line 87 indicates that the "--detachflag remains for explicit detached HEAD". These statements contradict each other.Please clarify whether the
--detachflag is removed or retained in the simplified interface.Also applies to: 87-87
Likely an incorrect or invalid review comment.
cmd/wtp/add.go (5)
25-31: Clear and helpful usage documentation.The simplified usage text and examples effectively communicate the three supported patterns for the
wtp addcommand.
78-85: Proper error handling for branch tracking resolution.The refactoring to extract branch tracking logic into a separate function improves code organization and maintainability.
343-346: Good use of wrapper function for backward compatibility.The wrapper maintains the existing API while delegating to the more detailed implementation.
348-368: Excellent user-friendly success messaging.The implementation provides clear, visually appealing feedback with appropriate emoji indicators for different states (branch vs commit). The integration with the consistent worktree naming convention is well done.
406-439: Well-structured branch tracking resolution.The function correctly handles the auto-resolution logic with appropriate precondition checks and specific error handling for the multiple remotes case.
cmd/wtp/add_test.go (2)
27-38: Test correctly validates simplified flag set.The test appropriately checks that only the
branchflag remains in the simplified interface.
707-806: Excellent test coverage for enhanced success messages.The tests comprehensively validate all success message scenarios including branch display, detached HEAD, and main worktree cases. The emoji and formatting assertions ensure consistent user experience.
- Remove Recent Changes sections from CLAUDE.md to eliminate documentation contradictions - Improve add command UsageText to avoid confusing vertical bar syntax - Remove unnecessary comment from worktree_test.go 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
1 participant
Summary
This PR dramatically simplifies the
wtp addcommand interface by removing complex features and flags that added cognitive overhead without significant value for most users.Removed Features
--detachflag and detached HEAD support--trackflag (auto-tracking of remote branches is now automatic)--forceflag (users can use git worktree directly for edge cases)--cd/--no-cdflags (shell integration handles directory switching)wtp add <worktree-name> <commit-ish>pattern (ambiguous with branch names)New Simplified Interface
wtp add <existing-branch>- Create worktree from existing local/remote branchwtp add -b <new-branch>- Create new branch and worktree from HEADwtp add -b <new-branch> <commit>- Create new branch from specific commitBenefits
-bfor new branchesImplementation Details
Philosophy
This change follows the principle that software should be easy to use correctly and hard to use incorrectly. By removing rarely-used complex features, we make the tool more approachable for the 90% use case while maintaining power where it matters.
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
--force,--detach,--track) and simplifying input validation.Tests
Documentation