-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Change struct expr pretty printing to match rustfmt style #93556
Conversation
r? @cjgillot (rust-highfive has picked a reviewer for you, use r? to override) |
I'd like to understand better the purpose of this crate. It seems to be partly redundant with rustfmt. Now that rustfmt is a subtree, could we reuse part of rustfmt to do what rustc_ast_pretty does? |
Rustfmt is less granular than ast_pretty. If you search zulip I had a discussion with @calebcartwright about it a while ago. |
Agreed. I think one day it would be good if we can get rustfmt's API to a point where it could be utilized for formatting individual AST nodes, but for today and the very foreseeable future it's very much towards the all-or-nothing end of the spectrum that would make rustfmt a poor fit for trying to format an individual construct |
@cjgillot, separately, rustfmt prioritizes quality over simplicity and performance, and is significantly slower than rustc_ast_pretty. If rustc needs to dump out megabytes of code from a |
} | ||
self.space(); | ||
} | ||
self.offset(-INDENT_UNIT); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you explain what this negative offset does? I have trouble understanding how this code manages to find the correct indentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The top of https://github.com/rust-lang/rust/blob/1.58.1/compiler/rustc_ast_pretty/src/pp.rs has an overview of the printing algorithm. Very briefly, when you make a sequence of cbox string break string break string break string end calls on the printer, either all of those breaks will turn into linebreaks, or none of them will turn into linebreaks. If they turn into linebreaks, the indentation at each one is the sum of the indentation of all the surrounding boxes.
So if we have:
ibox(4)cbox(0)Struct {breaka: Abreakb: Bbreak}endend
and it does not fit on the line, then you'd expect to get:
Struct {
a: A
b: B
}
That's why every break also holds its own offset which is relative to the sum of the surrounding boxes' indents, so that we can set -4 on the last break and get:
Struct {
a: A
b: B
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the detailed explanation. I'm surprised such negative offset facility was not used before, but that's maybe for another refactoring.
|
1f7b0a0
to
ca3057f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, this PR makes a nicer output.
@bors r+ rollup
} | ||
self.space(); | ||
} | ||
self.offset(-INDENT_UNIT); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the detailed explanation. I'm surprised such negative offset facility was not used before, but that's maybe for another refactoring.
@bors r+ rollup |
📌 Commit ca3057f has been approved by |
Change struct expr pretty printing to match rustfmt style This PR backports trailing comma support from https://github.com/dtolnay/prettyplease into rustc_ast_pretty and uses it to improve the formatting of struct expressions. Example: ```rust macro_rules! stringify_expr { ($expr:expr) => { stringify!($expr) }; } fn main() { println!("{}", stringify_expr!(Struct { a: Struct { b, c }, })); println!("{}", stringify_expr!(Struct { aaaaaaaaaa: AAAAAAAAAA, bbbbbbbbbb: Struct { cccccccccc: CCCCCCCCCC, dddddddddd: DDDDDDDDDD, eeeeeeeeee: EEEEEEEEEE, }, })); } ``` 🤮 Before: ```console Struct{a: Struct{b, c,},} Struct{aaaaaaaaaa: AAAAAAAAAA, bbbbbbbbbb: Struct{cccccccccc: CCCCCCCCCC, dddddddddd: DDDDDDDDDD, eeeeeeeeee: EEEEEEEEEE,},} ``` After: ```console Struct { a: Struct { b, c } } Struct { aaaaaaaaaa: AAAAAAAAAA, bbbbbbbbbb: Struct { cccccccccc: CCCCCCCCCC, dddddddddd: DDDDDDDDDD, eeeeeeeeee: EEEEEEEEEE, }, } ```
…askrgr Rollup of 9 pull requests Successful merges: - rust-lang#91939 (Clarify error on casting larger integers to char) - rust-lang#92300 (mips64-openwrt-linux-musl: Add Tier 3 target) - rust-lang#92383 (Add new target armv7-unknown-linux-uclibceabi (softfloat)) - rust-lang#92651 (Remove "up here" arrow on item-infos) - rust-lang#93556 (Change struct expr pretty printing to match rustfmt style) - rust-lang#93649 (Add regression tests for issue 80309) - rust-lang#93657 (Update CPU idle tracking for apple hosts) - rust-lang#93659 (Refactor conditional) - rust-lang#93669 (Resolve lifetimes for const generic defaults) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
This PR backports trailing comma support from https://github.com/dtolnay/prettyplease into rustc_ast_pretty and uses it to improve the formatting of struct expressions.
Example:
🤮 Before:
After: