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

deno doc --html panics when no file paths are provided #21067

Closed
EdJoPaTo opened this issue Nov 3, 2023 · 7 comments · Fixed by #21072
Closed

deno doc --html panics when no file paths are provided #21067

EdJoPaTo opened this issue Nov 3, 2023 · 7 comments · Fixed by #21072
Labels
bug Something isn't working correctly docs

Comments

@EdJoPaTo
Copy link

EdJoPaTo commented Nov 3, 2023

I read about the new deno doc --lint command and wanted to try it in a project.
I never used deno doc before but know and like cargo doc a lot.

First I used just deno doc --lint which printed out a huge unrelated list which ended with some Web Assembly stuff which isn't directly related to any source code I wrote. From a lint command I expected lint output which wasn't the case here. --html is new, maybe it builds the HTML docs then and output will only contain the lints I want?

$ deno doc --lint --html
error: the following required arguments were not provided:
  --name=<name>

Usage: deno doc --lint --html --name=<name> [source_file]...

For more information, try '--help'.

Alright, I'll add a --name=bla.

$ deno doc --lint --html --name=bla

============================================================
Deno has panicked. This is a bug in Deno. Please report this
at https://github.com/denoland/deno/issues/new.
If you can reliably reproduce this panic, include the
reproduction steps and re-run with the RUST_BACKTRACE=1 env
var set and include the backtrace in your report.

Platform: linux x86_64
Version: 1.38.0
Args: ["deno", "doc", "--lint", "--html", "--name=bla"]

thread 'main' panicked at /build/.cargo/registry/src/index.crates.io-6f17d22bba15001f/deno_doc-0.72.2/src/html/mod.rs:638:3:
assertion failed: !paths.is_empty()
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

After that I tried deno doc and found out that it also prints this long list of project unrelated documentation when not specified with a source code file which doesn't feel intuitive to me. deno lint or deno fmt don't need arguments, why does deno doc needs some? deno check enforces me to specify some (which is annoying) but deno doc just quietly accepts no arguments there. It's inconsistent (and still annoying to specify some, after all I still use it within my project, I don't want to specify more stuff, it should be as simple as cargo doc (or cargo check for that matter). deno.jsonc probably needs to know an entry point for that (Rust just assumes src/main.rs or src/lib.rs).

Also, why is the doc --lint command outputting documentation and not just lints? It doesn't seem that helpful and concise to me. After playing around a bit I found out that it is outputting the documentation when there is no lint. Why isn't it just showing "everything is fine" and leave the output to its specific command unrelated to --lint to be less confusing?

To end this more positively: I like the idea of lints to improve the documentation a lot. cargo doc does a great job here. I like that Deno tries to have something like that for TypeScript too and I want to use that!

@dsherret dsherret changed the title deno doc --lint usage is strange and it panics deno doc --html panics when no file paths are provided Nov 3, 2023
@dsherret dsherret added bug Something isn't working correctly docs labels Nov 3, 2023
@dsherret
Copy link
Member

dsherret commented Nov 3, 2023

It looks like it's the html code that's panicking? I opened a separate issue about lint here: #21070

I think your confusion is around not providing any file paths. For example, you should do deno doc --lint --html mod.ts

Also, why is the doc --lint command outputting documentation and not just lints?

deno doc is the sub command to generate the documentation and --lint is an extra verification step that runs before generating the documentation.

@KnorpelSenf
Copy link
Contributor

I believe this issue was rather meant as a suggestion to improve usability, not a question on how to use the CLI. It's an amazing feature and a great step forward, but the above journey highlights some rough edges that hint at ways to make the thing more intuitive.

@dsherret
Copy link
Member

dsherret commented Nov 3, 2023

@KnorpelSenf there were some bugs reported here (as per the title, deno doc panicking) and I believe part of the confusion was caused by the incorrect usage that deno doc allowed (deno doc --lint should now properly error). I'd recommend opening separate issues for suggestions.

@dsherret
Copy link
Member

dsherret commented Nov 3, 2023

What I'm thinking will make this more clear is for deno doc --lint mod.ts to output how many files it checked, but for deno doc --lint --json mod.ts or --html to output the docs.

@dsherret
Copy link
Member

dsherret commented Nov 3, 2023

I opened #21084 -- Thanks for the feedback!

dsherret added a commit that referenced this issue Nov 4, 2023
…ed (#21084)

As pointed out in #21067 , it's
confusing that `deno doc --lint mod.ts` outputs the documentation to
stdout on success. Instead, it would be better if it outputted how many
files were checked similar to what `deno lint` does on success. It still
outputs the documentation if `--lint` or `--html` are provided so this
is non-breaking.
@EdJoPaTo
Copy link
Author

EdJoPaTo commented Nov 4, 2023

there were some bugs reported here (as per the title, deno doc panicking) and I believe part of the confusion was caused by the incorrect usage that deno doc allowed (deno doc --lint should now properly error). I'd recommend opening separate issues for suggestions.

I feel a bit trolled here as you first edited the title and then reference it as it's about something else. This issue is about my confusion using deno doc (usage is strange and also panics). There clearly happened misunderstanding and it's probably not intentional.

The blog post stated it's heavily inspired by the Rust tooling which I know and like so I expected something similar and was as you said confused by it's usage then. Yes it also paniced on my journey of trying it but mainly my journey was horrible due to inconsitencies. For example because of a --lint command not behaving as other commands printing out lints. (Yes, #21084 is a step in a good direction, thank you for that!)

The idea of having a tool that is similar to what Rust offers is great but Deno isn't there yet. It's promising but I was pointing out whats lacking and confusing in my opinion. As I said I am used to Rust and cargo is an easy to use tool helping me with my project. Deno definitely can and wants to be as easy to use and I wanted to point out pain points with my issue to get there.

I think your confusion is around not providing any file paths. For example, you should do […]

Personally I think when the intuitive usage is not working the tool should be more intuitive instead of suggesting how to use it correctly.

deno doc is the sub command to generate the documentation and --lint is an extra verification step that runs before generating the documentation.

I am not sure which user needs that in the first place. Either I want the documentation then I don't care about lints or I want to ensure good documentation quality then I don't need the docs printed out, only what can be improved. For example a CI step ensuring good documentation doesn't need to print out the documentation in the process.

I'd recommend opening separate issues for suggestions.

My first intention when reading this was to just copy the text and create a new issue with my initial title and remove the part about the panic (It's only in the title and never mentioned in my text, only in the plaintext block). That's clearly not helpful and I went to bed. #21084 showed that you actually read my indented goal of my issue which I didn't expected after yesterdays response.

I still would like to have TypeScript tooling which is as great as cargo is. As my attempt with this issue didn't went as planned: How should I suggest something then?

Personally I see these "suggestions" in my initial text:

  • doc --lint doesn't behave as a lint command and sometimes print stuff unrelated to lint hints (the docs) when there are no hints. Funnily it doesn't when there are lint hints so it's even inconsistent with itself. --lint is new with this release (I assume from the blogpost?), it's goal is to provide lints. It shouldn't output unrelated stuff.

    I don't see anything breaking there in fixing it to get it's useful behavior as a lint tool which it should have been since release (→ fix). If doc should print out documentation on whatever argument is specified why isn't it still printing it out when --html also outputs to the filesystem?
    Otherwise --lint should be deprecated and added to deno lint (which can already partly do stuff like that when manually enabling rules like explicit-module-boundary-types).

  • Deno sometimes being project specific and sometimes file specific. Subcommands sometimes require arguments and sometimes not. deno doc printing out unrelated things to my working directory. Deno tries to be both project tooling and only script specific which creates inconsistencies. Or to phrase it more as a suggestion: Remove the inconsistencies of subcommands (mainly lint and check). It should either always require arguments or always default to *.ts (more like **/*.{md,ts,js,…}).

    Maybe an entry point settings in deno.jsonc is needed as Rust assumes src/lib.rs while Deno doesn't assume mod.ts and therefore sometimes requires additional arguments?

    deno doc only highlights this underlying issue even more. Its a symptom not the root cause.

@dsherret
Copy link
Member

dsherret commented Nov 4, 2023

I feel a bit trolled here as you first edited the title and then reference it as it's about something else.

Sorry, when an issue is opened with a bug (panic as was in the original title), then that will likely become what the issue is about and it will likely be closed by the fix. I edited the issue title to make it easier for people who have the same issue to search for it. As I mentioned before, it's best to keep issues focused to one topic.

Maybe an entry point settings in deno.jsonc is needed as Rust assumes src/lib.rs while Deno doesn't assume mod.ts and therefore sometimes requires additional arguments?

Remove the inconsistencies of subcommands (mainly lint and check). It should either always require arguments or always default to .ts (more like **/.{md,ts,js,…}).

Yup, entrypoints for libraries are coming in a future release, which should make this easier (I'm actually working on some of this next week, but it probably won't be released for several months because it's part of Deno 2.0). We also want to use that for some type checking optimizations. There is currently no way to know what the entrypoints are which is why explicit arguments are required for deno doc.

For type checking, see #20813

Otherwise --lint should be deprecated and added to deno lint (which can already partly do stuff like that when manually enabling rules like explicit-module-boundary-types).

The documentation output is necessary in order to do these checks. I have been thinking that once entrypoints lands then the lint command with no args could run these extra lint checks as well, butdeno doc --lint mod.ts is still useful in other cases.

Anyway, I think #21084 has largely addressed the feedback here. Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly docs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants