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

Add external doc attribute to rustc #1990

Merged
merged 6 commits into from
Sep 21, 2017
Merged

Add external doc attribute to rustc #1990

merged 6 commits into from
Sep 21, 2017

Conversation

mgattozzi
Copy link
Contributor

@mgattozzi mgattozzi commented May 4, 2017

RFC Accepted (tracking issue)

This RFC proposes an addition to be able to pull in documentation for items from
an external source at compile time, rather than requiring them to be in the
source code itself.

Rendered

This RFC proposes an addition to be able to pull in documentation for items from
an external source at compile time, rather than requiring them to be in the
source code itself.
@Manishearth
Copy link
Member

An alternate syntax, which fits in more with how #[doc] gets used, could be #[doc(include = "foo.md")]

And code like this:

```rust
#[external_doc("docs/example.md", "line")]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the line/mod stuff is necessary, just have #[external_doc("foo.md")] and #![external_doc("foo.md")]

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed; just use it in place of the doc attribute.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a better idea. I'll update the RFC when I can with this syntax as I find it more ergonomic and simmilar to what already exists in Rust.

@killercup
Copy link
Member

A few times, I've tried to use #[doc = include_str!("foo.md")], so I think include_doc might be a better name (or the include = "foo" syntax as suggested by #1990 (comment)). external makes it look related to extern fn IMHO.

Also, using #![attr] is far superior to adding a "mod" parameter.

learn to use Rust. It should be taught to existing users by updating
documentation to show it in use and to include in in the Rust Programming
Language book to teach new users. Currently the newest version of The Rust
Programming Language does not include doc comments as part of it. This would
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do include doc comments in the second edition, it's in the publishing a crate to crates.io section.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah see I thought it would have been under the comments section which is why I missed that.

@mgattozzi
Copy link
Contributor Author

I've updated the syntax and clarified the section about the Rust book in how we teach this.

@nrc nrc added T-lang Relevant to the language team, which will review and decide on the RFC. T-dev-tools T-doc Relevant to the documentation team, which will review and decide on the RFC. labels May 11, 2017
@nikomatsakis
Copy link
Contributor

I'm a fan of this general concept. I dislike having pages and pages of stuff inline, if nothing else, but also agree that there can be a lot of duplication between README.md etc.

@aturon aturon removed the T-lang Relevant to the language team, which will review and decide on the RFC. label May 11, 2017
@joshtriplett
Copy link
Member

Conclusion from the language team: generally quite favorable, but this should fall entirely within the purview of the doc and/or dev-tools teams, rather than lang.

@nikomatsakis
Copy link
Contributor

cc @rust-lang/docs

@nrc
Copy link
Member

nrc commented May 11, 2017

A possible extension to this is also allowing links into a section of a file. I imagine that some people might want to have all docs outside of the code file, and having a separate doc file for every function seems tedious. (Whether we want to encourage such a style is a valid question). I imagine we could allow something like include = "foo.md#section" where section is a markdown title in foo.md - I assume there is some markdown standard for doing this.

@jugglerchris
Copy link

A mostly lurker here throwing my thoughts...

Instead of adding an attribute to every item whose documentation is in a separate file, which is also sort of tedious, it could be implicit (or have an implicit default). So:

// module level
#![doc(external = "mymod.md")]

// no doc comment: rustdoc would look for a section "function_foo" or "fn.foo" or something like that
// in mymod.md
fn foo() -> usize { .. }

// This one specifies a different file.
#[doc(external = "mymod_bar.md")]
fn bar() { .. }

I'm not sure about the value of allowing item documentations both inline and separately for the same module, except perhaps when half way through converting from one format to the other.

I would definitely want the missing_docs lint to still work, and if the implicit link I suggest were done I'd also want a lint against documenting the same thing twice.

@mgattozzi
Copy link
Contributor Author

@nrc I think that would be a good idea since putting a separate file for each item seems excessive and it might be nicer on a per module basis. Usually in markdown a section is denoted with something like # or ## you could even use the --- notation to split things up per section. I think maybe something like # for types and then ## for sub-types like enums or fields in a struct might be a good way to split things up.

@frewsxcv
Copy link
Member

Love this proposal. I've read it over twice and haven't thought of any issues. I'm particularly looking forward to deduplicating text/code shared on the top-level docs and the README.

I imagine we could allow something like include = "foo.md#section" where section is a markdown title in foo.md - I assume there is some markdown standard for doing this.

This RFC is currently documentation format agnostic and has no knowledge or opinions about the underlying markdown syntax. Right now, there's nothing technically stopping someone from creating a rustdoc-like alternative that uses a different documentation format for all their doc comments and rendering it appropriately. Assuming this is a deliberate design decision (of keeping markdown as the docs format and the Rust language separate), seems like we can't add markdown-specific features (headings).

@mgattozzi
Copy link
Contributor Author

@frewsxcv If we want to keep it format agnostic would there be a syntax we should approach to denote what goes where if this is implemented?

@frewsxcv
Copy link
Member

I don't really understand your question @mgattozzi, can you rephrase it? The proposal as written is documentation format agnostic, which I think is a good thing.

@mgattozzi
Copy link
Contributor Author

Ah my bad @frewsxcv. I guess I should have said what should we do about having multiple items in a file? @jugglerchris brings up a good point about not wanting to have to annotate things multiple times or having lots of files. If we do have multiple items in a file we need a way to tell the compiler how to split up the documentation.

@Havvy
Copy link
Contributor

Havvy commented May 27, 2017

We should punt on multiple items being documented in a single file for another RFC. Get this change accepted first, as it's minimal and any sectional solution can base itself on this. Avoid unnecessary scope creep.

@mgattozzi
Copy link
Contributor Author

That's a good point. I'd be fine with opening another RFC to discuss all these extra expansions after this RFC gets accepted.

@steveklabnik
Copy link
Member

We talked about this in the docs team today, and everyone is pretty favorable for this. There's just one concern though, and that's about the "implementation section". Right now, /// is sugar for #[doc], so it seems strange that we'd have #[doc(include)] be sugar for /// which is sugar for #[doc]. That shouldn't actually affect the RFC though.

@eddyb
Copy link
Member

eddyb commented May 31, 2017

That makes me wonder if #[doc = include_str!("foo.rs")] will "happen" to work in the near future.
cc @jseyfried

@QuietMisdreavus
Copy link
Member

Alternately, it may be prudent to rephrase what it actually wants to do. For me specifically, the phrase "expand out ... at compile time" was what threw me off. Currently, rustdoc doesn't even completely "compile" crates to generate their documentation. It gets as far as an AST, and for any given item it checks for any #[doc] attributes. (The /// sugar is processed before this.) With this implementation in mind, it's possible to say that that's "compile time", but that wasn't my first impression, and it would be worth elaborating what's intended there.

Moreover, there's currently plans to rewrite rustdoc to use rls-analysis and the save-analysis data emitted from the compiler. That makes it a little clearer, since we could make sure to state that the file substitution occurs when that analysis data is saved, but it's still different from my first impression when I see "expanded at compile time".

Something else I thought of while considering the #[doc = include_str!("foo.md")] rabbit-hole: Where are paths in doc(include) attributes based? include! and friends base their paths starting from the directory that the file is in, but the RFC acts like they're based on the current-working-directory based on the time of compilation, i.e. the docs folder is sitting alongside src, rather than inside it. This should be specified in the RFC, or at least codified for the implementation.


In all, I'm very much in favor of the idea, and the syntax, but that implementation note is what tripped me up. Though to be honest, in writing up those first two paragraphs, I cleared up my own misconceptions about it, so maybe it's not as bad as I originally thought. >_>

@nrc
Copy link
Member

nrc commented Jun 6, 2017

We discussed this today in the dev-tools meeting (https://github.com/nrc/dev-tools-team/blob/master/minutes/meeting%20notes%202017-06-05.md). We generally approve of the RFC. We thought it was important to address #1990 (comment) - this might be a good alternative to this RFC.

I would also like to think about longer term plans. Do we want to be able to link a section of a file to become documentation (e.g., doc="foo.rs#bar"). If so, that would be a mark against using include_str!. This question (in my mind) comes down to 'do we want to encourage out-of-line documentation?', i.e., separating docs from code. I think we probably do not, since it makes source code less readable.

Further, do we want to just facilitate including README.md into the head of a crate, or is there a more general problem here? Could Rustdoc do a better job of documenting crates with a facility like this? I'd love to hear use cases beyond README.md, even if they'd need quite advanced machinery in Rustdoc. (I don't think we should necessarily have a more general or complex RFC, but I would like to be sure we are heading in the right direction and not just adding a bandaid that will become obsolete in the long term).

@mgattozzi
Copy link
Contributor Author

@QuietMisdreavus I was unsure as I was just banging this out. Thanks for the clarification :D

@behnam
Copy link

behnam commented Sep 28, 2017

Thanks, @mgattozzi. Yes, it's clear now. A couple of suggestions then:

  • Maybe you can put something along these lines in some informative sections of the RFC text itself, to keep it more self-sufficient?

  • Since one of the motivations of putting pieces of documents in external files is to be able to support multiple languages, I think it still remains an open question how to do so more efficiently, specially for internal documentations. In other words, something like the example below still needs some love to be practical, IMHO. Maybe you want to add something about this to the "Unresolved questions" section?

#[cfg_attr(lang = "en", doc("This is a struct to do blah blah blah."))]
#[cfg_attr(lang = "jp", doc("これはストルクトだ"))]
#[cfg_attr(lang = "fa", doc("این یک ساختار است برای فلان و بهمان."))]
pub struct ThisStruct;

@behnam
Copy link

behnam commented Sep 28, 2017

By the way, is there any discussion for rustdoc's support for multiple languages?

The solution proposed here is kind of in contradiction with the solution suggested here, and may be a bit in conflict: https://internals.rust-lang.org/t/pre-rfc-localization-for-rustdoc/3190

There's also mentions of localization support for rustdoc here, with no details, though: rust-community/team#178

@mgattozzi
Copy link
Contributor Author

@behnam now that the RFC is accepted I don't think it can be changed beyond little fixes for typos. I could be wrong but generally for history sake things shouldn't be changed as what was accepted was the text above.

There was talk about localizing error messages here but nothing definite. The pre-rfc you mentioned was from Feb of last year. This hadn't begun to be written until shortly after RustFest in Kiev after having discussed it with a few people after the conference. Looks like there was a pre-rfc over on the community repo from the localisation team, but nothing since Aug 16th. According to the this issue any kind of localisation for error messages would require an RFC. I haven't seen any yet, but now that this is accepted it could easily be used as the basis for another RFC focusing on that.

As for current rustdoc I don't see anything there. Nor on the new version that I'm helping work on during the impl period. Any major changes like this would require an RFC if it's to be used for current official Rust projects as these are pretty big changes the maintainer teams will be stuck with, including updating translations of docs.

@behnam
Copy link

behnam commented Sep 28, 2017

Uh, I didn't notice that this PR is closed! The labels still say "final-comment-period" and "S-waiting-on-community-feedback". I suppose it's enough to have this conversation here for the record then. Thanks anyway! 😄

@Manishearth
Copy link
Member

(clarified in the rfc PR body, usually they link to the tracking issue iirc)

@Manishearth Manishearth removed final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. S-waiting-on-community-feedback labels Sep 28, 2017
@mgattozzi
Copy link
Contributor Author

Hahaha okay this makes a bit more sense @behnam. I definitely would love to see a localisation PR at some point for sure. I think it would be an huge gain!

@chriskrycho
Copy link
Contributor

Is this actually working? Or not yet/only on @QuietMisdreavus's fork?

@QuietMisdreavus
Copy link
Member

@chriskrycho As rust-lang/rust#44781 is not yet merged, "official" rustdoc does not yet have this feature. If you'd like, you can check out my branch, build it, and try it out, but until that PR is merged that's the only way to try it out.

QuietMisdreavus added a commit to QuietMisdreavus/rust that referenced this pull request Nov 21, 2017
Partial implementation of rust-lang/rfcs#1990
(needs error reporting work)

cc rust-lang#44732
bors added a commit to rust-lang/rust that referenced this pull request Nov 22, 2017
rustdoc: include external files in documentation (RFC 1990)

Part of rust-lang/rfcs#1990 (needs work on the error reporting, which i'm deferring to after this initial PR)

cc #44732

Also fixes #42760, because the prep work for the error reporting made it easy to fix that at the same time.
@sunjay
Copy link
Member

sunjay commented Dec 7, 2017

Will doctests within external files still work?

@QuietMisdreavus
Copy link
Member

Yes, anything loaded in the external file is treated the same as if it were written inline.

(Please note that farther discussion of the feature - especially the implementation - should happen on the rust-lang/rust tracking issue. >_>)

@sunjay
Copy link
Member

sunjay commented Dec 7, 2017

Thanks! Sorry for asking in the wrong place!

@petrochenkov petrochenkov added the T-dev-tools Relevant to the development tools team, which will review and decide on the RFC. label Jan 30, 2018
kennytm added a commit to kennytm/rfcs that referenced this pull request Feb 9, 2018
aturon added a commit that referenced this pull request Feb 14, 2018
@Centril Centril added the A-attributes Proposals relating to attributes label Nov 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-attributes Proposals relating to attributes T-dev-tools Relevant to the development tools team, which will review and decide on the RFC. T-doc Relevant to the documentation team, which will review and decide on the RFC. T-rustdoc Relevant to rustdoc team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.