search

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

Support other PlantUML tags #395

Closed thirtque closed 11 months ago

thirtque commented 1 year ago

The extension only supports uml and salt tags, but those are not the only ones users can choose to use.

I've been pondering how to implement all of them, here are some of my ideas:

  1. Follow the current convention of [plantuml] and [salt], and add [ebnf], [json], [yaml] etc., but this seems too generic and could cause problems in other extensions.
  2. Prefix everything with plantuml-, change [salt] to [plantuml-salt], and add [plantuml-ebnf], [plantuml-json], [plantuml-yaml] etc., seems a little bit messy.
  3. Add a new tag option, replace [salt] with [plantuml,tag=salt], add [plantuml,tag=ebnf], [plantuml,tag=json], [plantuml,tag=yaml] etc., cleaner, but probably would require a more extensive refactor.
  4. Remove [salt] and just leave [plantuml], and ask users to provide @start{tag}/@end{tag} themselves since those are parts of the PUML grammar.

This PR only implements ebnf tag for now as a proof of concept to better understand the codebase.

pepijnve commented 1 year ago

Option 4 should already work today. When the code inside the block contains the @start and @end markers, then those are used as is. That's done at https://github.com/asciidoctor/asciidoctor-diagram/blob/master/lib/asciidoctor-diagram/plantuml/converter.rb#L114 At the moment the check is simply "does the provided code contain @start and @end". Might be a good idea to tighten that up a bit.

The addition of a specific salt block type was triggered by https://github.com/asciidoctor/asciidoctor-diagram/issues/66 I never bothered adding the others simply because nobody asked for it and option 4 was already available. Additionally, because PlantUML is constantly in flux you end up chasing a moving target.

thirtque commented 1 year ago

Ah, yes, you're right, option 4 does work. I got a little bit confused by the error I got when using the older PlantUML version, sorry.

I guess just adding some examples that use @start.../@end... in the documentation should help?