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

C4 PlantUML elements with parameters passed with $ are not recognized #100

Open
philippsimon opened this issue Oct 23, 2023 · 5 comments
Open
Assignees
Labels
bug Something isn't working

Comments

@philippsimon
Copy link

philippsimon commented Oct 23, 2023

Describe the bug

When I add elements and pass the arguments like it's shown on the C4 PlantUML page with a $-character ($descr="description") these elements are not recognized.

In general nearly all examples on the C4 PlantUML page can't be displayed because of that issue.

To Reproduce
Steps to reproduce the behavior:

  1. Go to '...'
  2. Click on '....'
  3. Scroll down to '....'
  4. See error

PlantUML

@startuml
Person(person_descr, "person", $descr="description")
Person(person_descr, "person", $descr="") ' an empty parameter also doesn't work
Person(person_tags_one, "person", $tags="one+two")
@enduml

Here more and it shows that it's a valid syntax

Expected behavior
The elements should be returned.

@philippsimon philippsimon added the bug Something isn't working label Oct 23, 2023
@Enteee
Copy link
Owner

Enteee commented Oct 26, 2023

@dgassma : since all the c4 stuff is coming from you, do you have an idea why this is happening?

@dgassma
Copy link
Contributor

dgassma commented Oct 29, 2023

@philippsimon : you are right and I am sorry for the inconvenience.
I misinterpreted the meaning of the $.

The current implementation is partly optional parameter parsing. Take a look at the C4 tests in plantuml-parser:

@startuml

("C4 - Person & System")

Person(person, "Person::label", "Person::descr")
Person_Ext (person_ext, "Person_Ext::label", "Person_Ext::descr")

System(system, "System::label", "System::descr")
System_Ext (system_ext, "System_Ext::label", "System_Ext::descr")

SystemDb (systemDb, "SystemDb::label", "SystemDb::descr")
SystemDb_Ext (systemDb_ext, "SystemDb_Ext::label", "SystemDb_Ext::descr")

SystemQueue (systemQueue, "SystemQueue::label", "SystemQueue::descr")
SystemQueue_Ext (systemQueue_ext, "SystemQueue_Ext::label", "SystemQueue_Ext::descr")

System (system, "System::label")

System (systemDescr, "System::label", "System::descr")

System (systemDescrSprite, "System::label", "System::descr", "System::sprite")

System (systemDescrSpriteTags, "System::label", "System::descr", "System::sprite", "System::tags")

System (systemDescrSpriteTagsLink, "System::label", "System::descr", "System::sprite", "System::tags", "System::link")

@enduml

I already have an idea how to fix it in my mind. However, I am not sure, if the library should support the old (wrong) parsing as well?
What is your opinion on that @philippsimon & @Enteee

I am going to try to provide a fix by the end of week.

@philippsimon
Copy link
Author

@philippsimon
Copy link
Author

I just saw that for mermaidjs they already created a parser for C4 diagrams:
https://github.com/mermaid-js/mermaid/blob/develop/packages/mermaid/src/diagrams/c4/parser/c4Diagram.jison
Maybe it can be reused or a collaboration can be started?

@Enteee
Copy link
Owner

Enteee commented Nov 21, 2023

@dgassma i don't have a strong opinion if we should support the old parsing as well. I rarely use C4 so I can not really speak from experience. Sorry..

@philippsimon looking at the parsing grammar in mermaid; looks cool and I would be super happy if I don't have to support C4 in a PEG in this project. But I am not quite sure if there's a working Jison to PEG translator and I am not even quite sure if our grammar isn't better, i think @dgassma did quite a good job :)

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

3 participants