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

[WIP] Reduce HTML output in rustdoc #41384

Closed
wants to merge 1 commit into from

Conversation

GuillaumeGomez
Copy link
Member

@GuillaumeGomez GuillaumeGomez commented Apr 19, 2017

 > du -sh normal/std/index.html
 60K	normal/std/index.html
 > du -sh build/x86_64-apple-darwin/doc/std/index.html
 36K	build/x86_64-apple-darwin/doc/std/index.html

r? @rust-lang/docs

@frewsxcv
Copy link
Member

@killercup
Copy link
Member

@GuillaumeGomez I'm pretty surprised to see your minifier drop so many closing tags. Especially the rules around omitting </li> seem to be error-prone to me (but I haven't dealt with stuff like that in quite a while). Two more things:

  1. Compare the gzip'd sizes as basically all HTTP traffic is compressed (as are blobs stored in a git repo IIRC)
  2. Have I linked to http://stackoverflow.com/a/1732454/1254484 recently? :)

@GuillaumeGomez
Copy link
Member Author

I'm pretty surprised to see your minifier drop so many closing tags. Especially the rules around omitting seem to be error-prone to me (but I haven't dealt with stuff like that in quite a while).

li, tr, td and equivalent can be unclosed considering they can't be directly in each others.

Compare the gzip'd sizes as basically all HTTP traffic is compressed (as are blobs stored in a git repo IIRC)

Hum, good point.

Have I linked to http://stackoverflow.com/a/1732454/1254484 recently? :)

The compression isn't "hardcore" so it leaves place for improvements. Using html5ever for this could be worth it, to be tested. This PR is a work in progress after all and isn't to be merged right now.

@aidanhs aidanhs added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Apr 19, 2017
@alexcrichton
Copy link
Member

Note that a tool like this seems like it has a high chance of accidentally introducing regressions, it'd be great if we could add some regression testing as well to this.

@GuillaumeGomez
Copy link
Member Author

@alexcrichton: it's not close at all to get merged. For now it was mostly to get some feedbacks. Do you have an idea by any chance on how I could test regressions in here?

@alexcrichton
Copy link
Member

To start this could use an html parser like html5ever and enusre the document structure doesn't change for standard Rust properties like:

  • The book
  • Standard library docs
  • Popular crates on docs.rs

@sophiajt
Copy link
Contributor

I'm not currently on rustdoc, so assigning to alex

r? @alexcrichton

@clarfonthey
Copy link
Contributor

It'd be nice if we could do this to rustdoc's CSS as well.

@GuillaumeGomez
Copy link
Member Author

@clarcharr: It's scheduled (at least I intend to do it).

@frewsxcv
Copy link
Member

frewsxcv commented May 3, 2017

In my opinion, minimizing HTML and CSS is something that would be cool to do for all pages generated by rustdoc, but it seems the underlying tool has some incubation to so. I have some concerns about some of the strategies it does and the lack of testing. I don't know if this PR is the most effective way to get feedback though

@GuillaumeGomez
Copy link
Member Author

Well, at least it opens the debate. I still have a lot to do on the minimizer tool.

@TimNN
Copy link
Contributor

TimNN commented May 10, 2017

@GuillaumeGomez: What's the current status of this PR?

@GuillaumeGomez
Copy link
Member Author

Quite busy, I still need to rewrite the HTML minifier (using html5ever). I'll try to get to it in the next weeks or so.

@bors
Copy link
Contributor

bors commented May 16, 2017

☔ The latest upstream changes (presumably #41843) made this pull request unmergeable. Please resolve the merge conflicts.

@alexcrichton
Copy link
Member

Ok I'm going to close this for now to help keep the queue clear, but please of course feel free to resubmit!

@GuillaumeGomez GuillaumeGomez deleted the minification branch November 24, 2017 20:55
@GuillaumeGomez GuillaumeGomez restored the minification branch November 24, 2017 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants