Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion extensions/git/scripts/bash/create-new-feature-branch.sh
Original file line number Diff line number Diff line change
Expand Up @@ -206,9 +206,18 @@ check_existing_branches() {
}

# Function to clean and format a branch name
#
# Three details keep this byte-identical to the Python and PowerShell twins:
# * LC_ALL=C -- in a UTF-8 locale glibc resolves the a-z *range* through
# collation, so [^a-z0-9] keeps accented lowercase letters that
# re.sub(r"[^a-z0-9]", ...) and .NET's -replace both strip.
# * `--*` instead of the GNU-only `\+`, which POSIX/BSD sed reads as a literal
# '+', leaving repeated separators uncollapsed on macOS.
# * printf instead of echo, so a name of "-n"/"-e"/"-E" is text, not options.
clean_branch_name() {
local name="$1"
echo "$name" | tr '[:upper:]' '[:lower:]' | sed 's/[^a-z0-9]/-/g' | sed 's/-\+/-/g' | sed 's/^-//' | sed 's/-$//'
local -x LC_ALL=C
printf '%s\n' "$name" | tr '[:upper:]' '[:lower:]' | sed 's/[^a-z0-9]/-/g' | sed 's/--*/-/g' | sed 's/^-//' | sed 's/-$//'
}

# ---------------------------------------------------------------------------
Expand Down Expand Up @@ -444,6 +453,10 @@ generate_branch_name() {

local stop_words="^(i|a|an|the|to|for|of|in|on|at|by|with|from|is|are|was|were|be|been|being|have|has|had|do|does|did|will|would|should|could|can|may|might|must|shall|this|that|these|those|my|your|our|their|want|need|add|get|set)$"

# LC_ALL=C for the same collation reason documented on clean_branch_name,
# and so the `grep -qw` acronym probe below uses ASCII word boundaries like
# the Python twin's (?<![0-9A-Za-z_]) lookarounds.
local -x LC_ALL=C

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Confirmed and fixed. "éDBé cache" gave db-cache from bash and cache from the extension's Python twin — LC_ALL=C makes the accent a word boundary for grep -qw,
while Unicode \b treats éDBé as one word.

Aligned to explicit ASCII lookarounds, matching what the core Python twin (scripts/python/create_new_feature.py:185) already did. Also fixed
scripts/powershell/create-new-feature.ps1, which is outside your comment but had the same \b against a core Python twin that already used ASCII boundaries, so core had the
divergence too.

Added test_acronym_adjacent_to_non_ascii_matches_python to the extension parity suite and an acronym_next_to_non_ascii case to test_python_branch_name_generation_matches_bash.

local clean_name=$(printf '%s' "$description" | tr '[:upper:]' '[:lower:]' | sed 's/[^a-z0-9]/ /g')

local meaningful_words=()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -420,11 +420,13 @@ function Get-BranchName {
if ($stopWords -contains $word) { continue }
if ($word.Length -ge 3) {
$meaningfulWords += $word
} elseif ($Description -cmatch "\b$($word.ToUpper())\b") {
} elseif ($Description -cmatch "(?<![0-9A-Za-z_])$($word.ToUpper())(?![0-9A-Za-z_])") {
# Case-sensitive (-cmatch) to mirror the bash twin's case-sensitive
# whole-word acronym match: keep a short word only when its UPPERCASE
# form appears in the original (an acronym). -match is case-insensitive
# and would keep every short word.
# and would keep every short word. ASCII boundaries rather than \b,
# which is Unicode-aware in .NET and would miss an acronym sitting
# next to an accented letter that bash's LC_ALL=C grep still splits on.
$meaningfulWords += $word
}
}
Expand Down
11 changes: 9 additions & 2 deletions extensions/git/scripts/python/create_new_feature_branch.py
Original file line number Diff line number Diff line change
Expand Up @@ -297,8 +297,15 @@ def generate_branch_name(description: str) -> str:
if len(word) >= 3:
meaningful_words.append(word)
# Keep short words only when they appear uppercased in the original
# description (acronyms like "API" or "DB").
elif re.search(rf"\b{re.escape(word.upper())}\b", description):
# description (acronyms like "API" or "DB"). The boundaries are spelled
# out as ASCII rather than using \b: \b is Unicode-aware on str, so
# "\u00e9DB\u00e9 cache" would hide the acronym behind a non-ASCII word
# character, while the bash twin's `grep -qw` runs under LC_ALL=C and
# sees a boundary there.
elif re.search(
rf"(?<![0-9A-Za-z_]){re.escape(word.upper())}(?![0-9A-Za-z_])",
description,
):
meaningful_words.append(word)

if meaningful_words:
Expand Down
17 changes: 15 additions & 2 deletions scripts/bash/create-new-feature.sh
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,18 @@ spec_prefix_exists() {
}

# Function to clean and format a branch name
#
# Three details keep this byte-identical to the Python and PowerShell twins:
# * LC_ALL=C -- in a UTF-8 locale glibc resolves the a-z *range* through
# collation, so [^a-z0-9] keeps accented lowercase letters that
# re.sub(r"[^a-z0-9]", ...) and .NET's -replace both strip.
# * `--*` instead of the GNU-only `\+`, which POSIX/BSD sed reads as a literal
# '+', leaving repeated separators uncollapsed on macOS.
# * printf instead of echo, so a name of "-n"/"-e"/"-E" is text, not options.
clean_branch_name() {
local name="$1"
echo "$name" | tr '[:upper:]' '[:lower:]' | sed 's/[^a-z0-9]/-/g' | sed 's/-\+/-/g' | sed 's/^-//' | sed 's/-$//'
local -x LC_ALL=C
printf '%s\n' "$name" | tr '[:upper:]' '[:lower:]' | sed 's/[^a-z0-9]/-/g' | sed 's/--*/-/g' | sed 's/^-//' | sed 's/-$//'
}

# Fit a feature prefix and suffix within GitHub's branch-name limit.
Expand Down Expand Up @@ -200,7 +209,11 @@ generate_branch_name() {
# Common stop words to filter out
local stop_words="^(i|a|an|the|to|for|of|in|on|at|by|with|from|is|are|was|were|be|been|being|have|has|had|do|does|did|will|would|should|could|can|may|might|must|shall|this|that|these|those|my|your|our|their|want|need|add|get|set)$"

# Convert to lowercase and split into words
# Convert to lowercase and split into words. LC_ALL=C for the same
# collation reason documented on clean_branch_name, and so the `grep -qw`
# acronym probe below uses ASCII word boundaries like the Python twin's
# (?<![0-9A-Za-z_]) lookarounds.
local -x LC_ALL=C
local clean_name=$(printf '%s' "$description" | tr '[:upper:]' '[:lower:]' | sed 's/[^a-z0-9]/ /g')

# Filter words: remove stop words and words shorter than 3 chars (unless they're uppercase acronyms in original)
Expand Down
6 changes: 4 additions & 2 deletions scripts/powershell/create-new-feature.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -145,11 +145,13 @@ function Get-BranchName {
# Keep words that are length >= 3 OR appear as uppercase in original (likely acronyms)
if ($word.Length -ge 3) {
$meaningfulWords += $word
} elseif ($Description -cmatch "\b$($word.ToUpper())\b") {
} elseif ($Description -cmatch "(?<![0-9A-Za-z_])$($word.ToUpper())(?![0-9A-Za-z_])") {
# Keep short words only if they appear as uppercase in original (likely
# acronyms). Use -cmatch so the comparison is case-sensitive, matching the
# bash script's case-sensitive grep; -match would be case-insensitive and
# would keep every short word.
# would keep every short word. The boundaries are spelled out as ASCII
# because .NET's \b is Unicode-aware, so an accented letter next to the
# acronym would suppress the match that bash's LC_ALL=C grep still makes.
$meaningfulWords += $word
}
}
Expand Down
90 changes: 84 additions & 6 deletions tests/extensions/git/test_git_extension_python_parity.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import pytest

from tests.conftest import requires_bash
from tests.parity_helpers import collation_range_locale
from tests.extensions.git.test_git_extension import (
_GIT_ENV,
_init_git,
Expand Down Expand Up @@ -139,20 +140,97 @@ def test_slug_generation_stop_words_and_acronyms(self, tmp_path: Path):
p = _run_py("create-new-feature-branch", py_proj, "--json", "--dry-run", description)
_assert_parity(b, p)

def test_short_name_cleaning(self, tmp_path: Path):
@pytest.mark.parametrize(
"description",
[
"Fix \u00e9DB\u00e9 sync",
"Tune the \u00fcUI\u00fc layer",
],
ids=["db_between_accents", "ui_between_accents"],
)
def test_acronym_adjacent_to_non_ascii_matches_python(
self, tmp_path: Path, description: str
):
"""A short acronym touching an accented letter is kept by both twins.

The bash twin probes for acronyms with `grep -qw` under LC_ALL=C, where
an accented letter is a non-word byte and therefore a word boundary.
Python's \\b and .NET's \\b are Unicode-aware and saw no boundary there,
so the twins disagreed on whether the acronym survived.
"""
bash_proj, py_proj = _twin_projects(tmp_path)
b = _run_bash(
"create-new-feature-branch.sh", bash_proj,
"--json", "--dry-run", description,
)
p = _run_py(
"create-new-feature-branch", py_proj,
"--json", "--dry-run", description,
)
_assert_parity(b, p)

@pytest.mark.parametrize(
("short_name", "expected"),
[
("User_Auth!", "001-user-auth"),
("User__Auth!!", "001-user-auth"),
("auth -- v2", "001-auth-v2"),
],
ids=["single_separators", "repeated_separators", "separator_run"],
)
def test_short_name_cleaning(
self, tmp_path: Path, short_name: str, expected: str
):
# Repeated separators included: the bash twin used to collapse them with
# sed 's/-\+/-/g', a GNU-ism that POSIX/BSD sed reads as a literal '+',
# so the runs survived on macOS.
bash_proj, py_proj = _twin_projects(tmp_path)
b = _run_bash(
"create-new-feature-branch.sh", bash_proj,
"--json", "--dry-run", "--short-name", short_name, "desc",
)
p = _run_py(
"create-new-feature-branch", py_proj,
"--json", "--dry-run", "--short-name", short_name, "desc",
)
_assert_parity(b, p)
assert json.loads(p.stdout)["BRANCH_NAME"] == expected

@pytest.mark.parametrize(
"description",
[
"Añadir autenticación de usuario",
"Ajouter la réservation hôtelière",
],
ids=["spanish", "french"],
)
def test_branch_name_ignores_locale_collation(
self, tmp_path: Path, description: str
):
"""The created git branch must not depend on the caller's locale.

The bash twin sanitizes with sed 's/[^a-z0-9]/-/g'. Under a locale whose
a-z range is collation-ordered that class keeps accented lowercase
letters, so bash checked out 001-ajouter-réservation-hôtelière where
the Python twin checks out 001-ajouter-servation-teli.
"""
locale_name = collation_range_locale()
if locale_name is None:
pytest.skip("no locale with collation-ordered [a-z] ranges available")
env_extra = {"LC_ALL": locale_name, "LANG": locale_name}

bash_proj, py_proj = _twin_projects(tmp_path)
# Single separator runs only: the bash twin's collapse step
# (sed 's/-\+/-/g') is a GNU-ism that BSD sed treats literally.
b = _run_bash(
"create-new-feature-branch.sh", bash_proj,
"--json", "--dry-run", "--short-name", "User_Auth!", "desc",
"--json", "--dry-run", description, env_extra=env_extra,
)
p = _run_py(
"create-new-feature-branch", py_proj,
"--json", "--dry-run", "--short-name", "User_Auth!", "desc",
"--json", "--dry-run", description, env_extra=env_extra,
)
_assert_parity(b, p)
assert json.loads(p.stdout)["BRANCH_NAME"] == "001-user-auth"
branch = json.loads(b.stdout)["BRANCH_NAME"]
assert branch.isascii(), branch

def test_numbering_from_specs_and_branches(self, tmp_path: Path):
bash_proj, py_proj = _twin_projects(tmp_path)
Expand Down
31 changes: 31 additions & 0 deletions tests/parity_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,37 @@ def clean_env() -> dict[str, str]:
return env


def collation_range_locale() -> str | None:
"""A locale whose ``[a-z]`` bracket range is collation-ordered, or ``None``.

glibc resolves a bracket-expression *range* through the locale's collation
table, so under ``en_US.UTF-8`` ``[^a-z0-9]`` leaves accented lowercase
letters alone while ``C.UTF-8`` and the POSIX locale strip them. Probe
``sed`` directly rather than trusting a locale name: the environments where
the divergence cannot be reproduced (no such locale installed, a non-glibc
libc, Git-for-Windows) are exactly the ones where the probe comes back
clean, so the caller can skip.
"""
for name in ("en_US.UTF-8", "en_US.utf8"):
env = clean_env()
env["LC_ALL"] = name
env["LANG"] = name
try:
probe = subprocess.run(
["sed", "s/[^a-z0-9]/-/g"],
input="é\n",
capture_output=True,
text=True,
check=False,
env=env,
)
except OSError: # pragma: no cover - sed missing entirely
return None
if probe.returncode == 0 and "é" in probe.stdout:
return name
return None


def run(
cmd: list[str], repo: Path, env: dict[str, str] | None = None
) -> subprocess.CompletedProcess[str]:
Expand Down
106 changes: 105 additions & 1 deletion tests/test_create_new_feature_python_parity.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
HAS_POWERSHELL,
bash_cmd,
break_wrap_layer,
clean_env,
collation_range_locale,
install_composition_stack,
install_scripts,
json_stdout,
Expand Down Expand Up @@ -117,8 +119,18 @@ def deny_listing(_path: Path):
"I want to add the new API rate limiting feature for users",
"Fix UI for DB sync",
"a to the of",
# An acronym touching an accented letter: bash probes with `grep -qw`
# under LC_ALL=C, where the accent is a word boundary, so the Python
# twin must use explicit ASCII lookarounds rather than a Unicode \b.
"Fix \u00e9DB\u00e9 sync",
],
ids=[
"plain",
"stop_words",
"acronyms",
"all_stop_words_fallback",
"acronym_next_to_non_ascii",
],
ids=["plain", "stop_words", "acronyms", "all_stop_words_fallback"],
)
def test_python_branch_name_generation_matches_bash(
repo: Path, description: str
Expand Down Expand Up @@ -1065,3 +1077,95 @@ def test_all_variants_corrected_prefix_skips_timestamp_collision(repo: Path) ->
assert json_stdout(py)["FEATURE_NUM"] == "20260320"
for result in (bash, ps, py):
assert "using 20260320 instead" in result.stderr


@requires_bash
@pytest.mark.parametrize(
"description",
[
"Añadir autenticación de usuario",
"Prüfung für Benutzer anlegen",
"Ajouter la réservation hôtelière",
],
ids=["spanish", "german", "french"],
)
def test_bash_branch_name_ignores_locale_collation(
repo: Path, description: str
) -> None:
"""Branch naming must not depend on the caller's locale.

``clean_branch_name``/``generate_branch_name`` sanitize with
``sed 's/[^a-z0-9]/-/g'``. Run under a collation-ordered locale that class
keeps accented lowercase letters, so bash produced
``001-ajouter-réservation-hôtelière`` where the Python and PowerShell twins
produce ``001-ajouter-servation-teli``: the same description yielded a
different ``specs/`` directory on two machines that differ only in ``LANG``.
"""
locale_name = collation_range_locale()
if locale_name is None:
pytest.skip("no locale with collation-ordered [a-z] ranges available")

env = clean_env()
env["LC_ALL"] = locale_name
env["LANG"] = locale_name

bash = run(bash_cmd(repo, SCRIPT, "--json", "--dry-run", description), repo, env)
py = run(py_cmd(repo, SCRIPT, "--json", "--dry-run", description), repo, env)

assert py.returncode == bash.returncode == 0
assert json_stdout(py) == json_stdout(bash)
branch = json_stdout(bash)["BRANCH_NAME"]
assert isinstance(branch, str) and branch.isascii(), branch


@requires_bash
@pytest.mark.parametrize(
("short_name", "expected"),
[
("My Fancy!! Name", "001-my-fancy-name"),
("auth -- v2", "001-auth-v2"),
],
ids=["punctuation_run", "separator_run"],
)
def test_bash_collapses_repeated_separators(
repo: Path, short_name: str, expected: str
) -> None:
"""Runs of non-alphanumeric characters collapse to a single hyphen.

The bash twin squeezed them with ``sed 's/-\\+/-/g'``. ``\\+`` is a GNU
extension, not POSIX BRE: BSD ``sed`` (macOS) reads it as a literal ``+``,
so nothing collapsed and the branch became ``001-my-fancy---name``.
"""
bash = run(
bash_cmd(repo, SCRIPT, "--json", "--dry-run", "--short-name", short_name, "x"),
repo,
)
py = run(
py_cmd(repo, SCRIPT, "--json", "--dry-run", "--short-name", short_name, "x"),
repo,
)

assert bash.returncode == py.returncode == 0
assert json_stdout(bash) == json_stdout(py)
assert json_stdout(bash)["BRANCH_NAME"] == expected


@requires_bash
@pytest.mark.parametrize("short_name", ["-n", "-e", "-E"], ids=["n", "e", "E"])
def test_python_dash_prefixed_short_name_matches_bash(
repo: Path, short_name: str
) -> None:
"""A short name that looks like an ``echo`` option is still text.

``clean_branch_name`` piped the raw value through ``echo "$name"``, so bash
consumed ``-n``/``-e``/``-E`` as options and emitted nothing, yielding the
suffix-less ``001-`` where Python yields ``001-n``.
"""
args = ("--json", "--dry-run", "--short-name", short_name, "x")
bash = run(bash_cmd(repo, SCRIPT, *args), repo)
py = run(py_cmd(repo, SCRIPT, *args), repo)

assert py.returncode == bash.returncode == 0
assert json_stdout(py) == json_stdout(bash)
expected = f"001-{short_name.lstrip('-').lower()}"
assert json_stdout(bash)["BRANCH_NAME"] == expected