Skip to content

fix(cli): normalize Windows backslashes in registry path matching#2933

Open
itssagarK wants to merge 2 commits into
Karanjot786:mainfrom
itssagarK:fix/cli-registry-windows-paths
Open

fix(cli): normalize Windows backslashes in registry path matching#2933
itssagarK wants to merge 2 commits into
Karanjot786:mainfrom
itssagarK:fix/cli-registry-windows-paths

Conversation

@itssagarK

@itssagarK itssagarK commented Jul 23, 2026

Copy link
Copy Markdown
Contributor
Closes Issue #2932 Under GSSoC

Pull Request Description

### Title
fix(cli): normalize Windows backslashes in registry path matching

### Summary of Changes
- Updated `detectCategory` and `detectPackage` in `scripts/build-registry.ts` to normalize

backslashes with .replace(/\\/g, '/') prior to pattern matching.
- Updated scanDirectory to normalize ROOT and full path strings for accurate relative
path construction.
- Added unit tests in scripts/build-registry.test.ts verifying Windows backslash path
resolution for category detection.

Type of Change

  • 🐛 Bug fix (type:bug)
  • ✨ Feature (type:feature)
  • 📝 Docs (type:docs)
  • 🧪 Tests (type:testing)
  • ♻️ Refactor (type:refactor)
  • 🎨 Design / UX (type:design)
  • ♿ Accessibility (type:accessibility)
  • ⚡ Performance (type:performance)
  • 🔧 DevOps / CI (type:devops)
  • 🔒 Security (type:security)

Checklist

  • ⭐ You starred the repo. The needs-star check blocks your merge otherwise.
  • Tests pass locally: bun vitest run
  • Build passes: bun run build
  • Typecheck passes: bun run typecheck
  • You read CONTRIBUTING.md.
  • Your PR title follows type: short description.
  • Widget state mutators call markDirty() (if your change affects rendering).
  • No new any types without an inline comment explaining why.
  • No unrelated refactors bundled into this PR.

GSSoC 2026 Participation

  • You are a GSSoC 2026 contributor.
  • Your GSSoC profile: https://gssoc.girlscript.org/profile/____

Screenshots / Recordings (UI changes)

Notes for the Reviewer

Summary by CodeRabbit

  • Bug Fixes
    • Improved cross-platform file path handling during registry generation.
    • Corrected category and package detection for Windows-style paths.
    • Ensured generated registry entries use consistent relative paths across operating systems.

@itssagarK
itssagarK requested a review from Karanjot786 as a code owner July 23, 2026 14:06
@github-actions github-actions Bot added type:testing +10 pts. Tests. type:bug +10 pts. Bug fix. labels Jul 23, 2026
@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: d29af2ea-9f10-4ef9-ab19-2fddfdbdddb9

📥 Commits

Reviewing files that changed from the base of the PR and between 452dcec and 368cda5.

📒 Files selected for processing (2)
  • scripts/build-registry.test.ts
  • scripts/build-registry.ts

📝 Walkthrough

Walkthrough

Registry path categorization and directory scanning now normalize Windows backslashes to forward slashes. Tests verify hook detection for a Windows-style path.

Changes

Path normalization

Layer / File(s) Summary
Normalize registry paths
scripts/build-registry.ts, scripts/build-registry.test.ts
detectCategory, detectPackage, and scanDirectory normalize path separators before detection and relative-path generation; tests cover Windows-style hook paths.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related issues

  • #2932 — Addresses Windows path normalization in registry category, package, and directory scanning logic.

Suggested reviewers: karanjot786

🚥 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
Title check ✅ Passed The title is concise, specific, and matches the main change to Windows path normalization.
Description check ✅ Passed The description covers the change, type, checklist, GSSoC, and notes, but omits the package(s) section and a clearer issue link.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@Karanjot786

Copy link
Copy Markdown
Owner

Thanks, but this reintroduces a double-push. scripts/build-registry.ts:374-377 — the new entries.push({ path: relPath, ... }) was added but the pre-existing entries.push({ path: full.replace(ROOT, "")..., ... }) right below it was not removed, so every scanned file is pushed into entries twice (duplicate registry entries with different path strings). Same regression as prior #2118. CI stays green because the tests exercise detectCategory/detectPackage with string literals, never scanDirectory output. Fix: replace the old push, do not add alongside it.

Checklist: [ ] one push per file (old push removed) [ ] a test asserting no duplicate entries

@Karanjot786 Karanjot786 added the quality:needs-work Needs changes before merge. label Jul 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

quality:needs-work Needs changes before merge. type:bug +10 pts. Bug fix. type:testing +10 pts. Tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants