asciidoctor / asciidoctor-diagram

:left_right_arrow: Asciidoctor diagram extension, with support for AsciiToSVG, BlockDiag (BlockDiag, SeqDiag, ActDiag, NwDiag), Ditaa, Erd, GraphViz, Mermaid, Msc, PlantUML, Shaape, SvgBob, Syntrax, UMLet, Vega, Vega-Lite and WaveDrom.
http://asciidoctor.org
MIT License
442 stars 107 forks source link

Consider adding support for bytefield-svg #276

Closed brunchboy closed 4 years ago

brunchboy commented 4 years ago

As part of a popular reverse-engineering project I needed to create some documents with extensive and complex byte field diagrams, and the only tool that had all the features I needed was the LaTeX bytefield package, which I used with success for a few years. But some of the other LaTeX packages I was using went off support, and I got tired of only having PDF as my output, so I decided to upgrade my documents to Antora sites written in Asciidoc. To enable that, I had to write my own update of bytefield, which I have called bytefield-svg.

It’s a node module, and I’m using it as an Antora extension. But @Mogztter suggested I add a CLI to make it easy to drive from asciidoctor-diagram, and let you know about it, in case you are interested in adding it as another available diagram type. Hopefully this will now be an easy task, and I am more than happy to answer any questions you might have about it if my documentation has gaps.

pepijnve commented 4 years ago

@brunchboy I had a go at making the integration already (see commit above). Test still fails with an internal error in bytefield-svg. Not really obvious what's wrong from the minified stack trace and error message.

/usr/local/lib/node_modules/bytefield-svg/lib.js:1261
function S3(a,b){return T3(a,b,null,null)}function T3(a,b,c,d){var e=ut.b(d,uZ(a));a=vI.b(d,tZ(a));throw On.b([p.a(b)," [at line ",p.a(a),", column ",p.a(e),"]"].join(""),ll.f(he([new h(null,3,[ds,oU,vI,a,ut,e],null),c])));}
                                                                                                   ^
Nn [Error]: The map literal starting with :span contains 5 form(s). Map literals must contain an even number of forms. [at line 16, column 50]
    at new Nn (/usr/local/lib/node_modules/bytefield-svg/lib.js:647:184)
    at Function.On.c (/usr/local/lib/node_modules/bytefield-svg/lib.js:649:71)
    at Function.On.b (/usr/local/lib/node_modules/bytefield-svg/lib.js:649:30)
    at T3 (/usr/local/lib/node_modules/bytefield-svg/lib.js:1261:109)
    at c4 (/usr/local/lib/node_modules/bytefield-svg/lib.js:1272:69)
    at e4 (/usr/local/lib/node_modules/bytefield-svg/lib.js:1276:205)
    at V3 (/usr/local/lib/node_modules/bytefield-svg/lib.js:1279:265)
    at U3 (/usr/local/lib/node_modules/bytefield-svg/lib.js:1262:166)
    at e4 (/usr/local/lib/node_modules/bytefield-svg/lib.js:1275:193)
    at V3 (/usr/local/lib/node_modules/bytefield-svg/lib.js:1279:265) {
  message: 'The map literal starting with :span contains 5 form(s). Map literals must contain an even number of forms. [at line 16, column 50]',
  data: h {
    v: null,
    A: 3,
    l: [ [y], [y], [y], 16, [y], 50 ],
    w: null,
    o: 16647951,
    F: 139268
brunchboy commented 4 years ago

Interesting! I will add a comment in the commit where the error is being reported, and see if that can help us understand what is happening.

pepijnve commented 4 years ago

🤦‍♂ I'm guessing Ruby string interpolation is kicking in since I embedded the test diagram source in the spec Ruby file...

brunchboy commented 4 years ago

That is consistent with the comment I just added. 😄

pepijnve commented 4 years ago

And presto, everything works! Thanks for pointing out what should have been obvious. RubyMine was even highlighting the problem.

Anyway, with that last commit you're welcome to kick the tires a bit. I'll update the README already in the meantime.

brunchboy commented 4 years ago

Sweet! I won’t have a chance to try this out until this evening, but thanks so much for your fast response.

brunchboy commented 4 years ago

All right, it seems to work! Since I haven’t been using Ruby for a decade or so it took me a while to figure out how to build and run it from source, but I was successful. The only problem I noticed is that the link in the README seems to be incorrectly formatted (what should be an opening square bracket is an opening curly brace). It is currently:

{uri-bytefield}{bytefield]

And it seems it should be:

{uri-bytefield}[bytefield]

I can certainly redo that note as a pull request if you like!

brunchboy commented 4 years ago

I did submit a PR against the bytefield branch with this tweak. I don’t know why it is not showing up in this discussion, perhaps because it is against a non-master branch?

In any case, I am glad you haven’t released this yet because I added a bunch of nice new features over the past week, with the help of my enthusiastic first user of the diagram generator.

pepijnve commented 4 years ago

@brunchboy odd indeed that the PR didn't show up. It's merged.

As you mentioned in the PR description, asciidoctor-diagram doesn't really depend on a specific version of the external tools (PlantUML and Ditaa being the exceptions since those are bundled). It just digs through $PATH looking for a binary with the right name. I've dealt with changes to CLI interfaces of various diagram rendering tools by first querying the version number (e.g., run bytefield-svg --version first) and then added branches in the code to generate the appropriate arguments.

What needs to be done in the diagram extension as you add options is to add logic that reads block attributes and converts them to the correct CLI arguments. See https://github.com/asciidoctor/asciidoctor-diagram/blob/master/lib/asciidoctor-diagram/syntrax/converter.rb for instance for a simple example.

brunchboy commented 4 years ago

I don’t think there are any arguments that require you to make them available as block attributes yet. I’ll try to let you know if that ever happens.

brunchboy commented 4 years ago

The arguments are there so that people who want to use it in different contexts (e.g. as an Asciidoctor.js extension, where the SVG should always be rendered as inline HTML) can do that. But configuration of diagrams from within the Asciidoc source is done by Clojure code, not block attributes.