Repository navigation
Split a glob class too large to list into its ranges [patch] - #196
Open
matt-edmondson wants to merge 2 commits into
Open
matt-edmondson wants to merge 2 commits into
matt-edmondson wants to merge 2 commits into
Conversation
ToGlobCharacterList gave up on a class that would list more than 4096 characters, and ResolveGlob read that as a half-typed token to ignore, so a complete class such as "[가-힣a-z]*" matched every item, and as an exclusion hid nothing. A class that is not negated is now split instead: each letter or digit range is matched natively, the rest is listed, and the token matches if any piece does. A negated class, or one whose split would give the token more than 16 alternatives, is listed in full however long it is, which is slower to match but no longer matches everything. A half-typed token is still ignored. Fixes #184 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017vfJW4tA1FYwsQrBAjGqs7
|
There was an error handling pipeline event 0de51f26-d7c3-42f0-bd95-aaa51dfe5fd9. |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Ports the fix from #186 (for #183). The README API test failed on main because the binary-compatibility overloads deliberately carry no defaults, so this PR's CI was red for a reason outside its change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017vfJW4tA1FYwsQrBAjGqs7
|
There was an error handling pipeline event fc412c32-bb93-4c29-9b3b-b251a9ccf69c. |
Contributor
Author
|
The first CI run failed in I ported the fix from open PR #186 into this branch (5ffdca0, identical diff), so this PR can go green on its own. With it, the full suite passes locally: 297 of 297. Whichever of #186 and this PR merges second will see the change already on Generated by Claude Code |
|
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.
Fixes #184
What was wrong
ToGlobCharacterListreturned null for a class that would list more thanMaxGlobClassMembers(4096) characters.ResolveGlobandDoesMatchGlobread null as a half-typed token to ignore. So a complete class such as[가-힣a-z]*matched every item, and as an exclusion it hid nothing.Change
ExpandGlobClassesnow returns the token's alternatives (IReadOnlyList<string>?), andResolveGlobparses each of them. The token matches if any alternative does, the same rule already used for the**/expansions.[가-힣],[a-z]), and the remaining members are listed ([_]).[가-힣a-z]*becomes[가-힣]*and[a-z]*.[A-힣]spans\) are listed in full. Matching a full list is slower: I measured about 15 µs per match against a 55k-member list, versus sub-microsecond natively. In exchange, the token no longer matches everything.file[%-!]) matches every item, while the letter/digit equivalent (file[9-0]) matches none #173 and is left for its own change.Tests
GlobClassTooLargeToListMatchesOnlyItsMembers, covering the issue's cases plus positive, negated, required and excluded variants, andGlobClassTooLargeToListStillLeavesAHalfTypedTokenIgnored.ExpandGlobClassesSplitsAClassTooLargeToListIntoItsRangesandExpandGlobClassesListsAClassTooLargeToListThatCannotBeSplit.[\u0001-�_]moved out of the "rejects" test into the latter.mainworktree: 12 rows and the half-typed test fail there. On this branch they all pass (297 total, 296 passed).IsMatch("README.md", "-[一-龥a-z]*")is true, not false. Glob matching is case-sensitive by default, soREADME.mdisn't a member ofa-zand isn't excluded. The test pins that, and also pins that-[一-龥A-Z]*does exclude it.EveryMethodInTheReadmeApiReferenceExistsWithItsDocumentedDefaultsalso fails onmainwithout this change. It is unrelated and untouched here.The open PRs #165 and #168 touch nearby glob-class code, so whichever of them lands second may need a small rebase.
Generated by Claude Code