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

Comments #17

Closed
nrc opened this issue Sep 29, 2016 · 45 comments · Fixed by #95

Comments

@nrc
Copy link
Member

commented Sep 29, 2016

As well as specifying formatting for comments, we should decide on how much we expect Rustfmt to enforce (at the least, it cannot enforce correct grammar). My experience has been that re-formatting comments is almost impossible to get right. However, it is possible that we might be able formulate acceptable heuristics if we want to go down that road.

This one will need some fleshing out, but to start with:

  • Prefer // comments to /* ... */ comments, use the latter only where there is code following on the same line.
  • Prefer /// for doc comments, use //! only for module/crate-level docs. Avoid the other forms.
  • Comments should be complete sentences. Start with a capital letter, (usually) end with a period (.).
  • Have a single space after the comment sigils: // A comment. or /// A doc comment.
  • Comments should be indented with the code they are 'attached' to.
  • Comments should be kept within the 80 char margin (longer comments are harder to read).

I do not believe we should get into what makes a comment good or bad, beyond its formatting.

I propose that Rustfmt should basically avoid formatting comments. It can move comments to make them aligned with code and can add or remove whitespace around comments (if necessary, and I suspect it won't be). But Rustfmt should not change the actual text of the comment at all. Rustfmt should warn if a comment runs over the maximum width, but shouldn't attempt to fix that. Rustfmt should not change the style of the comment, nor the whitespace inside a comment.

I'd be happy to have more sophisticated comment formatting (word-wrapping, etc.) behind an option, if that is of interest to users.

@Mark-Simulacrum

This comment has been minimized.

Copy link

commented Sep 29, 2016

Adding something about avoiding the ///////////////////////// style comment "header" and/or suggestion as to how rustfmt should deal with them. I know one of the recent re-format PRs to rustc had a problem with it reformatting a // section to /// because of such a block.

@ubsan

This comment has been minimized.

Copy link
Contributor

commented Sep 29, 2016

I think we should use // for line comments, and /* */ for longer comments. Unlike C/C++/lots, we don't have the confusing issue with closing tags, and they're better for accessibility (cc @camlorn). Similarly, doc comments should almost always use the /** form.

Something like the following would be my preferred solution:

/*! Module comment
    this is the documentation for a module
*/

/** Ooh, check out this type. It's so nice and typey.
    It's an example struct, wee!
*/
struct Type;

fn main() {
    /*  This is the main function
        it's nice, I guess
    */

    // oh nice, line comment! adding together stuff!
    println!("{}", !1 - !2);
}
@alexcrichton

This comment has been minimized.

Copy link

commented Sep 29, 2016

Rustfmt should warn if a comment runs over the maximum width

I'd personally be wary of adding warnings for this, sometimes the reason a comment is long is hard to fix (e.g. a long url). If rustfmt always emitted a warning for this unconditionally it may be annoying to turn off the warning on a case-by-case basis.

@nrc

This comment has been minimized.

Copy link
Member Author

commented Sep 29, 2016

My reasoning for always using line comments:

  • internal consistency - better to have one kind of comment than two
  • external consistency - most [citation needed] C/C++ code guides mandate line comments
  • less ambiguity - what is a long comment vs a short comment
  • tooling - at least in the editors I use, it is easier to format a block with line comments than block comments. Also some editors don't handle nested block comments as well as the language does.
@nrc

This comment has been minimized.

Copy link
Member Author

commented Sep 29, 2016

@alexcrichton does make tidy have a solution for this?

I could imagine allowing some kind of rustfmt::skip directive inside comments, although that might be more annoying than the warnings

@alexcrichton

This comment has been minimized.

Copy link

commented Sep 29, 2016

@nrc for make tidy we've got // ignore-tidy-linelength. I think that means that if the string "ignore-tidy-linelength" shows up anywhere in the file that the whole file is ignored for line lengths.

It's true though that the only reason I've ever seen at least to have a super long line is for literally URLs, so maybe there could just be an exception where if the line of the comment that's over the limit contains "http" rustfmt just conveniently doesn't warn about it

@ubsan

This comment has been minimized.

Copy link
Contributor

commented Sep 29, 2016

@nrc

  • internal consistency - I personally don't care very much about this argument. Rust is, and always has been, very TIMTOWTDI
  • external consistency - (gnu)[https://www.gnu.org/prep/standards/html_node/Comments.html#Comments] recommends block comments
  • ambiguity - "anything over a line" or "anything over two lines"
  • tooling - in the editor I use (vim), it's easier to format a block with block comments, because
// Foo
// Bar
// Baz

/*  Foo
    Bar
    Baz
*/

In the line comment case, if you hit >>, then it'll indent everything (including the //) to the right, as opposed to just the comment itself (this is really annoying for code blocks inside of comments, as in an example).

I don't know of any editor that, when using rust code highlighting, doesn't support nested block comments.

@skade

This comment has been minimized.

Copy link

commented Sep 29, 2016

I agree that rustfmt shouldn't change the comment style. It was probably picked for a reason.

I would appreciate trimming of whitespace and word wrapping a lot, though, as re-justifying comments is a real annoyance.

Some linting and spell-checking would be very cool :).

@pnkfelix

This comment has been minimized.

Copy link

commented Sep 29, 2016

The current bug description does not address the issue that external tools may be using sigils within comments to denote certain things.

One obvious example of this is markdown, which I imagine makes it hard to detect adherence to the rules about having periods at the end of sentences. (Plus english itself makes it hard, see?)

Another example that is of particular importance to me is Tango, which uses an @ character immediately after the // comment sigils (with no intervening whitespace between the // and the @) to signal that the tool should treat this comment specially.

While I could have Tango accept (and perhaps even emit) // @ instead of //@, I would prefer if the rules in the description were relaxed to allow for an arbitrary series of non-whitespace, non-alphanumeric characters to follow the //, then a space, and then the comment text.


Update: I did not read the description carefully enough; I had thought it was proposing the 80 character width rule be imposed with more than a warning.

The description mentions moving comments and altering the whitespace "around" comments; I infer this is actually describing something much more constrained, namely reindenting comments, which solely, consists of modifying the amount of whitespace that falls before the // comment sigil. Is this a correct inference?

@nrc

This comment has been minimized.

Copy link
Member Author

commented Sep 29, 2016

@skade

I would appreciate trimming of whitespace and word wrapping a lot

Unfortunately this is the hardest bit because we need to be smart enough to guess when a comment is a code block and we should preserve whitespace and when it is not and we should wrap. We could trim trailing whitespace at the least, although I seem to remember even that can be significant for some systems :-(

@pnkfelix

Yeah, the bullet points should be read as "should"s not "must"s and my current thinking is that they should not be enforced by tools. Rustfmt would have to move comments around, but that should probably be the limit of what it does.

@solson

This comment has been minimized.

Copy link
Contributor

commented Sep 29, 2016

We could trim trailing whitespace at the least, although I seem to remember even that can be significant for some systems :-(

Even in markdown, in fact, where it inserts a break within a paragraph. :/

@joshtriplett

This comment has been minimized.

Copy link
Contributor

commented Sep 29, 2016

@nrc For code, we could look for triple-backquoted code blocks and leave them alone.

@nrc

This comment has been minimized.

Copy link
Member Author

commented Sep 29, 2016

@nrc For code, we could look for triple-backquoted code blocks and leave them alone.

We considered this but, a lot peoples' use cases did not fit this pattern. E.g., people temporarily commenting out code and running rustfmt on save. Or people just not using markdown for their comments (surprisingly common outside the core Rust projects).

@pnkfelix

This comment has been minimized.

Copy link

commented Sep 30, 2016

@nrc wrote:

Rustfmt would have to move comments around, but that should probably be the limit of what it does.

Can you confirm the inference I made when I asked earlier:

I infer this is actually describing something much more constrained, namely reindenting comments, which solely consists of modifying the amount of whitespace that falls before the // comment sigil. Is this a correct inference?

@camlorn

This comment has been minimized.

Copy link

commented Sep 30, 2016

I didn't know the discussion as to whether block or line comments for docs was still open, but suffice it to say that hearing "slash slash slash" at the beginning of every line is indeed stupid and annoying. I can't turn off reporting of / either because that's the division operator.

I don't have a concrete reason to prefer one over the other beyond this and the possibly insignificant factor that some editors don't allow you to comment/uncomment lines. I have never been able to get a sighted person to give me a reason for this preference either. I would be interested to know if someone actually has a concrete reason for the unconditional use of line comments.

@steveklabnik

This comment has been minimized.

Copy link

commented Sep 30, 2016

@camlorn , @nrc had some bulletted reasons in the thread above: #17 (comment)

They're the same reason I prefer them. Though I can only imagine how annoying "slash slash slash" must be. There's no way to treat them as one token?

@camlorn

This comment has been minimized.

Copy link

commented Oct 1, 2016

@steveklabnik
I can also make the case that some editors don't provide the facilities required to format with line comments at all, so that one seems weak. Perhaps my editor has it and I just don't know about it. I can see the point for consistency, but making an exception for doc comments seems reasonable. For what it's worth, most blind people would probably agree with me that unconditionally using block comments is annoying.

There are two problems with the line comment approach as it specifically relates to docs. One is the aforementioned /// reading at the beginning of every line. The other is that screen readers indicate indentation by counting the whitespace at the beginning of the line, and it is not reasonable to expect the readers to learn about commenting styles for source code. If the line starts with non-whitespace characters, the screen reader assumes that there is no indent. This latter point is important for properly formatting examples, etc.

I might be able to make a dictionary entry for ///, but it still has to say something. If I take it out entirely, whether or not I'm looking at a comment at all becomes ambiguous. If it starts or ends with anything looking like code, nothing would remain to differentiate it. It may be possible to learn to do it entirely off context as the people I know who code in Python without indentation indication on do, but this isn't straightforward. One thing I have been pushing for in my screen reader is the ability to use sounds in the middle of speech, so perhaps it will in future be possible to replace the comment symbols with something shorter, like a distinctive click. But this is a long, long way away.

I actually use both styles in my own code. /**/ is for long comments, where reading large blocks of text without spam is important. // is for when knowing that a comment is present is important. A couple lines of // emphasize that there is something to be looked at. It's hard to articulate when I specifically make this choice, beyond saying that I almost unconditionally prefer block for any sort of documentation. And again: blind people don't really have a consistent style, so probably the next person does something completely different. Whether that be unconditional use of one or the other, or just making different choices about when to use each, I don't know.

@joshtriplett

This comment has been minimized.

Copy link
Contributor

commented Oct 3, 2016

Conservative conclusion from the 2016-10-03 style meeting: recommend but don't require line comments (@camlorn's accessibility concerns were quite persuasive there); allow block comments if people want; fix rustfmt to not mangle either kind of comment (move but don't change); possibly in the future allow rustfmt to reformat doc comments only (not non-doc comments).

If someone wants to convert doc comments from block to line (or have an option for the reverse), they can file a separate fmt-rfcs PR with a patch to rustfmt.

@ahmedcharles

This comment has been minimized.

Copy link

commented Oct 14, 2016

For the screenreader case, are there any readers that are extensible enough to have them understand that multiple lines beginning with /// or // form a grouping, much like a paragraph. I mean, I assume they recognize English paragraphs and signal those. If they supported arbitrary grouping constructs, you could have them read out "begin doc comment" and then read the comment without the slashes and then "end doc comment" at the end.

Note: I'm fine with the conservative conclusion.

@camlorn

This comment has been minimized.

Copy link

commented Oct 17, 2016

@ahmedcharles
No. They don't recognize paragraphs outside rich text editors, either. If the app doesn't provide it, the screen reader doesn't usually add it. Screen readers do not do any semantic analysis: they read only what the app provides and how the app provides it. Anything else is a large and difficult technical challenge. Technically possible, mind you, but not likely to happen soon. Rule 1: screen readers are almost completely dumb and provide no additional interpretation.

Provided that there was a text editor that was capable of exposing the info, it would still need screen reader scripting because none of the accessibility APIs are flexible enough to let you do this without cooperation on both sides. Even if it did recognize that it was a comment, it wouldn't have any way of knowing what the comment symbol was, and thus the comment symbol wouldn't be removed unless the app did it on its end. Even if NVDA did remove the commenting symbol, doc comments would still have the single /. And after all that, it would be ap-specific. This wouldn't help code read on GitHub issues, for instance, or on a computer that's not mine, or in a terminal.

If NVDA did the speech refactor this year and it's done to the point where I would like it to be and assuming that someone had a synth that understood text formatting, it would be possible to get most of the general case. This would still require implementing the accessibility API for a rich text control that is capable of exposing code formatting info, then teaching the screen readers to understand it. The soonest it could happen is a year, assuming NVDA got funding; realistically, being able to even begin a worthwhile attempt in the first place is at least a year off. No one has a synth capable of understanding text formatting and running at appropriate programmer speeds, though, and I don't know that I can convince them to pass that information through for something that's only hypothetical right now (writing such a synth becomes a higher priority every day).

And, after all that, we have no control over any of the other screen readers because they're all proprietary. Microsoft, Apple, and Google do not care and have a long history of only doing the minimum; Jaws, while still the most popular, is both proprietary and effectively dead in another few years; everyone else has a userbase that's so small that it's not worth it. Technically you could do something like this to Orca on Linux, but using Linux as a desktop as a blind person has much more fundamental problems.

So anyway, no, this won't get fixed on my end. I understand why it is the way it is and why we're not changing it, but it's an annoyance that will be with me forever to one degree or another.

@ahmedcharles

This comment has been minimized.

Copy link

commented Oct 18, 2016

Yeah, the state of affairs is pretty sad. I once was assigned an accessibility bug in an error dialog of a popular productivity suite. As part of reproducing the issue, I tried using windows with my eyes closed for almost 5 minutes and then I stopped. I did a similar experiment when I bought a macbook and the setup window offered an accessibility option and I figured I should close my eyes and try it.

Anyways, thanks for your perspective.

@camlorn

This comment has been minimized.

Copy link

commented Oct 18, 2016

@ahmedcharles
Nah, it's not as bad as you think. That's equivalent to the whole blindfolded dinners thing, and everyone who is actually blind and knows anything hates those because you come away with the wrong impression about everything and feel sorry for us or whatever. This issue is just hitting one of the hard cases that's paradoxically only really annoying, meaning that no one is going to invest resources.

Learning a screen reader is like learning Vim or Emacs: very frustrating for the first weeks or months. Especially if you're sighted and have to first unlearn sighted usage patterns (like, say, glancing) and can't use synths at insanely insane speeds. I don't suggest that sighted people test themselves; find a blind person to test it for you if possible.

The problem with semantic analysis is twofold: the AT has to run on not-so-powerful computers and what if it gets it wrong (say, on that really important credit card information form)? We could make the API expose more semantic information, but we already have enough trouble having apps be accessible in the first place so making it harder isn't really such a brilliant idea.

But we're now off topic, I suppose.

@ubsan

This comment has been minimized.

Copy link
Contributor

commented Nov 14, 2016

The style team discussed this a few weeks ago, but my life got in the way of writing it up, so I'm doing it now. We generally agreed that line comments are the standard way Rust does it by now, and it's unlikely to change (and will also probably be very unpopular); however, we also want to support other usecases. Therefore, the default recommendation will be line comments, but rustfmt will not change comment style, from line to block or block to line, and block comments will not be actively discouraged.

The recommendation for line width will likely be 80 characters, and rustfmt may eventually (hopefully!) line wrap.

@zetok

This comment has been minimized.

Copy link

commented Dec 5, 2016

Prefer // comments to /* ... */ comments, use the latter only where there is code following on the same line.

.

Therefore, the default recommendation will be line comments, but rustfmt will not change comment style, from line to block or block to line, and block comments will not be actively discouraged.

You got it backwards. Disappointing that rustfmt after all will be useless.

@steveklabnik

This comment has been minimized.

Copy link

commented Dec 6, 2016

@zetok please keep commentary constructive.

@zetok

This comment has been minimized.

Copy link

commented Dec 7, 2016

@steveklabnik care to elaborate on that?
How exactly an information that the tool is (will be) useless for a part of the target audience is not constructive?

@joshtriplett

This comment has been minimized.

Copy link
Contributor

commented Dec 7, 2016

@zetok You provided no information to back up your opinion; all you did was denigrate the work of the style team and the rustfmt developers. If you had provided specific explanations for your position, that would help; for instance, if you had some specific feedback or new information that explained your preference, that would provide a constructive contribution to the discussion. Instead, you just called the decision backward (with no explanation), and then put down the work of the rustfmt developers and contributors as useless.

Hopefully you can see why that provides us with no information, in addition to making our days a little bit more unpleasant.

@zetok

This comment has been minimized.

Copy link

commented Dec 7, 2016

@joshtriplett

You provided no information to back up your opinion; all you did was denigrate the work of the style team and the rustfmt developers. If you had provided specific explanations for your position, that would help; for instance, if you had some specific feedback or new information that explained your preference, that would provide a constructive contribution to the discussion.

First of all, it's not an opinion. If it was about my opinion, I'd say that I don't care, and couldn't care less.
As far as I was concerned, there were no actual differences between either "style".

If not for one thing, namely @camlorn's post on some other RFC about usability of either style for the blind.

So, there is only 1 difference between styles: one works for everyone, while the other makes life harder for the blind. Difference between the right thing to do, and wrong.

It's quite clear what the default choice should be.

And yet, here we are, with the wrong result.

Instead, you just called the decision backward (with no explanation), and then put down the work of the rustfmt developers and contributors as useless.

I call things for what they are. This decision makes rustfmt useless for me, as I have no time or will to check whether every single little thing it can do was done right or wrong. Note that checking everything would be required only because even the most basic choice about the right or wrong default has gone wrong.

Hopefully you can see why that provides us with no information (…)

You already had all the necessary information. The only thing that you were missing was the information about the impact of the decision on usability of the rustfmt, which I've provided.

(…) in addition to making our days a little bit more unpleasant.

Your make your own day.

@ubsan

This comment has been minimized.

Copy link
Contributor

commented Dec 7, 2016

@zetok Please leave if you can't be nice.

For anybody else: do note that rustfmt will not change comment styles. We recommend line comments, but block comments are not discouraged (I personally use block comments for long comments).

@solson

This comment has been minimized.

Copy link
Contributor

commented Dec 7, 2016

So, there is only 1 difference between styles: one works for everyone, while the other makes life harder for the blind.

This is clearly not the case. The preceding comments have discussed pros and cons of both styles that have nothing to do with blindness, so it is not a black and white decision as you claim. Even so, accessibility concerns were not ignored, either. From @joshtriplett's earlier comment:

recommend but don't require line comments (@camlorn's accessibility concerns were quite persuasive there)

Everything has to be weighed and the fact remains that most of the community uses and prefers line comments. It's not the job of the style team to dictate the style, but to listen to the community and develop a set of defaults that most people will want to use.

Inevitably, not everyone will be happy with every decision (even on the style team), but we should try to work with each other and not against each other. We'll get a better result if we're kind to each other.

@chriskrycho

This comment has been minimized.

Copy link

commented Dec 7, 2016

To clarify by way of repetition:

@ubsan: Therefore, the default recommendation will be line comments, but rustfmt will not change comment style, from line to block or block to line, and block comments will not be actively discouraged.

The style guide will recommend line comments; but the rustfmt tool will not change whatever comment style you use. So if you write /* ... */, rustfmt won't argue with that choice at all.

This seems like a pretty reasonable move for me.

@camlorn

This comment has been minimized.

Copy link

commented Dec 7, 2016

@zetok
If I was concerned, I would have come in and said I was concerned.

The long and the short of it is that I can use line comments. They are very annoying, but they do not prevent me from contributing; in fact, I have 2 closed and 1 pending pull requests, and two of them are more or less major refactors in trans.

if we lived in a world in which I couldn't use line comments, I wouldn't even be here. I would have failed out of school or otherwise realized that this issue means that I can't have a career, then gone and done something else.

But anyway, if a moral defense needed to be made then I would have made it myself. If this would have excluded me I would have said so. But demonstrably it doesn't.

@zetok

This comment has been minimized.

Copy link

commented Dec 8, 2016

@solson

So, there is only 1 difference between styles: one works for everyone, while the other makes life harder for the blind.

This is clearly not the case. The preceding comments have discussed pros and cons of both styles that have nothing to do with blindness, so it is not a black and white decision as you claim. (…)

As far as I can see, most of other arguments are irrelevant. They're significant enough to consider only in a situation where you don't have actual arguments and are grasping at straws.

(…) Even so, accessibility concerns were not ignored, either. From @joshtriplett's earlier comment:

recommend but don't require line comments (@camlorn's accessibility concerns were quite persuasive there)

Given the outcome, it's rather clear that the argument was mostly ignored.

Everything has to be weighed and the fact remains that most of the community uses and prefers line comments.

Is community really preferring line comments, or has it been discouraged to use block comments?

Lack of documentation for a very long time (is it still missing?), proposals to remove block comments entirely, bugs that affected only block comments for a very long time, etc. have made people think that either block comments are already not supported, or that Rust is going to drop them in due time.

rust-lang/rust#38173 (comment) ← it's really interesting how even people who are core(?) Rust contributors were made to think that block comments aren't a thing.

I'd like you to not pretend that community had a choice to prefer either, or that supposed choice was made knowingly. Certainly I didn't get to know about block comments from the documentation when I started learning Rust, and from what I've seen same goes a lot of people. Check, a lot of people are still unaware that block comments are possible.

Everything that could have been done was done to push people in direction of line comments, spare for explicitly officially saying that they shouldn't be used. But hey, this RFC does that job quite well by suggesting that line comments are the way to go.

It's not the job of the style team to dictate the style, but to listen to the community and develop a set of defaults that most people will want to use.

You're pushing people. Now, if you want to listen to people and improve things, it's quite clear that the default should be to suggest block comments, while not discouraging line comments.

The preference of "the community" could be another argument beside usability for the blind to consider, but is it really valid, given that there was a clear movement to push community in a certain direction?

So, again, there is only 1 relevant argument, namely usability for the blind.
The other argument could have been community's preference, but I don't see how can this be considered, given that community has been pushed to the line comments.

"we did what we could to make people prefer line comments, and look, they prefer them!"

"The community" prefers whatever the default is. You're defaulting to line comments. Guess what.

Inevitably, not everyone will be happy with every decision (even on the style team), but we should try to work with each other and not against each other. We'll get a better result if we're kind to each other.

Sure :)

@chriskrycho

This seems like a pretty reasonable move for me.

Well, reasonable would be suggesting to use block comments while in no way discouraging line comments.

I.e. suggesting the right thing while not explicitly discouraging people from using line comments since they're used a lot.

@camlorn
Yes, I get that. Line comments make things more annoying for you, and you can work with them. The thing is, there could be less annoyance for you, while usability for other people wouldn't be affected. It seems logical to just set the default for whatever that causes least amount of friction, in this case block comments.

@camlorn

This comment has been minimized.

Copy link

commented Dec 8, 2016

@zetok
Actually, there is a reason for line comments being the default: they are the default in many other languages. C++ tends to use them heavily. Python doesn't have block comments at all. SO too what javascript I've seen.

Instead of finding reasons why the community is wrong, tell us why you personally are attached to block comments. It is probably too late for argument: the point of team decisions is that they be somewhat final. But you're essentially saying that my argument for accessibility is the only reason why and that all other reasons are irrelevant and I, the blind person, do not buy such an argument. Despite being the one person on this thread who literally doesn't understand formatting from the sighted perspective.

Discussions cannot go on forever. Some choice has to be made, and someone is going to be dissatisfied. If there were malice here, I would have left the community, but Rust has done an excellent job. This community is the only time I've gotten a 24 hour turnaround on accessibility issues, outside my screen reader which is maintained by blind people for blind people.

Please leave the accessibility argument to the people affected. If you see a problem, by all means say you think there's a problem and ping me in your comment. I was more upset by the RFC thread because it removed the choice for my personal projects, and I want my personal projects to be convenient. But there is no point being adamant here. I could cry accessibility on everything, if I wanted. But then it loses its value.

@solson

This comment has been minimized.

Copy link
Contributor

commented Dec 8, 2016

@zetok

As far as I can see, most of other arguments are irrelevant.

Others don't agree with you that they are irrelevant. It's coming across like you personally want block comments and are trying to discount the arguments of others without actually addressing them.

Given the outcome, it's rather clear that the argument was mostly ignored.

Obviously I still disagree, but I don't think I can change your mind.

The preference of "the community" could be another argument beside usability for the blind to consider, but is it really valid, given that there was a clear movement to push community in a certain direction?

There wasn't a movement to push the community. That's bordering on a conspiracy theory, and implies ill motives and manipulation where none were present. The preference of the community is a valid and important factor.

The fact is that line comments were just historically popular in the Rust community from early on. Then, the more people used them, the more others wanted to follow suit. Block comments naturally fell by the wayside to some extent because people didn't want multiple comment types, or didn't learn about them (because they weren't popular), etc.

If block comments were the style that was popular early on, we might be having a different discussion, but there's a strong pressure to pick a style that most of the community considers normal right now, so that rustfmt doesn't come along and make Rust code suddenly different from what everyone's used to.

A similar problem came up with 2-space vs. 4-space indent, for example. A significant group of people preferred to change the default to 2-space (myself included), but the fact is that the vast majority are using 4-space, so the arguments would have to be very strong indeed to override the community norm.

Similarly, the accessibility issue was important enough that we decided not to change block comments to line comments by default, but not strong enough to override the community norm of recommending line comments.

Finally, to reiterate the conclusion again, block comments will be supported and allowed by default. Line comments will be recommended as they are the community norm. Neither will be converted into the other by default.

@johnthagen

This comment has been minimized.

Copy link
Contributor

commented Jan 27, 2017

I hadn't seen it discussed so I thought I would bring up the question "How many (if any) spaces should be used between code and an inline comment?"

Currently rustfmt adds 1:

let a = 1;// Hi

to

let a = 1; // Hi

An alternative, from PEP8:

Inline comments should be separated by at least two spaces from the statement.

Which would be:

let a = 1;// Hi

to

let a = 1;  // Hi

I don't have a strong preference. For Python code I think it makes the code slightly easier to read. I think in Rust it's probably less important to have two spaces because most of the the time there will be a ; before it. Either way, I think it would be good to make an official decision in the RFC.

@solson

This comment has been minimized.

Copy link
Contributor

commented Jan 27, 2017

The Google C++ style guide also suggests two spaces: "lines that are non-obvious should get a comment at the end of the line. These end-of-line comments should be separated from the code by 2 spaces."

I've used both 1 and 2 spaces and I don't have a strong preference, either.

@steveklabnik

This comment has been minimized.

Copy link

commented Jan 27, 2017

@joshtriplett

This comment has been minimized.

Copy link
Contributor

commented Jan 27, 2017

In the context of Rust code, I don't see a strong benefit to the extra space there; I personally always use one. (To the extent I put comments on the same line as code at all; I tend to put them on a separate line above, instead.)

@chriskrycho

This comment has been minimized.

Copy link

commented Jan 27, 2017

@nrc

This comment has been minimized.

Copy link
Member Author

commented Mar 14, 2017

We've changed our process: rather than requiring a full RFC, a PR to close this issue only requires making the changes discussed here to the style guide. You can see more details about the process in the readme for this repo.

If you'd like to help with this work, let me know, I'd be very happy to help you help us.

@johnthagen

This comment has been minimized.

Copy link
Contributor

commented Mar 23, 2017

Comments should be kept within the 80 char margin (longer comments are harder to read).

I have no problems with this conceptually, but I wonder about how practical this will be to do manually. For instance, most editors I've used before (IntelliJ, Sublime Text, Eclipse, Vim, etc) typically have a single "right margin" line that shows you where the end of the line is. But with this guideline we no potentially have two margins, one set at 100 for code and one set at 80 for comments. The "invisible" 80 seems like it could be frustrating to manually deal with if you can't see it and many tools aren't set up to wrap lines of different types differently.

The argument that "longer comments are harder to read" doesn't always apply the same way if you start comments pretty far indented. For comments that start indented is the intent that they should be wrapped at indent + 80 or at absolute 80 (which makes them even smaller than 80)?

If I comment out code, would rustfmt then wrap it at 80? (Granted if it was already at ~98 columns, the extra // might cause a wrap anyway).

I say all this just to discuss the idea of whether to only edict a single right margin, and maybe soften the comment style to something like "prefer wrapping comments more tightly when it aids readability".

In Python, PEP8 states comments and docstrings should be wrapped closer than the normal width, but flake8 as of 3.3.0, for example, only actually checks against the single right margin and does not consider comments separately. This could be considered a bug or maybe it's for pragmatic reasons.

@joshtriplett

This comment has been minimized.

Copy link
Contributor

commented Mar 23, 2017

@johnthagen I think we ended up deciding that it's too dangerous for rustfmt to word-wrap comments at all, because unlike Rust code, rustfmt can't necessarily know whether your comments have some reason for their wrapping (pseudocode, diagrams, etc). I might have misremembered though; it's been a while since we talked about comments.

(Personally, I use 100 characters for both code and comments.)

@ssokolow

This comment has been minimized.

Copy link

commented Mar 23, 2017

It's not as if Rust comments are plain text. Markdown has defined rules for when whitespace is significant that rustfmt can be taught and I enable what auto-rewrap I can get in any language which provides that functionality.

Given how much plain English I write when documenting, I'm far more likely to want to my text to be reformatted like that than not and, with my Vim showing the boundary column (column 100 for Rust, column 80 for Python), it's easy to plan ahead in situations where something has significant whitespace.

(Of course, it probably helps that I never use doctests because of shortcomings in how they integrate with some of the tooling I use.)

@johnthagen

This comment has been minimized.

Copy link
Contributor

commented Mar 23, 2017

@joshtriplett Thanks for the clarification. If 80 columns is more of a guideline I think that sounds good. I too use 100 characters for both code and comments as the hard limit just for simplicity, but will often wrap comments earlier manually if it makes it more readable.

@nrc nrc referenced this issue Aug 4, 2017
29 of 29 tasks complete
nrc added a commit that referenced this issue Sep 13, 2017
Comments
closes #17
cc #89
@nrc nrc referenced this issue Sep 13, 2017
nrc added a commit that referenced this issue Nov 13, 2017
Comments
closes #17
cc #89

@nrc nrc closed this in #95 Nov 13, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.