feat(pattern)!: expand * in command-name patterns as a glob#343
Merged
Conversation
intent(pattern): let rules like `deny: '/* *'` or `allow: 'pre-* --help'` match by command-name prefix instead of being silently dead. Previously only a standalone `*` was a wildcard in command position; partial patterns were compared as literal strings. decision(pattern): re-export `token_matching::literal_matches` as `pub(crate)` from `pattern_matcher::mod` and call it from `CommandPattern::matches`, so command-name and argument-token literals share the same `*`/`\*` semantics. rejected(pattern): duplicating `glob_match`/`unescape_and_match` inside `pattern_parser` — would diverge from the token-position implementation as it evolves.
intent(releases): correct a misleading line that said `'*' --help` and `* --help` parse to the same rule — bare `*` (Wildcard) expands across multiple command-name tokens (e.g. `docker compose --help`) while quoted `'*'` (Literal) only consumes a single token.
intent(releases): move the entry into Highlights and frame it as breaking because rules whose command-name part contains a non-standalone `*` (e.g. `deny: 'g* *'`) silently change from dead to matching every command with that prefix, which can flip an evaluation from `allow` to `deny` for users who already had such patterns.
* in command-name patterns as a glob* in command-name patterns as a glob
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #343 +/- ##
==========================================
- Coverage 89.06% 89.05% -0.01%
==========================================
Files 53 53
Lines 12657 12657
==========================================
- Hits 11273 11272 -1
- Misses 1384 1385 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Code Review
This pull request enables '*' glob expansion for command names in patterns, allowing for more flexible rule definitions. Documentation and release notes have been updated to reflect this change. The reviewer suggests consolidating the newly added unit tests in src/rules/pattern_parser.rs into a single parameterized rstest function to improve maintainability and adhere to the project's style guide.
…st fn intent(pattern): the literal and alternation cases share identical assertion logic — collapse them into a single parameterized function so adding a new case does not require deciding which sibling table to extend.
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
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.
Purpose
deny: '/* *'andallow: 'pre-* --help'*is a wildcard in command position; partial patterns like/*orpre-*are compared as literal strings, so a single rule cannot cover commands invoked by absolute path (e.g./usr/local/bin/foo) or commands sharing a name prefix*inside literal tokens already works for argument tokens (fix(pattern): apply glob matching to literal tokens containing *); only command position was left behindApproach
CommandPattern::matchesthrough the sameliteral_matchesused in token position, so*glob and\*escape semantics apply identically in command positionpattern_matcher::token_matching::literal_matchesfrom thepattern_matchermodule aspub(crate)sopattern_parsercan call itb*ina|b*) is glob-matched independently under the same rulesDesign decisions
Where the glob logic lives
token_matching::literal_matchesvia apub(crate)re-export*/\*behaviour stays in lockstep between token position and command position by constructionglob_match/unescape_and_matchprivately insidepattern_parserBreaking changes
*other than a standalone*now matches strictly more commands than beforedeny: 'g* *'used to be dead (no command is literally namedg*); it now denies every command whose name starts withg'*'in command position (e.g.'*' --help) used to only match a command literally named*; it now matches any single-token command name* --help(Wildcard) still expands across multi-token command names likedocker compose --help, while'*' --help(Literal("*")) consumes only one token — that asymmetry is unchanged*behaviour, escape it:\*works the same in command position as in token position