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

fix intra-link resolution spans in block comments #56010

Merged
merged 1 commit into from
Dec 12, 2018
Merged

fix intra-link resolution spans in block comments #56010

merged 1 commit into from
Dec 12, 2018

Conversation

euclio
Copy link
Contributor

@euclio euclio commented Nov 16, 2018

This commit improves the calculation of code spans for intra-doc
resolution failures. All sugared doc comments should now have the
correct spans, including those where the comment is longer than the
docs.

It also fixes an issue where the spans were calculated incorrectly for
certain unsugared doc comments. The diagnostic will now always use the
span of the attributes, as originally intended.

Fixes #55964.

r? @QuietMisdreavus

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 16, 2018
QuietMisdreavus
QuietMisdreavus approved these changes Nov 16, 2018
View reviewed changes
Copy link
Member

@QuietMisdreavus QuietMisdreavus left a comment

Choose a reason for hiding this comment

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

This is brilliant, thank you so much! I was expecting the fix to be more complicated than this, but your PR is great!

@QuietMisdreavus
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Nov 16, 2018

📌 Commit 9893407d87e81c2069ca06c416d316adbf562946 has been approved by QuietMisdreavus

@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 Nov 16, 2018
@kennytm kennytm mentioned this pull request Nov 17, 2018
Closed
@bors
Copy link
Contributor

bors commented Nov 19, 2018

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

@bors bors added 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 19, 2018
@euclio
Copy link
Contributor Author

euclio commented Nov 19, 2018

@QuietMisdreavus Rebased.

@euclio
Copy link
Contributor Author

euclio commented Nov 26, 2018

Ping @QuietMisdreavus

@QuietMisdreavus
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Dec 3, 2018

📌 Commit ea5843c has been approved by QuietMisdreavus

@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 Dec 3, 2018
kennytm added a commit to GuillaumeGomez/rust that referenced this pull request Dec 5, 2018
@kennytm
Rollup merge of rust-lang#56010 - euclio:intra-doc-spans, r=QuietMisd…
f0fbfe2 
…reavus

fix intra-link resolution spans in block comments

This commit improves the calculation of code spans for intra-doc
resolution failures. All sugared doc comments should now have the
correct spans, including those where the comment is longer than the
docs.

It also fixes an issue where the spans were calculated incorrectly for
certain unsugared doc comments. The diagnostic will now always use the
span of the attributes, as originally intended.

Fixes rust-lang#55964.
@GuillaumeGomez GuillaumeGomez mentioned this pull request Dec 5, 2018
Closed
@bors bors added 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 5, 2018
@ollie27 ollie27 mentioned this pull request Dec 5, 2018
Merged
@euclio
Copy link
Contributor Author

euclio commented Dec 6, 2018

@QuietMisdreavus Added a test and made sure the compiler docs can be generated successfully. I think this solution is cleaner than the original, too!

@euclio euclio changed the title [WIP] fix intra-link resolution spans in block comments fix intra-link resolution spans in block comments Dec 11, 2018
@euclio
Copy link
Contributor Author

euclio commented Dec 11, 2018

@QuietMisdreavus Ok, I think this is ready for bors to have another whack at it.

@QuietMisdreavus
Copy link
Member

Curious about what caused that error in the first place, but if we can get around it, that's probably best for now. The new code looks good, so here goes...

@bors r+

@bors
Copy link
Contributor

bors commented Dec 11, 2018

📌 Commit 56413ec has been approved by QuietMisdreavus

@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 Dec 11, 2018
@bors
Copy link
Contributor

bors commented Dec 11, 2018

⌛ Testing commit 56413ec with merge c4b3fc4bc2f0f9e068c9bf5b6bba2f0f240fd7d0...

@bors
Copy link
Contributor

bors commented Dec 11, 2018

💔 Test failed - status-appveyor

@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 Dec 11, 2018
@rust-highfive
Copy link
Collaborator

Your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log. 
Checking out files: 100% (17616/17616), done.
travis_time:end:3a0fcb18:start=1544571021577328000,finish=1544571032075818000,duration=10498490000
$ cd rust-lang/rust
$ git checkout -qf c4b3fc4bc2f0f9e068c9bf5b6bba2f0f240fd7d0
fatal: reference is not a tree: c4b3fc4bc2f0f9e068c9bf5b6bba2f0f240fd7d0
The command "git checkout -qf c4b3fc4bc2f0f9e068c9bf5b6bba2f0f240fd7d0" failed and exited with 128 during .
Your build has been stopped.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@alexcrichton
Copy link
Member

@bors: retry

  • CI automation hiccup

@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 Dec 11, 2018
@bors
Copy link
Contributor

bors commented Dec 12, 2018

⌛ Testing commit 56413ec with merge 6375777bb438d115b298003e93197110f881bba0...

@bors
Copy link
Contributor

bors commented Dec 12, 2018

💔 Test failed - status-appveyor

@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 Dec 12, 2018
@alexcrichton
Copy link
Member

@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 Dec 12, 2018
@bors
Copy link
Contributor

bors commented Dec 12, 2018

⌛ Testing commit 56413ec with merge a64cdec...

bors added a commit that referenced this pull request Dec 12, 2018
@bors
Auto merge of #56010 - euclio:intra-doc-spans, r=QuietMisdreavus
a64cdec 
fix intra-link resolution spans in block comments

This commit improves the calculation of code spans for intra-doc
resolution failures. All sugared doc comments should now have the
correct spans, including those where the comment is longer than the
docs.

It also fixes an issue where the spans were calculated incorrectly for
certain unsugared doc comments. The diagnostic will now always use the
span of the attributes, as originally intended.

Fixes #55964.

r? @QuietMisdreavus
@bors
Copy link
Contributor

bors commented Dec 12, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: QuietMisdreavus
Pushing a64cdec to master...

@bors bors merged commit 56413ec into rust-lang:master Dec 12, 2018
@euclio euclio deleted the intra-doc-spans branch December 12, 2018 13:35
@SimonSapin SimonSapin mentioned this pull request Dec 27, 2018
Merged
@bryanburgers bryanburgers mentioned this pull request May 29, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ollie27 ollie27 ollie27 left review comments

@QuietMisdreavus QuietMisdreavus QuietMisdreavus approved these changes

Assignees

@QuietMisdreavus QuietMisdreavus

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

rustdoc: intra-doc link errors give incorrect spans on certain block doc comments
7 participants
@euclio @QuietMisdreavus @bors @rust-highfive @kennytm @alexcrichton @ollie27