Skip to content

Rustdoc label trait feature#157058

Open
ThierryBerger wants to merge 7 commits into
rust-lang:mainfrom
ThierryBerger:rustdoc_label_trait
Open

Rustdoc label trait feature#157058
ThierryBerger wants to merge 7 commits into
rust-lang:mainfrom
ThierryBerger:rustdoc_label_trait

Conversation

@ThierryBerger
Copy link
Copy Markdown

@ThierryBerger ThierryBerger commented May 28, 2026

View all comments

  • Part of Tracking Issue for doc_label_trait #156865
  • parse #[doc(label_trait)]
  • render it in html
  • add some parsing tests (for the unstable feature)
  • add some html tests
  • Render a color from a hash from its full path -> ⚠️ some colors may be ugly, but we have a path forward with the color argument.

Should the following be out of scope ?

  • Parse color (#[doc(label_trait(color="0xff0000")])
  • display a small colored indicator (with a hover/first letter?) when a type implementing a label_trait is listed outside of its main page
  • Some Integration with rust analyzer
Screenshot 2026-05-28 at 14 06 01

@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 28, 2026

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_passes/src/check_attr.rs

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez, @lolbinarycat

rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead.

cc @rust-lang/rust-analyzer

Some changes occurred in compiler/rustc_hir/src/attrs

cc @jdonszelmann, @JonathanBrouwer

@rustbot rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) A-rustdoc-js Area: Rustdoc's JS front-end A-rustdoc-json Area: Rustdoc JSON backend S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rust-analyzer Relevant to the rust-analyzer team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output. labels May 28, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 28, 2026

r? @fmease

rustbot has assigned @fmease.
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

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: rustdoc
  • rustdoc expanded to 9 candidates
  • Random selection from GuillaumeGomez, camelid, fmease, lolbinarycat, notriddle

@rust-log-analyzer

This comment has been minimized.

Comment thread src/doc/rustdoc/src/unstable-features.md Outdated
Comment thread src/doc/rustdoc/src/unstable-features.md Outdated
Copy link
Copy Markdown
Author

@ThierryBerger ThierryBerger May 28, 2026

Choose a reason for hiding this comment

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

This is mostly a copy from notable_trait, should be updated.

View changes since the review

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should not be here, only in rustdoc book considering it's a rustdoc attribute.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I removed it, but seeing src/doc/unstable-book/src/language-features/doc-notable-trait.md is making me push back lightly on that, are you sure about your feedback? - or should we clean up other doc attributes that shouldn't be here (in other pr)?

Comment on lines +58 to +59
/// Relative URL to the trait page, or empty when not linkable.
href: String,
Copy link
Copy Markdown
Author

@ThierryBerger ThierryBerger May 28, 2026

Choose a reason for hiding this comment

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

should it be an option?

View changes since the review

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we should always link to the trait, so I'd say no.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Maybe doc_hidden would make us not be able to do so ? It would be quite far-fetched to add label_trait to doc_hidden but... Maybe other cases like... visibility? not sure.

Comment thread compiler/rustc_feature/src/unstable.rs Outdated
@GuillaumeGomez
Copy link
Copy Markdown
Member

Render a color from a hash from DefId -> ⚠️ is that an issue? color won't be stable between releases.

I would generated the color hash from the trait path (so crate::Trait for current crate, etc). Like that it will be stable across releases.

Comment thread src/librustdoc/html/render/mod.rs Outdated
Comment thread src/librustdoc/html/render/mod.rs Outdated
Comment thread src/librustdoc/html/render/mod.rs Outdated

let Some(impls) = cx.cache().impls.get(&did) else { return Vec::new() };

let mut out: Vec<LabelTraitInfo> = impls
Copy link
Copy Markdown
Member

@GuillaumeGomez GuillaumeGomez May 28, 2026

Choose a reason for hiding this comment

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

Instead of using a Vec, would be better to use a BTreeMap. It's sorted on insert and prevents duplications.

View changes since the review

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I changed to do that but then collect it to Vec, keeping this discussion opened as I'm not sure if you meant something else.

Comment thread src/librustdoc/html/render/print_item.rs Outdated
Comment thread src/librustdoc/html/render/mod.rs Outdated
Comment thread src/librustdoc/html/templates/print_item.html Outdated
Comment thread src/librustdoc/html/templates/print_item.html
Comment thread src/librustdoc/html/templates/print_item.html Outdated
Comment thread src/librustdoc/html/templates/print_item.html Outdated
Comment on lines +1040 to +1041
// FIXME: valid for traits, should be checked in attr_parsing
label_trait: _,
Copy link
Copy Markdown
Author

@ThierryBerger ThierryBerger May 28, 2026

Choose a reason for hiding this comment

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

should we have strong verification on which items those are applied?

View changes since the review

Copy link
Copy Markdown
Member

@Veykril Veykril left a comment

Choose a reason for hiding this comment

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

Could you remove the two rust-analyzer changes please? They don't do anything as is so they will likely cause unnecessary conflicts for us

View changes since this review

@rust-log-analyzer

This comment has been minimized.

@ThierryBerger ThierryBerger force-pushed the rustdoc_label_trait branch from 2977320 to 6cddf7b Compare May 28, 2026 19:50
@rust-log-analyzer

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-attributes Area: Attributes (`#[…]`, `#![…]`) A-rustdoc-js Area: Rustdoc's JS front-end A-rustdoc-json Area: Rustdoc JSON backend S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rust-analyzer Relevant to the rust-analyzer team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants