Skip to content

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

Merged
rust-bors[bot] merged 2 commits into
rust-lang:mainfrom
lolbinarycat:rustdoc-sidebar-disambig-142147
Oct 16, 2026
Merged

rust-bors[bot] merged 2 commits into
rust-lang:mainfrom
lolbinarycat:rustdoc-sidebar-disambig-142147

Conversation

@lolbinarycat

@lolbinarycat lolbinarycat commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor

View all comments

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

This comment has been minimized.

@lolbinarycat
lolbinarycat force-pushed the rustdoc-sidebar-disambig-142147 branch from 9bab947 to 46fca93 Compare October 15, 2026 16:20
@rustbot

rustbot commented Oct 15, 2026

Copy link
Copy Markdown
Collaborator

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
Copy Markdown
Member

Nice, thanks!

@bors r+ rollup

@rust-bors

rust-bors Bot commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 46fca93 has been tentatively approved by GuillaumeGomez

It will be put into the queue for this repository once PR CI succeeds.

@rust-bors rust-bors Bot added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Oct 15, 2026
@rust-bors rust-bors Bot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 15, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Oct 15, 2026
…big-142147, r=GuillaumeGomez

rustdoc: add disambiguators for identically named method in sidebar

fixes rust-lang#142147

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

before:
<img width="264" height="439" alt="image" src="https://github.com/user-attachments/assets/8347c2b8-88cb-470f-beb3-6cb377ca37f0" />

after:
<img width="693" height="576" alt="image" src="https://github.com/user-attachments/assets/478d1ef5-7a1e-4024-a796-d2b0fd0f5a49" />
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Oct 15, 2026
…big-142147, r=GuillaumeGomez

rustdoc: add disambiguators for identically named method in sidebar

fixes rust-lang#142147

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

before:
<img width="264" height="439" alt="image" src="https://github.com/user-attachments/assets/8347c2b8-88cb-470f-beb3-6cb377ca37f0" />

after:
<img width="693" height="576" alt="image" src="https://github.com/user-attachments/assets/478d1ef5-7a1e-4024-a796-d2b0fd0f5a49" />
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Oct 15, 2026
…big-142147, r=GuillaumeGomez

rustdoc: add disambiguators for identically named method in sidebar

fixes rust-lang#142147

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

before:
<img width="264" height="439" alt="image" src="https://github.com/user-attachments/assets/8347c2b8-88cb-470f-beb3-6cb377ca37f0" />

after:
<img width="693" height="576" alt="image" src="https://github.com/user-attachments/assets/478d1ef5-7a1e-4024-a796-d2b0fd0f5a49" />
rust-bors Bot pushed a commit that referenced this pull request Oct 15, 2026
…uwer

Rollup of 8 pull requests

Successful merges:

 - #164271 (Add release notes for 1.99.1)
 - #164361 (delegation: map return even if there is no target expression)
 - #162778 (rustdoc: add disambiguators for identically named method in sidebar)
 - #163224 ([rustdoc] Add tabs to settings popover)
 - #164220 (Test the interaction between `--remap-path-prefix` and doctests)
 - #164279 (Make the `documentation` scope a sub-scope of the `object` scope)
 - #164318 (Fix check with `is_impl_trait_in_trait()` before expecting local id)
 - #164360 (Fix(lib/sys): Restore Win7 compilation)
rust-bors Bot pushed a commit that referenced this pull request Oct 16, 2026
…uwer

Rollup of 7 pull requests

Successful merges:

 - #164175 (Drop WC_ERR_INVALID_CHARS from Windows stdio)
 - #164361 (delegation: map return even if there is no target expression)
 - #162778 (rustdoc: add disambiguators for identically named method in sidebar)
 - #163224 ([rustdoc] Add tabs to settings popover)
 - #164220 (Test the interaction between `--remap-path-prefix` and doctests)
 - #164318 (Fix check with `is_impl_trait_in_trait()` before expecting local id)
 - #164360 (Fix(lib/sys): Restore Win7 compilation)
@rust-bors
rust-bors Bot merged commit 3235e54 into rust-lang:main Oct 16, 2026
14 checks passed
@rustbot rustbot added this to the 1.101.0 milestone Oct 16, 2026
rust-bors Bot pushed a commit that referenced this pull request Oct 16, 2026
Rollup merge of #162778 - lolbinarycat:rustdoc-sidebar-disambig-142147, r=GuillaumeGomez

rustdoc: add disambiguators for identically named method in sidebar

fixes #142147

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

before:
<img width="264" height="439" alt="image" src="https://github.com/user-attachments/assets/8347c2b8-88cb-470f-beb3-6cb377ca37f0" />

after:
<img width="693" height="576" alt="image" src="https://github.com/user-attachments/assets/478d1ef5-7a1e-4024-a796-d2b0fd0f5a49" />
@rust-timer

Copy link
Copy Markdown
Collaborator

Note

This PR was benchmarked as part of triage of its containing rollup: triage URL.

Finished benchmarking commit (7278160): comparison URL.

Overall result: ❌ regressions - please read:

Our benchmarks found a performance regression caused by this PR.
This might be an actual regression, but it can also be just noise.

Next Steps:

  • If the regression was expected or you think it can be justified,
    please write a comment with sufficient written justification, and add
    @rustbot label: +perf-regression-triaged to it, to mark the regression as triaged.
  • If you think that you know of a way to resolve the regression, try to create
    a new PR with a fix for the regression.
  • If you do not understand the regression or you think that it is just noise,
    you can ask the @rust-lang/wg-compiler-performance working group for help (members of this group
    were already notified of this PR).

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
2.4% [2.4%, 2.4%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Max RSS (memory usage)

This perf run didn't have relevant results for this metric.

Cycles

This perf run didn't have relevant results for this metric.

Binary size

This perf run didn't have relevant results for this metric.

Artifact size: 406.81 MiB -> 406.97 MiB (0.04%)

@rustbot rustbot added the perf-regression Performance regression. label Oct 16, 2026
@Mark-Simulacrum

Copy link
Copy Markdown
Member

Regression is real and persists, but limited to just one doc scenario and seems likely expected from disambiguating methods in the sidebar (more work to do, probably slightly more output).

@Mark-Simulacrum Mark-Simulacrum added the perf-regression-triaged The performance regression has been triaged. label Oct 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

perf-regression Performance regression. perf-regression-triaged The performance regression has been triaged. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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

6 participants