Skip to content

Match paths with a leading ./ - #54

Open
Bortlesboat wants to merge 2 commits into
hmarr:mainfrom
Bortlesboat:fix-leading-dot-slash
Open

Bortlesboat wants to merge 2 commits into
hmarr:mainfrom
Bortlesboat:fix-leading-dot-slash

Conversation

@Bortlesboat

Copy link
Copy Markdown

Paths like ./src/file.go never matched anchored rules, so codeowners ./src/file.go fell through to the * rule (or showed unowned) while codeowners src/file.go worked. This strips a leading ./ before matching, since paths are relative to the repo root anyway. It also covers .\src\file.go on Windows, because separators are normalised first.

Fixes #17

@hmarr hmarr left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks, the fix looks right. One test suggestion inline.


Generated by Claude Code

Comment thread testdata/patterns.json Outdated
Comment on lines +344 to +349
"name": "unanchored pattern with leading dot-slash path",
"pattern": "*.txt",
"paths": {
"./foo.txt": true,
"./foo/bar.txt": true,
"./foo.go": false

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

These cases pass without the fix: *.txt compiles to \A(?:.+/)?..., and the leading .+/ already matches the ./. So only the /foo/ block above exercises the change, and that only covers the literal-prefix fast path.

Could you swap this for an anchored pattern that goes through the regex path? Something like:

{
   "name": "anchored wildcard pattern with leading dot-slash path",
   "pattern": "/src/*.go",
   "paths": {
      "./src/a.go": true,
      "./lib/src/a.go": false
   }
}

Generated by Claude Code

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.

Good catch, swapped it for /src/*.go. Confirmed it fails without the match.go change and passes with it.

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.

Does not show correct results for file path including leading ./

2 participants