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

Warn more gently on ignored example code #44927

Closed
bluss opened this issue Sep 29, 2017 · 16 comments
Closed

Warn more gently on ignored example code #44927

bluss opened this issue Sep 29, 2017 · 16 comments
Labels
A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools C-enhancement Category: An issue proposing an enhancement or a PR with one. P-medium Medium priority T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@bluss
Copy link
Member

bluss commented Sep 29, 2017

Source code (rendered):

//! ```ignore
//! use itertools::Itertools;
//! ```

This renders with a warning triangle in the crate's docs. "Be careful when using this code, it's not being tested!"

Can we be more gentle with the warnings? This warning reflects badly on the crate, and we cast doubt on it that is unfounded. Any author that uses ignore probably knew why they did so.

In this case the warning is especially visible and unlucky, and the user can easily be confused, is "the code" referring to the whole crate?

@bluss bluss added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Sep 29, 2017
@GuillaumeGomez GuillaumeGomez added the A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools label Sep 29, 2017
@GuillaumeGomez
Copy link
Member

We'll need to discuss it in the next team meeting.

@est31
Copy link
Member

est31 commented Sep 30, 2017

cc #44397 . I was totally okay with the previous behaviour and don't really like the new one either.

@TimNN TimNN added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Oct 1, 2017
@steveklabnik
Copy link
Member

So, this is one of the longest-standing requests for rustdoc, and we get issues about it all the time. People expect to be able to run examples, and so if the example doesn't run, we should tell them in some fashion.

Is there some way we can address both concerns here?

@bluss
Copy link
Member Author

bluss commented Oct 2, 2017

I guess it's just an issue of how to formulate it.

Something more matter of fact would be good like "This example is not tested automatically." Warning triangle can stay?

@est31
Copy link
Member

est31 commented Oct 3, 2017

Is there some way we can address both concerns here?

My main problem is the inaccuracy. Whether a test is ignored or not is not the same as whether a test actually compiles and runs. Just look at the example @bluss linked. The test obviously will compile, but its ignored. The reverse can be true as well: if you have changed an API but forgot to run cargo test before uploading, the doc tests might not have been tested.

Its the same issue I have with this crates.io batches thing: I want the info to be accurate, and not just be some indicator. This is Rust, not C!

I'd agree with the feature if rustdoc would actually test each single example for whether it compiles and runs, and display that info instead.

@GuillaumeGomez
Copy link
Member

Well, every non-ignored code block is tested and run, so I'm not sure to understand what you're talking about.

@arielb1
Copy link
Contributor

arielb1 commented Oct 3, 2017

I think as a workaround you could comment the code out, e.g.

//! ```rust,ignore
//! # #[cfg(never)] mod foo {
//! use itertools::Itertools;
//! # }
//! ```

@Rufflewind
Copy link
Contributor

I sometimes write Rust-like pseudocode to explain concepts where normal Rust code would be too verbose or detract from the main topic. The presence of syntax highlighting improves readability.

I find this warning rather discouraging, especially the exclamation mark that is normally used for unsafe code. Besides, there's ways to work around this kind of stuff, so all it does is to force doc writers to find more unsightly hacks like @arielb1's example, or give up syntax highlighting for no good reason.

I think rather than to disparage code blocks that are untested with scary signs, consider adding some positive decoration to the code blocks that are tested, like "this code is tested" with a glowy green border or something, perhaps with some buttons for copy-pasting the original code and/or exposing the # lines.

@est31
Copy link
Member

est31 commented Oct 9, 2017

perhaps with some buttons for copy-pasting the original code

First, on all operating systems I've tried on you can just select code and copy it by pressing some keys, so its not really needed, second usually example code is not properly licensed. Most times its under the same license as the code around it, so you'd have to copy the entire attribution/license headers as well. So I don't think one should go down that path.

@steveklabnik steveklabnik added the P-medium Medium priority label Oct 31, 2017
@QuietMisdreavus
Copy link
Member

I'm looking at touching this, at least to tweak the phrasing. How do the phrases "This sample is not tested" and "This sample deliberately fails to compile" sound?

I'm also looking for a better icon, but sadly INFORMATION SOURCE is a box on my work computer, so i'd need to find one that is provided by the fonts we bundle with rustdoc.

@est31
Copy link
Member

est31 commented Nov 6, 2017

@QuietMisdreavus what about ⓘ ?

@bluss
Copy link
Member Author

bluss commented Nov 6, 2017

The former sounds good. Should it say "example" instead of sample?

@QuietMisdreavus
Copy link
Member

@est31 That character works! Thanks!

@bluss "Example" is probably a better word - i'll work that in.

@QuietMisdreavus
Copy link
Member

I put these string tweaks in #45815 - feel free to bikeshed over there.

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Nov 14, 2017
…s, r=GuillaumeGomez

rustdoc: tweak notes on ignore/compile_fail examples

Part of rust-lang#44927

This is a softening of these notices to mention *why* a given example has a given callout, rather then telling viewers to be careful with an example. It also changes the character used for these samples from a warning logo to a circle-i/information logo.

![image](https://user-images.githubusercontent.com/5217170/32464361-5fbb5d9e-c305-11e7-8482-ce71b97a54df.png)
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Nov 14, 2017
…s, r=GuillaumeGomez

rustdoc: tweak notes on ignore/compile_fail examples

Part of rust-lang#44927

This is a softening of these notices to mention *why* a given example has a given callout, rather then telling viewers to be careful with an example. It also changes the character used for these samples from a warning logo to a circle-i/information logo.

![image](https://user-images.githubusercontent.com/5217170/32464361-5fbb5d9e-c305-11e7-8482-ce71b97a54df.png)
@QuietMisdreavus
Copy link
Member

Now #45815 is merged, where does that leave this issue?

@bluss
Copy link
Member Author

bluss commented Nov 14, 2017

Icon and text are improved, I think this is fine to close. Thanks @QuietMisdreavus

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools C-enhancement Category: An issue proposing an enhancement or a PR with one. P-medium Medium priority 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

8 participants