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

Permit trailing commas in macro invocations #6

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Permit trailing commas in macro invocations #6

wants to merge 1 commit into from

Conversation

myrrlyn
Copy link

@myrrlyn myrrlyn commented Feb 17, 2020

This commit uses the ? operator to support optional trailing commas in
macro calls without requiring the call to use the panic-message arms.

The ? macro operator was stabilized in 1.32 for the 2018 edition,
and 1.37 for the 2015 edition. Applying this commit requires raising
the crate MSRV to one of these two versions; for the lower, the crate
must be explicitly set to the edition = "2018" key in its manifest.

This commit uses the `?` operator to support optional trailing commas in
macro calls without requiring the call to use the panic-message arms.

The `?` macro operator was stabilized in `1.32` for the 2018 edition,
and `1.37` for the 2015 edition. Applying this commit requires raising
the crate MSRV to one of these two versions; for the lower, the crate
must be explicitly set to the `edition = "2018"` key in its manifest.
@myrrlyn
Copy link
Author

myrrlyn commented Feb 17, 2020

I wrote this patch because I noticed that my rustfmt settings were causing macro calls to break. Insertion of the trailing commas forced the macro expansion into the arms expecting a panic message.

As noted, it forces an MSRV. Please feel completely free to reject the PR if this is unacceptable.

@murarth
Copy link
Owner

murarth commented Feb 19, 2020

I have to say, I'm not persuaded that there's a good reason to accept this proposed change and I'm inclined to reject it.

It seems to me that rustfmt, as a rule, should not change the tokens within a macro invocation. Indeed, it seems that the default settings of rustfmt do not change macro tokens. Further, the matches macro recently added to std does not accept a trailing comma (although that could change between now and the time of its stabilization).

@myrrlyn
Copy link
Author

myrrlyn commented Feb 19, 2020

My motivation for submitting this change was predicated solely on the fact that all the std macros started out without trailing comma support, and then added it in 1.26. I elected to use the ? operator instead of duplicate arms only for simplicity. If you're opposed to the requisite MSRV that imposes, I'm happy to rewrite to the std style; if you're opposed syntactically, I have absolutely no hard feelings about closing this.

@nothingelsematters
Copy link

I've stumbled upon this issue recently. I have to notice that the matches! std macro already accepts a trailing comma. Moreover, I do not see any reason not to accept an optional trailing comma here, I suppose that it should be the user's choice which code style to use.

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.

3 participants