fix: resolve shell hook interference with tab completion #25
Conversation
The shell hook function was intercepting --generate-shell-completion calls, preventing the completion system from working correctly. When zsh's completion system invoked the command, the hook would try to execute cd logic instead of passing through to the binary. Additionally, the asterisk marker (*) used to indicate the current worktree was being included in completion output, causing shell glob expansion issues. Changes: - Hook functions now detect and pass through completion requests - Completion output strips asterisk markers before display - Fixes apply to bash, zsh, and fish hooks Fixes tab completion for 'wtp cd <tab>' command.
WalkthroughUpdates the cd worktrees output to strip trailing asterisks before printing, and augments generated shell hook scripts (bash/zsh/fish) to detect and forward the --generate-shell-completion flag directly to the main wtp command. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User Shell
participant H as Hook Script (bash/zsh/fish)
participant W as wtp Binary
rect rgb(245,248,252)
Note over U,H: Startup / user command
U->>H: source hook + run shell function
alt Arg includes --generate-shell-completion (new)
H->>W: wtp --generate-shell-completion
W-->>H: completion script output
H-->>U: prints completion data
else Normal invocation
H->>W: wtp [other args]
W-->>H: command output
H-->>U: forwards output
end
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
cmd/wtp/cd.go (1)
365-371: Consider checking scanner error for robustness.The scanner error is not checked after the loop completes. While scanner errors are rare for in-memory buffers and the function signature does not allow error propagation, checking
scanner.Err()would make the implementation more complete.Add after the loop:
for scanner.Scan() { line := scanner.Text() // Remove trailing asterisk that marks current worktree line = strings.TrimSuffix(line, "*") fmt.Println(line) } + // Note: scanner errors cannot be propagated due to completion handler signature + if err := scanner.Err(); err != nil { + // Could log here if logging is available in completion context + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
cmd/wtp/cd.go(1 hunks)cmd/wtp/hook.go(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Run gofmt and goimports; keep imports grouped and organized; local import prefix follows module path github.com/satococoa/wtp
Adhere to golangci-lint rules configured for the project (vet, staticcheck, gosec, mnd, lll=120, etc.)
Errors should be wrapped with context and must not be ignored
Use snake_case for Go filenames; document exported identifiers when non-trivial
Files:
cmd/wtp/hook.gocmd/wtp/cd.go
cmd/wtp/**
📄 CodeRabbit inference engine (AGENTS.md)
cmd/wtp/**: CLI entrypoint and commands are implemented under cmd/wtp
Update CLI help text to reflect user-facing changes
Command behavior: wtp cd prints only the absolute worktree path with no side effects
Command behavior: wtp completion generates pure completion scripts via urfave/cli
Command behavior: wtp hook emits shell functions that intercept wtp cd; wtp shell-init combines completion and hook output
Files:
cmd/wtp/hook.gocmd/wtp/cd.go
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
PR: satococoa/wtp#0
File: AGENTS.md:0-0
Timestamp: 2025-10-07T15:56:11.502Z
Learning: Applies to cmd/wtp/** : Command behavior: wtp completion <shell> generates pure completion scripts via urfave/cli
Learnt from: CR
PR: satococoa/wtp#0
File: AGENTS.md:0-0
Timestamp: 2025-10-07T15:56:11.502Z
Learning: Applies to cmd/wtp/** : Command behavior: wtp hook <shell> emits shell functions that intercept wtp cd; wtp shell-init <shell> combines completion and hook output
Learnt from: CR
PR: satococoa/wtp#0
File: AGENTS.md:0-0
Timestamp: 2025-10-07T15:56:11.502Z
Learning: Applies to cmd/wtp/** : Command behavior: wtp cd prints only the absolute worktree path with no side effects
📚 Learning: 2025-10-07T15:56:11.502Z
Learnt from: CR
PR: satococoa/wtp#0
File: AGENTS.md:0-0
Timestamp: 2025-10-07T15:56:11.502Z
Learning: Applies to cmd/wtp/** : Command behavior: wtp hook <shell> emits shell functions that intercept wtp cd; wtp shell-init <shell> combines completion and hook output
Applied to files:
cmd/wtp/hook.go
📚 Learning: 2025-10-07T15:56:11.502Z
Learnt from: CR
PR: satococoa/wtp#0
File: AGENTS.md:0-0
Timestamp: 2025-10-07T15:56:11.502Z
Learning: Applies to cmd/wtp/** : Command behavior: wtp completion <shell> generates pure completion scripts via urfave/cli
Applied to files:
cmd/wtp/hook.go
📚 Learning: 2025-10-07T15:56:11.502Z
Learnt from: CR
PR: satococoa/wtp#0
File: AGENTS.md:0-0
Timestamp: 2025-10-07T15:56:11.502Z
Learning: Applies to cmd/wtp/completion.go : Use getWorktreeNameFromPath() to resolve worktree display names consistently across completion, errors, and parsing
Applied to files:
cmd/wtp/cd.go
📚 Learning: 2025-10-07T15:56:11.502Z
Learnt from: CR
PR: satococoa/wtp#0
File: AGENTS.md:0-0
Timestamp: 2025-10-07T15:56:11.502Z
Learning: Applies to cmd/wtp/** : Command behavior: wtp cd prints only the absolute worktree path with no side effects
Applied to files:
cmd/wtp/cd.go
🔇 Additional comments (4)
cmd/wtp/cd.go (1)
365-371: Asterisk stripping correctly prevents shell glob expansion.The implementation correctly strips trailing asterisks from worktree names before printing them for completion. This prevents the shell from interpreting patterns like
@*as glob expressions, which was causing the completion failure described in the PR.cmd/wtp/hook.go (3)
76-82: LGTM: Completion passthrough correctly prevents hook interference.The early detection of
--generate-shell-completionensures that completion requests bypass the hook's cd logic and reach the binary directly. This fixes the completion issue where the hook was attempting to execute cd logic instead of delegating to the completion system.
105-111: LGTM: Consistent completion handling for zsh.The zsh hook implementation mirrors the bash approach with appropriate shell-specific syntax. The completion passthrough logic is correctly implemented.
134-140: LGTM: Fish shell syntax correctly adapted.The fish hook implementation correctly adapts the completion passthrough logic using fish-specific syntax (
$argv,test,$status,end). The behavior is consistent with the bash and zsh implementations.
|
Thank you for your contribution! I’ve just merged PR #27, which resolves the regression this PR highlighted. With that merged I’ll go ahead and close this PR. Really appreciate you taking the time to open it! |
2 participants
Summary
Fixes tab completion for
wtp cd <tab>by resolving two issues:Problem
When using
wtp completion zshwithwtp hook zsh, tab completion failed because:wtp()shell function intercepted--generate-shell-completionflag*markers (e.g.,@*) which caused shell expansion errorsSolution
Hook functions (bash, zsh, fish)
--generate-shell-completionflagcommand wtpwhen detectedCompletion output (cd.go)
*marker from worktree nameslistcommand but breaks completionTest Plan
Manual testing completed:
wtp cd <tab>shows all worktreeswtp cd <worktree>navigates correctlyFiles Changed
cmd/wtp/cd.go: Strip asterisk from completion outputcmd/wtp/hook.go: Pass through completion requests in all hooksSummary by CodeRabbit