Skip to content

fix(scripts): portable uppercase for branch-name acronym retention (bash 3.2)#3192

Open
PascalThuet wants to merge 2 commits into
github:mainfrom
PascalThuet:fix/branch-name-acronym-bash32
Open

fix(scripts): portable uppercase for branch-name acronym retention (bash 3.2)#3192
PascalThuet wants to merge 2 commits into
github:mainfrom
PascalThuet:fix/branch-name-acronym-bash32

Conversation

@PascalThuet

Copy link
Copy Markdown
Contributor

Description

Branch-name generation keeps short uppercase acronyms (e.g. AI) by re-checking the lowercased word against the original description with ${word^^}. That parameter expansion is bash 4+ only; on macOS's default bash 3.2 it errors with bad substitution, so the acronym / short-word retention branch never matches and those words are silently dropped (go AI now yields 001-now instead of 001-ai-now).

Replace ${word^^} with tr '[:lower:]' '[:upper:]', which is portable. Applies to both the core scripts/bash/create-new-feature.sh and the git extension's extensions/git/scripts/bash/create-new-feature-branch.sh. No behavior change on bash 4+.

CI runs on bash 4+/Linux, so the existing tests passed there; the bug only shows on the default macOS shell.

Testing

  • On macOS bash 3.2.57: test_branch_name_short_word_case_sensitivity and test_short_word_retention[go AI now-001-ai-now] now pass (they failed before this fix).
  • Full branch-naming suites green: test_timestamp_branches, test_git_extension, test_branch_numbering (99 passed, 35 skipped).
  • shellcheck --severity=error clean on both scripts.

AI Disclosure

  • I did not use AI assistance for this contribution
  • I did use AI assistance (describe below)

An AI coding agent surfaced the failing tests while running the suite on macOS and pinned the root cause (${word^^} vs bash 3.2); the fix was written and reviewed by me.

Branch-name generation keeps short uppercase acronyms (e.g. "AI") by re-checking
the lowercased word against the original description with ${word^^}. That
parameter expansion is bash 4+ only; on macOS's default bash 3.2 it errors with
"bad substitution", so the acronym/short-word retention branch never matches and
those words are dropped ("go AI now" yields 001-now instead of 001-ai-now). Use
tr '[:lower:]' '[:upper:]' instead, which is portable.

Applies to both the core create-new-feature.sh and the git extension's
create-new-feature-branch.sh. The existing
test_branch_name_short_word_case_sensitivity / test_short_word_retention tests
cover this and now pass on bash 3.2 (CI runs on bash 4+/Linux, so they passed
there already).

(Disclosure: an AI coding agent surfaced the failure while running the suite on
macOS and pinned the root cause; fix written and reviewed by me.)
@PascalThuet PascalThuet requested a review from mnriem as a code owner June 27, 2026 06:17
- core create-new-feature.sh: match the acronym with `grep -qw` (POSIX
  whole-word) instead of `\b...\b` (GNU/BSD-only), matching the git extension
  and dropping a non-POSIX construct.
- lint: add a CI guard rejecting bash 4+ case-modification expansions in *.sh.
  shellcheck assumes bash 4+ from the shebang and can't flag them, and CI has no
  bash-3.2 lane, so this prevents silently re-shipping the macOS regression this
  PR fixes.
- update a stale PowerShell extension comment that cited the removed bash idiom.

(Disclosure: prompted by an AI code review of the PR; written and reviewed by me.)

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.

Pull request overview

This PR fixes branch-name generation on macOS’s default Bash 3.2 by removing Bash-4-only ${var^^} case expansion from the short-word/acronym retention logic, replacing it with a portable tr-based uppercase conversion and a whole-word match.

Changes:

  • Replace ${word^^} with tr '[:lower:]' '[:upper:]' for acronym detection in both core and git-extension Bash scripts.
  • Align matching behavior to use grep -w whole-word matching rather than \b word boundaries.
  • Add a CI lint guard to prevent future use of Bash 4+ case-modification expansions in *.sh files, and document the fix in the changelog.
Show a summary per file
File Description
scripts/bash/create-new-feature.sh Uses portable uppercase conversion + whole-word matching to retain acronyms on Bash 3.2.
extensions/git/scripts/bash/create-new-feature-branch.sh Applies the same portable acronym retention logic in the git extension’s branch script.
extensions/git/scripts/powershell/create-new-feature-branch.ps1 Comment-only clarification about case-sensitive acronym matching parity.
CHANGELOG.md Documents the portability fix.
.github/workflows/lint.yml Adds a CI guard to reject Bash 4+ case-modification expansions in shell scripts.

Review details

Tip

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

  • Files reviewed: 5/5 changed files
  • Comments generated: 2
  • Review effort level: Low

Comment thread CHANGELOG.md

<!-- insert new changelog below this comment -->

- fix(scripts): make branch-name acronym retention portable (uppercase via `tr` instead of bash-4-only `${word^^}`, whole-word match via `grep -w` instead of GNU/BSD-only `\b`), fixing acronym/short-word retention on macOS's default bash 3.2
Comment on lines +61 to +63
- name: Reject bash 4+ case-modification expansions
run: |
matches=$(git ls-files -z -- '*.sh' | xargs -0 grep -nE '\$\{[A-Za-z_][A-Za-z0-9_]*(\[[^]]*\])?(\^|,)' || true)

@mnriem mnriem 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.

Please address Copilot feedback

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