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

Add list patterns proposal #3245

Merged
merged 16 commits into from
Mar 19, 2021
Merged

Add list patterns proposal #3245

merged 16 commits into from
Mar 19, 2021

Conversation

alrz
Copy link
Member

@alrz alrz commented Mar 2, 2020

Tracking issue: #3435
Discussion: #1510, #1039, #898
Prototype: dotnet/roslyn@main...alrz:list-patterns
Proposal: Rendered

@gafter
Copy link
Member

gafter commented Mar 3, 2020

Really nice work!

List initializers (e.g. in an object creation expression) look like { 1, 2, 3 }, so by A design principle for Programming With Data in #3107 the pattern form "should" use curly braces instead of square brackets. Do you see issues with that? I don't think there is a clash because a property pattern contains PropertyName: before each pattern.

proposals/list-patterns.md Outdated Show resolved Hide resolved
proposals/list-patterns.md Outdated Show resolved Hide resolved
@alrz
Copy link
Member Author

alrz commented Mar 3, 2020

the pattern form "should" use curly braces instead of square brackets. Do you see issues with that?

Unfortunately, with braces, empty list pattern would be ambigious with property pattern clause.

proposals/list-patterns.md Outdated Show resolved Hide resolved
@gafter gafter mentioned this pull request May 7, 2020
@YairHalberstadt
Copy link
Contributor

IEnumerable support can def be added

I would suggest allowing pattern based enumeration as well, same as foreach.

@alrz

This comment has been minimized.

@alrz

This comment has been minimized.

proposals/list-patterns.md Outdated Show resolved Hide resolved
@alrz

This comment has been minimized.

@alrz alrz closed this Feb 11, 2021
@alrz alrz reopened this Mar 2, 2021
@alrz
Copy link
Member Author

alrz commented Mar 2, 2021

Updated based on the latest LDM decisions. @333fred @jcouv please review.

proposals/list-patterns.md Outdated Show resolved Hide resolved
proposals/list-patterns.md Outdated Show resolved Hide resolved
proposals/list-patterns.md Show resolved Hide resolved
proposals/list-patterns.md Outdated Show resolved Hide resolved
proposals/list-patterns.md Outdated Show resolved Hide resolved
proposals/list-patterns.md Outdated Show resolved Hide resolved
proposals/list-patterns.md Outdated Show resolved Hide resolved
proposals/list-patterns.md Outdated Show resolved Hide resolved
Base automatically changed from master to main March 12, 2021 19:15
proposals/list-patterns.md Outdated Show resolved Hide resolved
proposals/list-patterns.md Outdated Show resolved Hide resolved
proposals/list-patterns.md Outdated Show resolved Hide resolved
Copy link
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done with review pass (iteration 11)

@alrz alrz requested a review from jcouv March 17, 2021 04:52
Copy link
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM Thanks (iteration 12) with minor follow-up comments.

@jcouv
Copy link
Member

jcouv commented Mar 17, 2021

Tagging @333fred for a second look. Thanks

proposals/list-patterns.md Outdated Show resolved Hide resolved
proposals/list-patterns.md Outdated Show resolved Hide resolved
proposals/list-patterns.md Outdated Show resolved Hide resolved
Copy link
Member

@333fred 333fred left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of clarity comments.

@alrz alrz requested a review from a team as a code owner March 19, 2021 10:03
@alrz alrz requested a review from 333fred March 19, 2021 10:06
@jcouv jcouv merged commit b864efc into dotnet:main Mar 19, 2021
@jcouv
Copy link
Member

jcouv commented Mar 19, 2021

Cool. Thanks for writing this up and sorting many questions. We'll start looking at your roslyn-side change now :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

10 participants