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.

Sign up for GitHub

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

Jump to bottom

Remove disambiguators from intra doc link text #76078

Merged
merged 6 commits into from
Sep 5, 2020
Merged

Remove disambiguators from intra doc link text #76078

merged 6 commits into from
Sep 5, 2020

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Aug 29, 2020

Closes #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.

@jyn514 jyn514 added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name labels Aug 29, 2020
@rust-highfive

This comment has been minimized.

Sign in to view
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 29, 2020
@jyn514 jyn514 added C-feature-accepted Category: A feature request that has been accepted pending implementation. and removed C-feature-accepted Category: A feature request that has been accepted pending implementation. labels Aug 29, 2020
@jyn514 jyn514 mentioned this pull request Aug 29, 2020
Closed
@jyn514 jyn514 force-pushed the no-disambiguator branch 2 times, most recently from c63852b to 92c2f22 Compare August 29, 2020 20:44
Manishearth
Manishearth approved these changes Aug 30, 2020
View reviewed changes
src/librustdoc/html/markdown.rs Outdated Show resolved Hide resolved
@Manishearth
Copy link
Member

i think some more comments in the markdown parser hook would be nice. Otherwise looks great!

@jyn514
Copy link
Member Author

jyn514 commented Aug 30, 2020

i think some more comments in the markdown parser hook would be nice

I found lots of bugs while adding these 😆 working on those now, will add tests to match the bugs.

@jyn514
Copy link
Member Author

jyn514 commented Aug 30, 2020

This is ready for re-review.

@jyn514
Copy link
Member Author

jyn514 commented Sep 3, 2020

ping @Manishearth - how do the new comments look?

Manishearth
Manishearth approved these changes Sep 3, 2020
View reviewed changes
Copy link
Member

@Manishearth Manishearth left a comment

Choose a reason for hiding this comment

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

r=me, maybe squash the commits a little first

@jyn514
Remove needless .to_owned() for link
89ae59a
@jyn514
Refactor ItemLink into its own struct
d5495e2
@jyn514
Refactor RenderedLink into its own type
31a7b6e
@jyn514
Remove disambiguators from link text
9815010 
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
@jyn514
display_for -> suggestion_for
9d7e797
@jyn514
Copy link
Member Author

jyn514 commented Sep 3, 2020

@bors r=manishearth

@bors
Copy link
Contributor

bors commented Sep 3, 2020

📌 Commit 025a533284abbfb0a5e4882dc7b75c6ee11278ea has been approved by manishearth

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 3, 2020
@bors
Copy link
Contributor

bors commented Sep 4, 2020

⌛ Testing commit 025a533284abbfb0a5e4882dc7b75c6ee11278ea with merge 2253f3aff4eb7eaca3a32476302efb3f01e7b636...

@bors
Copy link
Contributor

bors commented Sep 4, 2020

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 4, 2020
@jyn514
Copy link
Member Author

jyn514 commented Sep 4, 2020

It tried to document the example! 😆

  Documenting rustdoc v0.0.0 (/checkout/src/librustdoc)
error: unresolved link to `f`
   --> src/librustdoc/clean/types.rs:442:37
    |
442 |     /// in an intra-doc link (e.g. [`fn@f`])
    |                                     ^^^^^^ unresolved link
    |
    = note: `-D broken-intra-doc-links` implied by `-D warnings`
    = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`

@jyn514
Misc cleanup
18c14fd 
- Preserve suffixes when displaying
- Rename test file to match `intra-link*`
- Remove unnecessary .clone()s
- Improve comments and naming
- Fix more bugs and add tests
- Escape intra-doc link example in public documentation
@jyn514
Copy link
Member Author

jyn514 commented Sep 4, 2020

@bors retry

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 4, 2020
@Dylan-DPC-zz
Copy link

@bors r=manishearth

@bors
Copy link
Contributor

bors commented Sep 4, 2020

📌 Commit 18c14fd has been approved by manishearth

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Sep 4, 2020
@Dylan-DPC
Rollup merge of rust-lang#76078 - jyn514:no-disambiguator, r=manishearth
4751df5 
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 Dylan-DPC-zz mentioned this pull request Sep 4, 2020
Closed
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Sep 4, 2020
@Dylan-DPC
Rollup merge of rust-lang#76078 - jyn514:no-disambiguator, r=manishearth
07a6c2c 
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 Dylan-DPC-zz mentioned this pull request Sep 4, 2020
Closed
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Sep 5, 2020
@Dylan-DPC
Rollup merge of rust-lang#76078 - jyn514:no-disambiguator, r=manishearth
baded64 
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.
This was referenced Sep 5, 2020
Closed
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 5, 2020
@bors
Auto merge of rust-lang#76376 - Dylan-DPC:rollup-8chsbw9, r=Dylan-DPC
7d289ae 
Rollup of 11 pull requests

Successful merges:

 - rust-lang#75695 (Add a regression test for issue-72793)
 - rust-lang#75741 (Refactor byteorder to std in rustc_middle)
 - rust-lang#75954 (Unstable Book: add links to tracking issues for FFI features)
 - rust-lang#75994 (`impl Rc::new_cyclic`)
 - rust-lang#76060 (Link vec doc to & reference)
 - rust-lang#76078 (Remove disambiguators from intra doc link text)
 - rust-lang#76082 (Fix intra-doc links on pub re-exports)
 - rust-lang#76254 (Fold length constant in Rvalue::Repeat)
 - rust-lang#76258 (x.py check checks tests/examples/benches)
 - rust-lang#76263 (inliner: Check for codegen fn attributes compatibility)
 - rust-lang#76285 (Move jointness censoring to proc_macro)

Failed merges:

r? @ghost
@bors bors merged commit ed39e6d into rust-lang:master Sep 5, 2020
@rustbot rustbot added this to the 1.48.0 milestone Sep 5, 2020
@jyn514 jyn514 deleted the no-disambiguator branch September 5, 2020 17:29
@jyn514 jyn514 mentioned this pull request Jan 10, 2021
Merged
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Mar 2, 2021
@JohnTitor
Rollup merge of rust-lang#80874 - jyn514:intra-doc-docs, r=Manishearth
a9339ed 
Update intra-doc link documentation to match the implementation

r? `@Manishearth`
cc `@camelid` `@m-ou-se`

Relevant PRs:
- rust-lang#74489
- rust-lang#80181
- rust-lang#76078
- rust-lang#77519
- rust-lang#73101

Relevant issues:
- rust-lang#78800
- rust-lang#77200
- rust-lang#77199 / rust-lang#54191

I haven't documented things that I consider 'just bugs', like rust-lang#77732, but I have documented features that aren't implemented, like rust-lang#78800.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pickfire pickfire pickfire left review comments

@Manishearth Manishearth Manishearth approved these changes

Assignees

@Manishearth Manishearth

Labels
A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name 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.
Projects
None yet
Milestone
1.48.0
Development

Successfully merging this pull request may close these issues.

Remove disambiguator from intra doc link text
7 participants
@jyn514 @rust-highfive @Manishearth @bors @Dylan-DPC-zz @pickfire @rustbot