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
434 stars 106 forks source link

Kroki with plantuml and include directive #335

Closed achimgrimm closed 3 years ago

achimgrimm commented 3 years ago

I started to change my diagram generation from lokal to kroki as a external diagram server as explained in https://github.com/asciidoctor/asciidoctor-diagram/issues/226#issuecomment-761841009 as this is a great feature that really helps the build process.

The rendered plantuml diagram does not show the data from the include directive.

Here is my example puml.

@startuml
!include roles.puml

rectangle checkout {
  customer -- (checkout)
  (checkout) .> (payment) : include
  (help) .> (checkout) : extends
  (checkout) -- clerk
}
@enduml

This is the roles.puml file

 @startuml
left to right direction
skinparam packageStyle rectangle
skinparam monochrome true
actor customer
actor clerk
@enduml
pepijnve commented 3 years ago

The delegation to kroki isn't triggering the preprocessing logic yet that handles PlantUML !include statements. As a consequence the !include roles.puml is being passed to kroki as is and that obviously can't resolve that file. I'll have to reorganise the code a bit to handle this case.

pepijnve commented 3 years ago

If you would like a short term workaround, you might be able to use an asciidoc include instead.

achimgrimm commented 3 years ago

Thank you for the very fast feedback. :-)

I think the same applies to the config attribute on the asciidoc directive.

pepijnve commented 3 years ago

Kroki does not support parameters at all so I don't think there's much I can do about that.

pepijnve commented 3 years ago

I was looking at this issue this morning again and I'm not sure how easy it's going to be to fix this. I don't actually have logic in place that inlines the included diagram. Implementing that correctly would require reimplementing all the include logic of the PlantUML preprocessor which is, in my opinion, out of scope for this gem. My worry here is that I might end up chasing a moving target since the PlantUML syntax and features keep evolving.

achimgrimm commented 3 years ago

That is a valid point.

Maybe there is a way to trigger the PlantUML preprocessor using there API?

I am not sure how it is done, but the IntelliJ asciidoc Plugin (https://intellij-asciidoc-plugin.ahus1.de/docs/users-guide/features/preview/diagrams.html#kroki) somehow manages the includes.

pepijnve commented 3 years ago

Maybe there is a way to trigger the PlantUML preprocessor using there API?

That's a valid idea, but what's the benefit of using kroki then? If you want to run the PlantUML preprocessor you need a JVM and the PlantUML jars and then you might as well just run PlantUML locally.

pepijnve commented 3 years ago

I am not sure how it is done, but the IntelliJ asciidoc Plugin somehow manages the includes.

I'll have a look at the IntelliJ plugin code. @ahus1 maybe you can answer that question faster?

pepijnve commented 3 years ago

https://github.com/asciidoctor/asciidoctor-intellij-plugin/blob/1f9d5fac603b47cc1fb34cbd9fb6701a29cfb8e1/src/main/resources/kroki-extension.rb#L78

The config seems to get prepended. I'm not seeing any evidence of !include being handled though.

achimgrimm commented 3 years ago

what's the benefit of using kroki then

There are some local dependency on the system needed to render diagrams with PlantUML. Those are not needed if you use kroki. The local dependencies get even worth if you start using more diagram types (like BPMN).

achimgrimm commented 3 years ago

The config seems to get prepended. I'm not seeing any evidence of !include being handled though.

I will create a test setup and come back to you.

pepijnve commented 3 years ago

There are some local dependency on the system needed to render diagrams with PlantUML

Yes, I understand that. My point is that you need precisely those local dependencies to trigger the PlantUML preprocessor so that doesn't seem to solve much.

achimgrimm commented 3 years ago

OK, I see your point here. I would hope that the preprocessor does not need graphviz installation while parsing the diagram code.

achimgrimm commented 3 years ago

The config seems to get prepended. I'm not seeing any evidence of !include being handled though.

I will create a test setup and come back to you.

I can confirm that the plugin does not work with includes either.

ahus1 commented 3 years ago

Hi - it seems that I'm a bit late for this discussion.

The code of the plugin has moved and evolved a bit, see here: https://github.com/Mogztter/asciidoctor-kroki/blob/master/ruby/lib/asciidoctor/extensions/asciidoctor_kroki/extension.rb

Also in this evolved version, includes with the "!" at the start are not supported.

Looking closer, there is the attribute "kroki-plantuml-include" - I gave that one a try, but it has a bug; instead of '\n' it should be "\n" - otherwise the literal text is prepended. The next (pre-)release 0.32.9 will contain this fix. Follow https://github.com/asciidoctor/asciidoctor-intellij-plugin/issues/639 for details.

After the fix, and after removing all @startuml/@enduml elements, the following worked for me:

sample:

:kroki-plantuml-include: roles.puml

[plantuml]
----
rectangle checkout {
  customer -- (checkout)
  (checkout) .> (payment) : include
  (help) .> (checkout) : extends
  (checkout) -- clerk
}
----

roles.puml:

left to right direction
skinparam packageStyle rectangle
skinparam monochrome true
actor customer
actor clerk

image

pepijnve commented 3 years ago

I'm curious why that kroki-plantuml-include exists.

This code will do the same thing. You just have to make sure to omit all @enduml lines since that will terminate the processing prematurely in PlantUML otherwise.

[plantuml]
----
include::roles.puml[]

rectangle checkout {
  customer -- (checkout)
  (checkout) .> (payment) : include
  (help) .> (checkout) : extends
  (checkout) -- clerk
}
----

It's a bit of a frankenstein asciidoc/plantuml mix that way of course, but that is (and always has been) supported.

ahus1 commented 3 years ago

@pepijnve - if you a lot of images, and all with the same prefix, the kroki-plantuml-include will come in handy.

achimgrimm commented 3 years ago

@pepijnve @ahus1 Do you think it is feasible for PlantUML to offer an API that just resolves the !include and renders a "plain" PlantUML diagram?

If so, we could have a look at this. As this is a topic that seem to be valid for all tools around Kroki and PlantUML.

ahus1 commented 3 years ago

@achimgrimm - as the focus of PlantUML is to render the diagrams, I would be surprised if they would provide a preprocessor-only API. Kroki is all about not installing PlantUML and other clients in the first place.

If you are on the road for Kroki-only rendering, asciidoctor-kroki's Ruby module would be the way to go, as I see it as the most light-weight approach to this.

There is already a TODO in the Ruby code about plantuml include pre-processing; if someone would provide a PR, I don't think the maintainer would object. There is also the hint of "this behaves different than the JS version", and looking at the JavaScript version there is include processing included.

pepijnve commented 3 years ago

I had a quick look at the JS preprocessing code. I can make a quick port of that to Ruby. asciidoctor-diagram already preprocesses PlantUML code to turn relative include paths into absolute ones. I'll go ahead and replace that with actual inclusion logic instead. That will both improve error reporting for incorrect include paths (PlantUML's error reporting is pretty awful) and solve this problem in one go.

pepijnve commented 3 years ago

And it will also solve cache invalidation issues when using includes. Triple win! 😄

achimgrimm commented 3 years ago

You make my day. :)

pepijnve commented 3 years ago

I started looking into this today by reviewing the JS code and then trying to compare it with what PlantUML actually does. Unfortunately one glance at the PlantUML source code reminded how much of a tangled mess it is. I came to the conclusion that it's probably preferable to get PlantUML to do the preprocessing rather than trying to reimplement the logic in the exact same way.

PlantUML itself does not offer this, but I've been able to make a little wrapper around it (a slight variant of the existing wrapper that generates images) that returns the preprocessed code. In the resulting output all !includes have been inlined and this can then be passed on safely to Kroki (or some other server). I'm planning on continuing with this approach for now.

This strategy will only require a JVM to be installed to run the PlantUML code. GraphViz is not required.

achimgrimm commented 3 years ago

I think that is a big improvement. For me the JVM, and problaby others, is not such a big problem. As many languages require that anyway.

Thank you very much for the very quick response.

If you like, I can give you a hand and update the documentation with the kroki integration feature.

pepijnve commented 3 years ago

06baa8ed2c841c8a6e32ed785302f98febce676e should fix the include issue

achimgrimm commented 3 years ago

Hi @pepijnve, do you know a release date for the change yet?

pepijnve commented 3 years ago

This should be fixed in release 2.1.1 which I built today.