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

Nightly warning for codeblock languages with non-alphanumeric characters #115938

Closed
cpu opened this issue Sep 18, 2023 · 9 comments · Fixed by #115947
Closed

Nightly warning for codeblock languages with non-alphanumeric characters #115938

cpu opened this issue Sep 18, 2023 · 9 comments · Fixed by #115947
Labels
C-bug Category: This is a bug. regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@cpu
Copy link

cpu commented Sep 18, 2023

Code

I tried this code with cargo doc:

/// This is a test:
/// ```ASN.1
/// foo
/// ```
fn main() {
    println!("Hello, world!");
}

I expected to see this happen: rust doc generated without any warnings.

Instead, this happened:

warning: unexpected character `.`
 --> src/main.rs:1:1
  |
1 | / /// This is a test:
2 | | /// ```ASN.1
3 | | /// foo
4 | | /// ```
  | |_______^
  |
  = note: `#[warn(rustdoc::invalid_codeblock_attributes)]` on by default

"ASN.1" is supported by Linguist. I also see the same result if I specify any codefence languages that include special characters (e.g. C++, C#).

Version it worked on

It most recently worked on: rustc 1.74.0-nightly (8142a319e 2023-09-13)

Version with regression

It fails with a warning on rustc 1.74.0-nightly (203c57dbe 2023-09-17)

rustc --version --verbose:

rustc 1.74.0-nightly (203c57dbe 2023-09-17)
binary: rustc
commit-hash: 203c57dbe20aee67eaa8f7be45d1e4ef0b274109
commit-date: 2023-09-17
host: x86_64-unknown-linux-gnu
release: 1.74.0-nightly
LLVM version: 17.0.0

Possibly related to #110800 ?

@rustbot modify labels: +regression-from-stable-to-nightly -regression-untriaged

@cpu cpu added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels Sep 18, 2023
@rustbot rustbot added needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. I-prioritize Issue: Indicates that prioritization has been requested for this issue. regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. and removed regression-untriaged Untriaged performance or correctness regression. labels Sep 18, 2023
@Noratrieb
Copy link
Member

fixed by #115914

@Noratrieb Noratrieb removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Sep 18, 2023
@cpu cpu changed the title Nightly rejecting codeblock languages with non-alphanumeric characters Nightly warning for codeblock languages with non-alphanumeric characters Sep 18, 2023
@cpu
Copy link
Author

cpu commented Sep 18, 2023

fixed by #115914

@Nilstrieb Apologies, I'm not sure I see how the linked PR fixes the issue. We're not using the new syntax or custom code classes. Using the codeblock fence syntax in the PR description shouldn't generate an error or a warning, no? Have I misunderstood something?

@Noratrieb
Copy link
Member

Hm, good point, I didn't look at this in detail (I wanted to but got distracted). The PR will be in nightly tomorrow, could you try it then to see whether it works? But you're right, looking at the error it looks like it might not.
@GuillaumeGomez

@GuillaumeGomez
Copy link
Member

It should generate a warning as the meaning of {} will change in the future. However it shouldn't emit an error and that's what was fixed in #115914.

@GuillaumeGomez
Copy link
Member

Oh wait, it should indeed not emit a warning (or at least not this one) in this case. I'll check what rustdoc did before.

@GuillaumeGomez
Copy link
Member

Wrote the fix in #115947.

So in your case, it'll still emit a warning, but the correct one this time: it'll tell you that the behaviour will change in the future.

So now about how A.B should behave, if you're not happy if the future update on this, please open a new issue where we can talk about it. The feature will be stabilized in a few months at the earliest so we have plenty of time to discuss it. :)

@apiraino apiraino added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Sep 19, 2023
@cpu
Copy link
Author

cpu commented Sep 19, 2023

Thanks @GuillaumeGomez

if you're not happy if the future update on this, please open a new issue where we can talk about it

I'll wait for this to land in a nightly release so I can see what the new warning looks like and we can go from there :-)

@bors bors closed this as completed in 52a0d13 Sep 19, 2023
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Sep 19, 2023
Rollup merge of rust-lang#115947 - GuillaumeGomez:custom_code_classes_in_docs-warning, r=notriddle

Custom code classes in docs warning

Fixes rust-lang#115938.

This PR does two things:
 1. Unless the `custom_code_classes_in_docs` feature is enabled, it will use the old codeblock tag parser.
 2. If there is a codeblock tag that starts with a `.`, it will emit a behaviour change warning.

Hopefully this is the last missing part for this feature until stabilization.

Follow-up of rust-lang#110800.

r? `@notriddle`
@cpu
Copy link
Author

cpu commented Sep 20, 2023

We're not seeing a warning or an error with the latest nightly. Thanks again for the quick fix :-)

@GuillaumeGomez
Copy link
Member

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants