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

Properly fill the title of a component #12

Closed
DaelDe opened this issue Aug 20, 2019 · 14 comments · Fixed by #16
Closed

Properly fill the title of a component #12

DaelDe opened this issue Aug 20, 2019 · 14 comments · Fixed by #16
Assignees
Labels
bug Something isn't working

Comments

@DaelDe
Copy link

DaelDe commented Aug 20, 2019

the diagram:

@startuml
interface _COMP_EXT1_IF1 as SwfComponentBase
component _COMP8 as RoadBoundaryFusion
@enduml

results in the formatted graph:

{
  "nodes": [
    {
      "name": "_COMP_EXT1_IF1",
      "members": [],
      "id": "_COMP_EXT1_IF1",
      "type": "Interface",
      "hidden": true
    },
    {
      "name": "_COMP8",
      "id": "_COMP8",
      "type": "Component",
      "title": "_COMP8",
      "hidden": true
    }
  ],
  "edges": [
    {
      "from": "_COMP8",
      "to": "",
      "name": "contains",
      "hidden": true
    }
  ]
}

ID, title and name of a component are identical but. But the title should be the string after as.

For example:
component _COMP8 as RoadBoundaryFusion
should result in

    {
      "name": "_COMP8",
      "id": "_COMP8",
      "type": "Component",
      "title": "RoadBoundaryFusion",
      "hidden": true
    }
@DaelDe DaelDe added the bug Something isn't working label Aug 20, 2019
@Enteee
Copy link
Owner

Enteee commented Aug 20, 2019

I think this related to #10 and should be fixed with #13 .

@Enteee
Copy link
Owner

Enteee commented Aug 20, 2019

when #13 gets merged, the diagram posted above should be parsed to:

{
  "nodes": [
    {
      "name": "diag",
      "diagrams": [
        {
          "elements": [
            {
              "name": "SwfComponentBase",
              "members": []
            },
            {
              "name": "RoadBoundaryFusion"
            }
          ]
        }
      ],
      "id": "diag",
      "type": "File",
      "hidden": true
    },
    {
      "name": "SwfComponentBase",
      "members": [],
      "id": "SwfComponentBase",
      "type": "Interface",
      "hidden": true
    },
    {
      "name": "RoadBoundaryFusion",
      "id": "RoadBoundaryFusion",
      "type": "Component",
      "title": "RoadBoundaryFusion",
      "hidden": true
    }
  ],
  "edges": [
    {
      "from": "diag",
      "to": "SwfComponentBase",
      "name": "contains",
      "hidden": true
    },
    {
      "from": "RoadBoundaryFusion",
      "to": "diag",
      "name": "contains",
      "hidden": true
    },
    {
      "from": "diag",
      "to": "RoadBoundaryFusion",
      "name": "contains",
      "hidden": true
    }
  ]
}

would this help you?

@DaelDe
Copy link
Author

DaelDe commented Aug 21, 2019

Thank you for the fast reaction.

The output above is still not what I need because there is information lost.
Given the component definition from the diagram:
component _COMP8 as RoadBoundaryFusion
_COMP8 is is like an identifier of the component and RoadBoundaryFusion is like an alias. I think plantUML has no strict definition here but I need both.

So I would expect:

{
      "name": "_COMP8",
      "id": "_COMP8",
      "type": "Component",
      "title": "RoadBoundaryFusion",
      "hidden": true
}

This is my interpretation but I think it makes sense. There is no dedicated id for the component, so the anme could be used because it has to be somehow unique among the diagram. The alias after as is then the title of the component.

@Enteee
Copy link
Owner

Enteee commented Aug 21, 2019

I understand, this is a bit of a bigger change because I currently just ignore the id. But this makes perfect sense. I will change that. Probably tomorrow though..

@DaelDe
Copy link
Author

DaelDe commented Aug 21, 2019

This would be really helpful.

I also tried to create a peg grammar for plantUML but yours is better so I will further test it with our diagrams. Unfortunately plantUML has no real language specification and is not strict in checking the syntax. This makes it really hard.

@Enteee
Copy link
Owner

Enteee commented Aug 21, 2019

Yeah, I was struggling with this as well. I had to painfully reverse-engineer by looking at the existing implementation (the source code is a pain to read though). Plantuml not having a proper language specification is the one big reason why I started this project. Your contribution is always welcome. Thank you.

@Enteee
Copy link
Owner

Enteee commented Aug 21, 2019

So currently, the id field is a feature of the graph-formatter and is always id = name . I am actually contemplating right now If i should add an id property to all classes. The logic would then be:

# with:
# component [name]
# component [name] as [title]
# component "[title]" as [name]
# component [name] as "[title]"

if there is no 'as':
   id = name
   name = name
else:
  id = name
  name = title

then we would not need this additional title attribute.

@Enteee
Copy link
Owner

Enteee commented Aug 21, 2019

I am not 100% convinced about the solution proposed in my last comment. Because an id should ideally be globally identifying and hence would need to include package and namespace information... I didn't implement this in the parser yet because this would be extremely complicated to do. Nevertheless having three attributes id, name, title seems to be a good alternative.

@DaelDe
Copy link
Author

DaelDe commented Aug 21, 2019

I also see these 2 options. However, in plantUML is no concept of a globally unique id. I think it is not needed, just has to be unique for the diagram where it appears.

However, if you for some reason need it to be globally unique in your code you can also use something like https://github.com/ai/nanoid . For me both solutions are fine.

You could stick to the easier solution if there are no immediate complaints.

This was referenced Aug 22, 2019
@Enteee
Copy link
Owner

Enteee commented Aug 22, 2019

@DaelDe see pull request #16 for a first draft of an implementation of this feature.

@Enteee
Copy link
Owner

Enteee commented Aug 22, 2019

I've decided to implement the following for now:

# with:
# component [name]
# component [title] as [name]
# component "[title]" as [name]
# component [name] as "[title]"

if there is no 'as':
   name = name
   title =  name
else:
  name = name
  title = title

changes from my previous design:

  • id still remains just a graph-formatter feature
  • fix component [name] as [title] -> component [title] as [name]

@DaelDe
Copy link
Author

DaelDe commented Aug 24, 2019

That's quite a change :). I will test i when it is released. Or is there a way for me to test it before?

@Enteee
Copy link
Owner

Enteee commented Aug 26, 2019

it's not that much. It's just most of the fixtures changing because of the new title attribute. If you want to have a look at the code just look at the files in the src/-folder not ending in .pegjs. If you want to run the code then just:

$ git clone --single-branch --branch feature/support-uml-element-titles https://github.com/Enteee/plantuml-parser.git
$ cd plantuml-parser
$ npm install
$ npm start 

should be enough.

@DaelDe
Copy link
Author

DaelDe commented Aug 26, 2019

I am not using the cli, but the library.

I will have a look at the modified tests, should be enough.

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

Successfully merging a pull request may close this issue.

2 participants