search

asciidoctor / docker-asciidoctor

:ship: A Docker image for using the Asciidoctor toolchain to process AsciiDoc content
https://hub.docker.com/r/asciidoctor/docker-asciidoctor/
Other
321 stars 151 forks source link

kroki support? #191

Closed mskyaxl closed 2 years ago

mskyaxl commented 3 years ago

Hi, have you considered adding asciidoctor-kroki in this container? https://github.com/Mogztter/asciidoctor-kroki

mojavelinux commented 3 years ago

I consider Kroki to be a partner community. Therefore, I think it's reasonable for it to be included in the container (assuming it doesn't cause too much weight). Though we are still facing the problem that this container is growing without bounds, so layers are something we really need as the number of projects people want to include increases.

SilentButeo2 commented 2 years ago

I don't know how Kroki matches up with the current implemented Asciidoctor Diagram. What I can say, is that although Asciidoctor Diagram supports a bunch of extensions, it seems that not all of them (or maybe most of them) are not working in this docker container.

Currently I have a document to convert where multiple diagram processor [svgbob][wavedrom][mermaid][plantuml]. And its only the [plantuml] that currently works.

So I think that Kroki would fix that part. Asciidoctor Diagram could be removed? Or don't include Kroki, but make Asciidoctor Diagram implementation work for all diagram processors?

dduportal commented 2 years ago

Hello @SilentButeo2 @mskyaxl , would you be interested in helping us by proposing a PR to install kroki in the container (+ add a test in the test harness with a minimalist example)? If you are interested but need help/mentoring I can be there for you.

mskyaxl commented 2 years ago

i have already a branch that does this, I can update it(if needed) and create a PR :)

dduportal commented 2 years ago

@mskyaxl that would be awesome! We'll check the size differences to ensure the new image is not bloated, but that could be great

mojavelinux commented 2 years ago

I would be supportive of adding Asciidoctor Kroki to the image, but not removing Asciidoctor Diagram. I don't want to create a situation where we put two projects in this ecosystem against each other. Asciidoctor Diagram and Kroki take different approaches to handling the work of diagram generation. The benefit of Asciidoctor Diagram is that it works completely locally. Asciidoctor Kroki offloads the work of generating the diagram to another server (which is why it's easier to support more diagram types without adding software). And since the provider is remote, adding the extension should be very lightweight.

If we get to a point where we offer several different images, I could see having one image that uses Asciidoctor Diagram and all its diagram tool dependencies and another image having Asciidoctor Kroki to keep the image more lean.

SilentButeo2 commented 2 years ago

The reason I only would like to have Asciidoctor Kroki support, is that the current Asciidoctor Diagram doesn't seem to support [svgbob][wavedrom][mermaid] (and maybe others also).

So support for Asciidoctor Kroki is as such not mandatory IMHO.

mojavelinux commented 2 years ago

Asciidoctor Diagram supports those formats (see https://docs.asciidoctor.org/diagram-extension/latest/#creating-a-diagram), they just may not be fully enabled in this particular image. Each diagramming tool adds bulk to the image, so choices have been made to support only the most popular ones, or ones that people have requested specifically. (See #109 for more context).

mskyaxl commented 2 years ago

I created the PR #226.

dduportal commented 2 years ago

Implemented in #226 and released in https://github.com/asciidoctor/docker-asciidoctor/releases/tag/1.13.0