Repository navigation
A command's name is read out of its source path by the glob engine that selected it, and a selected source with no name is reported - #343
Merged
HackingGate merged 1 commit intoOct 10, 2026
Conversation
…at selected it, and a selected source with no name is reported
`commands-resolve` reads each `command_sources` pattern twice: with
`{}` widened to `*` it selects the source files through the same
override matcher as `files.glob`, and then a regex reads the command's
name out of each selected path. That regex was translated from the
pattern by hand, and the translation disagreed with the matcher. It
spelled `/**/` as one directory or more where gitignore semantics read
zero or more, and it anchored a slashless pattern at the root where the
matcher lets it match at any depth. Under the documented example
`*/cmd/{}/**/*.go`, a source directly in `tool/cmd/foo/` was selected,
could not be named, and was dropped by a silent `continue`: the run
printed "1 command(s) discovered", judged only the other command, and
passed the dropped command's documents unread. `{}.go` lost
`deep/x/foo.go` the same way.
`command_name_pattern` in src/scan.rs, with its inner `as_regex`, was
that hand translation; both are removed. A `command_name_pattern` of the
same name now lives in src/selection.rs beside the override builder
whose reading it must match, taking the whole pattern instead of the two
halves around the placeholder, and validation calls it too.
The name regex now comes from globset itself. The pattern goes through
the line normalization of `ignore`'s `GitignoreBuilder::add_line` (the
builder `OverrideBuilder` wraps) and is compiled with that builder's
options; the regex globset produces is used with the alphanumeric
literal that stood for the placeholder swapped for a one-segment
capture. That edit is checked rather than assumed: the pattern with
`*` in the placeholder's place must compile to the identical regex with
`[^/]*` where the capture goes, so anything that stops the placeholder
being a wildcard of its own is refused with the reason. A unit test asks
the override matcher about the same paths, so a later `ignore` that
normalizes differently fails there.
A selected path the name cannot be read from, which can still happen
where the `*` matched nothing (`cmd/.go` under `cmd/{}.go`), is now a
failure listing the path beside whatever the named commands found,
rather than a file that vanished from the count. The reference already
promised that.
Load-time validation keeps refusing `?`, bracket classes and brace
alternation, with the reason corrected: the old comment said only `*`,
`**`, `/` and literal text mean the same to both readings, which was
wrong about `**` and slashless patterns. It now also refuses a `*`
beside the placeholder, a backslash before it, and a leading `!` or `#`,
which selection reads as a doubled star, an escaped star, an exclusion
or a comment. No existing test encoded the old behaviour; one test's
comment restated the wrong claim and is corrected.
Claude-Session: https://claude.ai/code/session_01TG5hbR4Re6gcEZTpBCBymc
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 41 minutes. View limit details
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. Comment |
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (97.51%) is below the target coverage (100.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #343 +/- ##
==========================================
+ Coverage 94.37% 94.41% +0.04%
==========================================
Files 46 46
Lines 23091 23251 +160
==========================================
+ Hits 21792 21953 +161
+ Misses 1299 1298 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
HackingGate
deleted the
command-names-read-the-glob-the-way-selection-does
branch
October 10, 2026 02:06
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
commands-resolvereads eachcommand_sourcespattern twice: with{}widened to*it selects source files through the same override matcher asfiles.glob, and then a regex reads the command's name out of each selected path. That regex was a hand translation of the glob, and it disagreed with selection. It read/**/as one directory or more where gitignore semantics (which selection gets fromignore) read zero or more, and it anchored a slashless pattern at the root where selection matches it at any depth. Under the documented example*/cmd/{}/**/*.go,tool/cmd/foo/main.gowas selected but could not be named, so a silentcontinuedropped it and its README verbs were never judged.{}.golostdeep/x/foo.gothe same way.The name regex is now built from the same normalization
ignoreapplies (the line handling ofGitignoreBuilder::add_line, whichOverrideBuilderwraps), compiled by globset with that builder's options. The placeholder is swapped for an alphanumeric marker, and the result is checked, not assumed: the marker must occur exactly once, the pattern with*in its place must compile to the same regex as the selection glob, and the result must have exactly one capture group. Placements that would change what the pattern means to selection (*{},{}*,\{}, a leading!or#) are refused at load, along with the?, bracket class and brace refusals that were already there. A path that is selected but has no readable name is now a reported Failure; it no longer quietly drops out of the count. The hand translator (command_name_pattern/as_regexin src/scan.rs) is removed.A unit test checks the name reading against
ignore'sOverrideBuilderdirectly, over 11 patterns × 15 paths: a path is named exactly when it is selected. If a laterignorerelease normalizes differently, that test fails.Tests added:
a_doublestar_between_slashes_is_zero_directories_as_well_as_several,a_pattern_without_a_slash_names_a_command_at_any_depth,a_leading_doublestar_is_zero_directories_too,a_leading_slash_anchors_at_the_root_and_is_not_part_of_the_path,the_placeholder_captures_exactly_one_segment,a_trailing_doublestar_is_everything_inside_the_directory,every_path_selection_takes_is_a_path_the_name_reading_namesa_doublestar_reads_as_zero_or_more_directories_when_naming_the_command,a_pattern_with_no_slash_names_a_command_at_any_depth,a_selected_source_the_pattern_cannot_name_is_reported,a_pattern_selection_reads_differently_from_its_text_is_refused_at_loaddocs/REFERENCE.md is updated to describe the shared reading and the refused placements.
https://claude.ai/code/session_01TG5hbR4Re6gcEZTpBCBymc