Skip to content

fix: preserve option-like feature descriptions - #5124

Open
WOLIKIMCHENG wants to merge 1 commit into
github:mainfrom
WOLIKIMCHENG:fix/5096-bash-description-parity
Open

WOLIKIMCHENG wants to merge 1 commit into
github:mainfrom
WOLIKIMCHENG:fix/5096-bash-description-parity

Conversation

@WOLIKIMCHENG

Copy link
Copy Markdown
Contributor

Description

  • Preserve literal option-like descriptions such as -n, -e, and -E in the Bash feature script and Git branch hook, matching Python behavior.
  • Add regression tests and update the bundled Git extension version to 1.0.2.

Fixes #5096.

Testing

  • Relevant Linux tests: 295 passed, 116 skipped.
  • Post-correction focused tests: 19 passed; CRLF-model cases: 14 passed.
  • In a generated Codex/sh project, exact -n created branch 001-n and the expected spec, state, and checklist.
  • bash -n and git diff --check passed.

AI Disclosure

  • I did use AI assistance (described below)

AI disclosure: AI-assisted tools were used to implement the fix and develop tests.

Assisted-by: Codex CLI (model: gpt-6.1-sol, autonomous)
@WOLIKIMCHENG
WOLIKIMCHENG requested a review from mnriem as a code owner October 26, 2026 08:14
@mnriem mnriem added the triage-can-wait Verdict: valid and in-scope but deprioritized; held behind the evidence gate label Oct 26, 2026
@mnriem
mnriem requested a max review from Copilot October 26, 2026 14:17

@yxshee yxshee left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

@mnriem
mnriem requested an auto review from Copilot and removed request for Copilot November 2, 2026 15:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 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.

1 open issue
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 echo with printf when 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)

Comment on lines +1673 to +1675
assert normalize_repo_paths(py.stderr, repo_b) == normalize_repo_paths(
bash.stderr, repo_a
)
@mnriem

mnriem commented Nov 2, 2026

Copy link
Copy Markdown
Collaborator

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.

@mnriem mnriem added the author-awaiting Waiting on author response label Nov 2, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author-awaiting Waiting on author response triage-can-wait Verdict: valid and in-scope but deprioritized; held behind the evidence gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: create-new-feature.sh rejects the feature descriptions -n, -e and -E, which the Python script accepts

4 participants