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

cargo doc --open error message is confusing when all targets are skipped #14776

Closed
profwpollock opened this issue Nov 3, 2024 · 10 comments · Fixed by #14969
Closed

cargo doc --open error message is confusing when all targets are skipped #14776

profwpollock opened this issue Nov 3, 2024 · 10 comments · Fixed by #14969
Labels
C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` Command-doc S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review

Comments

@profwpollock
Copy link

profwpollock commented Nov 3, 2024

Problem

I use an alias for this command:

$ cargo doc -v --open --workspace --document-private-items --all-features --no-deps --examples

When first developing a new project, there are no examples yet. The output is:

warning: target filter `examples` specified, but no targets matched; this is a no-op
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.01s
error: no crates with documentation

So it is NOT a "no-op". I believe it should be. The warning alone is fine.

Proposed Solution

Instead of failing to provide crate documentation, it should create crate documentation even if there's no examples yet.

Notes

No response

@profwpollock profwpollock added C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-triage Status: This issue is waiting on initial triage. labels Nov 3, 2024
@epage epage added Command-doc S-needs-team-input Status: Needs input from team on whether/how to proceed. and removed S-triage Status: This issue is waiting on initial triage. labels Nov 4, 2024
@ehuss ehuss added S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review and removed S-needs-team-input Status: Needs input from team on whether/how to proceed. labels Dec 3, 2024
@ehuss
Copy link
Contributor

ehuss commented Dec 3, 2024

The Cargo team briefly looked at this, and agree that it would be helpful to update the error message to make it clearer that this is related to the --open flag. We probably want to keep the error itself, since the user explicitly asked to open a web browser, but if there are no crates, then there is nothing to open.

The no-op aspect is just an artifact that it is indeed a no-op to skip over a target selection flag that selects nothing (--examples), and that is handled generically for all commands at a very early stage. I suppose we could also reword that a little to say something else like "ignoring" or "skipping" or something like that, instead of "no-op". (Though sometimes people also get confused when saying "ignoring" when there are subtle differences between not passing the flag at all, so any rewording would need to be done with care.)

@ehuss ehuss changed the title cargo doc --examples ... aborts if there are no examples cargo doc --open error message is confusing when all targets are skipped Dec 3, 2024
@profwpollock
Copy link
Author

Actually the error is wrong. The crate does have documentation, just not examples (yet). As long as some crate documentation is produced, --examples should be a no-op and not produce any error; the warning is fine and the browser should open.

My workaround for this bug is to have two aliases, one with --examples and the other without.

Please revert the change in the title of this issue. Thanks for listening!

@weihanglo
Copy link
Member

weihanglo commented Dec 4, 2024

If there were some examples documented after running cargo doc --open --examples, the first example will be opened. That is the expected behavior of the --examples flag, as well as --example <ex> flag which will directly open a certain example.

In this case, there was no example, so no crate doc was produced. Not even the doc of the lib crate. Hence cargo was not able to open anything.

@profwpollock
Copy link
Author

If that's the intended behavior, then I agree with rewording the error message. However, the warning is wrong in that case and should be removed.

But I wish the intended behavior would be to produce crate docs if any, and that --examples was indeed a no-op if there is documentation but no examples. It seems inconvenient to require a developer to use two slightly different commands to produce documentation during development.

@epage
Copy link
Contributor

epage commented Dec 4, 2024

However, the warning is wrong in that case and should be removed.

How is the warning wrong?

But I wish the intended behavior would be to produce crate docs if any, and that --examples was indeed a no-op if there is documentation but no examples. It seems inconvenient to require a developer to use two slightly different commands to produce documentation during development.

In this case, cargo is doing exactly what you told it to do. imo we shouldn't be second guessing the user.

Personally, I've stopped using --open and instead click the file link that is printed (I switched from tmux to zellij because tmux stripped links at the time). If nothing else, I dislike that --open doesn't play well with my shell history in that I can't blindly run a command from my history but have to either add --open or remove it. Its also more consistent with other commands that would be awkward to have an --open, like cargo build --timings

@profwpollock
Copy link
Author

The warning is wrong because it is not a no-op. It is an error and there's an error message produced already, so the erroneous warning is not only wrong but redundant.

The command with --examples along with --open may be working as intended. But that is most definitely not as I expected. I expect the browser to open if there's any crate documentation at all and produce a warning if --examples is specified but there are no examples. If that's not what cargo maintainers desire for the behavior, I understand. But in that case, the warning is incorrect and should be eliminated or at least reworded.

I will leave this issue to your judgment for resolution at this point. I do appreciate you taking the time to correspond with me and appreciate the great work you do. Even if you don't agree with me! :-)

@epage
Copy link
Contributor

epage commented Dec 5, 2024

Two different things are being reported

  • --examples did nothing warning
  • No requested docs available to --open error

There is overlap (nothing). Whether something should be done depends on a couple of factors

  • Whether the warning shows up for each set of flags passed in or only if none of the selected flags produce something (i.e. can we see the warning without the error)
  • How well cargo internals can track all of the states that determine if the warning will show up at the same time knowing whether the error will show up and decide to suppress the warning

@ibilalkayy
Copy link
Contributor

ibilalkayy commented Dec 12, 2024

Hey @epage,

You're right that the examples did nothing wrong. It was executed through the command, but it didn't find the examples file and gave the error.

If a person just wants to open the file, then writing the --open would also be fine without --examples.

But now the problem arises, if the lib.rs or main.rs contains the docs but after executing the cargo doc command with the examples and open flags, should it open the documentation in the browser?

It has two parts.

Part No. 1

At this point, @profwpollock also has a strong point to open the browser and an error message should be given that the examples directory is missing.

If you agree on this point then we can change the message in which we can write that the docs of other files are provided except the examples and that's why

  1. Remove the examples directory if it exists
  2. OR fill the examples directory with docs

After some findings, I noticed that this behavior also exists in git where it takes the add, commit, and push commands. It adds and commits the changes successfully but when the wrong branch name is provided, then it does not push the code. All the things happen in just one line of command.

Here is the git commands to showcase:

Image

Part No. 2:

Now if you want to keep the docs strict so that it gives you the error message without opening it in the browser. then it is also fine because

  1. The user will be focused on the problem instead of getting distracted.
  2. The user will be disturbed by switching from the browser to the code editor to see the result every time and maybe the user will say that the doc is generated but after coming back to the editor, he will notice that the example docs are not even present yet and there is an error.

I presented both points that could be implemented and I want to know what is your priority to implement. Let me know.

Thank you!

@weihanglo
Copy link
Member

But now the problem arises, if the lib.rs or main.rs contains the docs but after executing the cargo doc command with the examples and open flags, should it open the documentation in the browser?

No. See #14776 (comment)
I would suggest being consistent and just fix the error message. Don't open the browser for a random URL from whatever on local cache that may eventually fail to find.

@ibilalkayy
Copy link
Contributor

@weihanglo Well, I was not aware of the expected behavior of examples. Now, here are 2 points about which I am confused.

  1. What if the lib contains the docs, will it be ignored if the examples are not provided?
  2. What should be an error message if I am consistent on that message only?

github-merge-queue bot pushed a commit that referenced this issue Dec 19, 2024
### What does this PR try to resolve?

This PR is resolving an
[issue](#14776 (comment))
where `cargo doc --open --examples` does not give a clear message when
the examples file is given to it.

I changed the message from `no crates with documentation` in the
[cargo_doc.rs](https://github.com/rust-lang/cargo/blob/58b2d609ece6bd0333e4d0f63a024fe3dd350b4b/src/cargo/ops/cargo_doc.rs#L62)
and
[doc.rs](https://github.com/rust-lang/cargo/blob/58b2d609ece6bd0333e4d0f63a024fe3dd350b4b/tests/testsuite/doc.rs#L1504)
to `requested crate documentation is not available to open`. Now it
becomes easy for the user to understand what they're missing.

 Fixes #14776

### How should we test and review this PR?

Here is the command through which you can test this change:

`cargo test SNAPSHOTS=overwrite -- doc::open_no_doc_crate`

<!--
Thanks for submitting a pull request 🎉! Here are some tips for you:

* If this is your first contribution, read "Cargo Contribution Guide"
first:
  https://doc.crates.io/contrib/
* Run `cargo fmt --all` to format your code changes.
* Small commits and pull requests are always preferable and easy to
review.
* If your idea is large and needs feedback from the community, read how:
  https://doc.crates.io/contrib/process/#working-on-large-features
* Cargo takes care of compatibility. Read our design principles:
  https://doc.crates.io/contrib/design.html
* When changing help text of cargo commands, follow the steps to
generate docs:

https://github.com/rust-lang/cargo/tree/master/src/doc#building-the-man-pages
* If your PR is not finished, set it as "draft" PR or add "WIP" in its
title.
* It's ok to use the CI resources to test your PR, but please don't
abuse them.

### What does this PR try to resolve?

Explain the motivation behind this change.
A clear overview along with an in-depth explanation are helpful.

You can use `Fixes #<issue number>` to associate this PR to an existing
issue.

### How should we test and review this PR?

Demonstrate how you test this change and guide reviewers through your
PR.
With a smooth review process, a pull request usually gets reviewed
quicker.

If you don't know how to write and run your tests, please read the
guide:
https://doc.crates.io/contrib/tests

### Additional information

Other information you want to mention in this PR, such as prior arts,
future extensions, an unresolved problem, or a TODO list.
-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` Command-doc S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review
Projects
None yet
5 participants