Skip to content

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

Open
terminalchai wants to merge 1 commit intounjs:mainfrom
terminalchai:fix/regex-hyphen-escape
Open

fix(_utils): move hyphens to start of character classes in parseGitURI regex#256
terminalchai wants to merge 1 commit intounjs:mainfrom
terminalchai:fix/regex-hyphen-escape

Conversation

@terminalchai
Copy link

@terminalchai terminalchai commented Mar 13, 2026

Summary

Closes #183

The inputRegex in src/_utils.ts had hyphens positioned inside character classes in ways that, while technically valid in ES5 mode, are ambiguous and can cause issues in strict Unicode mode (u/v flags) or with stricter regex linters:

// Before
const inputRegex = /^(?<repo>[\w.-]+\/[\w.-]+)(?<subdir>[^#]+)?(?<ref>#[\w./@-]+)?/;

In [\w.-], the sequence .- looks like a range (though - at end-of-class is treated as a literal in practice). In [\w./@-], similarly @- before ] is ambiguous to readers and tooling.

Fix

Move the hyphen to the start of each character class, where it is unambiguously a literal hyphen in all regex modes — no escape needed, no linter warning:

// After
const inputRegex = /^(?<repo>[-\w.]+\/[-\w.]+)(?<subdir>[^#]+)?(?<ref>#[-\w./@]+)?/;

This is the idiomatic JavaScript pattern (MDN, ECMAScript spec) for a literal hyphen inside [].

Tests

Added two regression cases to test/utils.test.ts that exercise hyphens in the owner/repo name and branch ref:

  • my-org/my-repo{ repo: "my-org/my-repo" }
  • my-org/my-repo#fix/MY-123{ repo: "my-org/my-repo", ref: "fix/MY-123" }

All 11 tests pass, oxlint 0 warnings/errors, oxfmt clean, tsgo --noEmit clean, build clean.

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced Git URI parsing to properly handle repository names and references containing hyphens.

@coderabbitai
Copy link

coderabbitai bot commented Mar 13, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6f70b521-1aee-4a95-918e-3ec29eb140e0

📥 Commits

Reviewing files that changed from the base of the PR and between f1bdc5d and 8d33ca3.

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

📝 Walkthrough

Walkthrough

The PR fixes a regex robustness issue in the Git URI parser by moving dashes to the start of character classes in both the repo and ref segments, ensuring they're treated as literal characters rather than potential range operators. Test cases validate hyphenated owner/repo names and refs parse correctly.

Changes

Cohort / File(s) Summary
Regex Pattern Improvement
src/_utils.ts
Moved dash to the start of character classes in repo and ref regex segments to ensure literal dash interpretation and avoid unintended range interpretation.
Parser Test Coverage
test/utils.test.ts
Added two test cases verifying parseGitURI correctly handles hyphenated inputs: "my-org/my-repo" and "my-org/my-repo#fix/MY-123".

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A dash in the right place, oh what delight,
No more regex ranges causing a fight,
Hyphens now literal, crisp and so clear,
Tests prove the parser works far and near! 🎉

🚥 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 improved robustness.
Linked Issues check ✅ Passed The PR directly addresses issue #183 by fixing the regex character class ambiguity that was causing errors in parseGitURI, with added test coverage for hyphen handling.
Out of Scope Changes check ✅ Passed All changes are within scope: regex fix in src/_utils.ts and regression tests in test/utils.test.ts directly address issue #183.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📝 Coding Plan
  • Generate coding plan for human review comments

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.

Missing escape slash on RegEx in Utils

1 participant