Add Cacoo support

[hnakamur]

Hi, I added the Cacoo support to the asciidoctor-diagram.
Please refer README.adoc for the usage.

I confirmed this pull request works successfully.
Could you review this?
Thanks.

[pepijnve]

No objections to merging this. The only thing I'm wondering is how incremental rendering of the asciidoc document should work for this type of diagram. At the moment asciidoctor-diagram only recreates images when the source code changes. In this case the source code is the diagram id, but that doesn't change when the diagram is updated on the web site. We could query the update timestamp via the cacoo api but that means an additional roundtrip to the server for each image. Alternatively we could just shortcut the conditional image rendering in this case, but that's a bit wasteful on bandwidth (and perhaps server processing work). Any thoughts on how you would like this to behave?
@mojavelinux any opinion on your part? I'm asking since you're keep a close eye on the performance of asciidoctor.

[mojavelinux]

In general, I like the idea of providing more options. However, I'm not sure how I feel yet about integrating proprietary services directly into Asciidoctor Diagram core. I think this is a case where we should draw the line and introduce a dedicated gem. The gem could depend on Asciidoctor Diagram, so you could still load it with a single commandline flag. But it allows these service integration to have their own space. It's really not that difficult to setup and publish a new gem, so we're not talking about a lot of extra work.

I propose the name of the gem to be: asciidoctor-diagram-cacoo. You could load it using:

$ asciidoctor -r asciidoctor-diagram-cacoo sample.adoc

As a side note, I think we should do the memegenerator.net integration using a similar separation.

As for the cache question, I think for now we should simply require that the .cache file be deleted to force an update. We could also consider performing an "update to date" check either at a fixed interval (1 hour, 1 day, etc) or a flag to force update to date checks. Something like -a update-diagrams.

[pepijnve]

memegenerator integration + deck.js backend makes for some nice slide decks :)

Agreed on drawing the line at integrating proprietary web services; that makes sense. That means I'm going to have to start thinking about public API, semantic versioning, etc... Life's so simple when you're the only API user.

So are you proposing to have an asciidoctor-diagram-cacoo gem under the asciidoctor umbrella or is this something @hnakamur should do instead?

[mojavelinux]

memegenerator integration + deck.js backend makes for some nice slide decks :)

Hell yeah! You can find the discussion about it in the asciidoctor-extensions-lab project: asciidoctor/asciidoctor-extensions-lab#8

[mojavelinux]

So are you proposing to have an asciidoctor-diagram-cacoo gem under the asciidoctor umbrella or is this something @hnakamur should do instead?

I think all new projects should incubate in the creators own namespace, then we migrate them into Asciidoctor when we get buy-in from the group. This gives equal opportunity to all and allows the group to be part of the process.

[hnakamur]

Thank you all for comments. I created https://github.com/hnakamur/asciidoctor-diagram-cacoo
Right now it always calls Cacoo APIs and creates PNG files.

As for image cache, I would like to write like:

["Cacoo", "cacoo-${my_cacoo_diagram_id_here}"]

and let asciidoctor-diagram-cacoo create cacoo-${my_cacoo_diagram_id_here}.png and cacoo-${my_cacoo_diagram_id_here}.png.cache.
In this way, you can remove the image cache file of the outdated diagrams easily.

Could you tell me how can I get the filename "cacoo-${my_cacoo_diagram_id_here}" above?

After that, I would like to create a rake task to delete the outdated image cache files using
https://cacoo.com/lang/en/api_diagrams API. I get the updated timestamps with this API and compare them to mtime of the image cache files.

[hnakamur]

I read the asciidoctor-diagram source code and found the target filename is set to a local variable at:

        target = attributes.delete('target')

However, target is not passed to the generator.

          result = generator_info[:generator].call(source.code, parent)

Can we modify the line like the below?

          result = generator_info[:generator].call(source.code, parent, target)

With this change, I can write like

["cacoo", "cacoo-CyAliDWeasJjVyv8"]

instead of

["cacoo", "cacoo-CyAliDWeasJjVyv8"]
----------------
CyAliDWeasJjVyv8
----------------

The former way is much better since I do not have to repeat the cacoo diagram ID both in the target filename and in the source code.

[pepijnve]

You can go even more concise by using the block macro syntax. That could take the form

cacoo::CyAliDWeasJjVyv8[]

in this syntax the bit between :: and [] is the value for the target attribute.

In order to support this I think I'm going to need to add some additional hooks in the diagram base class. One of the things I'm thinking of exposing is a method that you can override that determines the target filename. The default would be the current behavior (value of target if set or hash of source code as fallback), but you could then override that to return "cacoo-#{target}" or something like that. I'm also thinking of making the logic that determines if an image needs to be regenerated overridable. The current behaviour will again be the default which you can then override to compare image mtime with update time from the cacoo api for instance. I'll have a look at making those changes this evening.

[hnakamur]

Thanks for considering those changes. It would be nice if the logic that checks images are outdated will be run once before each image generation code will be called. Since I would like to decrease Cacoo API call roundtrips, it would be nice that I can use https://cacoo.com/lang/en/api_diagrams API just once and check each Cacoo image in the document is outdated.

I am looking forward to see those changes. Thanks!

[pepijnve]

@hnakamur I've made some progress on defining a proper API that you can use. Could you have a look at https://github.com/pepijnve/asciidoctor-diagram/tree/afbc882cef8fbdc06f2504196c5fbac903800a37 to see if this is turning out to be usable for you.
I modified your Cacoo implementation for the restructured API at https://gist.github.com/pepijnve/2d4059db5d827462b5cd to give you an idea of how things will work.
@mojavelinux as always your opinion is also more than welcome :)

[hnakamur]

@pepijnve Thanks a lot for the update of asciidoctor-diagram and my Cacoo implementation!
I made a working version based on your gist and modified asciidoctor-diagram.
Could you have a look at https://github.com/hnakamur/asciidoctor-diagram-cacoo/commits/enhance_by_pepijnve ?
I added your name to Credits section of README. Is this OK with you?

I also created the example project at https://github.com/hnakamur/asciidoctor-diagram-cacoo-example
You can see the example output at http://hnakamur.github.io/asciidoctor-diagram-cacoo-example/sample.html

Right now, it does not check images are up to date. I will work on that form now.

Can I ask you a question? I had to add the code section like below between two ---- lines.

["cacoo", "f0MLos8tgXXxaTBv"]
----
f0MLos8tgXXxaTBv
----

In this way, CacooBlockProcessor#create_source is called.

However, when I tried an example without the code like:

["cacoo", "f0MLos8tgXXxaTBv"]

I suppose CacooBlockMacroProcessor#create_source will be called, but actually it was not called and no images were created.
In what circumstances CacooBlockMacroProcessor#create_source will be called?

One more thing. I looked the generated source code at http://hnakamur.github.io/asciidoctor-diagram-cacoo-example/sample.html and found the alt attribute of the img tag is set to the Cacoo diagram ID.

<img src="cacoo-f0MLos8tgXXxaTBv.png" alt="f0MLos8tgXXxaTBv" width="481" height="394">

However, I would like to set the alt text other than the diagram ID. Could you suggest a way to write the alt text in asciidoc files? Maybe something like:

["cacoo", "f0MLos8tgXXxaTBv", "png", "alt text"]

[mojavelinux]

I'm excited to see the progress on this issue! Thanks for setting up the new gem @hnakamur! I'll keep following along and chime in when ever I can help out!

[pepijnve]

The block macro syntax takes the general form

<macro>::<target>[<attributes>]

So to trigger your CacooBlockMacroProcessor you would have to write

cacoo::f0MLos8tgXXxaTBv[]

Regarding the <img/> alt attribute, any named attribute that you pass specify that doesn't have a particular meaning for the diagram processor should be getting passed through to the generated block. Asciidoctor-diagram generates image blocks which means you can use any attribute that applies to image blocks on a diagram block (or block macro). To get the alt attribute filled in write

cacoo::f0MLos8tgXXxaTBv["png", alt="alt text"]

[pepijnve]

I've reviewed the updated cacoo code. The only thing I would tweak is to change

class Source < API::FileSource

to

class Source < API::BasicSource

The cacoo Source class is overriding all of the methods from FileSource so it doesn't make sense to extend from it.

[hnakamur]

@pepijnve Thanks for your explanation and review! I updated my gem hnakamur/asciidoctor-diagram-cacoo and an example code hnakamur/asciidoctor-diagram-cacoo-example and an example page.
I'm looking forward for your work to be merged to https://github.com/asciidoctor/asciidoctor-diagram

@mojavelinux I'm excited to see the achievement on this issue too!

Thanks again both of you for your great support. It was very helpful.

[mojavelinux]

Thanks again both of you for your great support. It was very helpful.

That's what I love to hear!

[hnakamur]

@pepijnve Hi, I updated hnakamur/asciidoctor-diagram-cacoo following changes by Rename Asciidoctor::Diagram::API to Asciidoctor::Diagram::Extensions · e04ef75 · pepijnve/asciidoctor-diagram.

By the way, I created another asciidoctor-diagram extension for including diagrams created with D3.js.

• extension source: hnakamur/asciidoctor-diagram-d3js
• example source: hnakamur/asciidoctor-diagram-d3js-example
• example html: http://hnakamur.github.io/asciidoctor-diagram-d3js-example/sample.html

Could you review this?
Thanks.

[mojavelinux]

@hnakamur I posted some feedback on the list. http://discuss.asciidoctor.org/d3-js-integration-for-Asciidoctor-Diagram-td2259.html

[pepijnve]

Closing this PR since the code was moved to a separate project instead and there are no further issues to be resolved for the time being.