asciidoctor / asciidoctor-kroki

Asciidoctor.js extension to convert diagrams to images using Kroki!
https://kroki.io/
MIT License
147 stars 50 forks source link

The PlantUML preprocessor should be executed when the diagram type is `c4plantuml` #126

Closed ggrossetie closed 3 years ago

ggrossetie commented 4 years ago

In this case, we should not try to resolve the following targets:

In fact, the above files will be resolved server-side. I think we should not resolve them only when the path is local:

mwabik commented 3 years ago

Could it be changed to be more generic than just for c4?

We have issue when pre-processing https://github.com/tmorin/plantuml-libs/tree/master/cloud inclusion in local mode:

!global $INCLUSION_MODE="local"
!global $LIB_BASE_LOCATION="./cloud/"
!include ./cloud/library.puml

However, our internal Kroki server handles that just fine.

ggrossetie commented 3 years ago

We already do that for remote files but not for local files:

https://github.com/Mogztter/asciidoctor-kroki/blob/762eb16ce99607b89ad02af01c1183e89cee244c/src/preprocess.js#L182-L184

The reasoning was that local files should be resolved locally. If I understand correctly the file ./cloud/library.puml is not available locally (i.e. only available on the Kroki server)?

We could either add an option or ignore the error if a local file cannot be resolved.

mwabik commented 3 years ago

Yes, those files only exist on the Kroki server. And they are not resolvable locally. Using same logic as for remote files pre-processing logic would solve it.

anb0s commented 3 years ago

Related to #147 ? If configured to ignore local include errors, this will work... or better as configuration for exclude-patterns?

ggrossetie commented 3 years ago

or better as configuration for exclude-patterns?

That's a good idea and probably more flexible than kroki-plantuml-unresolved-local-include

ggrossetie commented 3 years ago

Yes, those files only exist on the Kroki server. And they are not resolvable locally. Using same logic as for remote files pre-processing logic would solve it.

For reference, the issue described by @mwabik was fixed in fe014dfa190efc0a64ef26b12627a2f009008257