Skip to content
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

Augment generated regex XML comment with original pattern and options #88175

Merged
merged 3 commits into from
Jun 29, 2023

Conversation

stephentoub
Copy link
Member

Fixes #88121

Before:
image

After:
image

@ghost
Copy link

ghost commented Jun 29, 2023

Tagging subscribers to this area: @dotnet/area-system-text-regularexpressions
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes #88121

Before:
image

After:
image

Author: stephentoub
Assignees: -
Labels:

area-System.Text.RegularExpressions

Milestone: -

@danmoseley
Copy link
Member

Is the comment in the second image quite correct? It says to match '2' and then begins an alternation, then match 5. I'd expect both in the alternation.

@danmoseley
Copy link
Member

Oh I see it's extracted the common prefix

@danmoseley
Copy link
Member

These could use the same language, though?

  • Match a character in the set [0-4].
  • Match '0' through '9'.

@stephentoub
Copy link
Member Author

These could use the same language, though?

They could... it's recognizing 0-9 as being the same as ECMAScript \d, which is hardcoded to be "'0' through '9'"

@danmoseley
Copy link
Member

I think it would be a little easier to read in fairly everyday cases like this one.

@danmoseley
Copy link
Member

Also just curious why does it call out greedily in (just) some cases, given that's the default?

@stephentoub
Copy link
Member Author

stephentoub commented Jun 29, 2023

Also just curious why does it call out greedily in (just) some cases, given that's the default?

It calls out for all loops (including ones with a min of 0 / max of 1, aka optional) whether they're greedy or lazy.

@stephentoub stephentoub merged commit 41a624d into dotnet:main Jun 29, 2023
@stephentoub stephentoub deleted the augmentxmlcomment branch June 29, 2023 13:52
@ghost ghost locked as resolved and limited conversation to collaborators Jul 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Please add the pattern to the summary of GeneratedRegex partial method
2 participants