asciidoctor / asciidoctor-kroki

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

Allow to configure the criticality when an include is missing #147

Open ggrossetie opened 4 years ago

ggrossetie commented 4 years ago

We might want to introduce an attribute to control the criticality when an include is missing:

kroki-plantuml-unresolved-local-include

We could do the same for remote include. The default value of local include would be error and the default value for remote include would be warn.

Follow-up #141

scphantm commented 3 years ago

This just bit me again. the fix of putting in

<c4/C4_Context.puml>

compiles correctly, as mentioned in my bug (https://github.com/Mogztter/asciidoctor-kroki/issues/49). but, the

##[error]Skipping preprocessing of PlantUML standard library include file '<c4/C4_Context.puml>'

line is dumped into stderr which my new build pipeline automatically assumes is an error. I am going to have to get creative with grep or something so i can filter out these errors vs actual errors in my pipeline configs.

just mentioning because it just ground me to a halt for the moment.

kirchsth commented 3 years ago

Would it be possible that (asciidoctor-)kroki support a special comment in the PlantUML diagram itself? Eg. if a line starts with 'optional include then no error is generated if the following include cannot be resolved.

This could solve the problem with https://github.com/plantuml-stdlib/C4-PlantUML/issues/156 if I would add a line like

' convert it with additional command line argument -DRELATIVE_INCLUDE="." to use locally
!if %variable_exists("RELATIVE_INCLUDE")
  ' optional include: Kroki and Antora can ignore the not supported calculated !include (no command line argument -D is supported at all) 
  !include %get_variable_value("RELATIVE_INCLUDE")/C4_Context.puml
!else
  !include https://raw.githubusercontent.com/kirchsth/C4-PlantUML/extended/C4_Context.puml
!endif

BR Helmut

ggrossetie commented 3 years ago

This just bit me again. the fix of putting in <c4/C4_Context.puml> compiles correctly, as mentioned in my bug (#49). but, the (...) line is dumped into stderr which my new build pipeline automatically assumes is an error. I am going to have to get creative with grep or something so i can filter out these errors vs actual errors in my pipeline configs.

Indeed, console.warn is an alias of console.error.

https://github.com/Mogztter/asciidoctor-kroki/blob/12f6970a56695f6b513260b810b91ee7e7193993/src/preprocess.js#L219

In this case, I think we should use console.log to print this message to stdout (instead of stderr). Arguably, we could also just suppress this message (by default).

Would it be possible that (asciidoctor-)kroki support a special comment in the PlantUML diagram itself? Eg. if a line starts with 'optional include then no error is generated if the following include cannot be resolved.

I'm not sure... maybe... 🤔

lind commented 2 years ago

console.log instead of error would be nice to be able to filter messages like this using Antora level-key

Skipping preprocessing of PlantUML include, because reading the referenced local file 'C4_Context.puml' caused an error:
Error: ENOENT: no such file or directory, open 'C4_Context.puml'

But maybe I'm missing something? These error messages does not format like the other Antora log messages:

[13:12:33.521] WARN (asciidoctor): unterminated listing block
    file: modules/ROOT/pages/information-products/power-redispatch.adoc:192
    source: ... (refname: main)

Maybe something more has to be done to send the logs through Antora logger?

ggrossetie commented 2 years ago

We are using console (https://developer.mozilla.org/en-US/docs/Web/API/Console) whereas Antora is using a proper logger called pino.

I also think that some logs are too pedantic... It's hard to find the right balance 😬

ggrossetie commented 2 years ago

I've decided to reduce the severity from warn to info. I think we should manage the criticality using the logger level.

lind commented 2 years ago

I've decided to reduce the severity from warn to info. I think we should manage the criticality using the logger level.

Seem like a good solution in https://github.com/Mogztter/asciidoctor-kroki/pull/324 👍