Improve internals to allow having much smaller HTML generated #262
Conversation
crates/anstyle-svg/src/lib.rs
Outdated
| /// Whether or not to generate HTML5 compatible tags. | ||
| pub const fn use_html5(mut self, use_html5: bool) -> Self { |
There was a problem hiding this comment.
What is the prevelance of html5 for whether we should just make this non-configurable?
There was a problem hiding this comment.
HTML5 is basically used everywhere nowadays. I'm not sure if formats like epub are fully-compliant though. So if you want to keep this crate useable by a maximum number of users, I'd say keep the possibility to disable HTML5. We can however discuss about whether or not it should be enabled or disabled by default.
There was a problem hiding this comment.
In that case, I think I'd prefer we switch over to HTML 5 and we can add an opt-out if someone has a need.
Based on usage, HTML 5 seems like the right choice. At that point, rather than speculatively have an opt-out, I'd prefer doing it when requested.
There was a problem hiding this comment.
Is there a reason use_html5 hasn't been removed?
GuillaumeGomez
force-pushed
the
improve-internals
branch
2 times, most recently
from
e865e6f to
e125d1b
Compare
|
Made atomic commits, moved function as suggested and created a new enum for span kinds (named |
Pull Request Test Coverage Report for Build 16598275897Details
💛 - Coveralls |
GuillaumeGomez
force-pushed
the
improve-internals
branch
3 times, most recently
from
f9eb36c to
e5c93d9
Compare
…g_span` functions
GuillaumeGomez
force-pushed
the
improve-internals
branch
from
e5c93d9 to
87825e6
Compare
|
Applied suggestions and merge tests updates within the relevant commits. |
| write_bg_span(buffer, "span", &element.style, &element.text); | ||
| } | ||
| writeln!(buffer, r#"<br />"#).unwrap(); | ||
| buffer.write_str(br).unwrap(); |
There was a problem hiding this comment.
Please restore the use of writeln! as that is not relevant to this commit (overlooked it until I could see how the tests changed
crates/anstyle-svg/src/lib.rs
Outdated
| /// Whether or not to generate HTML5 compatible tags. | ||
| pub const fn use_html5(mut self, use_html5: bool) -> Self { |
There was a problem hiding this comment.
Is there a reason use_html5 hasn't been removed?
crates/anstyle-svg/src/lib.rs
Outdated
| if span != SpanKind::Tspan && classes.is_empty() && element.url.is_none() { | ||
| // No need to create an element if there is no class or href. | ||
| write!(buffer, "{fragment}").unwrap(); | ||
| } | ||
| let mut need_closing_a = false; | ||
|
|
||
| write!(buffer, r#"<{span}"#).unwrap(); |
There was a problem hiding this comment.
We are writing out the fragment with and without a span in this commit
crates/anstyle-svg/src/lib.rs
Outdated
| // No need to create an element if there is no class or href. | ||
| write!(buffer, "{fragment}").unwrap(); | ||
| } | ||
| let mut need_closing_a = false; |
There was a problem hiding this comment.
If already splitting everything, why not have this in the match?
|
|
||
| let br = if self.use_html5 { "<br>" } else { "<br />" }; | ||
| writeln!(buffer, r#" <div class="container {FG}">"#).unwrap(); | ||
| write!(buffer, r#"<div class="container {FG}">"#).unwrap(); |
There was a problem hiding this comment.
Please revert this commit
I doubt this makes enough of a difference to be worth it and part of the intent for ansstyle-svg is to be usable for tests which involves diffing the content
3 participants
First commit comes from #261.
For
triagebot"enhanced" log pages, it makes a HUGE difference to not have all these tags rendered (takes me more than one minute to load a page with anstyle before this PR).