Add some JSDoc comments to rustdoc JS #92026
Conversation
|
Some changes occurred in HTML/CSS/JS. |
This comment has been minimized.
This comment has been minimized.
|
|
||
| /** | ||
| * @typedef {{ | ||
| * doc: string, |
There was a problem hiding this comment.
I'm not sure how useful this comment is...
There was a problem hiding this comment.
The main goal for this comment is to describe the schema of these input rows from search-index.js, in a way that can be automatically checked by Closure Compiler.
I went ahead and wrote a longer comment trying to describe the schema better. I had to put "is a mystery" for some of the fields because I couldn't figure them out. Maybe you can help me fill in the details. :-)
|
The adds of the comment looks good overall. My issue is about the newly created global variables: we tried to avoid that as much as possible originally because it's super easy in JS to overwrite them without taking care in a sub-function... |
Can you document this somewhere other than the PR description, please? Maybe the dev-guide or at the top of one of the KS files? |
Yeah, these are a bit odd- but that is how Closure Compiler does typedefs. Essentially you define a dummy variable that doesn't do anything, just to attach the type information to it. We're not really at risk of overwriting these (CrateCorpus, Corpus, ParsedQuery, Row), because we don't actually rely on their values anywhere. They're purely a place to attach type information.
I put it in a README in the JS directory. |
It seems very wrong to me: we're adding more (unnecessary) code to satisfy the doc generator... Not only that but you need to create a file
I don't think this is the right place to put this documentation. What about putting it on the function generating the search index? It's where I would look if I wanted to know what the argument is doing. And that would also allow to getting rid of these newly added variables.
👍 |
In this case we're not using it as a doc generator but a type checker. I did realize I can move these typedefs into externs.js, where they won't generate a Can you explain why you think externs.js is bad? It is not built into rustdoc and it doesn't wind up in the minified output.
Moved to buildIndex(). |
It was because it only contained two lines which didn't seem to have any link to the rest. So for potential new contributors, very confusing. Could you add a comment at the top of the file explaining why it exists please? |
This follows the Closure Compiler dialect of JSDoc, so we can use it to do some basic type checking. We don't plan to compile with Closure Compiler, just use it to check types. See https://github.com/google/closure-compiler/wiki/ for details.
|
Nit fixed! |
|
Thanks! @bors: r+ rollup |
|
📌 Commit 7ba086c has been approved by |
…askrgr Rollup of 7 pull requests Successful merges: - rust-lang#88858 (Allow reverse iteration of lowercase'd/uppercase'd chars) - rust-lang#91544 (Fix duplicate derive clone suggestion) - rust-lang#92026 (Add some JSDoc comments to rustdoc JS) - rust-lang#92117 (kmc-solid: Add `std::sys::solid::fs::File::read_buf`) - rust-lang#92139 (Change Backtrace::enabled atomic from SeqCst to Relaxed) - rust-lang#92146 (Don't emit shared files when scraping examples from dependencies in Rustdoc) - rust-lang#92208 (Quote bat script command line) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
This follows the Closure Compiler dialect of JSDoc, so we can use it to do some basic type checking. We don't plan to compile with Closure Compiler, just use it to check types. See https://github.com/google/closure-compiler/wiki/ for details.
To try checking the annotations, run:
You'll see some warnings that "String continuations are not recommended". I'm not addressing those right now.
Discussed on Zulip.
r? @GuillaumeGomez