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

Add warning for invalid start of code blocks in rustdoc #48596

Merged

Conversation

GuillaumeGomez
Copy link
Member

Follow up of #48382.

Still two things to consider:

  1. Adding test for rustdoc output (but where? In UI or in rustdoc tests?).
  2. Try to fix the span issue.

r? @QuietMisdreavus

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 28, 2018
@QuietMisdreavus
Copy link
Member

Code looks good. As to your points:

  1. rustdoc tests are currently just focused on the html output of rustdoc, but this changes the stdout/stderr output of rustdoc, which (for rustc) would get tested in UI tests. However, i'm not aware of a way to tell UI tests to execute rustdoc instead of rustc. Maybe there's a way? @Mark-Simulacrum?
  2. What span issue are you referring to?

@GuillaumeGomez
Copy link
Member Author

It's not referring to the "correct" place. I have some ideas to fix it, shouldn't be long.

@pietroalbini pietroalbini 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-review Status: Awaiting review from the assignee but also interested parties. labels Mar 5, 2018
@pietroalbini
Copy link
Member

Ping from triage @GuillaumeGomez! Is 11 days still not long enough?

@GuillaumeGomez
Copy link
Member Author

Been busy. I'll try to fix the issue when I have time.

@GuillaumeGomez GuillaumeGomez force-pushed the invalid-code-block-start branch from ee2ad41 to 7786f70 Compare March 18, 2018 19:35
@GuillaumeGomez
Copy link
Member Author

Updated.

@pietroalbini pietroalbini added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 18, 2018
@QuietMisdreavus
Copy link
Member

I forget what the code looked like before. Does this fix the "span issue" to your liking?

@GuillaumeGomez
Copy link
Member Author

Yes! :)

@QuietMisdreavus
Copy link
Member

Works for me!

@bors r+

@bors
Copy link
Contributor

bors commented Mar 20, 2018

📌 Commit 7786f70 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 Mar 20, 2018
kennytm added a commit to kennytm/rust that referenced this pull request Mar 21, 2018
…tart, r=QuietMisdreavus

Add warning for invalid start of code blocks in rustdoc

Follow up of rust-lang#48382.

Still two things to consider:
 1. Adding test for rustdoc output (but where? In UI or in rustdoc tests?).
 2. Try to fix the span issue.

r? @QuietMisdreavus
kennytm added a commit to kennytm/rust that referenced this pull request Mar 22, 2018
…tart, r=QuietMisdreavus

Add warning for invalid start of code blocks in rustdoc

Follow up of rust-lang#48382.

Still two things to consider:
 1. Adding test for rustdoc output (but where? In UI or in rustdoc tests?).
 2. Try to fix the span issue.

r? @QuietMisdreavus
bors added a commit that referenced this pull request Mar 22, 2018
@alexcrichton alexcrichton merged commit 7786f70 into rust-lang:master Mar 22, 2018
@GuillaumeGomez GuillaumeGomez deleted the invalid-code-block-start branch March 23, 2018 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants