tidy: Run tidy style against markdown files. #81402
Conversation
|
(rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
The job Click to see the possible cause of the failure (guessed by this bot) |
| @@ -47,16 +47,15 @@ all type errors and name resolution errors with function bodies. Note that this | |||
| work for anything outside a function body: since Rustdoc documents your types, it has to | |||
| know what those types are! For example, this code will work regardless of the platform: | |||
|
|
|||
| <!-- `ignore` because doc-tests are run with `rustc`, not `rustdoc` --> | |||
| ```ignore | |||
| ```rust,ignore (platform-specific) | |||
There was a problem hiding this comment.
This doesn't preserve the info from the comment IMO. The issue is that - regardless of whether rustc gives an error or not - it may not match rustdoc's behavior, so it would be confusing to show it.
| ```rust,ignore (platform-specific) | |
| ```rust,ignore (tests rustdoc, not rustc) |
There was a problem hiding this comment.
I don't think that is a good explanation of why it is ignored. If it was just about the different output, then noplayground would be a more appropriate marker.
The important difference is that with ignore, there's nothing to validate that the example contains valid rust code. With noplayground, it'll be validated, but won't show the play button (which I assume is what the concern is about). Generally it is best to avoid ignore if at all possible.
That being said, I added an annotation for that.
There was a problem hiding this comment.
. If it was just about the different output, then noplayground would be a more appropriate marker.
It's not about the output - there is code that rustdoc accepts that rustc doesn't. So if this is marked with no_playground a) it will break on Linux because it's tested with rustc and b) even if it didn't break it would be misleading, because rustdoc and rustc are fundamentally treating this code differently.
If it makes you feel better, ignore still validates that the syntax is valid, just not that the program typechecks.
There was a problem hiding this comment.
Sorry, I didn't mean to seem like I was contradicting you; I could have worded that better. I was just trying to explain the reasoning I had for changing it. I understand there is a subtle reasoning here.
ignore does not validate syntax for tests. That validation is only run when building API documentation with rustdoc (and even then, I think it is only a warning). When running rustdoc --test you can put any text in an ignore block and it is completely ignored, and that's how the books are tested.
There was a problem hiding this comment.
Ah ok, that makes sense.
ignore does not validate syntax for tests.
Well, maybe it shouldn't, but currently it does: #30032 Never mind, you were talking specifically about --test I think.
| pub fn f() { | ||
| use std::os::windows::ffi::OsStrExt; | ||
| } | ||
| ``` | ||
|
|
||
| but this will not, because the unknown type is part of the function signature: | ||
|
|
||
| ```ignore | ||
| ```rust,ignore (platform-specific) |
There was a problem hiding this comment.
| ```rust,ignore (platform-specific) | |
| ```rust,ignore (tests rustdoc, not rustc) |
jyn514
added
A-testsuite
|
The other changes LGTM, feel free to r? me if you like. |
|
r? @jyn514 |
|
Did he mean |
|
@bors r+ |
|
📌 Commit 07cd499 has been approved by |
bors
added
S-waiting-on-bors
tidy: Run tidy style against markdown files. This adds tidy checks for markdown files. I think it is useful to have some style enforcement (for the same reasons the style is enforced on other files). I think it is worthwhile to avoid `ignore` on rust examples since having broken code in documentation is frustrating. Avoiding trailing whitespace is good because it has semantic meaning in markdown, which I think should be avoided.
| global_asm!(include_str!("something_neato.s")); | ||
| ``` | ||
|
|
||
| And a more complicated usage looks like this: | ||
|
|
||
| ```rust,ignore | ||
| ```rust,no_run |
There was a problem hiding this comment.
bors
added
S-waiting-on-author
The cfg isn't actually necessary, since `--emit=metadata` does not actually do enough validation for it to fail on any platform. However, I feel a little more comfortable leaving it in. Unhide the feature flag, since I think that is important to display.
|
I have tweaked the test to work on non-x86 platforms. I don't have an aarch64 linux system to test, but I manually tested a variety of arm targets and at least the unstable book passes the tests. @bors r=jyn514 |
|
📌 Commit 900648c has been approved by |
bors
added
S-waiting-on-bors
…as-schievink Rollup of 7 pull requests Successful merges: - rust-lang#81402 (tidy: Run tidy style against markdown files.) - rust-lang#81434 (BTree: fix documentation of unstable public members) - rust-lang#81680 (Refactor `PrimitiveTypeTable` for Clippy) - rust-lang#81737 (typeck: Emit structured suggestions for tuple struct syntax) - rust-lang#81738 (Miscellaneous small diagnostics cleanup) - rust-lang#81766 (Enable 'task list' markdown extension) - rust-lang#81812 (Add a test for escaping LLVMisms in inline asm) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
This adds tidy checks for markdown files. I think it is useful to have some style enforcement (for the same reasons the style is enforced on other files). I think it is worthwhile to avoid
ignoreon rust examples since having broken code in documentation is frustrating. Avoiding trailing whitespace is good because it has semantic meaning in markdown, which I think should be avoided.