Conversation
…l terminal access
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (2)
WalkthroughAdds two-line fzf output parsing and explicit key handling in the init script: captures Changes
Sequence DiagramsequenceDiagram
actor User
participant fzf
participant Shell as init.sh
participant Action as "git gtr AI/Editor"
participant Dir as "Shell (cd)"
User->>fzf: open picker
fzf->>Shell: output two-line selection ("\n" => key + selection)
Shell->>Shell: parse _gtr_key and _gtr_line
alt _gtr_key is ctrl-a or ctrl-e
Shell->>Action: invoke with _gtr_line
Action-->>User: perform AI/editor action
else no special key
Shell->>Dir: extract path from _gtr_line and cd
Dir-->>User: directory changed
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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: 1
🧹 Nitpick comments (1)
tests/init.bats (1)
241-246: Strengthen the fish “enter” assertion to validate parsing, not justcdpresence.This test currently passes on any
cdsubstring and won’t catch_gtr_selection/_gtr_lineparsing regressions. Assert the expected extraction lines explicitly.✅ Suggested test tightening
`@test` "fish fzf enter extracts path from selection and cd" { run cmd_init fish [ "$status" -eq 0 ] - # Fish uses string split or cut to extract path - [[ "$output" == *'cd '* ]] + [[ "$output" == *'set -l _gtr_line "$_gtr_selection[2]"'* ]] + [[ "$output" == *'set dir (string split \t -- "$_gtr_line")[1]'* ]] + [[ "$output" == *'cd $dir'* ]] }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/init.bats` around lines 241 - 246, The test "fish fzf enter extracts path from selection and cd" only checks for the substring "cd " and should instead assert that the shell output shows the actual parsing and a valid extracted path; update the assertions to verify the presence of the selection-parsing variables (_gtr_selection and/or _gtr_line) and that the emitted cd command is followed by a plausible path (e.g., matches "cd " + a path starting with "/", "~", or "."), so replace the loose [[ "$output" == *'cd '* ]] check with explicit checks for _gtr_selection/_gtr_line and a regex/assertion that the cd argument is a non-empty path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/commands/init.sh`:
- Around line 326-338: fzf --expect can return a single element in Fish when the
key is empty, so the current indexing of _gtr_selection (setting _gtr_key to [1]
and _gtr_line to [2]) will be wrong when Enter was pressed; update the handling
around _gtr_selection to detect its length and assign _gtr_key and _gtr_line
accordingly (e.g., if count _gtr_selection is 1 then set _gtr_key to "" and
_gtr_line to _gtr_selection[1], otherwise keep _gtr_key=_gtr_selection[1] and
_gtr_line=_gtr_selection[2]), then proceed with the existing ctrl-a/ctrl-e
branches and the set dir (string split \t -- "$_gtr_line")[1] usage.
---
Nitpick comments:
In `@tests/init.bats`:
- Around line 241-246: The test "fish fzf enter extracts path from selection and
cd" only checks for the substring "cd " and should instead assert that the shell
output shows the actual parsing and a valid extracted path; update the
assertions to verify the presence of the selection-parsing variables
(_gtr_selection and/or _gtr_line) and that the emitted cd command is followed by
a plausible path (e.g., matches "cd " + a path starting with "/", "~", or "."),
so replace the loose [[ "$output" == *'cd '* ]] check with explicit checks for
_gtr_selection/_gtr_line and a regex/assertion that the cd argument is a
non-empty path.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
lib/commands/init.shtests/init.bats
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
lib/commands/init.sh (2)
204-213:⚠️ Potential issue | 🔴 CriticalCritical: same orphaned command substitution in the Zsh block.
Identical to the Bash block — line 205 opens a dangling
$(…)that swallows lines 206–226. Apply the same fix: remove line 205 and keep only thelocaldeclarations for_gtr_keyand_gtr_line.🐛 Proposed fix
- local _gtr_selection _gtr_key _gtr_line - _gtr_selection="$(command git gtr list --porcelain | fzf \ + local _gtr_key _gtr_line local _gtr_porcelain🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/commands/init.sh` around lines 204 - 213, The Zsh block has the same dangling command substitution: remove the stray opening '$(command git gtr list --porcelain | fzf \' (the orphaned command substitution) and instead declare the needed locals, e.g. keep the local declarations for _gtr_key and _gtr_line and set _gtr_porcelain with _gtr_porcelain="$(command git gtr list --porcelain)"; then use printf ... | fzf to populate _gtr_selection as done in the Bash block so the subshells are balanced and no lines are swallowed.
86-95:⚠️ Potential issue | 🔴 CriticalCritical: Lines 86–87 and 204–205 create broken command substitutions in generated Bash and Zsh init functions.
Line 87 (Bash) opens
_gtr_selection="$(command git gtr list --porcelain | fzf \with a continuation, but line 88 containslocal _gtr_porcelain— a shell keyword, not a valid fzf argument. This causes the$(...)command substitution to span lines 87–108, consuming all intermediate statements as fzf arguments, resulting in a syntax error when users eval the output.The same pattern breaks the Zsh block at lines 204–205. Lines 86–87 and 204–205 are leftover orphaned code that must be removed; the actual working fzf invocation is correctly placed at lines 95+ and 213+ respectively.
Proposed fix: Remove lines 87 and 205 entirely; adjust lines 86 and 204 to declare only
local _gtr_key _gtr_line(the redeclaration of_gtr_selectionalready happens later).Proposed diff for Bash
- local _gtr_selection _gtr_key _gtr_line - _gtr_selection="$(command git gtr list --porcelain | fzf \ + local _gtr_key _gtr_line local _gtr_porcelainProposed diff for Zsh (lines 204–206)
- local _gtr_selection _gtr_key _gtr_line - _gtr_selection="$(command git gtr list --porcelain | fzf \ + local _gtr_key _gtr_line local _gtr_porcelain🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/commands/init.sh` around lines 86 - 95, Remove the orphaned, broken command-substitution lines that start a partial fzf invocation and wrongly place shell keywords inside $(...), specifically delete the stray `_gtr_selection="$(command git gtr list --porcelain | fzf \` occurrences in both the Bash and Zsh init blocks; instead ensure the local declaration reads only `local _gtr_key _gtr_line` (do not redeclare `_gtr_selection` there) so the later correct `_gtr_porcelain` + `_gtr_selection="$(printf '%s\n' "$_gtr_porcelain" | fzf \` invocation remains intact (update the Bash function and the Zsh function to remove the stray `_gtr_selection` lines).
♻️ Duplicate comments (1)
lib/commands/init.sh (1)
347-359:⚠️ Potential issue | 🔴 CriticalFish: pressing Enter (cd) is still broken due to empty-line collapse in command substitution.
Fish collapses the empty first line from
fzf --expectin command substitution, so when the user presses Enter,_gtr_selectioncontains only one element._gtr_selection[2]evaluates to empty, causing the function to bail out at line 350 without ever changing directory.Use
string split --allow-emptyto preserve the empty element, or check(count $_gtr_selection)and adjust indices:🐛 Proposed fix
test -z "$_gtr_selection"; and return 0 # --expect gives two lines: key (index 1) and selection (index 2) - set -l _gtr_key "$_gtr_selection[1]" - set -l _gtr_line "$_gtr_selection[2]" + # Fish collapses empty lines; when Enter is pressed, only 1 element remains + if test (count $_gtr_selection) -eq 1 + set -l _gtr_key "" + set -l _gtr_line "$_gtr_selection[1]" + else + set -l _gtr_key "$_gtr_selection[1]" + set -l _gtr_line "$_gtr_selection[2]" + end test -z "$_gtr_line"; and return 0🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/commands/init.sh` around lines 347 - 359, The fzf --expect result collapses the empty first element so _gtr_selection[2] can be missing; update the handling around _gtr_selection, _gtr_key and _gtr_line so the empty line is preserved: either use string split --allow-empty when splitting the command-substitution result into _gtr_selection, or explicitly check (count $_gtr_selection) before indexing and adjust indices (treat a single-element result as Enter/dir-only case). Ensure subsequent uses of _gtr_line and (string split \t -- "$_gtr_line")[1] still work when the selection had an empty second element.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@lib/commands/init.sh`:
- Around line 204-213: The Zsh block has the same dangling command substitution:
remove the stray opening '$(command git gtr list --porcelain | fzf \' (the
orphaned command substitution) and instead declare the needed locals, e.g. keep
the local declarations for _gtr_key and _gtr_line and set _gtr_porcelain with
_gtr_porcelain="$(command git gtr list --porcelain)"; then use printf ... | fzf
to populate _gtr_selection as done in the Bash block so the subshells are
balanced and no lines are swallowed.
- Around line 86-95: Remove the orphaned, broken command-substitution lines that
start a partial fzf invocation and wrongly place shell keywords inside $(...),
specifically delete the stray `_gtr_selection="$(command git gtr list
--porcelain | fzf \` occurrences in both the Bash and Zsh init blocks; instead
ensure the local declaration reads only `local _gtr_key _gtr_line` (do not
redeclare `_gtr_selection` there) so the later correct `_gtr_porcelain` +
`_gtr_selection="$(printf '%s\n' "$_gtr_porcelain" | fzf \` invocation remains
intact (update the Bash function and the Zsh function to remove the stray
`_gtr_selection` lines).
---
Duplicate comments:
In `@lib/commands/init.sh`:
- Around line 347-359: The fzf --expect result collapses the empty first element
so _gtr_selection[2] can be missing; update the handling around _gtr_selection,
_gtr_key and _gtr_line so the empty line is preserved: either use string split
--allow-empty when splitting the command-substitution result into
_gtr_selection, or explicitly check (count $_gtr_selection) before indexing and
adjust indices (treat a single-element result as Enter/dir-only case). Ensure
subsequent uses of _gtr_line and (string split \t -- "$_gtr_line")[1] still work
when the selection had an empty second element.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
lib/commands/init.sh
The merge of main into fix-open-ai inserted new --expect code above the existing porcelain/empty-state logic instead of replacing it, leaving a dangling _gtr_selection subshell and duplicate variable declarations that produced syntax errors in the generated shell code. Move the _gtr_key/_gtr_line declarations to after the empty-state guard and remove the orphaned half-finished assignment lines so bash -n passes cleanly on the generated init output.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
lib/commands/init.sh (1)
343-346:⚠️ Potential issue | 🟠 MajorFish still mis-parses Enter selection from
fzf --expect.When Enter is pressed, Fish can collapse the empty first line, so
_gtr_selection[1]becomes the selected row and_gtr_selection[2]is empty. That makes_gtr_lineempty and exits withoutcd.🔧 Suggested fix
- # --expect gives two lines: key (index 1) and selection (index 2) - set -l _gtr_key "$_gtr_selection[1]" - set -l _gtr_line "$_gtr_selection[2]" + # --expect gives key + selection; Fish may collapse empty first line (Enter) + set -l _gtr_key + set -l _gtr_line + if test (count $_gtr_selection) -eq 1 + set _gtr_key "" + set _gtr_line "$_gtr_selection[1]" + else + set _gtr_key "$_gtr_selection[1]" + set _gtr_line "$_gtr_selection[2]" + end test -z "$_gtr_line"; and return 0#!/usr/bin/env bash # Verify Fish collapsing behavior for empty first line from fzf --expect style output. fish -c ' set -l out (printf "\n/path\tbranch\n") echo "count="(count $out) for i in (seq (count $out)) echo "$i=<$out[$i]>" end 'Also applies to: 348-355
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/commands/init.sh` around lines 343 - 346, The fzf --expect parsing fails in Fish when the leading empty line is collapsed, leaving only one element in _gtr_selection so _gtr_line ends up empty and aborts; change the logic around _gtr_selection handling (variables _gtr_key and _gtr_line) to detect the single-element collapse and treat that single element as the selection: if _gtr_line is empty but _gtr_key is non-empty (or count of _gtr_selection equals 1), move _gtr_key into _gtr_line and clear _gtr_key so the code proceeds to cd; apply the same defensive fix to the analogous block referenced later (the 348-355 area) that parses _gtr_selection.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@lib/commands/init.sh`:
- Around line 343-346: The fzf --expect parsing fails in Fish when the leading
empty line is collapsed, leaving only one element in _gtr_selection so _gtr_line
ends up empty and aborts; change the logic around _gtr_selection handling
(variables _gtr_key and _gtr_line) to detect the single-element collapse and
treat that single element as the selection: if _gtr_line is empty but _gtr_key
is non-empty (or count of _gtr_selection equals 1), move _gtr_key into _gtr_line
and clear _gtr_key so the code proceeds to cd; apply the same defensive fix to
the analogous block referenced later (the 348-355 area) that parses
_gtr_selection.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
lib/commands/init.sh
Fish command substitution collapses empty lines, so when Enter is pressed with --expect, the empty key line disappears and the array has only 1 element instead of 2. Detect this by checking count and adjust indices accordingly. Also strengthen the fish enter test to validate string split + set dir parsing rather than just checking for 'cd '.
2 participants
Pull Request
Description
Motivation
Fixes # (issue)
Type of Change
Testing
Manual Testing Checklist
Tested on:
Core functionality tested:
git gtr new <branch>- Create worktreegit gtr go <branch>- Navigate to worktreegit gtr editor <branch>- Open in editor (if applicable)git gtr ai <branch>- Start AI tool (if applicable)git gtr rm <branch>- Remove worktreegit gtr list- List worktreesgit gtr config- Configuration commands (if applicable)Test Steps
Expected behavior:
Actual behavior:
Breaking Changes
Checklist
Before submitting this PR, please check:
git gtr(production) and./bin/gtr(development)Additional Context
License Acknowledgment
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache License 2.0.
Summary by CodeRabbit
New Features
Tests