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

HTML generated for code blocks can be improved (and shortened) #74180

Closed
GuillaumeGomez opened this issue Jul 9, 2020 · 7 comments
Closed

HTML generated for code blocks can be improved (and shortened) #74180

GuillaumeGomez opened this issue Jul 9, 2020 · 7 comments
Assignees
Labels
T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@GuillaumeGomez
Copy link
Member

For example, we can have occurrences of:

<span class="op">+</span><span class="op">=</span>

In this case, we could simply have:

<span class="op">+=</span>

We also have some weird generation sometimes:

</span> <span class="ident">x</span> <span class="kw">in</span> <span class="kw-2">&amp;</span><span class="ident">set</span>

In this case, I'm not sure what <span class="kw-2">&amp;</span> is actually doing...

@GuillaumeGomez GuillaumeGomez added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Jul 9, 2020
@GuillaumeGomez GuillaumeGomez self-assigned this Jul 9, 2020
@Lonami
Copy link
Contributor

Lonami commented Jul 9, 2020

I'm not sure what <span class="kw-2">&amp;</span> is actually doing...

Knowing nothing about the generation, seems it's just applying a different "keyword" style to & (x in &set).

@GuillaumeGomez
Copy link
Member Author

Thanks for the clarification! Then this span is not an issue. :)

@Lonami
Copy link
Contributor

Lonami commented Jul 9, 2020

Another question worth raising is whether whitespace should be ignored when merging spans. That is, if:

<span class="op">=</span> <span class="op">-</span>

could become

<span class="op">= -</span>

I think it should be fine as long as the styles do not alter the font size in ways it would change how spaces look (same for tabulators/newlines).

Also, I think class=op is valid HTML5 (and seems even since HTML2!) which would save two bytes per span. Maybe not worth doing if gzip compression is used when serving the pages anyway. I guess the main issue here is complexity, not space.

@GuillaumeGomez
Copy link
Member Author

Normally it depends on the font, so we use the same font in span, it should keep the spaces. Also in the case you showed, they're not directly following each other, but it might be interesting to merge them.

As for class=op, I have to admit I never thought about it. The only thing to be checked is if it's working on older browser rustdoc supports.

But this is a lot of good ideas, thanks for bringing them up!

@ollie27
Copy link
Member

ollie27 commented Jul 15, 2020

#73807 might fix this.

@GuillaumeGomez
Copy link
Member Author

At least improve the situation. I'll take another look once it'll merged to see if there is still weird DOM.

@GuillaumeGomez
Copy link
Member Author

#100429 and #100775 added the finishing touch. I don't think it can be improved much further at this point. Closing then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants