Skip to content

Conversation

@GuillaumeGomez
Copy link
Member

Fixes #148648.
First part of #148547.

The last part will be about handle AssocItemLink differently (either remove it or change how we do it).

r? @lolbinarycat

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. 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 Nov 17, 2025
@lolbinarycat
Copy link
Contributor

I think we should block this on #149026 just to be safe.

Copy link
Contributor

@lolbinarycat lolbinarycat left a comment

Choose a reason for hiding this comment

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

Looks pretty good, just a few nits.

I am somewhat tempted to say in the future we should also unify the impl block section naming into the same function so that this same logic can be used for sidebar generation, but I'm not sure if that would be better than the status quo or not.

View changes since this review

Comment on lines 174 to 177
DefKind::AssocFn if tcx.associated_item(def_id).defaultness(tcx).has_value() => {
Self::Method
}
DefKind::AssocFn => Self::TyMethod,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I would personally use an if/else here instead of a match guard, but it's subjective.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't prefer either so I'll update to your preference. :)

let item_type =
ItemType::from_def_kind(def_kind, Some(tcx.def_kind(parent_def_id)));
format!("#{}.{}", item_type.as_str(), tcx.item_name(did))
pub(crate) fn anchor(did: DefId, tcx: TyCtxt<'_>) -> String {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would call this fragment, since "anchor" can also refer to the whole <a> element.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fair enough.

@GuillaumeGomez
Copy link
Member Author

I think we should block this on #149026 just to be safe.

I actually tested with this one but I don't mind waiting. 😉

@GuillaumeGomez GuillaumeGomez force-pushed the unify-anchor branch 2 times, most recently from 3ad2a47 to 772c662 Compare November 17, 2025 21:18
@GuillaumeGomez
Copy link
Member Author

Applied suggestions.

@rustbot
Copy link
Collaborator

rustbot commented Nov 18, 2025

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@GuillaumeGomez
Copy link
Member Author

Rebased on #149026.

let item_type =
ItemType::from_def_kind(def_kind, Some(tcx.def_kind(parent_def_id)));
format!("#{}.{}", item_type.as_str(), tcx.item_name(did))
pub(crate) fn fragment(did: DefId, tcx: TyCtxt<'_>) -> String {
Copy link
Contributor

Choose a reason for hiding this comment

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

any reason you moved from returning impl Display to returning String?

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

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. 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.

[rustdoc] html::format::print_anchor doesn't handle fields of fields correctly

3 participants