Repository navigation
fix: preserve option-like feature descriptions - #5124
WOLIKIMCHENG wants to merge 1 commit into
Conversation
Assisted-by: Codex CLI (model: gpt-6.1-sol, autonomous)
yxshee
left a comment
There was a problem hiding this comment.
I checked this against main (7370e59). The change is right: in bash, $(echo "-n") is empty while $(printf '%s\n' "-n") is -n, so the trim step was the only thing turning these descriptions into the "cannot be empty" error.
I also looked for the same pattern elsewhere so a follow-up isn't needed. Both places that pipe the raw description through echo are covered here: scripts/bash/create-new-feature.sh:91 and extensions/git/scripts/bash/create-new-feature-branch.sh:103. The other $(echo "$var" | ...) calls in those two scripts (create-new-feature.sh:122, create-new-feature-branch.sh:120 and :155) only see directory or branch names already matched against ^[0-9]{3,}-, so they can't start with a dash. In generate_branch_name, echo "$result" and echo "$cleaned" only get words split out by unicode_words or the output of clean_branch_name, and neither one starts with -.
The branch merges cleanly with current main, and it also merges cleanly alongside #5201, which edits the same git extension script.
Posted on behalf of @yxshee. AI disclosure: Claude Code (model: Claude Opus 5.5, autonomous) read the diff, searched the bash scripts on main for the same pattern, test-merged the branch, and drafted this review. I did not run the PR branch's tests.
Generated by Claude Code
There was a problem hiding this comment.
🟡 Changes recommended
The parity test compares incompatible PowerShell and Bash persistence hints and assumes matching Windows and MSYS paths, risking failures under Git-for-Windows Bash.
What changed in this PR
This PR fixes #5096 by preserving option-like feature descriptions in the Bash scripts, aligning their behavior with Python.
Changes:
- Replace
echowithprintfwhen trimming descriptions in both feature-creation scripts. - Add parity tests for literal descriptions, whitespace rejection, creation, and dry-run behavior.
- Bump the bundled Git extension to version 1.0.2.
| File | Description |
|---|---|
tests/test_create_new_feature_python_parity.py |
Add core-script regression and parity coverage. |
tests/extensions/git/test_git_extension.py |
Update the expected extension version. |
tests/extensions/git/test_git_extension_python_parity.py |
Add branch-hook parity and side-effect checks. |
scripts/bash/create-new-feature.sh |
Preserve option-like descriptions during trimming. |
extensions/git/scripts/bash/create-new-feature-branch.sh |
Apply the same trimming fix to branch creation. |
extensions/git/extension.yml |
Bump the extension version to 1.0.2. |
extensions/catalog.json |
Synchronize the catalog version. |
Review effort: Balanced (auto)
| assert normalize_repo_paths(py.stderr, repo_b) == normalize_repo_paths( | ||
| bash.stderr, repo_a | ||
| ) |
|
Could you split this into two independently mergeable PRs: one for the Git extension fix, keeping its tests, version bump, and catalog update together, and one for the core script fix and its regression tests? Neither fix depends on the other, so this would let us review and land the extension change separately while assessing the core change on its own evidence. The Windows parity concern raised in this review belongs with the core PR; please address the shell-specific persistence hints and native/MSYS path differences there. AI disclosure: Drafted for @mnriem using GitHub Copilot, powered by GPT-6.1 Sol, in an interactive, user-directed session; AI inspected the diff and review feedback and drafted this response. |

Description
-n,-e, and-Ein the Bash feature script and Git branch hook, matching Python behavior.Fixes #5096.
Testing
-ncreated branch001-nand the expected spec, state, and checklist.bash -nandgit diff --checkpassed.AI Disclosure
AI disclosure: AI-assisted tools were used to implement the fix and develop tests.