Skip to content

Fix indentation of website code snippets #13571

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.

Sign up for GitHub

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
Oct 26, 2024
Merged

Fix indentation of website code snippets #13571

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion clippy_config/src/conf.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,9 @@ pub fn sanitize_explanation(raw_docs: &str) -> String {
// Remove tags and hidden code:
let mut explanation = String::with_capacity(128);
let mut in_code = false;
for line in raw_docs.lines().map(str::trim) {
for line in raw_docs.lines() {
let line = line.strip_prefix(' ').unwrap_or(line);
Copy link
Member

Choose a reason for hiding this comment

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

The indent will still be removed with this change.

Copy link
Member Author

Choose a reason for hiding this comment

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

It removes the single space ///[here]text, strip_prefix does not remove multiple elements

Copy link
Member

Choose a reason for hiding this comment

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

Arf indeed, thought it was stripping as long as it found the character. But then it creates an unbalance in case you have no whitespace character at the beginning of all lines (for whatever reason) and you then have a block.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah though this is exactly how the procedural macro did it as well. I think this (there being no space) is a non-issue overall but if this was already fixed in your PR I think that's a better way of doing it, afaik both rustfmt and clippy never catch this likely typo.

Copy link
Member

Choose a reason for hiding this comment

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

Which procedural macro?

Also, I think it's about handling the fact that most of the time, you write code after a whitespace (/// bla instead of ///bla). It's handled in rustdoc by finding out what's the minimum indent and then stripping according to that. Hum, might be worth to do that actually. Gonna update my PR.

Copy link
Member

Choose a reason for hiding this comment

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

In the prior PR, declare_clippy_lint was turned from a procedural macro into a declarative macro. It had the same logic before as this PR has

Copy link
Member

@flip1995 flip1995 Oct 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I like this solution better, as when copying the code block from the Clippy documentation, you don't have the leading in every line.

I don't think the non-leading whitespace in documentation is a concern, as ... we should just not do this in our documentation 😅

Copy link
Member

Choose a reason for hiding this comment

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

Ah - didn't realize it'd leave a leading space. The issue was about it being unintentional, and hard to spot, rather than a style choice

Copy link
Member

Choose a reason for hiding this comment

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

Hum, might be worth to do that actually. Gonna update my PR.

I don't think this is worth the effort/code. This is just to generate the Clippy documentation from the lint documentation in the Clippy code base. This code doesn't have to handle all possible doc writing styles people come up with, like rustdoc does.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah we can't reuse the fns rustdoc uses here unlike for our lints since we only get them as strings, but I think it's fine to assume our docs are reasonably formatted


if let Some(lang) = line.strip_prefix("```") {
let tag = lang.split_once(',').map_or(lang, |(left, _)| left);
if !in_code && matches!(tag, "" | "rust" | "ignore" | "should_panic" | "no_run" | "compile_fail") {
Expand Down