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

format_code_in_doc_comments formats some long line comments to block comments #5533

Closed
pan93412 opened this issue Sep 6, 2022 · 3 comments · Fixed by #5536
Closed

format_code_in_doc_comments formats some long line comments to block comments #5533

pan93412 opened this issue Sep 6, 2022 · 3 comments · Fixed by #5536
Labels
a-comments good first issue Issues up for grabs, also good candidates for new rustfmt contributors only-with-option requires a non-default option value to reproduce

Comments

@pan93412
Copy link
Contributor

pan93412 commented Sep 6, 2022

Meta

Description

After enabling format_code_in_doc_comments, rustfmt formats the following line comments to block comments – which is neither a documented nor an expected behavior.

struct TestStruct {
    position_currency: String, // Currency for position of this contract. If not null, 1 contract = 1 positionCurrency.
    pu: Option<i64>, // Previous event update sequense ("u" of previous message), -1 also means None
}

Expectation

rustfmt should not touch this.

struct TestStruct {
    position_currency: String, // Currency for position of this contract. If not null, 1 contract = 1 positionCurrency.
    pu: Option<i64>, // Previous event update sequense ("u" of previous message), -1 also means None
}

Actual Result

struct TestStruct {
    position_currency: String, /* Currency for position of this contract. If not null, 1 contract = 1 positionCurrency. */
    pu: Option<i64>, /* Previous event update sequense ("u" of previous message), -1 also means None */
}

Screenshot

rustfmt with format_code_in_doc_comments changes the long comments to block comments

Related

#3348

@ytmimi ytmimi added a-comments only-with-option requires a non-default option value to reproduce labels Sep 6, 2022
@ytmimi
Copy link
Contributor

ytmimi commented Sep 6, 2022

Confirming I can reproduce this with the latest master 38659ec.

@ytmimi
Copy link
Contributor

ytmimi commented Sep 6, 2022

I did some digging and It seems that block_style is being set to true in write_list

rustfmt/src/lists.rs

Lines 447 to 461 in 38659ec

let block_style = if !formatting.ends_with_newline && last {
true
} else if starts_with_newline(comment) {
false
} else {
comment.trim().contains('\n') || unicode_str_width(comment.trim()) > width
};
rewrite_comment(
comment.trim_start(),
block_style,
comment_shape,
formatting.config,
)
};

so when we call CommentRewrite::new from rewrite_comment_inner, we set things up to write a block comment.

rustfmt/src/comment.rs

Lines 564 to 568 in 38659ec

let ((opener, closer, line_start), style) = if block_style {
(
CommentStyle::SingleBullet.to_str_tuplet(),
CommentStyle::SingleBullet,
)

For anyone interested in working on this I think there are two avenues to explore in order to address the issue:

  1. Investigate how we might update the logic in write_list to pass block_style=false to rewrite_comment. write_list is a generic function used throughout the codebase so it might be difficult to make changes there without unintentionally breaking the behavior elsewhere.

  2. Take the is_doc_comment boolean and the config.format_code_in_doc_comments() config value into account to try to move code execution into the light_rewrite_comment code path instead of the rewrite_comment_inner code path from within identify_comment.

rustfmt/src/comment.rs

Lines 368 to 382 in 38659ec

} else if !config.normalize_comments()
&& !config.wrap_comments()
&& !config.format_code_in_doc_comments()
{
light_rewrite_comment(first_group, shape.indent, config, is_doc_comment)
} else {
rewrite_comment_inner(
first_group,
block_style,
style,
shape,
config,
is_doc_comment || style.is_doc_comment(),
)?
};

@ytmimi ytmimi added the good first issue Issues up for grabs, also good candidates for new rustfmt contributors label Sep 6, 2022
@pan93412
Copy link
Contributor Author

pan93412 commented Sep 6, 2022

Maybe I can help this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-comments good first issue Issues up for grabs, also good candidates for new rustfmt contributors only-with-option requires a non-default option value to reproduce
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants