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

feat: Add "deno doc" subcommand #4500

Merged
merged 15 commits into from
Mar 28, 2020
Merged

feat: Add "deno doc" subcommand #4500

merged 15 commits into from
Mar 28, 2020

Conversation

bartlomieju
Copy link
Member

@bartlomieju bartlomieju commented Mar 27, 2020

This PR integrates Rust part of documentation generator we were working on.

It provides simple terminal output as well as structured JSON output that can be used to generate documentation website.

For now generator only treats exported members of module. There are still a lot of missing details (filled with <TODO> or <UNIMPLMENETED> strings), but we're able to generate documentation for Deno namespace.

Huge thanks to @lucacasonato who worked on terminal output and web frontend.

Co-authored-by: Luca Casonato [email protected]

Closes #3021

@bartlomieju bartlomieju requested a review from ry March 27, 2020 10:06
@bartlomieju
Copy link
Member Author

Ref #3021

let doc_nodes = match doc_parser.parse(source_file, source_code) {
Ok(nodes) => nodes,
Err(e) => {
eprintln!("Failed to parse documentation:");
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this say: Failed to parse source: ?

@cknight
Copy link
Contributor

cknight commented Mar 27, 2020

This looks great! Really excited by this functionality. And I like that the overloaded methods are separated out, much easier to read.

Co-authored-by: Luca Casonato <[email protected]>
}

fn find_node_by_name(doc_nodes: Vec<DocNode>, name: String) -> Option<DocNode> {
let node = doc_nodes.iter().find(|node| node.name == name);
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this would be more useful if the matching was similar to how cargo test $FILTER works. That is, instead use node.name.contains(name) ?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a bit tricky because of namespaces... Can we leave it as is for first iteration and change in follow ups?

cli/doc/printer/mod.rs Outdated Show resolved Hide resolved
cli/doc/tests/mod.rs Outdated Show resolved Hide resolved
cli/doc/tests/mod.rs Outdated Show resolved Hide resolved
cli/doc/ts_type.rs Show resolved Hide resolved
cli/flags.rs Outdated Show resolved Hide resolved
cli/lib.rs Outdated Show resolved Hide resolved
cli/lib.rs Outdated Show resolved Hide resolved
@lucacasonato
Copy link
Member

I just want to mention that the whole printer is still very much a first pass, there is a lot of duplicate code, and I don't like that we print straight to stdout. I will have time to change this and add proper tests for the printer next week (in another PR).

@bartlomieju
Copy link
Member Author

I just want to mention that the whole printer is still very much a first pass, there is a lot of duplicate code, and I don't like that we print straight to stdout. I will have time to change this and add proper tests for the printer next week (in another PR).

@lucacasonato I'll change TerminalPrinter to return formatted string and add some basic tests before landing this PR; If you could build on that later and add color output that'd be awesome 🚀

swc_common = "=0.5.9"
swc_ecma_ast = "=0.18.1"
swc_ecma_parser = "=0.21.6"
swc_ecma_parser_macros = "=0.4.1"
Copy link
Member

@dsherret dsherret Mar 28, 2020

Choose a reason for hiding this comment

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

Note that I will be removing this explicitly defined dependency in dprint soon. Right now swc_ecma_parser depends on 0.4, but it really depends on 0.4.1. I just opened a PR in swc to fix this.

Copy link
Member

Choose a reason for hiding this comment

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

It might be helpful for us if dprint re-exported the swc namespace so we could use that rather than trying to keep the cargo versions in sync.

Copy link
Member

Choose a reason for hiding this comment

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

I was wondering if there was a better way. Opened https://github.com/dsherret/dprint/issues/164

cli/doc/ts_type.rs Outdated Show resolved Hide resolved
@bartlomieju bartlomieju requested a review from ry March 28, 2020 09:02
Copy link
Member

@ry ry left a comment

Choose a reason for hiding this comment

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

LGTM - awesome work @bartlomieju and @lucacasonato !

There's obviously much more to do, but we can build on this.

@ry ry merged commit 3fac487 into denoland:master Mar 28, 2020
dubiousjim added a commit to dubiousjim/deno that referenced this pull request Mar 29, 2020
* denoland/master: (35 commits)
  Ignore flaky test cafile_info (denoland#4517)
  fix(inspector): proper error message on port collision (denoland#4514)
  feat: Added colors to doc output (denoland#4518)
  v0.38.0
  feat: Add "deno doc" subcommand (denoland#4500)
  Update to Prettier 2 and use ES Private Fields (denoland#4498)
  upgrade: dprint 0.9.6 (denoland#4509)
  upgrade: rusty_v8 to v0.3.9 (denoland#4505)
  feat: Support Inspector / Chrome Devtools (denoland#4484)
  Improve isatty and kill API docs; Deno.kill() - throw on Windows (denoland#4497)
  refactor: rename ConsoleOptions to InspectOptions (denoland#4493)
  upgrade: dprint 0.9.5 (denoland#4491)
  feat: window.close() (denoland#4474)
  errors: replace .lines with explicit .split newline (denoland#4483)
  doc: improve various API docs and include examples (denoland#4486)
  hide source line if error message longer than 150 chars (denoland#4487)
  fix: add fsEvent notify::Error casts (denoland#4488)
  feat: add queueMicrotask to d.ts (denoland#4477)
  Revert "avoid using same port number for test (denoland#4147)"
  docs: update manual about how to run tests for std (denoland#4462)
  ...
dubiousjim added a commit to dubiousjim/deno that referenced this pull request Mar 29, 2020
* denoland/master:
  Ignore flaky test cafile_info (denoland#4517)
  fix(inspector): proper error message on port collision (denoland#4514)
  feat: Added colors to doc output (denoland#4518)
  v0.38.0
  feat: Add "deno doc" subcommand (denoland#4500)
  Update to Prettier 2 and use ES Private Fields (denoland#4498)
  upgrade: dprint 0.9.6 (denoland#4509)
  upgrade: rusty_v8 to v0.3.9 (denoland#4505)
  feat: Support Inspector / Chrome Devtools (denoland#4484)
  Improve isatty and kill API docs; Deno.kill() - throw on Windows (denoland#4497)
  refactor: rename ConsoleOptions to InspectOptions (denoland#4493)
  upgrade: dprint 0.9.5 (denoland#4491)
  feat: window.close() (denoland#4474)
  errors: replace .lines with explicit .split newline (denoland#4483)
  doc: improve various API docs and include examples (denoland#4486)
  hide source line if error message longer than 150 chars (denoland#4487)
  fix: add fsEvent notify::Error casts (denoland#4488)
@nayeemrmn nayeemrmn mentioned this pull request Apr 18, 2020
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.

"deno doc lib.ts"
6 participants