run rustfmt on libsyntax_ext folder#35614
run rustfmt on libsyntax_ext folder#35614bors merged 1 commit intorust-lang:masterfrom srinivasreddy:syntax_ext_rustfmt
Conversation
|
(rust_highfive has picked a reviewer for you, use r? to override) |
| prec), | ||
| self.ecx.field_imm(sp, | ||
| self.ecx.ident_of("width"), | ||
| width)]); |
There was a problem hiding this comment.
That was the first impression i got too. Let's see what @nrc says .
There was a problem hiding this comment.
It's kind of edge-casey because the better formatting requires breaking after the ( which is usually avoided. You could add a skip annotation to the statement, or just ignore it - I'm willing to eat the occasional poor formatting in exchange for automated formatting everywhere.
There was a problem hiding this comment.
Why not make a rule where for every broken ( the appropriate ending ) is also broken?
That way it would look visually consistent; would solve this edge case.
There was a problem hiding this comment.
That's not really the problem. The problem is that we have no heuristics for when to break after the ( in the first place.
There was a problem hiding this comment.
Can we switch to a style that doesn't have these kinds of awful edge cases? These continue to pop up. We have an alternative (finally), based on winapi:
There was a problem hiding this comment.
@ubsan Wow, thanks for writing that up -- I really like that style!
#34584 (comment) summarizes @petrochenkov's and my thoughts on these PRs. I really dislike rustfmt's non-block indenting ("visual indenting") that makes this example so horrifying (and also appears to be the main cause of the problems you referenced in the "Why we need this" section).
There was a problem hiding this comment.
The style I prefer is this:
self.ecx.expr_struct(sp, path, vec![
self.ecx.field_imm(sp, self.ecx.ident_of("fill"), fill),
self.ecx.field_imm(sp, self.ecx.ident_of("align"), align),
self.ecx.field_imm(sp, self.ecx.ident_of("flags"), flags),
self.ecx.field_imm(sp, self.ecx.ident_of("precision"), prec),
self.ecx.field_imm(sp, self.ecx.ident_of("width"), width)
]);EDIT: Apparently this is "block indenting" which is part of @ubsan's style guide.
There was a problem hiding this comment.
That's block style :)
last argument is multi-line
(note that I just added this; I used to only do that for multi-line single-argument functions, but I realized that this was a failure I needed to address, so I made sure it was all functions which ended with a multi-line argument).
|
@bors r+ rollup |
|
📌 Commit d652639 has been approved by |
This is also my POV, though if someone wants to file an issue on rustfmt, that seems fine. (I don't think this PR is the place to debate what rustfmt's heuristics unless there is a genuine bug.) |
|
@nikomatsakis the issue was, until about 3 hours ago, we did not have a team to debate this :P |
| vec![self_ty], | ||
| lifetimes, | ||
| self_params, | ||
| Vec::new()) |
There was a problem hiding this comment.
This function call can remain on one line -- not sure why rustfmt breaks it up...
…=nikomatsakis run rustfmt on libsyntax_ext folder
…=nikomatsakis run rustfmt on libsyntax_ext folder
10 participants
No description provided.