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

Allow trailing comma #603

Closed

Conversation

HyeonuPark
Copy link

Allow trailing comma within macro call.
And some tiny fixes I found.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 87.092% when pulling 143ecda on HyeonuPark:allow-trailing-comma into 3dd6672 on Geal:master.

1 similar comment
@coveralls
Copy link

coveralls commented Nov 5, 2017

Coverage Status

Coverage remained the same at 87.092% when pulling 143ecda on HyeonuPark:allow-trailing-comma into 3dd6672 on Geal:master.

@Geal
Copy link
Collaborator

Geal commented Nov 26, 2017

hmm, that's a lot of changes for something I'm not really convinced is useful. Why do you need trailing commas in argument lists?

@HyeonuPark
Copy link
Author

Nom macros tend to be deeply nested, so it should usually be split into multiple lines.

And there's two major style rules for multiline function call: comma-first and comma-last

I saw you prefer comma-first style for your own code, but many people in Rust community likes comma-last with trailing comma.

So it will be nice to allow trailing comma in nom macros!

@perlun
Copy link

perlun commented Dec 8, 2017

I saw you prefer comma-first style for your own code, but many people in Rust community likes comma-last with trailing comma.

To give my 0,02€ of this, we really don't know this. We just know that those who like trailing comma tend to raise their voice louder and share their opinion.

Until someone really does an exhaustive poll/examination of some sort, I think we should be careful to draw too far-reaching conclusions from that discussion.

(Disclaimer: I am the author of the linked issue, where I advocated that the default Rust style should not advocate trailing comma. 😃 Since writing that flamebait, I have given trailing comma a chance in a minor TypeScript project, and it's not that bad once you get used to it.)

@Geal Geal force-pushed the master branch 3 times, most recently from 6c99077 to e7ca818 Compare May 14, 2018 12:06
@Geal
Copy link
Collaborator

Geal commented Aug 18, 2018

closing this, it's not really useful for nom, sorry

@Geal Geal closed this Aug 18, 2018
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.

4 participants