-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Add support for Graphviz diagrams #2052
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
Conversation
layouts/_default/baseof.html
Outdated
<script type="text/javascript"> | ||
(() => { | ||
let vizInstance | ||
[...document.querySelectorAll("pre[class=graphviz]")].forEach(async (x) => { | ||
if (!vizInstance) vizInstance = await Viz.instance() | ||
const options = { | ||
graphAttributes: { | ||
bgcolor: "transparent", | ||
}, | ||
engine: x.getAttribute("engine") || "dot", | ||
} | ||
const svg = vizInstance.renderSVGElement(x.innerText, options) | ||
svg.classList.add("graphviz") | ||
x.parentNode.insertBefore(svg, x); | ||
x.style.display = 'none' | ||
}); | ||
})(); | ||
</script> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I guess there's a point to be made about moving this logic to application.js
and only keep a call to Graphviz.render()
here, right? I'll do that real quick.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
02adff6
to
a3d3ba3
Compare
assets/sass/dark-mode.css
Outdated
@@ -152,6 +152,10 @@ | |||
#l10n-versions-dropdown footer a { | |||
color: #6969dd; | |||
} | |||
|
|||
svg.graphviz { | |||
filter: invert(60%); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I saw that, and changed it from 60% to 67%, maybe you also like that better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
content/about/distributed.html
Outdated
edge [fontname="Helvetica", penwidth=3]; | ||
|
||
// Nodes | ||
blessed_repo [label="blessed\nrepository", fillcolor=teal, fontcolor=white, pos="6,4!"]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe match the design of the existing diagrams a bit more closely? For example the "blessed repository" node used to be very big and now it's small. Also I think I liked the gray arrows better with the smaller arrowheads, the black is a bit harsh and draws attention to the arrows instead of the nodes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I can see what I can do. But as you probably noticed, I am much better with Node.JS trickery than with graphical design ;-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
totally! personally I kind of enjoy trying to copy an existing design as closely as possible as a way to try to improve my marginal design skills, maybe I'll give it a shot later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems that adding margin="0.3"
in the blessed_repo
nodes' attributes does the trick.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I liked the gray arrows better with the smaller arrowheads, the black is a bit harsh and draws attention to the arrows instead of the nodes.
Oy, I had forgotten to address this valuable feedback. Pushed now!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a3d3ba3
to
cf7fe87
Compare
This is awesome! I noticed the images lost their alt text, even if it was pretty minimal to begin with. Should that be an option to the |
While Mermaid diagrams are more and more common on the internet, thanks to GitHub's support in their GitHub-flavored Markdown codeblocks, it is often frustratingly difficult to make elegant diagrams with them and to avoid overlapping or unnecessarily long edges. Graphviz, in contrast, allows much more control over the layout. The downside is that Graphviz is a command-line program, not a JavaScript library, and would require the diagrams to be pre-rendered and committed into the repository (a recipe for out-of-sync files). Enter `viz.js` (https://github.com/mdaines/viz-js). Essentially, it is a WebAssembly build of Graphviz. Therefore, it _does_ run in the browser. The downside, compared to Mermaid, is the size: While `mermaid.min.js` weighs around 25 kB, `viz-global.js` weighs around 1.4 MB. And I did not find any CDN that serves a current version, therefore I had to download it: VIZ_VERSION=3.17.0 npm pack @viz-js/viz@$VIZ_VERSION tar Oxvf viz-js-viz-$VIZ_VERSION.tgz package/dist/viz-global.js \ >static/js/viz-global.js With this commit, Graphviz dot diagrams can be added like this: {{< graphviz >}} digraph { a -> b -> c } {{< /graphviz >}} While I tried to make it as painless to work on these diagrams locally, there are more efficient ways than editing the HTML files, running Hugo and then reloading the page, for example Graphviz Online: https://dreampuf.github.io/GraphvizOnline/?engine=neato. This page works entirely in the browser, and uses a slightly outdated version of `viz.js`. Signed-off-by: Johannes Schindelin <[email protected]>
There are some issues when adding SVGs inside <svg> HTML elements, e.g. that positioning can be harder than with <img> ones, as pointed out at git#2052 (comment). Let's use <img> elements instead, passing the SVG via data URLs. There are more benefits to that: For example, most modern browsers allow copying an image into the clipboard that is specified as an `<img>` element, but not `<svg>` ones. Likewise, the "Open Image in New Tab" functionality typically only works with the former but not with the latter. Signed-off-by: Johannes Schindelin <[email protected]>
No need to set a background color, is there? Signed-off-by: Johannes Schindelin <[email protected]>
This can be done like so: {{< graphviz engine="neato" >}} Signed-off-by: Johannes Schindelin <[email protected]>
Range-diff
|
cf7fe87
to
3f412a7
Compare
This may also be interesting for Progit indeed. |
@jnavila but ProGit2 uses AsciiDoc, where I would not really know how to integrate the Graphviz step. But seeing as AsciiDoc needs to be run "server-side", anyway, Graphviz diagrams should be much easier to integrate into that process than they were here, with a Hugo-based process (I did spend 3 dozen hours on this). The major caveat I'd like to call out is that git-scm.com post-processes the AsciiDoc sources from ProGit2 in a quite "home-grown" way, via a Ruby script, and any changes you make in ProGit2 would require changes to that script, too. Unless you integrate Graphviz in a way that the AsciiDoc refers already to the Graphviz output, in which case nothing really changes for downstream users like git-scm.com. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dscho This is nice work. To be honest, I like the graphs more than the original. They are more consistent.
I've added some thoughts and questions.
The WebAssembly version of Graphviz is not exactly light (1.4 megabytes at the time of writing). And it wasteful to let everybody re-render the SVG client-side over and over again even though the diagram's definition hasn't changed. So let's add a script that performs that rendering "on the server side" (more precisely: during deployment). This script takes no arguments and post-processes the output of Hugo in `public/`. For performance reasons, it requires the list of files that contain Graphviz diagrams. With other sites, it might not matter much because there might only be a handful pages. However, on git-scm.com there _are_ thousands of files that do not contain any Graphviz diagrams, and therefore it is a necessary optimization to process only the files that _do_ contain Graphviz diagrams. To get that list, a new layout and page are added that Hugo processes, identifying said list of files and writing the result to `public/diagram-list.html`. Since this script runs via Node.JS and therefore lacks the convenient built-in HTML parser of browser-based JavaScript engines, a prerequisite is to run `npm install` so that the `node-html-parser` package is available. A note about the slightly inelegant way the `graphviz` class is added to the generated SVGs: Sadly, `node-html-parser` is not smart enough to handle `element.classList.add(...)` followed by `element.toString()` correctly, and as a result the output would _not_ have the added class. But there's always crude string processing to save the day. Signed-off-by: Johannes Schindelin <[email protected]>
Since we're generating `<img>` elements anyway, might just as well keep that accessibility support. Signed-off-by: Johannes Schindelin <[email protected]>
This avoids the 1.4MB download on the client-side, in favor of inlined, small SVG elements. Signed-off-by: Johannes Schindelin <[email protected]>
This topic corresponds to git#2052. Signed-off-by: Johannes Schindelin <[email protected]>
3f412a7
to
884b895
Compare
Range-diff
|
This topic corresponds to git#2052. Signed-off-by: Johannes Schindelin <[email protected]>
884b895
to
a2c59d6
Compare
I wanted to reply to this earlier, to say that I implemented this in 66d59f7 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dscho Tiny comment on the last commit message. From my point of view it's ready to ship.
@@ -27,22 +27,101 @@ <h4>Subversion-Style Workflow</h4> | |||
A centralized workflow is very common, especially from people transitioning from a centralized system. Git will not allow you to push if someone has pushed since the last time you fetched, so a centralized model where all developers push to the same server works just fine. | |||
</p> | |||
<p class="center"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the commit message:
With this change, the text in those diagrams is copy/paste-able.
@dscho That no longer seems to be the case now <svg>
is no longer is inlined. It's a nitpick on the commit message, I don't consider this an issue that it's not the case. Do you think it worthy to edit the commit message?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True. I force-pushed a fix:
Range-diff
-
1: a2c59d6 ! 1: 352dbbf Replace the diagrams in
about/distributed
with Graphviz versions@@ Commit message Replace the diagrams in `about/distributed` with Graphviz versions This commit recreates those diagrams that have been provided as `.png` - files before. - - With this change, the text in those diagrams is copy/paste-able. But of - course that's not the real reason I made this change, the real reason is + files before. Of course, this is not strictly necessary because the + previous `.png` diagrams served their purpose well enough. But I wanted to show off that we can now use Graphviz diagrams on git-scm.com. + Note: When developing this patch, I had considered inlining the SVGs as + `<svg>` instead of `<img>`. That would have had the following + advantages: + + - The text in those diagrams would have been copy/paste-able. + + - We could have defined explicit colors for dark mode, as the `<style>` + element would have been able to refer to the HTML document's `:root` + element's dark mode definitions. + + However, having SVGs embedded in `<img>` elements have a few benefits: + + - Current browsers allow to `Open Image in New Tab` (where the text + would be copy/paste-able again). + + - Current browsers allow to `Save Image` (even if there is no way to + specify a filename, see https://github.com/whatwg/html/issues/2722). + + - By using `<img>` elements, we can reuse the previous work on images in + dark mode. + Signed-off-by: Johannes Schindelin <[email protected]> ## content/about/distributed.html ##
This commit recreates those diagrams that have been provided as `.png` files before. Of course, this is not strictly necessary because the previous `.png` diagrams served their purpose well enough. But I wanted to show off that we can now use Graphviz diagrams on git-scm.com. Note: When developing this patch, I had considered inlining the SVGs as `<svg>` instead of `<img>`. That would have had the following advantages: - The text in those diagrams would have been copy/paste-able. - We could have defined explicit colors for dark mode, as the `<style>` element would have been able to refer to the HTML document's `:root` element's dark mode definitions. However, having SVGs embedded in `<img>` elements have a few benefits: - Current browsers allow to `Open Image in New Tab` (where the text would be copy/paste-able again). - Current browsers allow to `Save Image` (even if there is no way to specify a filename, see whatwg/html#2722). - By using `<img>` elements, we can reuse the previous work on images in dark mode. Signed-off-by: Johannes Schindelin <[email protected]>
This topic corresponds to git#2052. Signed-off-by: Johannes Schindelin <[email protected]>
a2c59d6
to
352dbbf
Compare
Changes
.png
s in https://git-scm.com/about/distributed with scalable versions.Context
Over in #2049, we discussed various options how to add elegant diagrams, and settled on Graphviz ones. I broke this out so that I don't have to derail the discussion about the cheat sheet anymore.
The solution presented here uses
viz.js
, a WebAssembly version of Graphviz. Since it weighs a bit much (1.4MB), this is used client-side only when developing the page locally; When the site gets deployed, the SVGs are "pre-rendered", i.e. generated and inserted in place of the<pre class="graphviz">
blocks.To demonstrate this new feature, I replaced the diagrams in https://git-scm.com/about/distributed with Graphviz ones:
The changes of this PR can also be seen in action in my fork: https://dscho.github.io/git-scm.com/about/distributed