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

PlantUML custom skin throws an exception on salt diagrams #204

Closed bcouetil closed 5 years ago

bcouetil commented 5 years ago

Hello,

First of all, thank you for the good work on this project ! Using it on a daily basis as part of my documentation.

I use a custom skin on my plantUML diagrams, using the plantumlconfig parameter to inject it. This works nicely, except when there is also a Salt GUI mockup somewhere, then it throws an exception :

java.lang.IllegalArgumentException at net.sourceforge.plantuml.salt.SaltUtils.createElement(SaltUtils.java:88) at net.sourceforge.plantuml.salt.PSystemSalt.exportDiagramNow(PSystemSalt.java:75) at net.sourceforge.plantuml.AbstractPSystem.exportDiagram(AbstractPSystem.java:130) at net.sourceforge.plantuml.SourceStringReader.outputImage(SourceStringReader.java:149) at net.sourceforge.plantuml.SourceStringReader.outputImage(SourceStringReader.java:121) at org.asciidoctor.diagram.plantuml.PlantUML.generate(PlantUML.java:101) at org.asciidoctor.diagram.CommandProcessor.processRequest(CommandProcessor.java:40) at org.asciidoctor.diagram.CommandProcessor.processRequest(CommandProcessor.java:19) at sun.reflect.GeneratedMethodAccessor19.invoke(Unknown Source) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:498) at org.jruby.javasupport.JavaMethod.invokeDirectWithExceptionHandling(JavaMethod.java:453)

I don't except the skin to work on salt GUI mockup, but ignoring the skinparam stuff would be great.

For I know, I have to remove all my salt blocks or remove my custom skin. tryied to put :plantumlconfig!: before the salt block, but does not seem to have any impact.

pepijnve commented 5 years ago

Could you provide a small example document that I can use to reproduce this problem?

bcouetil commented 5 years ago

Preparing a use case, I found something interesting.

This works flawlessly, pasting the skinparam stuff inline before @startsalt :

[plantuml, diagram-user-interface, png]
----
skinparam backgroundColor transparent
skinparam defaultFontColor #519BC8
skinparam defaultFontStyle bold
skinparam arrowColor #519BC8
skinparam arrowThickness 2
skinparam shadowing false
skinparam objectBorderColor red

skinparam actor {
    borderColor #EF525B
    backgroundColor #EF525B
    fontColor #EF525B
}
@startsalt
{+
{* File | Edit | Source | Refactor 
 Refactor | New | Open File | - | Close | Close All }
{/ General | Fullscreen | Behavior | Saving }
{
    { Open image in: | ^Smart Mode^ }
    [X] Smooth images when zoomed
    [X] Confirm image deletion
    [ ] Show hidden images 
}
[Close]
}
@endsalt
----

But separating in config and adoc files throws an exception :

plantuml.cfg

skinparam backgroundColor transparent
skinparam defaultFontColor #519BC8
skinparam defaultFontStyle bold
skinparam arrowColor #519BC8
skinparam arrowThickness 2
skinparam shadowing false
skinparam objectBorderColor red

skinparam actor {
    borderColor #EF525B
    backgroundColor #EF525B
    fontColor #EF525B
}

test.adoc

[plantuml, diagram-user-interface, png]
----
@startsalt
{+
{* File | Edit | Source | Refactor 
 Refactor | New | Open File | - | Close | Close All }
{/ General | Fullscreen | Behavior | Saving }
{
    { Open image in: | ^Smart Mode^ }
    [X] Smooth images when zoomed
    [X] Confirm image deletion
    [ ] Show hidden images 
}
[Close]
}
@endsalt
----
pepijnve commented 5 years ago

This looks like a plantuml bug to me. I saved your config file and plantuml to files and ran plantuml from the command line. Results in the same error (using PlantUML 1.2018.13). Probably best to report this in the plantuml project issue tracker.

Cerris:lib pepijn$ java -jar plantuml.jar -config test.cfg test.puml
L = data=[, borderColor #EF525B, backgroundColor #EF525B, fontColor #EF525B, }, {+, {* File | Edit | Source | Refactor, Refactor | New | Open File | - | Close | Close All }, {/ General | Fullscreen | Behavior | Saving }, {, { Open image in: | ^Smart Mode^ }, [X] Smooth images when zoomed, [X] Confirm image deletion, [ ] Show hidden images, }, [Close], }]
java.lang.IllegalArgumentException
    at net.sourceforge.plantuml.salt.PSystemSalt.createElement(PSystemSalt.java:195)
    at net.sourceforge.plantuml.salt.PSystemSalt.exportDiagramNow(PSystemSalt.java:104)
    at net.sourceforge.plantuml.AbstractPSystem.exportDiagram(AbstractPSystem.java:130)
    at net.sourceforge.plantuml.PSystemUtils.exportDiagramsDefault(PSystemUtils.java:152)
    at net.sourceforge.plantuml.PSystemUtils.exportDiagrams(PSystemUtils.java:95)
    at net.sourceforge.plantuml.SourceFileReaderAbstract.getGeneratedImages(SourceFileReaderAbstract.java:148)
    at net.sourceforge.plantuml.Run.manageFileInternal(Run.java:488)
    at net.sourceforge.plantuml.Run.processArgs(Run.java:393)
    at net.sourceforge.plantuml.Run.manageAllFiles(Run.java:362)
    at net.sourceforge.plantuml.Run.main(Run.java:184)
bcouetil commented 5 years ago

Thank you for your lightning fast responses.

Indeed PlantUML should behave better in case of wrong data in header file, but this is not exactly a PlantUML problem : we are feeding parameters that are for core plantUML diagrams to salt, that is offered to users by PlantUML, but that has nothing in common.

I understand that not feeding the skin file to [plantuml] blocks in case of salt syntax may be complicated or questionnable, but could we at least ignore the file for [salt] blocks ? or any other way to specify which diagrams should (not) get the skin file ?

[salt,format="svg"]
----
{{T
+ src
++ main
+++ java
+++ test
}}
----
pepijnve commented 5 years ago

That will actually already work. plantumlconfig is a backwards compatibility key for older versions of the diagram extension. If you change that to plantuml-config it will only get picked up by plantuml blocks. Likewise salt-config will only get picked up by salt blocks.

The general pattern for block attributes is that you can either specify them per block (e.g., config on an individual block) or globally by adding <blockname>- as prefix (e.g., plantuml-config).

bcouetil commented 5 years ago

This works great with plantuml-config ! thanks !

Did not know for the block's config part, something like [plantuml, png, config=./config/my-file.cfg] ?

pepijnve commented 5 years ago

something like [plantuml, png, config=./config/my-file.cfg] ?

exactly. There's a section in the README that describes the various attributes that each block type supports.