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

Add links for primitives in "jump to definition" feature #88033

Merged
merged 2 commits into from
Sep 14, 2021
Merged

Add links for primitives in "jump to definition" feature #88033

merged 2 commits into from
Sep 14, 2021

Conversation

GuillaumeGomez
Copy link
Member

@GuillaumeGomez GuillaumeGomez commented Aug 14, 2021 •
edited
Loading

Follow-up of #84176.

I created a function primitive_from_str which is code that was originally in collect_intra_doc_links::resolve_primitive to prevent code duplication.

I also created the primitive_link_url function which is somewhat similar to primitive_link but too much different to merge both of them.

Part of #89095.

r? @jyn514

@GuillaumeGomez GuillaumeGomez added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Aug 14, 2021
@rust-highfive
Copy link
Collaborator

Some changes occurred in intra-doc-links.

cc @jyn514

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 14, 2021
jyn514
jyn514 reviewed Aug 14, 2021
View reviewed changes
src/librustdoc/html/format.rs Outdated Show resolved Hide resolved
src/librustdoc/html/format.rs Outdated Show resolved Hide resolved
src/librustdoc/html/format.rs Outdated Show resolved Hide resolved
@jyn514 jyn514 added the A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name label Aug 14, 2021
jyn514
jyn514 reviewed Aug 16, 2021
View reviewed changes
Copy link
Member

@jyn514 jyn514 left a comment

Choose a reason for hiding this comment

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

I think this may be fixed without having to add all this extra work once #87073 is merged - do you mind waiting for that? You could also try rebasing over that in the meantime to see if the tests pass without your changes.

src/librustdoc/html/format.rs Outdated Show resolved Hide resolved
@GuillaumeGomez
Copy link
Member Author

I think this may be fixed without having to add all this extra work once #87073 is merged - do you mind waiting for that? You could also try rebasing over that in the meantime to see if the tests pass without your changes.

I'm totally fine with waiting for it!

@JohnCSimon JohnCSimon added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 6, 2021
@JohnCSimon
Copy link
Member

triage: fyi #87073 is still open

@jyn514 jyn514 added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 6, 2021
@GuillaumeGomez GuillaumeGomez added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Sep 13, 2021
@GuillaumeGomez
Copy link
Member Author

#87073 has been merged, the PR has been rebased and updated to take the changes into account making it much smaller. Ready for the next review round! :)

jyn514
jyn514 suggested changes Sep 13, 2021
View reviewed changes
src/librustdoc/html/render/span_map.rs Outdated Show resolved Hide resolved
src/librustdoc/html/render/span_map.rs Show resolved Hide resolved
src/test/rustdoc/check-source-code-urls-to-def.rs Show resolved Hide resolved
@jyn514
Copy link
Member

jyn514 commented Sep 13, 2021

You're right, this is a lot simpler now :)

@jyn514 jyn514 added requires-nightly This issue requires a nightly compiler in some way. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 13, 2021
jyn514
jyn514 reviewed Sep 13, 2021
View reviewed changes
src/librustdoc/clean/utils.rs Outdated Show resolved Hide resolved
src/librustdoc/html/highlight.rs Outdated Show resolved Hide resolved
src/librustdoc/html/render/span_map.rs Outdated Show resolved Hide resolved
src/test/rustdoc/check-source-code-urls-to-def-std.rs Outdated Show resolved Hide resolved
@GuillaumeGomez
Copy link
Member Author

Updated!

@jyn514
Copy link
Member

jyn514 commented Sep 13, 2021

r=me once CI is passing

@GuillaumeGomez
Copy link
Member Author

@bors: r=jyn514

@bors
Copy link
Contributor

bors commented Sep 13, 2021

📌 Commit d73c0a3 has been approved by jyn514

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 13, 2021
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Sep 13, 2021
@GuillaumeGomez
Rollup merge of rust-lang#88033 - GuillaumeGomez:jump-to-def-primitiv…
f789108 
…e, r=jyn514

Add links for primitives in "jump to definition" feature

Follow-up of rust-lang#84176.

I created a function `primitive_from_str` which is code that was originally in `collect_intra_doc_links::resolve_primitive` to prevent code duplication.

I also created the `primitive_link_url` function which is somewhat similar to `primitive_link` but too much different to merge both of them.

r? `@jyn514`
This was referenced Sep 13, 2021
Closed
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 14, 2021
@bors
Auto merge of rust-lang#88914 - GuillaumeGomez:rollup-h5svc6w, r=Guil…
ec9a1bd 
…laumeGomez

Rollup of 7 pull requests

Successful merges:

 - rust-lang#88033 (Add links for primitives in "jump to definition" feature)
 - rust-lang#88722 (Make `UnsafeCell::get_mut` const)
 - rust-lang#88851 (Fix duplicate bounds for const_trait_impl)
 - rust-lang#88859 (interpreter PointerArithmetic: use new Size helper methods)
 - rust-lang#88885 (Fix jump def background)
 - rust-lang#88894 (Improve error message for missing trait in trait impl)
 - rust-lang#88896 (Reduce possibility of flaky tests)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit b3bb786 into rust-lang:master Sep 14, 2021
@rustbot rustbot added this to the 1.57.0 milestone Sep 14, 2021
@GuillaumeGomez GuillaumeGomez deleted the jump-to-def-primitive branch September 14, 2021 11:55
@GuillaumeGomez GuillaumeGomez mentioned this pull request Sep 19, 2021
Open
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jyn514 jyn514 jyn514 requested changes

Assignees

@jyn514 jyn514

Labels
A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name requires-nightly This issue requires a nightly compiler in some way. 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.57.0
Development

Successfully merging this pull request may close these issues.
6 participants
@GuillaumeGomez @rust-highfive @JohnCSimon @jyn514 @bors @rustbot