-
Notifications
You must be signed in to change notification settings - Fork 1k
Fixes adjustment of max_width within macros #6651
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -423,6 +423,63 @@ macro_rules! create_config { | |
| )+ | ||
| } | ||
|
|
||
| /// Reduces the maximum width of the configuration. | ||
| /// | ||
| /// This method is intended to be used when creating a forked | ||
| /// configuration for a particular formatting context in which the | ||
| /// total available width needs to be reduced. This reduces the | ||
| /// size of max_width, and all other properties affected by small | ||
| /// heuristics, without treating those adjustments as overrides. | ||
| /// | ||
| /// As an example use case, this method is used when formatting | ||
| /// arms within a declarative macro. | ||
| #[allow(unreachable_pub)] | ||
| pub fn reduce_max_width(&mut self, delta: usize) { | ||
| self.adjust_max_width(-1 * delta as isize); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remark: in theory, if we want to be very careful around handling of large widths, this has a potential problem where However, I don't think this is a blocking concern for this PR and should not be something that's run into super frequently in practice (esp. if host pointer width is 64+). AFAICT in other parts of the code base we are also not super careful about large widths, so even if this wraps around there are other parts of the codebase that will have similar problems, AFAIK. |
||
| } | ||
|
|
||
| /// Increases the maximum width of the configuration. | ||
| /// | ||
| /// This method is intended to be used when creating a forked | ||
| /// configuration for a particular formatting context in which the | ||
| /// total available width needs to be reduced. This reduces the | ||
| /// size of max_width, and all other properties affected by small | ||
| /// heuristics, without treating those adjustments as overrides. | ||
| /// | ||
| /// As an example use case, this method is used when formatting | ||
| /// arms within a declarative macro. | ||
| #[allow(unreachable_pub)] | ||
| pub fn increase_max_width(&mut self, delta: usize) { | ||
| self.adjust_max_width(delta as isize); | ||
| } | ||
|
|
||
| /// Adjusts the maximum width of the configuration. | ||
| /// | ||
| /// This method is intended to be used when creating a forked | ||
| /// configuration for a particular formatting context in which the | ||
| /// total available width needs to be reduced. This reduces the | ||
| /// size of max_width, and all other properties affected by small | ||
| /// heuristics, without treating those adjustments as overrides. | ||
| /// | ||
| /// As an example use case, this method is used when formatting | ||
| /// arms within a declarative macro. | ||
| fn adjust_max_width(&mut self, delta: isize) { | ||
| let adjust = |value: usize| (value as isize + delta) as usize; | ||
|
|
||
| self.array_width.2 = adjust(self.array_width.2); | ||
| self.attr_fn_like_width.2 = adjust(self.attr_fn_like_width.2); | ||
| self.chain_width.2 = adjust(self.chain_width.2); | ||
| self.fn_call_width.2 = adjust(self.fn_call_width.2); | ||
| self.single_line_if_else_max_width.2 = adjust(self.single_line_if_else_max_width.2); | ||
| self.single_line_let_else_max_width.2 = | ||
| adjust(self.single_line_let_else_max_width.2); | ||
| self.struct_lit_width.2 = adjust(self.struct_lit_width.2); | ||
| self.struct_variant_width.2 = adjust(self.struct_variant_width.2); | ||
|
|
||
| self.max_width.2 = adjust(self.max_width.2); | ||
| self.set_heuristics(); | ||
| } | ||
|
|
||
| fn set_width_heuristics(&mut self, heuristics: WidthHeuristics) { | ||
| let max_width = self.max_width.2; | ||
| let get_width_value = | | ||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question:
Is there a (albeit synthetic) test that can demonstrate the after effect (of this edge case)?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've added a prior commit in the chain that adds test fixtures for these macro use cases, with both default heuristics, and manually crafted widths that will trigger this edge case when the macro body indent exceeds the difference between the custom width and Note that the error log currently happens whenever the "clipping" behavior occurred, which is for any file with a macro body if you have custom width values close to / equal to This change fixes the error logging, but it only effects formatting when the length of a line is specifically within the gap between The When it comes to this being formatting affecting:
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the detailed explanation, this is what I was having trouble figuring out 👍 Note I consider this review chain resolved, but intentionally leaving open for other reviewers for context.
jieyouxu marked this conversation as resolved.
jieyouxu marked this conversation as resolved.
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| // rustfmt-format_macro_bodies: true | ||
| // rustfmt-max_width: 82 | ||
| // rustfmt-fn_call_width: 76 | ||
|
|
||
| macro_rules! short { | ||
| () => { | ||
| m!(a, b, c); | ||
| }; | ||
| } | ||
|
|
||
| macro_rules! medium { | ||
| () => { | ||
| m!(aaaaaaaaaaaaaaaa, bbbbbbbbbbbbbbbb, cccccccccccccccc, ddddddddd); | ||
| }; | ||
| } | ||
|
|
||
| macro_rules! long { | ||
| () => { | ||
| m!(aaaaaaaaaaaaaaaa, bbbbbbbbbbbbbbbb, cccccccccccccccc, ddddddddddddddd); | ||
| }; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| // rustfmt-format_macro_bodies: true | ||
|
|
||
| macro_rules! short { | ||
| () => { | ||
| m!(a, b, c); | ||
| }; | ||
| } | ||
|
|
||
| macro_rules! medium { | ||
| () => { | ||
| m!(aaaaaaaaaa, bbbbbbbbbb, cccccccccc, ddddddddd); | ||
| }; | ||
| } | ||
|
|
||
| macro_rules! long { | ||
| () => { | ||
| m!(aaaaaaaaaaaaaaaa, bbbbbbbbbbbbbbbb, cccccccccccccccc, ddddddddddddddd); | ||
| }; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| // rustfmt-format_macro_bodies: true | ||
| // rustfmt-max_width: 82 | ||
| // rustfmt-fn_call_width: 76 | ||
|
|
||
| macro_rules! short { | ||
| () => { | ||
| m!(a, b, c); | ||
| }; | ||
| } | ||
|
|
||
| macro_rules! medium { | ||
| () => { | ||
| m!(aaaaaaaaaaaaaaaa, bbbbbbbbbbbbbbbb, cccccccccccccccc, ddddddddd); | ||
| }; | ||
| } | ||
|
|
||
| macro_rules! long { | ||
| () => { | ||
| m!( | ||
| aaaaaaaaaaaaaaaa, | ||
| bbbbbbbbbbbbbbbb, | ||
| cccccccccccccccc, | ||
| ddddddddddddddd | ||
| ); | ||
| }; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| // rustfmt-format_macro_bodies: true | ||
|
|
||
| macro_rules! short { | ||
| () => { | ||
| m!(a, b, c); | ||
| }; | ||
| } | ||
|
|
||
| macro_rules! medium { | ||
| () => { | ||
| m!(aaaaaaaaaa, bbbbbbbbbb, cccccccccc, ddddddddd); | ||
| }; | ||
| } | ||
|
|
||
| macro_rules! long { | ||
| () => { | ||
| m!( | ||
| aaaaaaaaaaaaaaaa, | ||
| bbbbbbbbbbbbbbbb, | ||
| cccccccccccccccc, | ||
| ddddddddddddddd | ||
| ); | ||
| }; | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.