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

Support leading pipe in match arms #93

Merged
merged 1 commit into from
Oct 4, 2018

Conversation

DJMcNab
Copy link
Contributor

@DJMcNab DJMcNab commented Oct 3, 2018

This adds support for match arms of the form:

<...>
| X | Y => <...>,
| X => <...>,
| 1..2 => <...>,
etc

Implementation discussion

This just naïvely 'eats' a leading pipe if one is available. The equivalent line in the reference libsyntax is in parse_arm.

As noted in the comment linked above, this feature was formally introduced as a result of rust-lang/rfcs#1925. This feature is in active use in the rust-analyzer codebase

I have added some tests for this feature, but maybe more would be required

EDIT: Always looking for feedback - is this PR description over-engineered?

@matklad
Copy link
Member

matklad commented Oct 4, 2018

bors r+

Thanks! This is exactly how I would have done it!

The PR description is very useful, I think I'll use this PR as an example of "how to change grammar" when I get to writing a useful contributing md.

That said, it probably isn't worth the effort to write such a detailed description every time, though linking to the origninal libsyntax2 is always helpful.

bors bot added a commit that referenced this pull request Oct 4, 2018
93: Support leading pipe in match arms r=matklad a=DJMcNab

This adds support for match arms of the form:
```rust
<...>
| X | Y => <...>,
| X => <...>,
| 1..2 => <...>,
etc
```

# Implementation discussion

This just naïvely 'eats' a leading pipe if one is available. The equivalent line in the reference `libsyntax` is in [`parse_arm`](https://github.com/rust-lang/rust/blob/441519536c8bd138e8c651743249acd6814747a1/src/libsyntax/parse/parser.rs#L3552).

As noted in the comment linked above, this feature was formally introduced as a result of rust-lang/rfcs#1925. This feature is in active use in the [`rust-analyzer` codebase](https://github.com/matklad/rust-analyzer/blob/c87fcb4ea5874a7307c1d9d1192e923f3ae2c922/crates/ra_syntax/src/syntax_kinds/generated.rs#L231)

I have added some tests for this feature, but maybe more would be required

EDIT: Always looking for feedback - is this PR description over-engineered?

Co-authored-by: Daniel McNab <36049421+djmcnab@users.noreply.github.com>
@matklad matklad mentioned this pull request Oct 4, 2018
@bors
Copy link
Contributor

bors bot commented Oct 4, 2018

@bors bors bot merged commit a55ef9b into rust-lang:master Oct 4, 2018
@DJMcNab DJMcNab deleted the match-inital-pipe branch October 4, 2018 15:44
@DJMcNab
Copy link
Contributor Author

DJMcNab commented Oct 4, 2018

Thanks!

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

Successfully merging this pull request may close these issues.

2 participants