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

Guide: show Rust has range comments; doc comments as convention. #17903

Closed
wants to merge 1 commit into from

Conversation

jkleint
Copy link

@jkleint jkleint commented Oct 10, 2014

Range comments are much more friendly for writing large blocks of text.
Reading the Guide, I was a bit alarmed that Rust might not have them.
Speaking of, they're very useful for doc comments, which are just a
convention for formatting comments, not a separate type of comment.

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @aturon (or someone else) soon.

@aturon
Copy link
Member

aturon commented Oct 10, 2014

r? @steveklabnik

@aturon aturon assigned steveklabnik and unassigned aturon Oct 10, 2014
@steveklabnik
Copy link
Member

We generally recommend against using these kinds of comments, and so I don't think we should bring them up here.

@reem
Copy link
Contributor

reem commented Oct 10, 2014

I was also under the impression that repeated /// was encouraged over /** for comments.

@jkleint
Copy link
Author

jkleint commented Oct 10, 2014

What is the advantage to that style? The style guide doesn't provide a rationale. Range comments would seem to be less mindless typing and less maintenance burden.

As a newbie, seeing all that inefficiency made me recoil a bit. I was actually worried Rust might not have range comments. Even if there's a good reason to prefer single-line, letting people know that range comments exist would let them make their own informed decision.

@tbu-
Copy link
Contributor

tbu- commented Oct 10, 2014

As it stands, the style guide would have to be changed first in order to make this happen.

@jkleint
Copy link
Author

jkleint commented Oct 11, 2014

OK, updated to reflect the preferred style of doc comments.

What's the best way to start a conversation about the style guide?

Thanks.

@sfackler
Copy link
Member

There's a thread on rust-dev somewhere talking about it, but we decided to go with /// because there's only one possible style, compared to

/**
  * This is a foo
  */

vs

/**
 * This is a foo
 */

vs

/**
    This is a foo
 */

etc.

@jkleint what do you mean by inefficiency? The prevailing style in C, Java, etc block comments is the first or second forms, so you still have to add a thing to each line. The vim and probably emacs and other syntax files in the rust repo already know how to automatically insert a new /// on newlines.

@jkleint
Copy link
Author

jkleint commented Oct 11, 2014

Ah, I had not thought of that. If absolute uniformity is the goal, I suppose that would be the way to go. I assumed the goal was, well, good style: easy-to-read, easy-to-write, elegant code.

I'll be honest: it's mostly a taste thing. Any variation of the range comment looks, to me, easier to read, write and just more elegant than a block of triple slashes. As mentioned, most every other language that could prefer line comments chooses not to -- maybe there's a reason? Rust is about practicality and efficiency; a systems programming language, where less is more, perhaps a nod to the days where every byte counted. The line comments shout "unconsidered waste." Upon first seeing it, the optimizer in me thought "I reeeeely hope there's better way to do that." It detracted from me learning Rust. I stopped reading and reached for the language reference hoping Rust was not actually that tedious. Plus, am I the only one for whom it evokes the ASCII-art crack-tro scene .NFO files of yore? It just seems a little gauche by association.

By inefficiency, I mean it's O(N) characters to comment N lines, where O(1) will do. Yes, your editor can do it for you, but just because it can generate reams of filler does not mean it should; and not everyone's editor supports Rust (yet!). In the case of documentation, you may be writing Rust "code" in the context of a Markdown or other file where Rust-specific assists are not available. (Incidentally, Pandoc does not seem to render triple-slash blocks correctly in the Guide.)

Personally I feel that much waste is distasteful enough that it will drive people to use range comments, and you will have even more divergence from the style guide than a space here or there. A new language doesn't have to follow the prevailing style -- in fact, the style guide might be a great place to change it.

Anyway, these aren't my ideas. From Code Complete, Chapter 32:

Use styles that don't break down or discourage modification. Any style that's too fancy is annoying to
maintain.
...
For longer comments, the task of creating long columns of double slashes, manually breaking lines of text between rows, and similar activities is not very rewarding, and so the /* … */ syntax is more appropriate for multiline comments.

@steveklabnik
Copy link
Member

Yes. Given that the guide is not intended to be comprehensive, and is supposed to show you the right way, I think adding this is a distraction. If we change the recommended style, I am open to applying this patch, but I don't think that it's right to do so now.

Thank you for the time and effort anyway.

lnicola pushed a commit to lnicola/rust that referenced this pull request Aug 29, 2024
Allow flycheck process to exit gracefully

Assuming it isn't cancelled. Closes rust-lang#17902.

The only place CommandHandle::join() is used is when the flycheck command
finishes, so this commit changes the behavior of the method itself.

The only reason I can see for the existing behavior is if the command is somehow holding onto a build lock longer than it should, this would force it to be released. But it would be a pretty heavy-handed way to solve that issue. I'm not aware of this occurring in practice.
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

Successfully merging this pull request may close these issues.

7 participants