-
-
Notifications
You must be signed in to change notification settings - Fork 178
Cross chunk matching #1648
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Cross chunk matching #1648
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refactors the cross-chunk matching logic used by various scrubbers (GuidScrubber, DateScrubber, DirectoryReplacements) into a shared, reusable utility class called CrossChunkMatcher. This eliminates code duplication and provides a consistent approach for matching and replacing patterns that may span across StringBuilder chunk boundaries.
Key Changes
- Introduced
CrossChunkMatcherclass that provides generic pattern matching across StringBuilder chunks - Refactored
GuidScrubberto use the newCrossChunkMatcherinfrastructure - Refactored
DateScrubberto use the newCrossChunkMatcherinfrastructure - Refactored
DirectoryReplacements_StringBuilderto use the newCrossChunkMatcherinfrastructure
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| CrossChunkMatcher.cs | New shared utility class for matching patterns across StringBuilder chunk boundaries; provides generic ReplaceAll method with callbacks for cross-chunk and within-chunk matching |
| GuidScrubber.cs | Refactored to use CrossChunkMatcher; removed duplicate FindMatches logic and internal Match struct |
| DateScrubber.cs | Refactored to use CrossChunkMatcher; replaced ReplaceFixedLength and ReplaceVariableLength methods with CrossChunkMatcher callbacks; renamed parameter from tryConvertDate to tryConvert for consistency |
| DirectoryReplacements_StringBuilder.cs | Refactored to use CrossChunkMatcher; moved FindMatches logic into callbacks; introduced MatchContext class to encapsulate matching state |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/Verify/Serialization/Scrubbers/DirectoryReplacements_StringBuilder.cs
Outdated
Show resolved
Hide resolved
| public static void ReplaceAll<TContext>( | ||
| StringBuilder builder, | ||
| int maxLength, |
Copilot
AI
Nov 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The XML documentation is missing the maxLength parameter. Add a <param name="maxLength"> tag to document this parameter, e.g., <param name="maxLength">Maximum length of patterns to search for</param>.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot open a new pull request to apply changes based on this feedback
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot open a new pull request to apply changes based on this feedback
src/Verify/Serialization/Scrubbers/DirectoryReplacements_StringBuilder.cs
Outdated
Show resolved
Hide resolved
src/Verify/Serialization/Scrubbers/DirectoryReplacements_StringBuilder.cs
Show resolved
Hide resolved
Co-authored-by: Copilot <[email protected]>
…gBuilder.cs Co-authored-by: Copilot <[email protected]>
|
@SimonCropp I've opened a new pull request, #1651, to work on those changes. Once the pull request is ready, I'll request review from you. |
…gBuilder.cs Co-authored-by: Copilot <[email protected]>
|
@SimonCropp I've opened a new pull request, #1652, to work on those changes. Once the pull request is ready, I'll request review from you. |
No description provided.