search

plantuml / plantuml

Generate diagrams from textual description
https://plantuml.com
Other
9.73k stars 881 forks source link

[v1.2023.0] Style management (error on `carbon-gray` theme) #1250

Closed The-Lum closed 1 year ago

The-Lum commented 1 year ago

Hello @arnaudroques, and all,

With the update of the theme Gallery, here are first tests with the v1.2023.0 and the theme carbon-gray seems to be broken...

Here is minimal example:

!theme carbon-gray
component a

See also error here (from Action on theme Gallery):

</themes/puml-theme-carbon-gray.puml>:770:error:Error in style definition: bad definition
net.sourceforge.plantuml.style.parser.StyleParsingException: bad definition
    at net.sourceforge.plantuml.style.parser.StyleParser.readValue(StyleParser.java:190)
    at net.sourceforge.plantuml.style.parser.StyleParser.parse(StyleParser.java:108)
    at net.sourceforge.plantuml.jsondiagram.StyleExtractor.applyStyles(StyleExtractor.java:116)
    at net.sourceforge.plantuml.jsondiagram.JsonDiagramFactory.createSystem(JsonDiagramFactory.java:94)
    at net.sourceforge.plantuml.PSystemBuilder.createPSystem(PSystemBuilder.java:[14](https://github.com/The-Lum/puml-themes-gallery/actions/runs/3883188154/jobs/6624205160#step:4:15)0)
    at net.sourceforge.plantuml.BlockUml.getDiagram(BlockUml.java:181)
    at net.sourceforge.plantuml.SourceFileReaderAbstract.getGeneratedImages(SourceFileReaderAbstract.java:[16](https://github.com/The-Lum/puml-themes-gallery/actions/runs/3883188154/jobs/6624205160#step:4:17)8)
    at net.sourceforge.plantuml.Run.manageFileInternal(Run.java:5[19](https://github.com/The-Lum/puml-themes-gallery/actions/runs/3883188154/jobs/6624205160#step:4:20))
    at net.sourceforge.plantuml.Run.processArgs(Run.java:402)
    at net.sourceforge.plantuml.Run.manageAllFiles(Run.java:369)
    at net.sourceforge.plantuml.Run.main(Run.java:[20](https://github.com/The-Lum/puml-themes-gallery/actions/runs/3883188154/jobs/6624205160#step:4:21)4)
net.sourceforge.plantuml.style.parser.StyleParsingException: Cannot understand <
    at net.sourceforge.plantuml.style.parser.StyleParser.parse(StyleParser.java:256)
    at net.sourceforge.plantuml.style.parser.StyleParser.parse(StyleParser.java:61)
    at net.sourceforge.plantuml.jsondiagram.StyleExtractor.applyStyles(StyleExtractor.java:116)
    at net.sourceforge.plantuml.jsondiagram.JsonDiagramFactory.createSystem(JsonDiagramFactory.java:94)
    at net.sourceforge.plantuml.PSystemBuilder.createPSystem(PSystemBuilder.java:140)
    at net.sourceforge.plantuml.BlockUml.getDiagram(BlockUml.java:181)
    at net.sourceforge.plantuml.SourceFileReaderAbstract.getGeneratedImages(SourceFileReaderAbstract.java:168)
    at net.sourceforge.plantuml.Run.manageFileInternal(Run.java:519)
    at net.sourceforge.plantuml.Run.processArgs(Run.java:402)
    at net.sourceforge.plantuml.Run.manageAllFiles(Run.java:[36](https://github.com/The-Lum/puml-themes-gallery/actions/runs/3883188154/jobs/6624205160#step:4:37)9)
    at net.sourceforge.plantuml.Run.main(Run.java:204)

See all log here:

I search what is broken...

Regards.

arnaudroques commented 1 year ago

I search what is broken...

The issue is on our side: we have completely rewritten the style parser. Don't waste any time: we are going to investigate.

Thanks for the report!

arnaudroques commented 1 year ago

It has just been fixed. Some other themes may have also issues: could you tell us if you find any errors? Thanks!

The-Lum commented 1 year ago

Hi @arnaudroques, and all,

After rebuild all the Theme Gallery this morning, it is better. But we always see $4 \times$:

net.sourceforge.plantuml.style.parser.StyleParsingException: Cannot understand <
    at net.sourceforge.plantuml.style.parser.StyleParser.parse(StyleParser.java:259)
    at net.sourceforge.plantuml.style.parser.StyleParser.parse(StyleParser.java:61)
    at net.sourceforge.plantuml.jsondiagram.StyleExtractor.applyStyles(StyleExtractor.java:116)
    at net.sourceforge.plantuml.jsondiagram.JsonDiagramFactory.createSystem(JsonDiagramFactory.java:94)
    at net.sourceforge.plantuml.PSystemBuilder.createPSystem(PSystemBuilder.java:140)
    at net.sourceforge.plantuml.BlockUml.getDiagram(BlockUml.java:181)
    at net.sourceforge.plantuml.SourceFileReaderAbstract.getGeneratedImages(SourceFileReaderAbstract.java:168)
    at net.sourceforge.plantuml.Run.manageFileInternal(Run.java:519)
    at net.sourceforge.plantuml.Run.processArgs(Run.java:402)
    at net.sourceforge.plantuml.Run.manageAllFiles(Run.java:369)
    at net.sourceforge.plantuml.Run.main(Run.java:204)

See Action Log here:

I search for which diagram that occurs...

And with this lines:

    at net.sourceforge.plantuml.jsondiagram.StyleExtractor.applyStyles(StyleExtractor.java:116)
    at net.sourceforge.plantuml.jsondiagram.JsonDiagramFactory.createSystem(JsonDiagramFactory.java:94)

That seems perhaps occurs with JSON or YAML diagram... (but I don't know with which theme!)

Regards.

The-Lum commented 1 year ago

[Just for traceability]

FYI @bharatrajagopalan (initial author of the carbon-gray theme [#1059, #1060]) See minor change/fix (to be conform with the new PlantUML style parser) on your theme here:

Regards.

The-Lum commented 1 year ago

Hi @arnaudroques,

Here is a complement:

See example on the corresponding doc pages. and here:

@startyaml
<style>
yamlDiagram {
  node {
    BackGroundColor lightblue
    LineColor lightblue
    FontName Helvetica
    FontColor red
    FontSize 18
    FontStyle bold
    BackGroundColor Khaki
    RoundCorner 0
    LineThickness 2
    LineStyle 10;5
    separator {
      LineThickness 0.5
      LineColor black
      LineStyle 1;5
    }
  }
  arrow {
    BackGroundColor lightblue
    LineColor green
    LineThickness 2
    LineStyle 2;5
  }
}
</style>
  -
    name: Mark McGwire
    hr:   65
    avg:  0.278
  -
    name: Sammy Sosa
    hr:   63
    avg:  0.288

@endyaml

Thanks for your analyse and correction... Regards.

The-Lum commented 1 year ago

Hi @arnaudroques,

A line of research...

Just change 

    LineStyle 10;5

to

    LineStyle 10-5
@startyaml
<style>
yamlDiagram {
  node {
    BackGroundColor lightblue
    LineColor lightblue
    FontName Helvetica
    FontColor red
    FontSize 18
    FontStyle bold
    BackGroundColor Khaki
    RoundCorner 0
    LineThickness 2
    LineStyle 10-5
    separator {
      LineThickness 0.5
      LineColor black
      LineStyle 1-5
    }
  }
  arrow {
    BackGroundColor lightblue
    LineColor green
    LineThickness 2
    LineStyle 2-5
  }
}
</style>
  -
    name: Mark McGwire
    hr:   65
    avg:  0.278
  -
    name: Sammy Sosa
    hr:   63
    avg:  0.288

@endyaml

Regards.

The-Lum commented 1 year ago

Test by test...

Just adding an empty style... [and this is the drama]

<style>
</style>

Could you compare:

@startyaml
!theme amiga
  -
    name: Mark McGwire
    hr:   65
    avg:  0.278
  -
    name: Sammy Sosa
    hr:   63
    avg:  0.288
@endyaml

VS

@startyaml
!theme amiga
<style>
</style>
  -
    name: Mark McGwire
    hr:   65
    avg:  0.278
  -
    name: Sammy Sosa
    hr:   63
    avg:  0.288
@endyaml

Thanks for your work...

arnaudroques commented 1 year ago
  • How manage this [not backward compatible] change?

I'm afraid that this will be an exception where we are not going to be backward compatible.

This is because we are now fully supporting real CSS format:

<style>
yamlDiagram {
  node {
    BackGroundColor: lightblue;
    LineColor: lightblue;
    FontName: Helvetica;
    FontColor: red;
    FontSize: 18;
    FontStyle: bold;
    BackGroundColor: Khaki;
    RoundCorner: 0;
    LineThickness: 2;
    LineStyle: 10-5;
    separator {
      LineThickness: 0.5;
      LineColor: black;
      LineStyle: "1;5";
    }
  }
  arrow { BackGroundColor: lightblue; LineColor: green; LineThickness: 2; LineStyle: "2;5"; }
}
</style>

The legacy "close-to-CSS" format is also supported:

<style>
yamlDiagram {
  node {
    BackGroundColor lightblue
    LineColor lightblue
    FontName Helvetica
    FontColor red
    FontSize 18
    FontStyle bold
    BackGroundColor Khaki
    RoundCorner 0
    LineThickness 2
    LineStyle "10;5"
    separator {
      LineThickness 0.5
      LineColor black
      LineStyle "1;5"
    }
  }
  arrow {
    BackGroundColor lightblue
    LineColor green
    LineThickness 2
    LineStyle 2-5
  }
}
</style>

So now LineStyle 10;5 is really too confusing for our parser, so you have to use LineStyle 10-5 or LineStyle "10;5"

I think it's an acceptable solution, since it only breaks the rendering, not the diagram itself (and only if LineStyle is used). Any though?

PS: Thanks about the <style></style> issue, we're going to fix it also.

The-Lum commented 1 year ago

Hi,

  1. If that is to be fully compliant with CSS style, a not backward compatibility is acceptable...
  2. Perhaps the <style></style> issue performs the StyleParsingException: Cannot understand <

To be continued.

The-Lum commented 1 year ago

Last issue fixed by:

Thanks, 👍 I can continue to improve theme...

The-Lum commented 1 year ago

See also question (about styler parser) here: