Skip to content
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

Remove disambiguator from intra doc link text #65354

Closed
dtolnay opened this issue Oct 13, 2019 · 9 comments · Fixed by #76078
Closed

Remove disambiguator from intra doc link text #65354

dtolnay opened this issue Oct 13, 2019 · 9 comments · Fixed by #76078
Assignees
Labels
A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name C-bug Category: This is a bug. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@dtolnay
Copy link
Member

dtolnay commented Oct 13, 2019

/// Trait: [`trait@Name`], fn: [`fn@Name`]
pub struct S;

pub trait Name {}

#[allow(non_snake_case)]
pub fn Name() {}

This is currently rendered as:

The links point to the right place (the trait and the fn respectively) but I believe the link text should be just "Name" for both.

Discovered in #65336.

Mentioning the tracking issue: #43466

This issue has been assigned to @jyn514 via this comment.

@dtolnay dtolnay added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. C-bug Category: This is a bug. A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name labels Oct 13, 2019
@GuillaumeGomez
Copy link
Member

I can do it if you want but maybe you prefer doing it?

@dtolnay
Copy link
Member Author

dtolnay commented Oct 14, 2019

I definitely don't prefer doing it. 😉 Feel free to go ahead. Thanks!

@GuillaumeGomez
Copy link
Member

Ok haha. Will try to do it not too far into the future.

@GuillaumeGomez GuillaumeGomez self-assigned this Oct 15, 2019
@jyn514
Copy link
Member

jyn514 commented Jul 6, 2020

@rustbot claim

@rustbot rustbot assigned rustbot and unassigned GuillaumeGomez Jul 6, 2020
@jyn514
Copy link
Member

jyn514 commented Jul 6, 2020

So, I tried implementing this just now and it's not possible with the current pulldown-cmark API. The issue is that while broken_link_callback can return a new title (for when you hover) and a new URL, it cannot change the text that is rendered for shortcut reference links: [fn@Name]. See https://github.com/raphlinus/pulldown-cmark/blob/master/src/parse.rs#L2213.

Maybe we could ask pulldown to extend the API to allow this? I had trouble with the last PR I made though, the link parsing is very complicated.

@GuillaumeGomez
Copy link
Member

I think it'll be the best situation to directly improve the parser's API, indeed.

@jyn514

This comment has been minimized.

@rustbot rustbot added the S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. label Jul 10, 2020
@jyn514
Copy link
Member

jyn514 commented Aug 23, 2020

The recommended solution @marcusklaas gave in pulldown-cmark/pulldown-cmark#466 (comment) was to add a second pass on top of the pulldown parser which removes the disambiguator from Unknown link kinds. Not yet sure how hard this will be to implement, but it wouldn't need pulldown changes. It would go somewhere around jyn514@1650fbe#diff-545d93b02fb448fc31d33a6b1e0e0c48.

@jyn514 jyn514 added E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. and removed S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. labels Aug 23, 2020
@jyn514
Copy link
Member

jyn514 commented Aug 29, 2020

Presumably you would still want to keep symbol disambiguators, right? So fn@Name would be rendered as Name, but Name() would still be rendered the same?

jyn514 added a commit to jyn514/rust that referenced this issue Sep 3, 2020
Related to rust-lang#65354

- Pass through the replacement text to `markdown.rs`
- Add some tests
- Add a state machine that actually replaces the text when parsing Markdown
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Sep 4, 2020
Remove disambiguators from intra doc link text

Closes rust-lang#65354.
r? @Manishearth

The commits are mostly atomic, but there might be some mix between them here and there. I recommend reading 'refactor ItemLink' and 'refactor RenderedLink' on their own though, lots of churn without any logic changes.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Sep 4, 2020
Remove disambiguators from intra doc link text

Closes rust-lang#65354.
r? @Manishearth

The commits are mostly atomic, but there might be some mix between them here and there. I recommend reading 'refactor ItemLink' and 'refactor RenderedLink' on their own though, lots of churn without any logic changes.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Sep 5, 2020
Remove disambiguators from intra doc link text

Closes rust-lang#65354.
r? @Manishearth

The commits are mostly atomic, but there might be some mix between them here and there. I recommend reading 'refactor ItemLink' and 'refactor RenderedLink' on their own though, lots of churn without any logic changes.
@bors bors closed this as completed in ed39e6d Sep 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name C-bug Category: This is a bug. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants