Skip to content

Second line of Comment is incorrectly indentated #4546

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

Open
davidBar-On opened this issue Nov 23, 2020 · 3 comments · May be fixed by #5649
Open

Second line of Comment is incorrectly indentated #4546

davidBar-On opened this issue Nov 23, 2020 · 3 comments · May be fixed by #5649
Assignees
Labels
1x-backport:pending Fixed/resolved in source but not yet backported to a 1x branch and release a-comments p-low poor-formatting

Comments

@davidBar-On
Copy link
Contributor

Input

fn main() {
    let v = [
        "A", /* item A comment */
        "BBBBBB", /* item BBBBBB comment line 1
                   * item BBBBBB comment line 2 */
        "CCC", /* item CCC comment */
    ];
}

Output

fn main() {
    let v = [
        "A", /* item A comment */
        "BBBBBB", /* item BBBBBB comment line 1
              * item BBBBBB comment line 2 */
        "CCC", /* item CCC comment */
    ];
}

Expected output

Same as input. The second line of the comment to "BBBBBB" is wrongly aligned to the "A" comment. (Based on test case from #4518.)

Meta

  • rustfmt version: rustfmt 2.0.0-rc.2-nightly (30bda450 2020-11-18)
  • From where did you install rustfmt?: self build
@calebcartwright
Copy link
Member

Another case where this is technically true, but it's really tough to justify mutli-line block comments that are trailing the element they are supposed to be associated to within a sequence. This seems like a very uncommon comment style, and I'd argue that it's significantly more difficult to read than the more common style of having multiline comments above the associated item.

Even with such a relatively simple example, it's tough from a visual glance to quickly grok what the second BBBBB line is supposed to be associated to.

If there's an easy, noninvasive code change that account for this then that'd be fine, but if it takes anything non trivial (either in implementation or runtime overhead) then I don't think that would be justified.

A big motivating factor that played into decisions when the style guide was defined was to try to avoid rightward drift and visual alignment. A comment style like this is much more likely to run out to the right and potentially run into width limits since every line of the comment block will be extended by the length of the element plus associated overhead

@davidBar-On
Copy link
Contributor Author

I believe there is a simple solution which is backward compatible when multiline comments are not used (which from what you wrote I understand is true for almost all cases). This is by taking the approach that each multiline comment is considered a "comments indentation group" of its own, so each multiline will be indented per the list item it belongs to.

I already have a draft change using this approach, which is basically adding two if statements, one in write_list() and one in max_width_of_item_with_post_comment(). If you believe this approach may be acceptable, I can submit a PR with the changes.

Taking the above approach, the output of the original example will be:

fn main() {
    let v = [
        "A", /* item A comment */
        "BBBBBB", /* item BBBBBB comment line 1
                   * item BBBBBB comment line 2 */
        "CCC", /* item CCC comment */
    ];
}

Following is a more comprehensive expected output example. Note that A / EEE are a group and CCC / DDDDD are a group. On the other hand both BBBBBB and F are on their own group.

fn main() {
    let v = [
        "A",   /* item A comment */
        "EEE", /* item E comment */
        "BBBBBB", /* item B comment line 1
                   * item B comment line 2 */
        "F", /* item F comment line 1
              * item F comment line 2 */
        "CCC",   /* item C comment */
        "DDDDD", /* item D comment */
    ];
}

@calebcartwright
Copy link
Member

If you believe this approach may be acceptable, I can submit a PR with the changes.

Feel free to open a PR with suggested changes, I was just speaking in general terms that uncommon/discouraged styles of comment places like this aren't a top priority, so any code changes have to be weighed accordingly

davidBar-On added a commit to davidBar-On/rustfmt that referenced this issue Dec 2, 2020
@ytmimi ytmimi added a-comments 1x-backport:pending Fixed/resolved in source but not yet backported to a 1x branch and release p-low labels Jul 26, 2022
davidBar-On added a commit to davidBar-On/rustfmt that referenced this issue Jan 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1x-backport:pending Fixed/resolved in source but not yet backported to a 1x branch and release a-comments p-low poor-formatting
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants