- Notifications
You must be signed in to change notification settings - Fork216
Anchorsupdate#588
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Anchorsupdate#588
Uh oh!
There was an error while loading.Please reload this page.
Conversation
dnfadmin commentedJan 25, 2021 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
|
As agreed, I added a public RegexOtions.DFA enum that triggers SRM from within the regex Run method. |
… full anchor support (work in progress)
Did a major rewrite of the core SRM matcher engine, now with full anchor support. |
…dfull of test that dont pass seem suspicious/wrong)
| @@ -1,7 +1,7 @@ | |||
| using System; | |||
| using System.Diagnostics; | |||
| namespaceMicrosoft.SRM | |||
| namespaceSystem.Text.RegularExpressions.SRM | |||
| { | |||
| [Serializable] | |||
| internal struct RegexOptions | |||
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.
Now that you're merging the code, seems the SRM versions of IgnoreCase, etc can be replaced by simply using the existing enum? And maybe retaining some of your helper methods here but no more need for any "mapping" from one to the other.
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.
Right, removed SRM.RegexOptions.
| InitializeReferences(); | ||
| } | ||
| /// <summary> | ||
| /// This method is here for perf reasons: if InitializeSRM is NOT called in the | ||
| /// Init method, we don't load SRM.Regex when instantiating a regex that does not use the DFA option |
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.
Now it's all in the same assembly, does no-inlining matter? The type will be loaded anyway, and Jitting will only happen if it ends up needing the code.
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.
Removed the comment and the complier method attribute. Added another note there as a reminder that in fact RightToLeft can be supported now -- fairly easily I think.
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.
I didn't review the SRM parts, but looks good to me
This is great! Thanks! |
I fixed the merge conflict in the project files. |
@veanes thanks, as you saw I did a big merge to bring everything up to date. That probably introduced the file header check as well. |
First major integration of SRM into .NET Regex.
Includes support for anchors and initial unit tests.