Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Closed
The-Lum opened this issue Jan 10, 2023 · 11 comments
Closed

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

The-Lum opened this issue Jan 10, 2023 · 11 comments

Comments

@The-Lum
Copy link
Collaborator

The-Lum commented Jan 10, 2023

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
Copy link
Contributor

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
Copy link
Contributor

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

@The-Lum
Copy link
Collaborator Author

The-Lum commented Jan 11, 2023

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
Copy link
Collaborator Author

The-Lum commented Jan 11, 2023

[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
Copy link
Collaborator Author

The-Lum commented Jan 11, 2023

Hi @arnaudroques,

Here is a complement:

  • It seems that style is without effect on JSON or YAML diagram.

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
Copy link
Collaborator Author

The-Lum commented Jan 11, 2023

Hi @arnaudroques,

A line of research...

Just change

    LineStyle 10;5

to

    LineStyle 10-5
  • How manage this [not backward compatible] change?
    On all the doc. and all theme using this functionality...
    [to debate: it is not a big issue, but just know it]
@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
Copy link
Collaborator Author

The-Lum commented Jan 11, 2023

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
Copy link
Contributor

  • 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
Copy link
Collaborator Author

The-Lum commented Jan 11, 2023

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
Copy link
Collaborator Author

The-Lum commented Jan 12, 2023

Last issue fixed by:

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

@The-Lum
Copy link
Collaborator Author

The-Lum commented Jan 14, 2023

See also question (about styler parser) here:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@arnaudroques @The-Lum