Skip to content

fix(_utils): move hyphens to start of character classes in parseGitURI regex#262

Open
terminalchai wants to merge 2 commits into
unjs:mainfrom
terminalchai:fix/regex-hyphen-escape
Open

fix(_utils): move hyphens to start of character classes in parseGitURI regex#262
terminalchai wants to merge 2 commits into
unjs:mainfrom
terminalchai:fix/regex-hyphen-escape

Conversation

@terminalchai

@terminalchai terminalchai commented Apr 27, 2026

Copy link
Copy Markdown
Contributor

Fixes a regex where hyphens inside character classes [...] were in a position that could be interpreted as a range operator.

Problem

In src/_utils.ts, the parseGitURI regex had character classes with hyphens in non-literal positions:

\\ s
// ambiguous — '-' between chars can mean a range
/[\w.-]/
\\

While V8 currently treats this as a literal hyphen, it is technically undefined behaviour per the spec and triggers lint warnings. ECMA-262 requires hyphens to be first, last, or escaped to be unambiguously literal.

Fix

Move hyphens to the start of each character class so they are unambiguously literal:

\\ s
/[-\w.]/
\\

Tests

Added test cases in \ est/utils.test.ts\ covering repo references that contain hyphens, ensuring they parse correctly.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed parsing of Git repository URLs to correctly support repository names and reference names (branches and tags) containing hyphens.

@coderabbitai

coderabbitai Bot commented Apr 27, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: eb490030-11f6-4e95-9051-6ff3553b088c

📥 Commits

Reviewing files that changed from the base of the PR and between 94e8233 and c84b73d.

📒 Files selected for processing (2)
  • src/_utils.ts
  • test/utils.test.ts

📝 Walkthrough

Walkthrough

The regex patterns in the parseGitURI function are adjusted to reorder character classes in repo and ref segments, moving hyphens to the start of bracket expressions. Two test cases are added to verify correct parsing of git URLs containing hyphens in repository names and reference segments.

Changes

Cohort / File(s) Summary
parseGitURI Regex Update
src/_utils.ts, test/utils.test.ts
Adjusts character class ordering in repo and ref regex patterns (moving hyphens to start of bracket expressions); adds two test cases validating hyphen support in repo names and refs.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

Suggested reviewers

  • pi0

Poem

🐰 A dash of improvement in just the right place,
Hyphens now leading with elegant grace,
Git URIs parse with a flick of the ear,
Each repo and ref becomes wonderfully clear! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: moving hyphens to the start of character classes in the parseGitURI regex for better ECMA-262 compliance.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
⚔️ Resolve merge conflicts
  • Resolve merge conflict in branch fix/regex-hyphen-escape

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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.

2 participants