Skip to content

URL in comment prevents formatting #5634

@krtab

Description

@krtab
Contributor

When using rustfmt to wrap doc comments, having a URL in the comments prevents any wrapping from happening.

Example:

/// This is indented fine. It is rather long but does not contains any link. Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.
struct Foo {}


/// This is not indented. The only real difference is the presence of a URL. Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum. Source: https://www.lipsum.com/
struct Bar {}

Becomes:

/// This is indented fine. It is rather long but does not contains any link.
/// Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod
/// tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam,
/// quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo
/// consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse
/// cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat
/// non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.
struct Foo {}

/// This is not indented. The only real difference is the presence of a URL. Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum. Source: https://www.lipsum.com/
struct Bar {}

The config used for the tests was:

unstable_features = true
wrap_comments = true
error_on_unformatted = true
error_on_line_overflow = true

Activity

krtab

krtab commented on Dec 22, 2022

@krtab
ContributorAuthor

According to the comments (cf below) in rustfmt source, this is intended behavior, but I don't understand why, and couldn't find a discussion of why in the tracking issues. Comments frequently have URL, including in markdown links in them, so I think it would be best not to have this limitation.

rustfmt/src/comment.rs

Lines 799 to 803 in ee2bed9

// We only want to wrap the comment if:
// 1) wrap_comments = true is configured
// 2) The comment is not the start of a markdown header doc comment
// 3) The comment width exceeds the shape's width
// 4) No URLS were found in the comment

ytmimi

ytmimi commented on Dec 22, 2022

@ytmimi
Contributor

Thanks for reaching out. This is indeed intended behavior. We've had several issues in the past where breaking urls has been an issue. In the worst case breaking urls would prevent rustdoc from rendering them properly so we're erring on the side of caution and not breaking those lines at all.

Check out the following issues for reference: #506, #3787, #5095, #5260

It would be great if someone could help update the docs for wrap_comments to better communicate those details 😁

krtab

krtab commented on Dec 22, 2022

@krtab
ContributorAuthor

I see how it could have break in the past but isn't it something that can be fixed? Would you be open to a PR restoring wrapping of comments containing urls? It seems to me that this is merely a case of correctly detecting all possible URL formats with an exhaustive enough regex. Am I missing something?

ytmimi

ytmimi commented on Dec 22, 2022

@ytmimi
Contributor

but isn't it something that can be fixed? Would you be open to a PR restoring wrapping of comments containing urls?

I think the current implementation could be made more permissive, but we still wouldn't want to introduce line break within a URL. You'll likely need to change how break_string works. You're welcome to give it a try. The simpler and more straightforward the patch the better!

It seems to me that this is merely a case of correctly detecting all possible URL formats with an exhaustive enough regex. Am I missing something?

Sorry, would you mind elaborating a little on this. Are you referring to the current implementation or what you'd need to do to solve the issue?

ytmimi

ytmimi commented on Dec 22, 2022

@ytmimi
Contributor

All that being said, I would still like to update the docs to better describe cases when we won't wrap comments

krtab

krtab commented on Dec 23, 2022

@krtab
ContributorAuthor

I've added documentation but still would like to give a go to the underlying issue and get formatting of comments with urls to work (without splitting urls themselves of course).

However, I believe this means the issue should be untagged "documentation" and possibly "good-first-issue".

ytmimi

ytmimi commented on Dec 23, 2022

@ytmimi
Contributor

@krtab I'd like to close this once we update the docs to better describe how the feature is currently intended to work.

That being said, I'd be more than happy to review a follow up PR to expand on the current behavior of wrap_comments 😁

mladedav

mladedav commented on Mar 7, 2023

@mladedav

@ytmimi The documentation seems to have been updated in #5637 if you want to close this (and/or create a separate issue)

ytmimi

ytmimi commented on Mar 7, 2023

@ytmimi
Contributor

@mladedav appreciate you pointing that out 😁

1 remaining item

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    documentationgood first issueIssues up for grabs, also good candidates for new rustfmt contributors

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @mladedav@ytmimi@krtab

        Issue actions

          URL in comment prevents formatting · Issue #5634 · rust-lang/rustfmt