Skip to content
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

Handle DefinitiveListTactic::SpecialMacro when writing pre-comments #5028

Merged
merged 1 commit into from
Oct 17, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 24 additions & 22 deletions src/lists.rs
Original file line number Diff line number Diff line change
Expand Up @@ -367,29 +367,31 @@ where
result.push_str(&comment);

if !inner_item.is_empty() {
if tactic == DefinitiveListTactic::Vertical || tactic == DefinitiveListTactic::Mixed
{
// We cannot keep pre-comments on the same line if the comment if normalized.
let keep_comment = if formatting.config.normalize_comments()
|| item.pre_comment_style == ListItemCommentStyle::DifferentLine
{
false
} else {
// We will try to keep the comment on the same line with the item here.
// 1 = ` `
let total_width = total_item_width(item) + item_sep_len + 1;
total_width <= formatting.shape.width
};
if keep_comment {
result.push(' ');
} else {
result.push('\n');
result.push_str(indent_str);
// This is the width of the item (without comments).
line_len = item.item.as_ref().map_or(0, |s| unicode_str_width(&s));
match tactic {
DefinitiveListTactic::SpecialMacro(_)
| DefinitiveListTactic::Vertical
| DefinitiveListTactic::Mixed => {
Comment on lines +370 to +373
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't feel obligated to change anything, just sharing for awareness. For future reference, you may want to consider utilizing the matches! macro in these types of cases because it could both express the condition more succinctly, and it helps avoid the extra level of indentation that comes with the match arm body, e.g.

                use DefinitiveListTactic::*;
                if matches!(tactic, Vertical | Mixed | SpecialMacro(_)) {

(with the wildcard import being another option to shorten the length of the pattern variants)

Copy link
Contributor Author

@ytmimi ytmimi Oct 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I appreciate you pointing this out. It didn't even occur to me to use the matches! macro. I went ahead and made the change in #5034!

// We cannot keep pre-comments on the same line if the comment is normalized
let keep_comment = if formatting.config.normalize_comments()
|| item.pre_comment_style == ListItemCommentStyle::DifferentLine
{
false
} else {
// We will try to keep the comment on the same line with the item here.
// 1 = ` `
let total_width = total_item_width(item) + item_sep_len + 1;
total_width <= formatting.shape.width
};
if keep_comment {
result.push(' ');
} else {
result.push('\n');
result.push_str(indent_str);
// This is the width of the item (without comments).
line_len = item.item.as_ref().map_or(0, |s| unicode_str_width(&s));
}
}
} else {
result.push(' ');
_ => result.push(' '),
}
}
item_max_width = None;
Expand Down
4 changes: 4 additions & 0 deletions tests/source/issue-4615/minimum_example.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
info!(//debug
"{}: sending function_code={:04x} data={:04x} crc=0x{:04X} data={:02X?}",
self.name, function_code, data, crc, output_cmd
);
5 changes: 5 additions & 0 deletions tests/target/issue-4615/minimum_example.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
info!(
//debug
"{}: sending function_code={:04x} data={:04x} crc=0x{:04X} data={:02X?}",
self.name, function_code, data, crc, output_cmd
);