Skip to content

Fix line display for hoedown - #41405

Merged
bors merged 1 commit into
rust-lang:masterfrom
GuillaumeGomez:rustdoc-warning-improvement
Apr 22, 2017
Merged

Fix line display for hoedown#41405
bors merged 1 commit into
rust-lang:masterfrom
GuillaumeGomez:rustdoc-warning-improvement

Conversation

@GuillaumeGomez

@GuillaumeGomezGuillaumeGomez commented Apr 19, 2017

Copy link
Copy Markdown
Member

Fixes#41401.

r? @rust-lang/docs

QuietMisdreavus
QuietMisdreavus reviewed Apr 20, 2017
View reviewed changes
Comment threadsrc/librustdoc/test.rs Outdated

@QuietMisdreavusQuietMisdreavusApr 20, 2017

Copy link
Copy Markdown
Contributor

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

frewsxcv commented Apr 20, 2017

Copy link
Copy Markdown
Contributor

@bors delegate=quietmisdreavus

In case you feel confident enough to review this :)

@bors

bors commented Apr 20, 2017

Copy link
Copy Markdown
Collaborator

✌️ @QuietMisdreavus can now approve this pull request

@kennytm

kennytm commented Apr 20, 2017•
edited
Loading

Copy link
Copy Markdown
Member

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))

@kennytmkennytm mentioned this pull request Apr 20, 2017
Closed
@GuillaumeGomez

GuillaumeGomez commented Apr 20, 2017

Copy link
Copy Markdown
MemberAuthor

This is really hell...

@GuillaumeGomez
GuillaumeGomezforce-pushed the rustdoc-warning-improvement branch from d5d9add to a654610CompareApril 20, 2017 18:20
@GuillaumeGomez

GuillaumeGomez commented Apr 20, 2017

Copy link
Copy Markdown
MemberAuthor

Updated.

@GuillaumeGomezGuillaumeGomez mentioned this pull request Apr 21, 2017
Merged
@steveklabnik

steveklabnik commented Apr 21, 2017

Copy link
Copy Markdown
Contributor

@bors: r+

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

@bors

bors commented Apr 21, 2017

Copy link
Copy Markdown
Collaborator

📌 Commit a654610 has been approved by steveklabnik

@ollie27

ollie27 commented Apr 21, 2017

Copy link
Copy Markdown
Contributor

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

GuillaumeGomez commented Apr 21, 2017

Copy link
Copy Markdown
MemberAuthor

Just improving priority a bit.

@bors: p=1

@shepmastershepmaster 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

bors commented Apr 21, 2017

Copy link
Copy Markdown
Collaborator

⌛ Testing commit a654610 with merge 609a7fb...

@bors

bors commented Apr 21, 2017

Copy link
Copy Markdown
Collaborator

💔 Test failed - status-appveyor

@alexcrichton

alexcrichton commented Apr 21, 2017

Copy link
Copy Markdown
Member

@bors: retry

@bors

bors commented Apr 21, 2017

Copy link
Copy Markdown
Collaborator

⌛ Testing commit a654610 with merge ce2d90e...

@arielb1

arielb1 commented Apr 21, 2017

Copy link
Copy Markdown
Contributor

@bors retry

  • mac builders not starting

@frewsxcv

frewsxcv commented Apr 22, 2017

Copy link
Copy Markdown
Contributor

@bors retry

trying to kick homu

@frewsxcv

frewsxcv commented Apr 22, 2017

Copy link
Copy Markdown
Contributor

@bors retry

@frewsxcvfrewsxcv reopened this Apr 22, 2017
@bors

bors commented Apr 22, 2017

Copy link
Copy Markdown
Collaborator

⌛ Testing commit a654610 with merge c99d381...

frewsxcv added a commit to frewsxcv/rust that referenced this pull request Apr 22, 2017
@frewsxcv
Rollup merge ofrust-lang#41405- GuillaumeGomez:rustdoc-warning-impr…
47c68ec
…ovement, r=steveklabnik
Fix line display for hoedown
Fixesrust-lang#41401.
r? @rust-lang/docs
@frewsxcvfrewsxcv mentioned this pull request Apr 22, 2017
Closed
@frewsxcv

frewsxcv commented Apr 22, 2017

Copy link
Copy Markdown
Contributor

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 ofrust-lang#41405- GuillaumeGomez:rustdoc-warning-impr…
aff40e2
…ovement, r=steveklabnik
Fix line display for hoedown
Fixesrust-lang#41401.
r? @rust-lang/docs
@frewsxcvfrewsxcv 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:masterApr 22, 2017
@GuillaumeGomez
GuillaumeGomez deleted the rustdoc-warning-improvement branch April 22, 2017 22:04
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@QuietMisdreavusQuietMisdreavusQuietMisdreavus left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

S-waiting-on-borsStatus: 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