Skip to content

Fix tests by adding markdown tags #922

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

Merged
merged 1 commit into from
Jul 28, 2025

Conversation

apiraino
Copy link
Contributor

@apiraino apiraino commented Jul 23, 2025

I noticed that we have a command mdbook test, I have run it and discovered a massive quantity of linting errors.

I did a random visual check and adding the console parameter to all these quoted texts does not seems to change anything but fixes the tests.

So maybe the question is, are we interested in fixing these tests? Are we using tests here anyway?

thanks for sharing an opinion :)

Note: I left out from this patch some lints failures because they involve Rust code and I suspect it's just not linting

Rendered

@rustbot
Copy link
Collaborator

rustbot commented Jul 23, 2025

r? @jieyouxu

rustbot has assigned @jieyouxu.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 23, 2025
Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if they help with better codeblock hinting (or avoiding trying to use Rust syntax highlighting for non-Rust snippets), it seems fine

So maybe the question is, are we interested in fixing these tests? Are we using tests here anyway?

Note that AFAIK none of these are tests, and are only codeblocks for stylistic/illustrative reasons.

@jieyouxu
Copy link
Member

(BTW, what does console actually correspond to?)

@apiraino
Copy link
Contributor Author

(BTW, what does console actually correspond to?)

MDbook uses Highlight.js and it seems to be just an alias for shell
https://github.com/highlightjs/highlight.js/blob/main/SUPPORTED_LANGUAGES.md

console is the most used in our book so I just stuck to that 🤷‍♂️

$ rg -I "\`\`\`shell" | tr -d "^$" | wc
      7       7      63
$ rg -I "\`\`\`console" | tr -d "^$" | wc
     22      22     242
$ rg -I "\`\`\`bash" | tr -d "^$" | wc
      2       2      16

@apiraino
Copy link
Contributor Author

I think if they help with better codeblock hinting (or avoiding trying to use Rust syntax highlighting for non-Rust snippets), it seems fine

Yes, that's also my understanding (i.e. cosmetic linting)

@jieyouxu
Copy link
Member

Yes, that's also my understanding (i.e. cosmetic linting)

Alright, the changes in this PR seems fine in that case. Just wanted to note that I don't think we're running mdbook test on the Forge book nor was that ever intended AFAIK.

@apiraino apiraino force-pushed the fix-tests-round-1 branch from 073d63a to 73ed15f Compare July 28, 2025 14:24
@apiraino apiraino force-pushed the fix-tests-round-1 branch from 73ed15f to 7bd597c Compare July 28, 2025 14:29
@apiraino apiraino requested a review from jieyouxu July 28, 2025 14:30
Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@jieyouxu jieyouxu merged commit 0bc0356 into rust-lang:master Jul 28, 2025
1 check passed
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants