UoY-RoboStar / robochart-textual

This repository contains the plugins for the RoboChart textual editor
Eclipse Public License 2.0
0 stars 1 forks source link

Role of diagram construct in the textual language #54

Open pefribeiro opened 2 years ago

pefribeiro commented 2 years ago

Yesterday, when looking over Matt's model I could see that diagram x is being used, perhaps inadvertently instead of package x. Throughout the model there are various import x::* that don't seem to refer to a proper x. I then checked the textual language and can see that under the rule for RCPackage we have:

https://github.com/UoY-RoboStar/robochart-textual/blob/3a40f3df4ca5e04aebf092114ff7550318131827/circus.robocalc.robochart.textual/src/circus/robocalc/robochart/textual/RoboChart.xtext#L20-L34

that is, diagram ID seems to be parsed without any assignment to a model element. As far as I can see the RoboChart metamodel also doesn't record anything about diagram names. Is this in the language for backward compatibility or is it related to the integration with Sirius, for example? If the former, I think we could perhaps warn that this is not used, otherwise it may cause confusion. I note that in the graphical editor it is possible to create diagrams with several names that refer to the same .rct resource, which is a nice feature, which I've used, for example, to show different views/layout of the same model.

The other question is whether an import xx::* that refers to something that doesn't exist should also flag a warning.

alvarohm commented 2 years ago

If I'm not mistaken, I used that as a trick to create an empty RCT file that could have an associated Sirius diagram. At the time, an empty RCT file would not produce an RCPackage, so I couldn't create an empty diagram from it. This may have changed and it is worth checking. If so, we can remove the diagram keyword (and name), but this may cause issues with older models.

On Wed, 15 Jun 2022, 19:03 Pedro Ribeiro, @.***> wrote:

Assigned #54 https://github.com/UoY-RoboStar/robochart-textual/issues/54 to @alvarohm https://github.com/alvarohm.

— Reply to this email directly, view it on GitHub https://github.com/UoY-RoboStar/robochart-textual/issues/54#event-6815085569, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABEHMYKNJG73GZ64F3LLBH3VPILGDANCNFSM5Y4FQ4EQ . You are receiving this because you were assigned.Message ID: @.*** com>

MattWindsor91 commented 2 years ago

Hmm. I have to admit I neither noticed the distinction between diagram and package (I must have picked the former up from somewhere by mistake?) and the imports are likely stale with the issue Pedro mentioned meaning I never noticed them becoming irrelevant.

It feels like in 99% of cases robochart will automatically do the right thing with resolving elements, anyway?

ana-york commented 2 years ago

Shouldn't older models use the older versions of the tool associated with them in the page? A

[image: https://robostar.cs.york.ac.uk/] https://robostar.cs.york.ac.uk/

On Thu, 16 Jun 2022 at 08:20, Alvaro Miyazawa @.***> wrote:

If I'm not mistaken, I used that as a trick to create an empty RCT file that could have an associated Sirius diagram. At the time, an empty RCT file would not produce an RCPackage, so I couldn't create an empty diagram from it. This may have changed and it is worth checking. If so, we can remove the diagram keyword (and name), but this may cause issues with older models.

On Wed, 15 Jun 2022, 19:03 Pedro Ribeiro, @.***> wrote:

Assigned #54 < https://github.com/UoY-RoboStar/robochart-textual/issues/54> to @alvarohm https://github.com/alvarohm.

— Reply to this email directly, view it on GitHub < https://github.com/UoY-RoboStar/robochart-textual/issues/54#event-6815085569 , or unsubscribe < https://github.com/notifications/unsubscribe-auth/ABEHMYKNJG73GZ64F3LLBH3VPILGDANCNFSM5Y4FQ4EQ

. You are receiving this because you were assigned.Message ID: @.*** com>

— Reply to this email directly, view it on GitHub https://github.com/UoY-RoboStar/robochart-textual/issues/54#issuecomment-1157322665, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABEHNI4TWDE7KUU3LUOQUA3VPLIVTANCNFSM5Y4FQ4EQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>

alvarohm commented 2 years ago

The "diagram NAME" was primarily used by the new file wizard so that it would be parsed into an RCPackage and a diagram could be created. I have changed my working copy of the graphical editor to not add "diagram NAME" and it still creates the associated diagram, so this trick is not necessary anymore.

By older versions of models, I mean anything created (using the new file wizard) before this change. So, if I remove "diagram NAME" from the syntax, and anyone tries to open a file created with the new file wizard previously, it will have a syntax error.

@pefribeiro I missed your question about imports. I'm using the default import mechanism of Xtext, so I'm not sure how simple it would be to add validations to this. An import is just a string.

alvarohm commented 2 years ago

I have modified the graphical editor wizard to no longer add the label "diagram NAME". I considered deprecating the syntax, but have not found an easy way to do it. The deprecation mechanism of Xtext did not work straightaway and I don't think it is worth investing time sorting this out.

alvarohm commented 2 years ago

I have made the changes discussed above in the commit e4c71a5 of the graphical editor.