fix(completions): remove dead branch referencing undefined variable in bash completion - #28744
fix(completions): remove dead branch referencing undefined variable in bash completion#28744JakubPecenka wants to merge 1 commit into
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📥 CommitsReviewing files that changed from the base of the PR and between 3756d6e1c6d06f1ced816793b6595e31772ba911 and 3e5d275. 📒 Files selected for processing (1)
WalkthroughSimplified Changes
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
completions/bun.bash (1)
56-66:⚠️ Potential issue | 🟡 MinorThe guard prevents the error, but
re_comp_word_scriptis never defined—leaving dead code.The
-nguard on line 60 correctly prevents the regex error. However, sincere_comp_word_scriptis never defined anywhere in this file, the condition will always evaluate to false, making the entire second branch of the OR unreachable dead code.Choose one of these approaches:
- If the variable was intended to be defined, add the missing definition (similar to
re_prev_scripton line 57)—possiblylocal re_comp_word_script="(^| )${cur_word}($| )";- If the branch is unnecessary, remove the dead code entirely for clarity
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@completions/bun.bash` around lines 56 - 66, The conditional uses an undefined variable re_comp_word_script, leaving dead/unreachable code; either define re_comp_word_script (e.g., mirror re_prev_script using cur_word: create a local re_comp_word_script="(^| )${cur_word}($| )" before the conditional) so the second branch can work, or remove the entire second branch that references re_comp_word_script and its related logic (the “-n \"${re_comp_word_script}\" && \"${COMPREPLY[*]}\" =~ ${re_comp_word_script}” part and any code only relevant to that branch) to eliminate the dead code while keeping behavior around re_prev_script, prev, cur_word, package_json_compreply, COMPREPLY and replaced_script unchanged.
🤖 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 `@completions/bun.bash`:
- Around line 56-66: The conditional uses an undefined variable
re_comp_word_script, leaving dead/unreachable code; either define
re_comp_word_script (e.g., mirror re_prev_script using cur_word: create a local
re_comp_word_script="(^| )${cur_word}($| )" before the conditional) so the
second branch can work, or remove the entire second branch that references
re_comp_word_script and its related logic (the “-n \"${re_comp_word_script}\" &&
\"${COMPREPLY[*]}\" =~ ${re_comp_word_script}” part and any code only relevant
to that branch) to eliminate the dead code while keeping behavior around
re_prev_script, prev, cur_word, package_json_compreply, COMPREPLY and
replaced_script unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3e272042-1857-4a7b-bb8f-9176797f065b
📥 Commits
Reviewing files that changed from the base of the PR and between 3ed4186 and 42c00dc8ae7a8e7bf3c3b3f1da62107f6bb79316.
📒 Files selected for processing (1)
completions/bun.bash
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 (1)
completions/bun.bash (1)
1-1: 🧹 Nitpick | 🔵 TrivialPre-existing: Shebang is missing the
!character.The shebang line should be
#!/usr/bin/env bashinstead of#/usr/bin/env bash. This is a pre-existing issue unrelated to this PR, but worth noting for a future fix.Suggested fix
-#/usr/bin/env bash +#!/usr/bin/env bash🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@completions/bun.bash` at line 1, The shebang line at the top of completions/bun.bash is missing the exclamation mark; update the first line from "#/usr/bin/env bash" to the correct shebang "#!/usr/bin/env bash" so the script is executed by the system's bash interpreter when run directly.
🤖 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 `@completions/bun.bash`:
- Line 1: The shebang line at the top of completions/bun.bash is missing the
exclamation mark; update the first line from "#/usr/bin/env bash" to the correct
shebang "#!/usr/bin/env bash" so the script is executed by the system's bash
interpreter when run directly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ec064ca4-a58d-443d-8bf5-c1f127e1c7f2
📥 Commits
Reviewing files that changed from the base of the PR and between 42c00dc8ae7a8e7bf3c3b3f1da62107f6bb79316 and 3756d6e1c6d06f1ced816793b6595e31772ba911.
📒 Files selected for processing (1)
completions/bun.bash
…rd_script `re_comp_word_script` is referenced on line 60 of `completions/bun.bash` but was never defined anywhere in the script (since the original PR oven-sh#403 in 2022). This causes `[[ ... =~ ${re_comp_word_script} ]]` to fail with: -bash: [[: invalid regular expression: empty (sub)expression every time tab completion triggers `_read_scripts_in_package_json` (e.g. `bun run <TAB>` or `bun <TAB>` in a directory with a package.json). Rather than guarding the undefined variable with `-n`, this removes the dead branch entirely. Since `re_comp_word_script` was never defined, the condition always evaluated to false — the branch was unreachable dead code. The first branch already correctly handles filtering scripts from completions once a script has been selected as the previous word.
3756d6e to
3e5d275
Compare
JakubPecenka
changed the title
|
Thanks for the fix, and sorry it sat this long. #26743, opened in February, removes the same dead |
2 participants
What does this PR do?
Fixes bash tab completion error:
-bash: [[: invalid regular expression: empty (sub)expression.re_comp_word_scriptis referenced on line 60 ofcompletions/bun.bashbut was never defined anywhere in the script (since the original PR #403 in 2022). This causes[[ ... =~ ${re_comp_word_script} ]]to fail with the above error every time tab completion triggers_read_scripts_in_package_json(e.g.bun run <TAB>orbun <TAB>in a directory with apackage.json).Rather than guarding the undefined variable with
-n, this removes the dead branch entirely. Sincere_comp_word_scriptwas never defined, the condition always evaluated to false — the branch was unreachable dead code. The first branch already correctly handles filtering scripts from completions once a script has been selected as the previous word.How did you verify your code works?
Tested with a bash test suite that simulates completion by setting
COMP_WORDS/COMP_CWORDand calling_bun_completionsdirectly — 39 tests covering:bun run <TAB>no stderrinvalid regular expression: empty (sub)expressionbun <TAB>no stderrbun dev <TAB>no stderrbun run dev <TAB>no stderrbun run <TAB>shows scripts)bun run d<TAB>→dev)bun run dev <TAB>filters other scripts)--cwd, no package.json, etc. (36 tests)