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 line display for hoedown #41405

Merged
merged 1 commit into from
Apr 22, 2017
Merged

Fix line display for hoedown #41405

merged 1 commit into from
Apr 22, 2017

Conversation

GuillaumeGomez
Copy link
Member

Fixes #41401.

r? @rust-lang/docs

QuietMisdreavus
QuietMisdreavus reviewed Apr 20, 2017
View reviewed changes
src/librustdoc/test.rs Outdated
}
if number < line {
line -= number;
}
Copy link
Member

Choose a reason for hiding this comment

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

What cases were you running into where the number of tests was more than the line number itself? Or is this a guard to prevent underflowing a usize?

@frewsxcv
Copy link
Member

@bors delegate=quietmisdreavus

In case you feel confident enough to review this :)

@bors
Copy link
Contributor

bors commented Apr 20, 2017

✌️ @QuietMisdreavus can now approve this pull request

@kennytm
Copy link
Member

kennytm commented Apr 20, 2017 •
edited
Loading

Sorry it may be hijacking, just inspecting the test result of this PR it seems to uncover additional false warnings not yet fixed.

  1. https://github.com/rust-lang/book/blame/ad7de198/first-edition/src/conditional-compilation.md#L58-L62 (Travis; rendered book)
  2. https://github.com/rust-lang/book/blame/ad7de198/first-edition/src/concurrency.md#L132-L141
  3. https://github.com/rust-lang/book/blame/ad7de198/first-edition/src/crates-and-modules.md#L76-L92 (additionally, a warning is placed on a ```rust,ignore code block 😕)
  4. https://github.com/rust-lang/book/blame/ad7de198/first-edition/src/match.md#L48-L59
    (... and a lot more from The Book. It seems most of these come after a ```text block)
  5. https://github.com/rust-lang/book/blame/ad7de198/first-edition/src/primitive-types.md#L32-L35 (no ```text block before, maybe a different issue.)
  6. https://github.com/rust-lang-nursery/reference/blame/6b0de90d/src/tokens.md#L92-L98 (the whole file is not tested)
  7. https://github.com/rust-lang-nursery/reference/blame/6b0de90d/src/types.md#L194-L201

The Travis log should provide more cases.

(Minimized to two additional test cases in #41401 (comment))

@kennytm kennytm mentioned this pull request Apr 20, 2017
Closed
@GuillaumeGomez
Copy link
Member Author

This is really hell...

@GuillaumeGomez GuillaumeGomez force-pushed the rustdoc-warning-improvement branch from d5d9add to a654610 Compare April 20, 2017 18:20
@GuillaumeGomez
Copy link
Member Author

Updated.

@GuillaumeGomez GuillaumeGomez mentioned this pull request Apr 21, 2017
Merged
@steveklabnik
Copy link
Member

@bors: r+

thanks @GuillaumeGomez ❤️ (and @kennytm for the extra cases)

@bors
Copy link
Contributor

bors commented Apr 21, 2017

📌 Commit a654610 has been approved by steveklabnik

@ollie27
Copy link
Member

ollie27 commented Apr 21, 2017

The latest travis log: https://travis-ci.org/rust-lang/rust/jobs/224065533 still has a lot of "Code block is not currently run as a test" warnings so this is clearly not yet fully fixed.

@GuillaumeGomez
Copy link
Member Author

Just improving priority a bit.

@bors: p=1

@shepmaster shepmaster added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Apr 21, 2017
@bors
Copy link
Contributor

bors commented Apr 21, 2017

⌛ Testing commit a654610 with merge 609a7fb...

@bors
Copy link
Contributor

bors commented Apr 21, 2017

💔 Test failed - status-appveyor

@alexcrichton
Copy link
Member

@bors: retry

@bors
Copy link
Contributor

bors commented Apr 21, 2017

⌛ Testing commit a654610 with merge ce2d90e...

@arielb1
Copy link
Contributor

arielb1 commented Apr 21, 2017

@bors retry

  • mac builders not starting

@frewsxcv
Copy link
Member

@bors retry

trying to kick homu

@frewsxcv
Copy link
Member

@bors retry

@frewsxcv frewsxcv closed this Apr 22, 2017
@frewsxcv frewsxcv reopened this Apr 22, 2017
@bors
Copy link
Contributor

bors commented Apr 22, 2017

⌛ Testing commit a654610 with merge c99d381...

frewsxcv added a commit to frewsxcv/rust that referenced this pull request Apr 22, 2017
@frewsxcv
Rollup merge of rust-lang#41405 - GuillaumeGomez:rustdoc-warning-impr…
47c68ec 
…ovement, r=steveklabnik

Fix line display for hoedown

Fixes rust-lang#41401.

r? @rust-lang/docs
@frewsxcv frewsxcv mentioned this pull request Apr 22, 2017
Closed
@frewsxcv
Copy link
Member

This failed on Windows, but bors never picked it up. Going to retry this in a rollup

@bors retry

frewsxcv added a commit to frewsxcv/rust that referenced this pull request Apr 22, 2017
@frewsxcv
Rollup merge of rust-lang#41405 - GuillaumeGomez:rustdoc-warning-impr…
aff40e2 
…ovement, r=steveklabnik

Fix line display for hoedown

Fixes rust-lang#41401.

r? @rust-lang/docs
@frewsxcv frewsxcv mentioned this pull request Apr 22, 2017
Merged
bors added a commit that referenced this pull request Apr 22, 2017
@bors
Auto merge of #41459 - frewsxcv:rollup, r=frewsxcv
ff13b7c 
Rollup of 4 pull requests

- Successful merges: #37658, #41405, #41432, #41435
- Failed merges:
@bors bors merged commit a654610 into rust-lang:master Apr 22, 2017
@GuillaumeGomez GuillaumeGomez deleted the rustdoc-warning-improvement branch April 22, 2017 22:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@QuietMisdreavus QuietMisdreavus QuietMisdreavus left review comments

Assignees
No one assigned
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.
10 participants
@GuillaumeGomez @frewsxcv @bors @kennytm @steveklabnik @ollie27 @alexcrichton @arielb1 @QuietMisdreavus @shepmaster