mikitex70 / plantuml-markdown

PlantUML plugin for Python-Markdown
BSD 2-Clause "Simplified" License
192 stars 55 forks source link

Support kroki's `c4plantuml` directive for its bundled C4 files #76

Closed nejch closed 1 year ago

nejch commented 1 year ago

Thanks for maintaining this extension @mikitex70. I know this must be a bit quick after just releasing initial remote kroki support ;)

Kroki enables C4-PlantUML (https://github.com/plantuml-stdlib/C4-PlantUML) by locally bundling files to include and then exposes that via the /c4plantuml/ endpoint (it disables remote includes by default, https://docs.kroki.io/kroki/setup/configuration/#_safe_mode).

This can then be accessed via

```c4plantuml
!include C4_Context.puml
...


Some examples are also seen here https://docs.gitlab.com/ee/administration/integration/kroki.html#create-diagrams.

Would you consider adding a `c4plantuml` directive in addition to `plantuml` and in case of kroki use that endpoint instead? I can open a small PR for this if you think this can be considered in scope for this extension :)
mikitex70 commented 1 year ago

Hi @nejch, thanks for your links and examples. I will implement C4 support, give me some time to do the job. Maybe I can implement it on the weekend.

nejch commented 1 year ago

Thanks @mikitex70! I had another look at the code now and I see you're doing some preprocessing to inline the !include code, so this differs slightly from kroki's approach (kroki would have the files available server-side, but the extension here would assume them to be available locally before making the requests).

Just something to keep in mind - not sure what the best approach is, maybe having an allowlist of C4 files to not try to resolve locally, with a little C4PlantUMLPreprocessor or so.

anb0s commented 1 year ago

May be it helps if behavior should be same - i've implemented the preprocessor in:

Do not hesitate to ask if need help or review etc.

mikitex70 commented 1 year ago

Hi all, I've pushed in the develop branch a preliminary version for the C4 support. Some notes:

With this implementation the example by @nejch is rendered correctly.

I need a refactoring to make the code cleaner and more readable and to update the documentation, but can you try it and tell me if the behavior meets your requirements?

nejch commented 1 year ago

Thanks a lot @mikitex70, I just tested this against a live kroki server via mkdocs and renders nicely. The regex approach and overriding via comments also seems very flexible and non-intrusive to me. :) Not sure if @anb0s has more comments.

(The only minor thought I had about the loose (c4)?plantuml language is maybe it'll behave differently between clients (e.g. C4 diagrams hosted on GitLab would not render with plantuml when shown in the GitLab UI markdown, but would render with python when built for GitLab Pages). But I guess that's to be expected in a wide ecosystem of extensions/preprocessors, so not a big deal. :))

anb0s commented 1 year ago

@mikitex70 Thank you 👍

I think the support of stdlib include is the main benefit here and also a portable solution, because also implemented in another clients. All other special handling of includes should be avoided. The comment keyword solution is a good way for now. Later/optional: if possible it would be perfect to have some user specified configuration option for whitelisting, so user can specify via regex what should be rendered at server or client... may be multiple rules would be also good.

Anyway i will try the available version by end of the week :)

mikitex70 commented 1 year ago

@neich, the c4plantuml language is available, it is handled as the plantuml language. If you think it might be useful to have it for greater compatibility with other tools, I leave it there.

@anb0s, the server_include_whilelist configuration is already a list o regular expressions, so a user can add custom rules describing which files can be handled by the server. The only caveat is to append the ^[Cc]4.*$ value to the list, if used. Example:

plantuml_markdown:
  server_include_whilelist:
    - "^[Cc]4.*$"
    - "^MyIncludes.*$"

or in json:

{
  "plantuml_markdown": {
    "server_include_whilelist": [ "^[Cc]4.*$", "^MyIncludes.*$" ]
  }
}
nejch commented 1 year ago

@mikitex70 great, this all looks good to me. Having that c4plantuml there might be useful as both kroki and GitLab.com have it documented, just in case.

Yeah, the regex list approach definitely looks powerful enough. Maybe comments aren't even needed, though they might be useful to avoid ugly regex exclusions ;) no strong opinions on this one :grin:

anb0s commented 1 year ago

@mikitex70 cool 👍 only one typo / nitpick server_include_whilelist -> server_include_whitelist

I think the inline comment support is not needed, but also no strong opinion

mikitex70 commented 1 year ago

Released as version 3.7.3. Closing this issue. Thanks again for your contribution.