Skip to content

wrap_comments breaks local links #4130

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

Closed
Emoun opened this issue Apr 19, 2020 · 3 comments
Closed

wrap_comments breaks local links #4130

Emoun opened this issue Apr 19, 2020 · 3 comments

Comments

@Emoun
Copy link

Emoun commented Apr 19, 2020

When wrap_comments=true, crate-local links in docs might get split over two lines, breaking them.

Where rustfmt won't break doc links that start with e.g. http, links starting with ../ are not recognized as links and may be split over two lines. Example:

/// [some_local_link](../super_package/module1/module2/module3/module4/struct.SomeStruct.html)
fn some_fn() {}

Running rustfmt results in the following:

/// [some_local_link](../super_package/module1/module2/module3/module4/struct.
/// SomeStruct.html)
fn some_fn() {}

This crate-local link will no longer work when rustdoc is run.

I have no prior experience with the rustfmt codebase, but I think the issue is with the following function:

// Path: rustfmt/rustfmt-core/rustfmt-lib/src/comments.rs

/// Returns `true` if the given string MAY include URLs or alike.
fn has_url(s: &str) -> bool {
    // This function may return false positive, but should get its job done in most cases.
    s.contains("https://") || s.contains("http://") || s.contains("ftp://") || s.contains("file://")
}

It seems to me to be the function that is responsible for URL recognition. It is clear it doesn't take crate-local links into account.
Looking at related issues, it seems rustfmt/rustfmt-core/rustfmt-lib/src/string::detect_url() has the same job, and might already take local links into account. Maybe use it instead?

I'd be willing to try and create a test and fix PR if you give the go-ahead.

@calebcartwright
Copy link
Member

hi @Emoun, thanks for reaching out as well as the interest and willingness to contribute a fix!

I'm fairly confident that this issue has already been fixed, but just hasn't been released yet. See #4023 where this was previously reported and #4042 which resolved the issue.

At the moment we're primarily focused on getting a 2.0 release of rustfmt shipped and active development for 2.0 is being done on the master branch. The fix for this issue is available on master/rustfmt 2.0, but it simply hasn't been backported to a published rustfmt 1.x release yet.

We're also trying to only publish new 1.x versions of rustfmt to address major breaking issues, but if/when we do have another 1.x release we'll try to bundle in this fix as well. In the interim you could consider using rustfmt 2.x by building from source, and/or temporarily disabling comment wrapping in your config.

Accordingly, I'm going to close this but please feel free to re-open if you are able to reproduce this with the 2.x version of rustfmt from source.

Hopefully we'll have the fix available in a released version of rustfmt soon!

@Emoun
Copy link
Author

Emoun commented Apr 20, 2020

I can confirm that building from source doesn't exhibit this problem. Probably should have tried that before making the issue.

@calebcartwright
Copy link
Member

No worries, thanks for reaching out and verifying!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants