toolmantim
commented
6 years ago Thanks! I’m curious what the use case was for wanting this?
Closed TheDen closed 5 years ago
toolmantim
commented
6 years ago Thanks! I’m curious what the use case was for wanting this?
TheDen
commented
6 years ago Hey @toolmantim 👋 I'm using terminal to generate a lot of files that are served over the web, so it makes sense to minify content. My initial commit was my attempt at not adding a new external dependency (though the bytes.Replace changes I added can be improved). The latest commit uses minify, and I now have a custom build from this commit to minify all my generated --preview pages.
I thought I'd share the changes—there's no pressure to merge if it's too edge-case for most users.
toolmantim
commented
6 years ago Oh wow, interesting. I mean, potentially it's a neat optimisation for Buildkite also. What sort of size reductions are you finding?
TheDen
commented
6 years ago It's okay, but not as much as I hoped unfortunately—on the fixtures/homer.sh.raw file
$ du -b *minified*
8201 minified.html
10246 non-minified.htmlThat's a saving of 2045 bytes—however scaling to ~1mil files you'd save ~2GBs.
Minifying the actual content breaks pre-wrap so I'm interested in finding a solution where the content can be minified whilst preserving white-space values. If that can be done even partially without breaking content the data saved will be much better and will scale with the actual content not just the header.
lox
commented
6 years ago I always wonder whether minification is worth it in a context like this when you have things like HTTP2 and decent transfer encoding compression.
TheDen
commented
6 years ago Good point @lox 🤔I was thinking in terms of stored data/filesize rather than transfer speed, but it seems that even with HTTP/2 minification is still recommended[1][2] (concatenation is not of course).
lox
commented
6 years ago Yeah, I've always thought of it as the cherry on top 🍨
TheDen
commented
6 years ago Agreed. Thanks for making terminal by the way it's been very useful!
toolmantim
commented
6 years ago @lox for us we also load the string into memory in the Rails process, and modify the string, so potentially the savings for us aren't just network based.
lox
commented
6 years ago Perhaps we could look at something like https://github.com/tdewolff/minify?
toolmantim
commented
6 years ago @lox isn't that what this PR is using? 😁
lox
commented
6 years ago 🤦♂️ I swear when I first looked it wasn't. Am I losing my mind?
toolmantim
commented
6 years ago Hehe, you're not going crazy! First version wasn't, but https://github.com/buildkite/terminal/pull/50/commits/c2542ac3a48766353469990a0e987ae69c03d942 added it (https://github.com/buildkite/terminal/pull/50#issuecomment-406984426)
TheDen
commented
6 years ago Haha yeah It's not you, I ditched using the hacky bytes.Replace for that nice go package (I didn't want to add a new dependency in a public PR). I noticed you guys switched to govendor in a commit so I used it (I'm new to govendor but I've migrated myself).
keithpitt
commented
5 years ago @TheDen thanks for taking the time to PR this up! Having done a proper look through the savings, I'm sorry to say I don't think they warrant the additional code changes.
For the most part, --preview is only really meant to be a "preview" of the output, with some recommended CSS that we provide, however it's by no means meant to be a production quality version of the HTML (i.e you probably don't want to be embedding the CSS into the HTML anyway, we just do it because it's easy)
If you run the homer fixture without --preview, you get this:
<div class="term-container"><span class="term-fg33"> ___ _____</span>
<span class="term-fg33"> .'/,-Y" "~-.</span>
<span class="term-fg33"> l.Y ^.</span>
<span class="term-fg33"> /\ _\_</span>
<span class="term-fg33">i ___/" "\</span>
<span class="term-fg33">| /" "\ o !</span>
<span class="term-fg33">l ] o !__./</span>
<span class="term-fg33"> \ _ _ \.___./ "~\</span>
<span class="term-fg33"> X \/ \ ___./</span>
<span class="term-fg33"> ( \ ___. _..--~~" ~`-.</span>
<span class="term-fg33"> ` Z,-- / \</span>
<span class="term-fg33"> \__. ( / ______)</span>
<span class="term-fg33"> \ l /-----~~" /</span>
<span class="term-fg33"> Y \ /</span>
<span class="term-fg33"> | "x______.^</span>
<span class="term-fg33"> | \</span>
<span class="term-fg33"> j Y</span>
<span class="term-fg33"> ->Homer<-</span></div>This is what we consume in a production environment and render it into the page. The CSS is already loaded and cached by the browser at that point, so there's no real gain from minification. Also - because whitespace is important, I don't think there's many savings that can be made to minifying the actual non-preview output.
Having said that, we could totally reduce the file size by renaming term-fg33 to just fg33 - but then that becomes a little opaque. And maybe there's a different HTML tag we can use that's not a span that's smaller in chars? Even though i is meant italic text, the semantic meaning is:
The HTML
<i>element represents a range of text that is set off from the normal text for some reason. Some examples include technical terms, foreign language phrases, or fictional character thoughts. It is typically displayed in italic type.
I'd happily accept a PR that made terminal-to-html configurable in that regard: --html-tag-name "i" --html-class-prefix "t"
Anyway, in your scenario @TheDen, I'd recommend instead of using --preview, perhaps have a script like this:
#!/bin/bash
echo '<!DOCTYPE html><html><head><title>Terminal to HTML Party!</title><link rel="stylesheet" href="/terminal.css" /></head><body>'
cat fixtures/homer.sh.raw | terminal-to-html
echo '</body></html>'Which outputs like this:
<!DOCTYPE html><html><head><title>Terminal to HTML Party!</title><link rel="stylesheet" href="/terminal.css" /></head><body>
<span class="term-fg33"> ___ _____</span>
<span class="term-fg33"> .'/,-Y" "~-.</span>
<span class="term-fg33"> l.Y ^.</span>
<span class="term-fg33"> /\ _\_</span>
<span class="term-fg33">i ___/" "\</span>
<span class="term-fg33">| /" "\ o !</span>
<span class="term-fg33">l ] o !__./</span>
<span class="term-fg33"> \ _ _ \.___./ "~\</span>
<span class="term-fg33"> X \/ \ ___./</span>
<span class="term-fg33"> ( \ ___. _..--~~" ~`-.</span>
<span class="term-fg33"> ` Z,-- / \</span>
<span class="term-fg33"> \__. ( / ______)</span>
<span class="term-fg33"> \ l /-----~~" /</span>
<span class="term-fg33"> Y \ /</span>
<span class="term-fg33"> | "x______.^</span>
<span class="term-fg33"> | \</span>
<span class="term-fg33"> j Y</span>
<span class="term-fg33"> ->Homer<-</span></body></html>That clocks in at 1240 bytes, whereas your minified version is 8201 bytes. I think that's probably a win!
keithpitt
commented
5 years ago <!DOCTYPE html><html><head><title>Terminal to HTML Party!</title><link rel="stylesheet" href="/terminal.css" /></head><body>
<i class="fg33"> ___ _____</i>
<i class="fg33"> .'/,-Y" "~-.</i>
<i class="fg33"> l.Y ^.</i>
<i class="fg33"> /\ _\_</i>
<i class="fg33">i ___/" "\</i>
<i class="fg33">| /" "\ o !</i>
<i class="fg33">l ] o !__./</i>
<i class="fg33"> \ _ _ \.___./ "~\</i>
<i class="fg33"> X \/ \ ___./</i>
<i class="fg33"> ( \ ___. _..--~~" ~`-.</i>
<i class="fg33"> ` Z,-- / \</i>
<i class="fg33"> \__. ( / ______)</i>
<i class="fg33"> \ l /-----~~" /</i>
<i class="fg33"> Y \ /</i>
<i class="fg33"> | "x______.^</i>
<i class="fg33"> | \</i>
<i class="fg33"> j Y</i>
<i class="fg33"> ->Homer<-</i></body></html>Ended up being 1042 bytes, only 198 saved (boo)
Added an option to minify the
HTMLheader andCSSin the--previewmode