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

[unstable option] wrap_comments #3347

Open
Tracked by #83 ...
scampi opened this issue Feb 13, 2019 · 19 comments
Open
Tracked by #83 ...

[unstable option] wrap_comments #3347

scampi opened this issue Feb 13, 2019 · 19 comments
Labels
unstable option tracking issue of an unstable option

Comments

@scampi
Copy link
Contributor

scampi commented Feb 13, 2019

Tracking issue for unstable option: wrap_comments

@scampi scampi added the unstable option tracking issue of an unstable option label Feb 13, 2019
@scampi scampi changed the title [unstable option] unstable option: wrap_comments [unstable option] wrap_comments Feb 18, 2019
@carllerche
Copy link
Member

What is left to stabilize this?

@scampi
Copy link
Contributor Author

scampi commented May 16, 2019

There is now a documented process describing stabilisation so we'd have to go through the conditions set there.

@benbrittain
Copy link
Contributor

Is there a link to the stabilization process? I can't find it from the README.

I'd like to see us go through this process for the wrap_comments feature

@calebcartwright
Copy link
Member

calebcartwright commented Oct 28, 2019

The process for stabilizing options is defined in https://github.com/rust-lang/rustfmt/blob/master/Processes.md

@scampi & @topecongiro - correct me if I'm wrong but my understanding is that the current focus is on rustfmt 2.0, and that stabilization of some of these currently-unstable config options won't come til after the 2.0 release (based on https://github.com/rust-lang/rustfmt/projects/2)

@jhpratt
Copy link
Member

jhpratt commented Oct 31, 2020

FWIW while I'd like to see this stabilized, this option can also break markdown. Last time I tried, a large markdown table I had was wrapped, which meant it was no longer valid markdown.

@bitdivine
Copy link

Is there a way of saying "Do not wrap this comment"? We have a similar problem with long maths formulae. It's not a blocker, it just takes a bit of effort to fix up the formulae afterwards, however it would be nice if we could make exceptions for some blocks of text. I suppose the markdown could be checked programatically before and after each conversion and the formatting could be skipped if the markdown is broken. That won't catch the formulae but it might help in your case @jhpratt . Maybe there is a nice markdown formatter out there in the wild that understands when it is looking at a table.

@daniel5151
Copy link

Any updates on this?

@jhpratt
Copy link
Member

jhpratt commented Dec 7, 2020

@daniel5151 Updates are posted as they come in. What you see in this thread is the current status.

@Nemo157
Copy link
Member

Nemo157 commented Dec 16, 2020

Would it be plausible to split this into wrap_comments and wrap_doc_comments, and make wrap_doc_comments markdown aware?

@calebcartwright
Copy link
Member

Would it be plausible to split this into wrap_comments and wrap_doc_comments, and make wrap_doc_comments markdown aware?

Could you elaborate a bit on the motivation? Is it just about wanting wrap_comments stabilized more quickly or are there cases where you'd want to wrap doc comments but not other comments or vice versa?

@Nemo157
Copy link
Member

Nemo157 commented Dec 17, 2020

I actually don't care about normal comments much, I want to be able to have rustfmt wrap my doc comments. My thought was that normal comments aren't required to be markdown, so it would make sense to split the implementation.

@jhpratt

This comment has been minimized.

@calebcartwright

This comment has been minimized.

mnmaita added a commit to mnmaita/bevy that referenced this issue Mar 9, 2021
bors bot pushed a commit to bevyengine/bevy that referenced this issue Mar 10, 2021
Aims to close #1594.

These options are unstable and depend on the following PR's:

[wrap_comments](https://rust-lang.github.io/rustfmt/?version=v1.4.36&search=#wrap_comments): rust-lang/rustfmt#3347

[comment_width](https://rust-lang.github.io/rustfmt/?version=v1.4.36&search=#comment_width): rust-lang/rustfmt#3349

[normalize_comments](https://rust-lang.github.io/rustfmt/?version=v1.4.36&search=#normalize_comments): rust-lang/rustfmt#3350

@alice-i-cecile do you think this will solve the issue? When enabled, running the formatter locally should take the configurations into account to format comments. `--check` runs should also be considering them. This should be testable on the `nightly` toolchain.

~I didn't delve into normalizing `//` vs `/* */` though, should I take a look into that too? [normalize_comments](https://rust-lang.github.io/rustfmt/?version=v1.4.36&search=#normalize_comments) seems to be the solution for that but it's also unstable (tracking issue: rust-lang/rustfmt#3350). I can also add this configuration (commented out, of course) if it's desirable.~ Added `normalize_comments` option.
@obsgolem
Copy link

The current implementation of this feature doesn't reflow short comments back into the lines above. Is this something that could be implemented?

@dcow
Copy link

dcow commented May 19, 2022

This feature does not remove a leading blank comment line but it removes trailing blank lines.

///
/// Foo is a newtype for bar that can be passed to baz(foo) ...end of long line.
///
struct Foo ...

becomes

///
/// Foo is a newtype for bar that can be passed to baz(foo)
/// ...end of long line.
struct Foo ...

It's rather common to have an empty comment line before the associated section of code. It feels like this should at least be configurable.

@tgross35
Copy link
Contributor

tgross35 commented Apr 1, 2023

Just brainstorming here... there's an actively maintained library comrak that does markdown formatting, and allows setting line width. Could it make sense to use this on doc comments? It already has to be aware of things like tables and other markdown constructs, so I feel like it would do a better job out of the box than the current implementation.

Basically it seems like rustdoc needs some markdown-aware comment formatting, and it doesn't make sense to reinvent the wheel

Edit: more discussion on this at #5782

@luxalpa
Copy link

luxalpa commented Oct 17, 2023

I would prefer a wrap_doc_comments because sometimes I comment out my code temporarily, and then the normal wrap_comments can do some very unfortunate wrapping (particularly on macros).

@VorpalBlade
Copy link

VorpalBlade commented Jul 28, 2024

It would be good to be able to turn this off for specific comments or files. My use case for this is things like clap, bpaf or strum where doc strings are used to generate command line help output and I want precise control over the formatting in just those comments, while elsewhere I don't want to have to think about this and just have rustfmt do a sensible default.

For example I have this comment that is used (with strum::EnumMessage) to turn part of an enum into detailed documentation output:

pub(crate) enum Transform {
    // Other options here, elided for brevity...

    /// Get the value for a key from the system keyring. Useful for passwords
    /// etc that you do not want in your dotfiles repo.
    ///
    /// Arguments:
    /// * service="service-name"  (service name to find entry in the keyring)
    /// * user="user-name"        (username to find entry in the keyring)
    ///
    /// On Linux you can add an entry to the keyring using:
    /// secret-tool store --label="Descriptive name" service "service-name" username "user-name"
    #[strum(serialize = "keyring")]
    Keyring,
}

Rustfmt with wrap_comments wants to wrap the line with the secret-tool command. Since this is not used as markdown (it is used as plain text on stdout), putting this is a markdown code block doesn't really work (well, at least not without post-processing to filter that out).

@sww1235
Copy link

sww1235 commented Oct 30, 2024

@VorpalBlade would using #[rustfmt::skip] work for your use case?

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

No branches or pull requests