Skip to content

rustc: Allow trailing comma in record fields #1207

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

Closed
wants to merge 1 commit into from

Conversation

lht
Copy link
Contributor

@lht lht commented Nov 21, 2011

@nikomatsakis
Copy link
Contributor

Do we want the trailing commas to be preserved? This patch always inserts them in the pretty printer, whether or not they were present in the original source.

@brson
Copy link
Contributor

brson commented Nov 21, 2011

Yeah, agreed that to do this the pretty printer needs to remember whether the comma was used or not. We'll need an additional pp-exact test case that ensures commas aren't added to records without them.

@nikomatsakis
Copy link
Contributor

It will be a rather uglier patch, though, as we will have to store in the AST whether there was a trailing comma, which will lead to modifications trickling around...and there are a lot of cases to consider. pat_rec, call arguments, and so forth. To what extent does the pretty printer preserve your formatting today? Maybe it should just never or always emit trailing commas and so forth.

@brson
Copy link
Contributor

brson commented Nov 21, 2011

The main place the pretty-printer attempts to preserve formatting now is with comments and whitespace. It's fair for the pp to just be opinionated, so maybe this is fine as-is and not worth the mess of tacking on a flag to the AST.

@lht
Copy link
Contributor Author

lht commented Nov 22, 2011

To make the result less "ugly", maybe the pp should only preserve the trailing comma if the record is not wrapped in a single line. I haven't read how pp works, but I guess no flag is needed because we can check if the line number is advanced.

@brson
Copy link
Contributor

brson commented Nov 22, 2011

I don't believe there is a general way for the pp to know that what it's printing is going to end up causing a line break.

@brson
Copy link
Contributor

brson commented Nov 22, 2011

I pulled this as-is

@brson brson closed this Nov 22, 2011
coastalwhite pushed a commit to coastalwhite/rust that referenced this pull request Aug 5, 2023
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