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 First Draft of Lint Listing Page #1091

Merged
merged 5 commits into from
Jul 19, 2016

Conversation

killercup
Copy link
Member

cf. #1088

Test:

clone this branch and run

$ python util/export.py
$ cd util/gh-pages
$ python -m SimpleHTTPServer 8080

@killercup
Copy link
Member Author

My lunch break is over now, so I haven't

  • made any custom design adjustments
  • set this up to automatically deploy on travis (maybe copy this)

@@ -16,3 +16,6 @@ Cargo.lock

# Generated by dogfood
/target_recur/

# hg pages docs
Copy link
Contributor

Choose a reason for hiding this comment

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

Didn't you mean gh pages?

Copy link
Member Author

Choose a reason for hiding this comment

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

haha, yes

@killercup killercup force-pushed the feature/gh-pages-docs branch from b0cb583 to 20ca605 Compare July 12, 2016 13:34
info("got %s lints" % len(lints))
with open("util/gh-pages/lints.json", "w") as file:
json.dump(lints, file, indent=2)
info("wrote JSON for greate justice")
Copy link
Contributor

Choose a reason for hiding this comment

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

great! But not greate. 😄

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I just fixed that useless debug message :D

@killercup killercup force-pushed the feature/gh-pages-docs branch from 20ca605 to ba9eda7 Compare July 12, 2016 14:31
@killercup
Copy link
Member Author

Anything else (more typos?) you want me to change here?

I can't test the travis integration, so someone with the ability to push to this repo needs to create an API key and adjust the travis config. Then we can get this on the road :)

@mcarton
Copy link
Member

mcarton commented Jul 14, 2016

Some comments:

  • You should chmod +x util/export.py because I'm lazy 😏.
  • All lints <article> should have an html id so that we can point to it as we currently do with the wiki.
  • The files have some space-only lines.
  • The level selector should have a Warn + Deny option since that's what user will have by default.
  • The html file has a “Error loading commits!” message (bad copy-pasta 😄).
  • Some lints have redundant sections:

Example

Example:

let mut a = 5;
...
a = a + b;
  • How hard would it be to have syntax highlighting (is the markdown rendered client side? 😕) ?
  • I'd like an “open/close all” option (bonus).
  • 👍

@mcarton
Copy link
Member

mcarton commented Jul 14, 2016

  • The tables in the ineffective_bit_mask and wrong_self_convention lints are not rendered.

@mcarton
Copy link
Member

mcarton commented Jul 14, 2016

  • The font size of citations is much bigger than regular text (linkedlist looks really weird).
  • unnecessary_mut_passed is rendered as

Known problems

None Example my_vec.push(&mut value)

  • similarly for used_underscore_binding and wrong_self_convention

- Section IDs, with handy anchor links
- Multiple filters for levels
- Table rendering, block quote size
- Nicer loading (hide un-rendered content)
- Code highlighting (only for Rust, of course!)
- Fix parsing of descriptions that have a newline after the section
  title (lead to duplicating the title, e.g., "Examples", in the
  content)
@killercup
Copy link
Member Author

I think I fixed everything you mentioned, @mcarton, except for the open all/close all feature (it's currently really lazy, and thus harder to change all values at once).

I have to say, I'm both surprised and shocked by how efficient implementing something like this in angular.js is. The code is anything but nice/performant/zen, but it works 😄

@mcarton
Copy link
Member

mcarton commented Jul 14, 2016

There is still the problem with unnecessary_mut_passed, used_underscore_binding and wrong_self_convention.

Also why is the inline code red?
screen shot 2016-07-14 at 21 35 33

Otherwise, LGTM.

@killercup
Copy link
Member Author

There is still the problem with unnecessary_mut_passed, used_underscore_binding and wrong_self_convention.

Hm, looks like it doesn't get that "example" should start the next section. I'll debug the python script (this might also be a problem when generating the wiki page!).

Also why is the inline code red?

That's Bootstrap's default style. Do you want me to make it grayscale instead?

@mcarton
Copy link
Member

mcarton commented Jul 14, 2016

this might also be a problem when generating the wiki page!

It looks fine

That's Bootstrap's default style. Do you want me to make it grayscale instead?

I though it was weird, but if it's Bootstrap leave it.

@mcarton
Copy link
Member

mcarton commented Jul 15, 2016

@killercup another thing: configuration variables do not appear in your doc (they do in the wiki, see eg. blacklisted_name).

@killercup
Copy link
Member Author

There is still the problem with unnecessary_mut_passed, used_underscore_binding and wrong_self_convention.

My regex was too good. These lint docs say "Example", not "Example:" (with the colon at the end) 😄

I though it was weird, but if it's Bootstrap leave it.

Haha, I did not realize saying "Bootstrap" works as an argument from authority!

configuration variables do not appear in your doc

Oh, they were in an entirely different dict that I did not merge with the lint data at all. Fixed that. Also learned some python tricks along the way!

@killercup
Copy link
Member Author

Anything else I need to fix? Someone still needs to add and test the travis config.

@mcarton
Copy link
Member

mcarton commented Jul 19, 2016

I'm fine with this.
I was going to deploy it just to realize I don't have access to https://github.com/Manishearth/rust-clippy/settings/keys. @Manishearth: it looks like you'll have to had a deploy key yourself for Travis.

@Manishearth
Copy link
Member

Huh, there's no way to give you access either. hold on.

@Manishearth
Copy link
Member

Emailed a deploy key. I assume you're able to run travis encrypt locally?

@mcarton mcarton mentioned this pull request Jul 19, 2016
@mcarton mcarton merged commit c2baf5f into rust-lang:master Jul 19, 2016
@mcarton
Copy link
Member

mcarton commented Aug 18, 2016

@killercup just realized when you have a filter and click a link to a lint that's filtered out, nothing happens. Is there some JS voodoo magic you could had here? 😄

Eg.:

@killercup
Copy link
Member Author

Oh, very interesting, @mcarton! I hadn't tested the inter-lint-links at all (didn't even know they existed tbh). Can you make that into a new issue and assign it to me, so I don't forget to fix this? :)

@mcarton
Copy link
Member

mcarton commented Aug 19, 2016

That one exists since https://github.com/Manishearth/rust-clippy/pull/1146, there might have been some other but that's a good idea IMO.

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.

4 participants