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.

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

diagram includes are not supported #25

Closed
ali-habibzadeh opened this issue Oct 26, 2019 · 4 comments
Closed

diagram includes are not supported #25

ali-habibzadeh opened this issue Oct 26, 2019 · 4 comments
Assignees
Labels
bug Something isn't working

Comments

@ali-habibzadeh
Copy link

ali-habibzadeh commented Oct 26, 2019

Describe the bug

In plantUML you can use !include and !includeurl for importing dependencies for a diagram (extensions, icons etc.) http://plantuml.com/preprocessing.

But if such imports are used the parser breaks.

return new peg$SyntaxError(
           ^
source-map-support.js:445
SyntaxError: Expected "'", "(", "/'", "@enduml", "[", "abstract ", "class ", "cloud", "component ", "database", "digraph", "enum ", "folder", "frame", "interface ", "namespace", "node", "note ", "package", "rectangle", "skinparam ", "state", "together ", "usecase ", [ \t], or [A-Za-z0-9._] but "}" found.

To Reproduce
Steps to reproduce the behavior:

  1. Use parse with content or use parseFile with given uml
  2. See error stack

PlantUML

@startuml
!includeurl https://raw.githubusercontent.com/RicardoNiepel/C4-PlantUML/release/1-0/C4.puml
!includeurl https://raw.githubusercontent.com/RicardoNiepel/C4-PlantUML/release/1-0/C4_Container.puml
!includeurl https://raw.githubusercontent.com/RicardoNiepel/C4-PlantUML/release/1-0/C4_Context.puml
!includeurl https://raw.githubusercontent.com/RicardoNiepel/C4-PlantUML/release/1-0/C4_Component.puml

System_Boundary(MetricsExecution, "Metrics Execution") {
    Component(apiRouter, "Express Routeer", "Api router")

    System_Ext(puppeteer, "puppeteer", "Headless Chrome Provider")

    Rel(apiRouter, PageRenderService, "url: string")
    Rel(PageRenderService, apiRouter, "IMetric<T>[]")
}
@enduml

Expected behavior
It should parse normally

Screenshots
N/A

Desktop (please complete the following information):

  • OS: Mac OS Mojave
  • Browser Node
  • Version 12

Additional context
N/A

@ali-habibzadeh ali-habibzadeh added the bug Something isn't working label Oct 26, 2019
@Enteee
Copy link
Owner

Enteee commented Oct 26, 2019

Thank you for reporting this @ali-habibzadeh.

!includeurl and some of its friends like !log, !function are all PlantUML prepreocessor featuers. I fear that implementing them means opening pandora's box. Anyways I do understand that the parser should not raise an exception when it encounters not preprocessed PlantUML diagrams and still try to do its best with the diagram given.

Therefore I'd suggest:

  • I will add test fixtures for all examples in: PlantUML prepreocessor
  • The parser should ignore all lines starting with a ! which will fix this bug.
  • In order to generate diagrams properly we still have to implement a PlantUML pre-processor (preprocess()), which takes a raw PlantUML diagram and returns a preprocessed version of that diagram. I will open a enhancement for this.

@Enteee
Copy link
Owner

Enteee commented Oct 26, 2019

Ok the simple test fixtures I pushed are parse in a low-effort manner. Which mean the !includeurl is just ignored by the parser. So what I wanted to do from my previous post:

  • The parser should ignore all lines starting with a ! which will fix this bug.

Does already work for simple one line preprocessor instructions.

The real reason for the parser exception is that without reading the includes, this part:

System_Boundary(MetricsExecution, "Metrics Execution") {
    Component(apiRouter, "Express Routeer", "Api router")

    System_Ext(puppeteer, "puppeteer", "Headless Chrome Provider")

    Rel(apiRouter, PageRenderService, "url: string")
    Rel(PageRenderService, apiRouter, "IMetric<T>[]")
}

is completely undefined syntax.

This means in order to fix this bug we actually have to implement #26 to its full extend.

Enteee added a commit that referenced this issue Oct 26, 2019
@Enteee
Copy link
Owner

Enteee commented Oct 26, 2019

@ali-habibzadeh since this bug requires effectively #26 in big parts, are you OK with the following plan?

Enteee added a commit that referenced this issue Dec 15, 2019
* Test fixture for #25: diagram includes are not supported
* Fixture for #25: diagram includes are not supported
* Test fixture output for #25
@Enteee
Copy link
Owner

Enteee commented Dec 15, 2019

Closed, tracking in #26

@Enteee Enteee closed this as completed Dec 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants