[bug-fix] Fix non-latin-feature-names: preserve Unicode feature names - #4780
github-actions[bot] wants to merge 4 commits into
Conversation
mnriem
added
the
triage-can-wait
Replace the initial proposed sanitizer with Unicode-aware name generation across Bash, PowerShell, and Python. Keep UTF-8 branch names within GitHub byte limits, preserve existing punctuation-only warnings, and update parity tests and documentation. Refs #4574. Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Windows Unicode output currently fails, and the three backends disagree for some Unicode number categories.
Review effort: Balanced
Findings: 1
Open (3)
What changed in this PR
Preserves Unicode feature names consistently across the core creation scripts and documents the updated naming policy.
Changes:
- Retains Unicode letters and digits across Bash, PowerShell, and Python.
- Enforces the 244-byte branch limit on UTF-8 boundaries.
- Adds cross-backend regression and parity coverage.
| File | Description |
|---|---|
scripts/bash/create-new-feature.sh |
Adds locale-aware Unicode naming and byte truncation. |
scripts/powershell/create-new-feature.ps1 |
Preserves Unicode categories and safely truncates UTF-8. |
scripts/python/create_new_feature.py |
Retains Unicode names and counts encoded bytes. |
tests/test_create_new_feature_python_parity.py |
Expands Unicode, locale, warning, and truncation tests. |
docs/reference/core.md |
Documents Unicode naming and Bash locale requirements. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
|
||
| def _clean_branch_name(name: str) -> str: | ||
| cleaned = re.sub(r"[^a-z0-9]", "-", name.lower()) | ||
| cleaned = re.sub(r"[^\w]|_", "-", name.translate(_ASCII_LOWER)) |
| 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/-$//' | ||
| local -x LC_ALL="$UNICODE_LOCALE" | ||
| printf '%s\n' "$name" | LC_ALL=C tr '[:upper:]' '[:lower:]' | sed 's/[^[:alnum:]]/-/g' | sed 's/--*/-/g' | sed 's/^-//' | sed 's/-$//' |
| if [ -z "$UNICODE_LOCALE" ]; then | ||
| UNICODE_LOCALE=C | ||
| if printf '%s' "${SHORT_NAME:-$FEATURE_DESCRIPTION}" | LC_ALL=C grep -q '[^ -~]'; then | ||
| echo "Error: A UTF-8 locale is required to create a Unicode feature name" >&2 | ||
| exit 1 |
Use Unicode letters and decimal digits consistently for feature names. Emit Python output as UTF-8 on Windows, decode parity subprocesses as UTF-8, and test the no-locale error and non-decimal number cases. Refs #4574. Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Use Python 3 for non-ASCII character classification where POSIX locale classes vary by platform. Preserve the shell-only ASCII path, report a clear error when a Unicode name lacks Python, and cover both paths. Refs #4574. Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Addressed this review in Validation: the naming/timestamp suites on the PR checkout passed (212 passed, 4 skipped), and the full pytest matrix now passes on Ubuntu, macOS, and Windows for Python 3.13 and 3.14. Ruff, shellcheck, markdownlint, and the other reported checks are green. The existing review request remains pending; I have not resolved the review threads. AI disclosure: On behalf of @mnriem, GitHub Copilot (GPT-6 Sol; autonomous code authoring in an interactive session) authored the follow-up code, tests, documentation, PR-body update, and this comment. Local and CI validations were automated; no human line-by-line review is claimed. |
Bug fix — non-latin-feature-names
Fixes #4574 using the UTF-8 naming policy clarified by the maintainer in the issue.
Summary
Feature descriptions and explicit short names retain Unicode letters and decimal digits across Bash, PowerShell, and Python. Non-Latin descriptions now produce readable branch and feature-directory names instead of
001-. Punctuation-only descriptions retain the existing empty-name warning. The three variants truncate UTF-8 branch names on character boundaries to meet GitHub's 244-byte limit.Changes
scripts/bash/create-new-feature.sh,scripts/powershell/create-new-feature.ps1, andscripts/python/create_new_feature.pyto share the Unicode letter/decimal-digit policy and enforce the byte limit. Bash uses a Python 3 interpreter to classify non-ASCII characters consistently across POSIX locales; ASCII names remain shell-only. Python emits UTF-8 on both stdout and stderr so Windows can report non-Latin feature names.tests/test_create_new_feature_python_parity.pyandtests/parity_helpers.pywith positive and negative cases for non-Latin names, Unicode number categories, CP1252 stdout/stderr, punctuation-only warnings, unavailable UTF-8 locales, unavailable Python for Bash Unicode names, and valid UTF-8 truncation. Correct the faulty expected word identified by the bug-test report and replace pre-fix ASCII-only expectations.docs/reference/core.mdto describe UTF-8 names, decimal-digit policy, and Bash requirements for a UTF-8 locale and Python 3 when using non-ASCII names.Verification and review
001-, expected001-添加用户. Review-round tests also reproduced the Windows CP1252UnicodeEncodeErrorand mismatchedx²category output before their fixes.f2c0f938removes Bash's dependence on locale character classes for Unicode classification.f2c0f938passes for Ubuntu, macOS, and Windows with Python 3.13 and 3.14, plus ruff, shellcheck, markdownlint, and other checks. Human review remains pending; reviewer conversations were left unresolved.Existing descriptions with non-ASCII letters will now generate different names. The optional
extensions/gitscripts are outside this create-new-feature change.Follow-up commits
89cee3e3,461d1583, andf2c0f938were authored autonomously by GitHub Copilot (GPT-6 Sol) on behalf of @mnriem. They update the original automated proposal; local test evidence is not a claim of human review. The initial proposal was generated by the bug-fix workflow below.