-
Notifications
You must be signed in to change notification settings - Fork 1.6k
doc_link_code: add check for links with code spans that render weird #14121
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
Conversation
b18e83d
to
566bb66
Compare
This is the lint described at rust-lang/rust#136308 (comment) that recommends using HTML to nest links inside code.
566bb66
to
045e36d
Compare
Seems surprising to make it a clippy lint. Why not a rustdoc lint instead? |
Implementing it in Clippy gives us a chance to shake out any unforeseen corner cases before uplifting it in Rustdoc. |
Oh gosh, I don't feel qualified for this PR at all. @GuillaumeGomez you're free to review this as you know rustdoc well, otherwise I'll reroll. |
I'll review soon. I'm done on my more pressing stuff. ;) |
clippy_lints/src/doc/mod.rs
Outdated
let mut events = events.peekable(); | ||
|
||
while let Some((event, range)) = events.next() { | ||
code_cluster = match (code_cluster, &event) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry to say but this but is absolutely unreadable. ^^'
Can't you check the tag instead and see if more than one code (even if embedded into an intra-doc link) instead? Code might be a bit longer but I also expect it to be much simpler to read.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The simpler way I could come up with to implement this requires the checker to be able to "skip" a Code
event.
To do that, I need an entirely separate loop just for this lint. But the code is a lot more readable that way.
By using a separate loop, I can just skip nodes that I don't want to process twice, instead of having to hand-build a state machine with an enum.
This looks much better, thanks! Looks all good to me @Centri3. :) |
Thanks for reviewing! |
This is the lint described at rust-lang/rust#136308 (comment) that recommends using HTML to nest links inside code.
changelog: [
doc_link_code
]: warn when a link with code and a code span are back-to-back