Skip to content

fix: correct rm --all worktree loop for bash (0-indexed arrays) - #28

Merged
siennathesane merged 1 commit into
craigsc:mainfrom
JSap0914:rm-all-bash-array-indexing
Jun 16, 2026
Merged

fix: correct rm --all worktree loop for bash (0-indexed arrays)#28
siennathesane merged 1 commit into
craigsc:mainfrom
JSap0914:rm-all-bash-array-indexing

Conversation

@JSap0914

Copy link
Copy Markdown
Contributor

cmux rm --all (_cmux_rm_all) silently leaves the first worktree behind and prints a spurious Failed: line when run under bash.

Cause

The dirs / branches arrays are built with +=, so they are 0-indexed in bash. But both loops in _cmux_rm_all iterate for (( i = 1; i <= ${#dirs[@]}; i++ )):

  • i starts at 1, so element 0 (the first worktree) is never previewed or removed.
  • i ends at count, which is one past the last valid index; ${dirs[$count]} is empty, so cd "" errors and git worktree remove "" fails, producing a blank Failed: entry and 1 failed.

zsh hid this because its arrays are 1-indexed.

Fix

Change both loops to 0-indexed, strict-less-than bounds:

for (( i = 0; i < ${#dirs[@]}; i++ )); do

This is correct in bash and remains correct in zsh, where ${arr[0]} is empty and ${arr[1..n]} hold the data — iterating 0..n-1 simply visits the first empty slot harmlessly while still covering every real element. (In practice cmux already runs the surrounding code in bash on Linux/WSL.)

Verification

Repro driver: create a temp git repo, add 3 worktrees under the default nested layout, source cmux.sh, and drive the non-interactive _cmux_rm_all path.

Before (unfixed): preview omits the first worktree, removal prints cd: : No such file or directory, a blank Failed:, and Done. 2/3 removed (1 failed). — the first worktree survives.

After (this change): preview lists all three, output is All 3 worktrees removed., and git worktree list shows zero cmux worktrees remaining.

Run on bash 5.2.21.

_cmux_rm_all built its dirs/branches arrays with += (0-indexed in bash) but iterated with for (( i=1; i<=${#dirs[@]}; i++ )). In bash this skips the first worktree and reads an empty element at i==count, producing a blank 'Failed:' line and '1 failed'. zsh masked it (1-indexed arrays). Use 0-indexed, strict-less-than loops; identical behavior in zsh and bash.
Copilot AI review requested due to automatic review settings June 16, 2026 02:37

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Fixes array iteration in _cmux_rm_all so all worktrees/branches are correctly listed and removed without skipping the first entry or indexing past the end.

Changes:

  • Update two for loops to iterate over Bash arrays using 0-based indexing (i=0; i<${#dirs[@]}; i++).
  • Aligns printed and removal logic with correct dirs[$i] / branches[$i] access.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@siennathesane siennathesane left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gotta love off-by-one errors! lgtm 🫶🏼

@siennathesane
siennathesane merged commit 864d41d into craigsc:main Jun 16, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants