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

Rewrite doc_markdown to use pulldown-cmark #1799

Merged
merged 10 commits into from
Jun 17, 2017
Merged

Rewrite doc_markdown to use pulldown-cmark #1799

merged 10 commits into from
Jun 17, 2017

Conversation

mcarton
Copy link
Member

@mcarton mcarton commented May 28, 2017

Rewrite doc_markdown to use pulldown-cmark.

This should fix a number of false positives and true negatives. It should mean that the lint can be extended more easily in the futur.

WIP: I have not got the spans quite right yet. The best I can do is line-wise (actually attribute-wise) spans, while the current lint report the actual problematic word.

Fix #1469.

@killercup
Copy link
Member

Yes! This is gonna be good

@mcarton mcarton changed the title [DO NOT MERGE, WIP] Rewrite doc_markdown to use pulldown-cmark Rewrite doc_markdown to use pulldown-cmark May 30, 2017
@mcarton
Copy link
Member Author

mcarton commented May 30, 2017

This is ready to be merged.

@mcarton
Copy link
Member Author

mcarton commented May 31, 2017

Appveyor build looks to have failed because of caching problem.

@mcarton mcarton reopened this May 31, 2017
@mcarton
Copy link
Member Author

mcarton commented Jun 1, 2017

The windows build is failing because of

------stderr------------------------------
error[E0464]: multiple matching crates for `pulldown_cmark`
  --> clippy_lints/src/lib.rs:55:1
   |
55 | extern crate pulldown_cmark;
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: candidates:
   = note: path: \\?\C:\projects\rust-clippy\clippy_tests\target\debug\deps\libpulldown_cmark-2b392b6bad2227f1.rlib
   = note: crate name: pulldown_cmark
   = note: path: \\?\C:\Users\appveyor\.rustup\toolchains\nightly-i686-pc-windows-gnu\lib\rustlib\i686-pc-windows-gnu\lib\libpulldown_cmark-650d3ff121d3d129.rlib
   = note: crate name: pulldown_cmark
error[E0463]: can't find crate for `pulldown_cmark`
  --> clippy_lints/src/lib.rs:55:1
   |
55 | extern crate pulldown_cmark;
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ can't find crate
error: aborting due to previous error(s)

Any idea what is going on?

@mcarton mcarton mentioned this pull request Jun 1, 2017
@oli-obk
Copy link
Contributor

oli-obk commented Jun 1, 2017

Rustc is distributing its own pulldown... somehow we're getting a conflict with the distributed one. Do we set any unusual path env var values in appveyor?

@mcarton
Copy link
Member Author

mcarton commented Jun 1, 2017

Looks like we do. Cc @llogiq, I've no idea how either Appveyor or Windows work.

@mcarton mcarton force-pushed the docs branch 3 times, most recently from 05377ff to 953be0f Compare June 10, 2017 17:52
@mcarton
Copy link
Member Author

mcarton commented Jun 10, 2017

This works \o/
Any objection to merging?

@Manishearth
Copy link
Member

No, but I would like a quick performance comparison.

@oli-obk
Copy link
Contributor

oli-obk commented Jun 10, 2017

You can remove my two commits then. Seems they had no effect and were messy to begin with

@mcarton
Copy link
Member Author

mcarton commented Jun 10, 2017

Before:
diesel: (3767 lines starting with //[/!])

time: 0.036; rss: 114MB	early lint checks
time: 0.036; rss: 113MB	early lint checks
time: 0.036; rss: 113MB	early lint checks

serde: (3111 lines starting with //[/!])

time: 0.034; rss: 111MB	early lint checks
time: 0.034; rss: 111MB	early lint checks
time: 0.034; rss: 111MB	early lint checks

clippy_lints: (10809 lines starting with //[/!])

time: 0.034; rss: 178MB	early lint checks
time: 0.029; rss: 178MB	early lint checks
time: 0.030; rss: 182MB	early lint checks

After:
diesel:

time: 0.038; rss: 113MB	early lint checks
time: 0.039; rss: 114MB	early lint checks
time: 0.036; rss: 114MB	early lint checks

serde:

time: 0.034; rss: 111MB	early lint checks
time: 0.033; rss: 113MB	early lint checks
time: 0.037; rss: 113MB	early lint checks

clippy_lints:

time: 0.033; rss: 179MB	early lint checks
time: 0.030; rss: 183MB	early lint checks
time: 0.030; rss: 181MB	early lint checks

Does not look like there is much of a difference. Any idea of a biggish crate with a lot of docs?
(It also indirectly found dead links in diesel) 😄

@Manishearth
Copy link
Member

(feel free to merge, fwiw)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

False doc_markdown warnings
4 participants