asciidoctor / asciidoctor-kroki

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

[Feature request] diagram options support #334

Closed yann-soubeyrand closed 1 year ago

yann-soubeyrand commented 2 years ago

Hello,

As discussed in #333, currently diagram options (https://docs.kroki.io/kroki/setup/diagram-options/) aren’t supported. A use case this support would help with is to be able to specify the view of a Structurizr diagram to render (currently, the rendered view is random).

ggrossetie commented 2 years ago

The main question is what naming convention should we use?

yann-soubeyrand commented 2 years ago

I personally don’t really like the first option, I prefer the second. Would it be possible to have something like

structurizr::partial$diagram.dsl[diagram-options="view-key=SystemLandscape,another-option=value"]

?

ggrossetie commented 2 years ago

Would it be possible to have something like (...)

That's another option. I feel like we are not using AsciiDoc with this syntax since we don't really use the attributes system. Some diagram options might expect a comma-separated list of flags/values. For instance, a font attribute where the value is the font name and the font size: font=Arial,11

We could workaround this "issue" by using an escape character \ or an another separator such as ;. Using an escape character might not work because it might be interpreted by Asciidoctor parser.

That's basically why I'm not a big fan of this solution because we are introducing our own syntax within an AsciiDoc attribute (and as a result, we will need a micro-parser).

I personally don’t really like the first option (...)

The first option aligns with Asciidoctor Diagram: https://docs.asciidoctor.org/diagram-extension/latest/#diagram-attributes May I ask why you don't like it?

(...) I prefer the second.

It could work but it's really verbose. Please note that we can't use options since it's already a named attribute in AsciiDoc: https://docs.asciidoctor.org/asciidoc/latest/attributes/options/

yann-soubeyrand commented 2 years ago

Would it be possible to have something like (...)

That's another option. I feel like we are not using AsciiDoc with this syntax since we don't really use the attributes system. Some diagram options might expect a comma-separated list of flags/values. For instance, a font attribute where the value is the font name and the font size: font=Arial,11

We could workaround this "issue" by using an escape character \ or an another separator such as ;. Using an escape character might not work because it might be interpreted by Asciidoctor parser.

That's basically why I'm not a big fan of this solution because we are introducing our own syntax within an AsciiDoc attribute (and as a result, we will need a micro-parser).

Yes, I agree with you.

I personally don’t really like the first option (...)

The first option aligns with Asciidoctor Diagram: https://docs.asciidoctor.org/diagram-extension/latest/#diagram-attributes May I ask why you don't like it?

I found the fact that we’ll had to either hardcode the list of possible arguments or to pass all the arguments including non diagram options not ideal. But if this is already done this way in Asciidoctor Diagram, maybe we should also go this way. Also, it seems that only the target and format atributes have to be filtered in their case. Would it be significantly difficult to filter out some attributes in our case?

(...) I prefer the second.

It could work but it's really verbose. Please note that we can't use options since it's already a named attribute in AsciiDoc: https://docs.asciidoctor.org/asciidoc/latest/attributes/options/

I may find the first option is the best in the end ;-)

ggrossetie commented 2 years ago

I found the fact that we’ll had to either hardcode the list of possible arguments or to pass all the arguments including non diagram options not ideal. But if this is already done this way in Asciidoctor Diagram, maybe we should also go this way. Also, it seems that only the target and format atributes have to be filtered in their case. Would it be significantly difficult to filter out some attributes in our case?

That's a good idea, it might indeed be easier to reverse the logic and filter built-in attributes 👍🏻

I may find the first option is the best in the end ;-)

Perfect 👌🏻

sixtysecrun commented 10 months ago

@ggrossetie I can see that you solved the problem of specifying view-key in #386 , but I don't understand how to use it 😞

Do you have an example of how I should specify view-key? I am trying the following ways 👇 , but none of it works. I am just served a random view.

[structurizr,diag1,svg,view-key=diag1]
----
include::diagram.dsl[]
----
structurizr::diagram.dsl[svg,view-key=diag2]
diagram.dsl

``` workspace { model { user = person "User" softwareSystem = softwareSystem "Software System" { webapp = container "Web Application" { user -> this "Uses!!!" } database = container "Database" { webapp -> this "Reads from and writes to" } } } views { systemContext softwareSystem "diag1" { include * autolayout lr } container softwareSystem "diag2" { include * autolayout lr } theme default } } ```

ggrossetie commented 10 months ago

@sixtysecrun Are you using the latest version of Asciidoctor Kroki? Please use the community chat to ask question: https://kroki.zulipchat.com/

sixtysecrun commented 10 months ago

Are you using the latest version of Asciidoctor Kroki?

I am using the latest version of Asciidoc Visual Code extension. I have just checked that it seems to work with Intellij Assidoc plugin.

Please use the community chat to ask question: https://kroki.zulipchat.com/

I will. Thank you!

ggrossetie commented 10 months ago

The Asciidoctor VS Code extension does not use the latest version that's probably why it's not working: https://github.com/asciidoctor/asciidoctor-vscode/blob/58adc632e6b9dfefd844efa66dd63ebdf9d1d78b/package.json#L658

Please open a new issue to request a version bump on asciidoctor-kroki.

yann-soubeyrand commented 3 months ago

Hey @ggrossetie, I realize I’ve never thanked you for this feature, so thanks a lot, it works like a charm!