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

rustdoc should understand javadoc-style comments #2498

Closed
brson opened this issue Jun 4, 2012 · 20 comments
Closed

rustdoc should understand javadoc-style comments #2498

brson opened this issue Jun 4, 2012 · 20 comments
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@brson
Copy link
Contributor

brson commented Jun 4, 2012

Both /** ... */ and /// syntaxes.

It should be relatively simple to write a rustdoc pass to pull out these comments. The only complications are that we need to heuristically associate comments with AST nodes, and we have to be able to identify and strip the common wall of stars:

/**
 *
 * <- these stars are not part of the documentation
 *
 */
@brson
Copy link
Contributor Author

brson commented Jun 4, 2012

The other downside is that this will make it more difficult to extract docs from compiled crates (which we can't actually do yet). Possibly we could have a rustc pass to promote doc comments to doc attributes.

@nikomatsakis
Copy link
Contributor

Maybe we should just treat Rustdoc comments as a kind of attribute and embed them in the AST/metadata like other attributes.

@eholk
Copy link
Contributor

eholk commented Jun 4, 2012

I kind of like the #"""This is a doc""" syntax from @brson's e-mail. I kind of like the idea of leaving comments completely unstructured for the most flexibility in explaining things to people reading the code, and having a lightweight, but more structured, syntax for documentation that gets extracted into a standalone entity.

@Dretch
Copy link
Contributor

Dretch commented Jun 10, 2012

OK then, given that this is marked easy, id like to work on it. Could @brson please point me in the right direction?

@brson
Copy link
Contributor Author

brson commented Jun 11, 2012

The basic strategy that I recommend is to modify the parser to treat doc comments as a special type of attribute, so that they are easily and automatically associated with the appropriate AST nodes.

The one significant wrinkle is that we have to distinguish, like attributes, between docs that apply to the thing that follows and docs that apply to the preceding thing. With attributes we currently use the trailing semi:

mod m {
   #[attr1]; // semi means this is an attribute of mod m
   #[attr2] // no semi means this is an attribute of fn f
   fn f() { }
}

Which is also how we distinguish crate attributes (semi-terminated attributes at the top of a .rc file apply to the 'containing' crate). I do want to change this syntax in a separate issue (#2569).

I suggest for doc comments that we use doxygen's convention for specifying docs that come after the thing they document: /**< */ and ///<, so crate docs would look like

/**<

The Rust core library

...
*/

It's unfortunate to have to distinguish between the two but not doing so would probably require much more subtle heuristics.

So here is an approximate plan of action:

  • Add a new lexer token to libsyntax/parse/token.rs called DOC_COMMENT(str_num, ast::attr_style), where str_num is an index to an interned string containing the entire comment and attr_style indicates whether it's an inner or outer attribute.
  • Add a boolean to ast::attribute_ in libsyntax/ast.rs indicating that the attribute is a sugared doc attribute.
  • Modify the lexer in libsyntax/parse/lexer.rs to emit doc comment tokens instead of passing over them as whitespace.
  • Mody attr parsing in libsyntax/parse/attr.rs to convert doc comment tokens to doc AST attributes.
  • Modify the pretty printer to understand doc comment attributes.

At this point rustdoc should automatically be extracting the doc comments from attributes, leaving all the decorating comment syntax (/// etc) intact.

  • Add strip_comment_pass to rustdoc to run over all the documentation and strip out comment syntax bits.

@Dretch hopefully this is enough to get started

@brson
Copy link
Contributor Author

brson commented Jun 11, 2012

@graydon you might want to be aware of this issue

@graydon
Copy link
Contributor

graydon commented Jun 12, 2012

Thanks. I am fine with reforming the applies-to-next / applies-to-enclosing syntax, but not sure angle bracket is the most suggestive. Don't feel Doxygen is an important enough precedent to drive this choice. Maybe play with a couple other variants visually in a scratch buffer to see if anything else looks a little better?

@graydon
Copy link
Contributor

graydon commented Jun 12, 2012

Also note that the lexer already has logic to extract comments and in particular grab the indentation-wall out of the way, it does so during the scan for comments-and-literals that it transplants into pretty-printed source.

@Dretch
Copy link
Contributor

Dretch commented Jun 12, 2012

Wow, thanks alot @brson (and @graydon), that is extremely helpful.

@Dretch
Copy link
Contributor

Dretch commented Jun 22, 2012

I have made some changes to hopefully fix this issue. You can see them here: Dretch@df2f4bb

Unfortunately I can't seem to make a pull request of just this commit - i keep getting this commit in the pull request too: Dretch@986662c. I think this may be because I have screwed up the branching somehow.

@brson
Copy link
Contributor Author

brson commented Jun 22, 2012

@Dretch that patch looks great! Exactly how I imagined, and the conversion script is a nice bonus.

Before we merge it can you change the inner doc comment syntax to be the one @graydon suggested in #2569 (//! and /*!)?

@brson
Copy link
Contributor Author

brson commented Jun 22, 2012

I did notice that the patch included a fix for a commented-out comment (////) that no longer even parses under this new scheme because it was not in attribute position. I wonder how big of a concern that is.

@Dretch
Copy link
Contributor

Dretch commented Jun 23, 2012

I did notice that the patch included a fix for a commented-out comment (////) that no longer even parses under this new scheme because it was not in attribute position. I wonder how big of a concern that is.

To avoid this, how about having //! ... and /*! ... */ for outer doc-comments and //!! ... and /*!! ... */ for inner ones?

@brson
Copy link
Contributor Author

brson commented Jun 25, 2012

@Dretch How would attribute syntax mirror that, still #[...] and #![...]?

I guess I'm not crazy about it but don't have any concrete explanation why. @graydon what do you think?

@graydon
Copy link
Contributor

graydon commented Jun 25, 2012

I agree with @brson, not too keen on //!!. Double exclamations get you into "reads like a comic strip" syntax :)

I'd suggest either inserting an extra space in that testcase (i.e. writing it as // // foo) or else making it recognize the 4-character sequences, specifically, ///[\t ] and /**[ \r\n\t] -- that is, include a single character of whitespace following the doc-comment open as part of how you recognize it.

(I gather sometimes people want long lines of /////////// or /************ to introduce doc-sections, so maybe the first fix is better?)

@Dretch
Copy link
Contributor

Dretch commented Jun 28, 2012

I was going to go with:

Before we merge it can you change the inner doc comment syntax to be the one @graydon suggested in #2569 (//! and /*!)?

..but then I noticed that this conflicts with the //! ERROR and //! WARNING scheme used by the test suite (https://github.com/mozilla/rust/wiki/Note-testsuite).

So how about using /// ... and /** ... */ for outer comments (as originally proposed), and ///! ... and /**! ... */ for inner ones (with three, not two, leading characters)?

@brson
Copy link
Contributor Author

brson commented Jun 28, 2012

I would rather change the error comment syntax in the test suite. Anything will do since it's not part of the language. How about //~?

@Dretch
Copy link
Contributor

Dretch commented Jun 28, 2012

I would rather change the error comment syntax in the test suite. Anything will do since it's not part of the language. How about //~?

That sounds ok to me.

@Dretch Dretch mentioned this issue Jun 30, 2012
brson pushed a commit that referenced this issue Jul 5, 2012
@Dretch
Copy link
Contributor

Dretch commented Jul 5, 2012

I have updated https://github.com/mozilla/rust/wiki/Doc-using-rustdoc to reflect the new syntax.

@brson
Copy link
Contributor Author

brson commented Jul 5, 2012

Thanks, Dretch! Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants