rustfmt
rustfmt copied to clipboard
Treat empty doc comments that precede doc attributes as significant
Fixes #5073
Normally we would want to remove any empty trailing doc comments however, as highlighted by 5073 by removing these trailing comments rustfmt is interfering with tools like rustdoc.
Checks have been added to allow rustfmt to keep empty trailing comments
when using wrap_comments = true if and only if the empty trailing
comments immideatly precede a doc attribute.
I agree there were a few signatures that needed to change. As you might tell from the code I was trying to find a way to dynamically determine if empty comments were significant. I'm definitely not married to this solution and open to suggestions you might have on how to fix this.
That being said, it's been a while since I submitted this PR, and revisiting this now has got me wondering if a simpler approach would be to introduce a new unstable configuration option like retain_empty_doc_comments (name suggestions are also appreciated!). The CommentRewrite struct gets a reference to the config, so we could just let users decide whether or not rustfmt should remove empty doc comments or not.
That being said, it's been a while since I submitted this PR, and revisiting this now has got me wondering if a simpler approach would be to introduce a new unstable configuration option like
retain_empty_doc_comments(name suggestions are also appreciated!). TheCommentRewritestruct gets a reference to the config, so we could just let users decide whether or not rustfmt should remove empty doc comments or not.
While that does sound like it'd make for a simpler implementation, do you think the categorical behavior associated with a config option will give users what they need in all contexts? My concern would be that the answer genuinely seems like "it depends" which is impossible to handle with a boolean-style config option, and I worry that an enum-style option would be challenging as well (what would the variants and their behaviors be?)
While that does sound like it'd make for a simpler implementation, do you think the categorical behavior associated with a config option will give users what they need in all contexts? My concern would be that the answer genuinely seems like "it depends" which is impossible to handle with a boolean-style config option, and I worry that an enum-style option would be challenging as well (what would the variants and their behaviors be?)
I think that's a really good point. Let me think on this a bit and get back to you. To your point, it might be better to avoid going down the configuration option route, but part of me still thinks I can come up with a slightly cleaner way to handle this.