fix: formatting macro with braces delimiter - #7031
Conversation
rustbot
added
the
S-waiting-on-review
| // For macro invocations with braces, always put a space between | ||
| // the `macro_name!` and `{ /* macro_body */ }` but skip modifying | ||
| // anything in between the braces (for now). | ||
| // the `macro_name!` and `{ /* macro_body */ }`. |
There was a problem hiding this comment.
This is not correct. We do modify it by trimming (sometimes).
| /// e.g. | ||
| /// | ||
| /// ```rust,compile_fail | ||
| /// foo!{ | ||
| /// x, | ||
| /// y, | ||
| /// foo( | ||
| /// a, | ||
| /// b, | ||
| /// c, | ||
| /// ), | ||
| /// } | ||
| /// ``` | ||
| /// | ||
| /// will become | ||
| /// | ||
| /// ```rust,compile_fail | ||
| /// foo!{ | ||
| /// x, | ||
| /// y, | ||
| /// foo( | ||
| /// a, | ||
| /// b, | ||
| /// c, | ||
| /// ), | ||
| /// } | ||
| /// ``` |
There was a problem hiding this comment.
The example seems wrong, or there is a regression.
The former won't be formatted: https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=232fbfe6b62a1ffcddd22f3045bdd306
There was a problem hiding this comment.
Yes, we traditionally don't format macro calls that are written with {} delimiters.
Instead of completely removing the example let's correct it.
| // If a macro delimiter and a string start, `"`, is in the same line, then skip trimming | ||
| // altogether. | ||
| if first_line_kind == FullCodeCharKind::StartString { | ||
| return Some(orig.to_string()); | ||
| } |
There was a problem hiding this comment.
How do we know that we're being called in the context of formatting a macro?
trim_left_preserve_layout also gets called when formatting comments.
| let mut veto_trim = false; | ||
| let mut vetoed = false; |
There was a problem hiding this comment.
Why did we rename veto_trim -> vetoed?
Does vetoed represent something else now?
There was a problem hiding this comment.
If the source and target are the same, then you only need the target file to check for idempotence.
| .map( | ||
| |&(trimmed, ref line, prefix_space_width)| match prefix_space_width { | ||
| _ if !trimmed => line.to_owned(), | ||
| Some(original_indent_width) => { | ||
| let new_indent_width = indent.width() | ||
| + original_indent_width.saturating_sub(min_prefix_space_width); | ||
| let new_indent = Indent::from_width(config, new_indent_width); | ||
| format!("{}{}", new_indent.to_string(config), line) | ||
| } | ||
| None => String::new(), | ||
| }, | ||
| ) | ||
| .map(|&(trimmed, ref line, prefix_space_width)| { | ||
| if !trimmed { | ||
| return line.to_owned(); | ||
| } | ||
|
|
||
| if let Some(original_indent_width) = prefix_space_width { | ||
| let new_indent_width = indent.width() | ||
| + original_indent_width.saturating_sub(min_prefix_space_width); | ||
| let new_indent = Indent::from_width(config, new_indent_width); | ||
| return format!("{}{}", new_indent.to_string(config), line); | ||
| } | ||
|
|
||
| String::new() | ||
| }) |
There was a problem hiding this comment.
Was this meaningfully changed?
| let line = if veto_trim || new_veto_trim_value { | ||
| veto_trim = new_veto_trim_value; | ||
| trimmed = false; | ||
| line | ||
|
|
||
| if vetoed || new_veto_trim_value { | ||
| vetoed = new_veto_trim_value; | ||
| trimmed_lines.push((false, line, prefix_space_width)); | ||
| } else { | ||
| line.trim().to_owned() | ||
| }; | ||
| trimmed_lines.push((trimmed, line, prefix_space_width)); | ||
| trimmed_lines.push((true, line.trim().to_owned(), prefix_space_width)); | ||
| } |
There was a problem hiding this comment.
So we didn't need to redefine line?
|
@rustbot author |
rustbot
added
S-waiting-on-author
3 participants
Fixes #6747.
This PR makes it so that
trim_left_preserve_layoutdoes nothing when the first line contains a string start.e.g., this will now not be formatted
I'm not too satisfied with this PR; I think the formatting for macros with braces
{,}needs to be reworked. I'm opening this PR as a starting point for that goal.