Skip to content

rustdoc: add disambiguators for identically named method in sidebar - #162778

Open
lolbinarycat wants to merge 2 commits into
rust-lang:mainfrom
lolbinarycat:rustdoc-sidebar-disambig-142147
Open

lolbinarycat wants to merge 2 commits into
rust-lang:mainfrom
lolbinarycat:rustdoc-sidebar-disambig-142147

Conversation

@lolbinarycat

@lolbinarycat lolbinarycat commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor

fixes #142147

also affects associated fns. main improvments are to pointer and box docs.

before:
image

after:
image

@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 Sep 14, 2026
@rustbot

rustbot commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator

r? @GuillaumeGomez

rustbot has assigned @GuillaumeGomez.
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 8 candidates
  • Random selection from GuillaumeGomez, notriddle

Comment thread src/librustdoc/html/render/sidebar.rs Outdated
Cow::Borrowed(s) => *s = &s[..len],
Cow::Owned(s) => s.truncate(len),
};
fn get_shortname<'a>(itm: Option<&'a Link<'a>>) -> Option<&'a str> {

@GuillaumeGomez GuillaumeGomez Sep 14, 2026 •

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.

Just a nit:

Suggested change
fn get_shortname<'a>(itm: Option<&'a Link<'a>>) -> Option<&'a str> {
fn get_short_name<'a>(item: Option<&'a Link<'a>>) -> Option<&'a str> {

View changes since the review

Comment thread src/librustdoc/html/render/sidebar.rs Outdated
})
}

fn postprocess_method_list(ml: &mut [Link<'_>]) {

@GuillaumeGomez GuillaumeGomez Sep 14, 2026 •

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.

Suggested change
fn postprocess_method_list(ml: &mut [Link<'_>]) {
fn postprocess_method_list(methods: &mut [Link<'_>]) {

View changes since the review

Comment thread src/librustdoc/html/render/sidebar.rs Outdated
Comment thread src/librustdoc/html/render/sidebar.rs Outdated
Comment thread src/librustdoc/html/render/sidebar.rs Outdated
Comment thread src/librustdoc/html/render/sidebar.rs Outdated
Comment on lines +809 to +814
let cur_sn = get_shortname(ml.get(i));
if get_shortname(ml.get(i.wrapping_sub(1))) != cur_sn
&& cur_sn != get_shortname(ml.get(i + 1))
{
let csnl = cur_sn.unwrap().len();
trunc_to(&mut ml[i].name, csnl);

@GuillaumeGomez GuillaumeGomez Sep 14, 2026 •

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.

Bigger suggestion then:

Suggested change
let cur_sn = get_shortname(ml.get(i));
if get_shortname(ml.get(i.wrapping_sub(1))) != cur_sn
&& cur_sn != get_shortname(ml.get(i + 1))
{
let csnl = cur_sn.unwrap().len();
trunc_to(&mut ml[i].name, csnl);
let Some(short_name) = get_short_name(methods.get(i)) else { continue };
if i > 0
&& get_short_name(methods.get(i - 1)) != Some(short_name)
&& Some(short_name) != get_short_name(methods.get(i + 1))
{
let current_len = short_name.len();
trunc_to(&mut methods[i].name, current_len);

View changes since the review

@lolbinarycat
lolbinarycat force-pushed the rustdoc-sidebar-disambig-142147 branch from 2dd6d30 to bf8114c Compare September 16, 2026 17:08
@lolbinarycat

Copy link
Copy Markdown
Contributor Author

the i > 0 change would break the code, as it would cause the first sidebar item to be always disambiguated.

@GuillaumeGomez

Copy link
Copy Markdown
Member

I like it a lot and the implementation looks good. Now just remains to add tests. :)

impl C<A> {
//@ has - "//a[@href='#method.dupe']" "dupe (C<A>)"
pub fn dupe(self) {}
//@ has - "//a[@href='#method.uniq1']" "uniq1"

@GuillaumeGomez GuillaumeGomez Sep 21, 2026 •

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.

Use matches with $ and ^ to ensure it's the full string instead of !has below.

View changes since the review

@lolbinarycat
lolbinarycat force-pushed the rustdoc-sidebar-disambig-142147 branch from 8080cd8 to d8272dc Compare September 21, 2026 20:02
pub struct C<T>(T);

impl C<A> {
//@ has - "//a[@href='#method.dupe']" "^dupe (C<A>)$"

@GuillaumeGomez GuillaumeGomez Sep 21, 2026 •

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.

Shouldn't it be matches here as well?

View changes since the review

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes...

@rust-log-analyzer

This comment has been minimized.

@lolbinarycat
lolbinarycat force-pushed the rustdoc-sidebar-disambig-142147 branch from d8272dc to 976084c Compare September 22, 2026 17:52
@rust-log-analyzer

This comment has been minimized.

@lolbinarycat
lolbinarycat force-pushed the rustdoc-sidebar-disambig-142147 branch from 976084c to 9bab947 Compare September 22, 2026 18:36
@rust-bors

rust-bors Bot commented Sep 24, 2026

Copy link
Copy Markdown
Contributor

☔ The latest upstream changes (presumably #163244) made this pull request unmergeable. Please resolve the merge conflicts by rebasing.

This branch has not been deployed

No deployments
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.

Duplicate entries in side bar in std docs for pointer

4 participants