Community
Participate
Working Groups
Since internal transition is not represented as graphical edge, they are not naturally created using the transition edge tool on the palette, but instead they need to be created in other ways. Two main options exist how they can be created: 1) Right click on a state in a state machine diagram and select a context menu for creating an "Internal Transition". This is actually a general limitation in Papyrus where a "new child menu" is not available on the context menu on model elements in a diagram. 2) Use the assistant ("hover menu") when hovering over the state where a creation tool can be provided to create an internal transition for that state. Compare for example with how to add an operation or a property to class when hovering over a class in a class diagram. When an internal transitions has been created it shall be shown in an "internal transitions compartment" on the state itself seen from the "outside" of the state. When a state has internal transitions then a small annotation should also be shown in the top right corner of the state (the same icon used for internal transition can be used). Preferably the internal transitions shall also be shown in a separate list ("external compartment") below the state frame "on the inside" of the state, in the case the state is a composite state and has its own state machine diagram. This is to ensure that the local transitions are shown, and are availble for editing, both "on the outside" as well as "on the inside" of the state. See also Bug 494284 for the creation of external and local transitions.
Please see Bug 493869 Comment 5 for a summary of proposed principles when it comes to the creation of composite states. Since adding an internal transitions requires that you create a region to hold the internal transition, this means that adding the first internal transition to a simple state, basically turns that into a composite state, which means that the region shall be created, the nested state machine diagram, adding entry/exit points on all incoming/outgoing transitions, and the composite state adornment is shown. Since the nested state machine diagram always is added at this stage, it is preferred if the internal transitions always is shown "on the inside" as well, in a separate compartment below the state frame itself.
Rémi, could you please look at this and assign?
assigning as part of the work on state machines.
Regarding 1) Right click on a state in a state machine diagram and select a context menu for creating an "Internal Transition". This is actually a general limitation in Papyrus where a "new child menu" is not available on the context menu on model elements in a diagram. I have a running prototype for this new child menu in the diagrams, but I am facing some design issues: - any contribution from model explorer is reused, so we have same configuration on the diagram than in the model explorer. Do you expect there to have a different configuration, with for example less elements or different ones ? - As for Model explorer or Property views, elements are created using only their semantic definition. In our case, this has the effect not to display the new element in the diagram, exept if the container is canonical, e.g. displays everything in its compartment. What is your expectation on such a menu?
Created attachment 265487 [details] sample of the prototype for new child menu in diagram
(In reply to Remi Schnekenburger from comment #4) > Regarding 1) Right click on a state in a state machine diagram and select a > context menu for creating an "Internal Transition". This is actually a > general limitation in Papyrus where a "new child menu" is not available on > the context menu on model elements in a diagram. > > I have a running prototype for this new child menu in the diagrams, but I am > facing some design issues: > - any contribution from model explorer is reused, so we have same > configuration on the diagram than in the model explorer. Do you expect there > to have a different configuration, with for example less elements or > different ones ? I am not sure. Gut feeling, and my own personal opinion, would be that it actually can be the same. But when checking the legacy tooling, there are in general fewer menu choices when right clicking on an element in a diagram compared to when right clicking on the same element the model explorer. Not sure if this is intentional or unintentional in the legacy tooling. Personally I prefer consistency, i.e. that they can be the same configuration. But on the other hand, I have feeling that the context menu in the diagram could benefit from possibly being even more filtered (as it seem to be done in the legacy tooling). Let's start with keeping it simple and have the same configuration. But as a general feature (which I guess should be in base Papyrus), I think that it would be good with the support of "new child" menu both in the model explorer and in the diagram, and that the "new child" menu by default can "inherit" the configuration from the model explorer, but that it can both "exclude" and "extend" the configuration with less respectively more menu choices if needed. > - As for Model explorer or Property views, elements are created using only > their semantic definition. In our case, this has the effect not to display > the new element in the diagram, exept if the container is canonical, e.g. > displays everything in its compartment. > > What is your expectation on such a menu? This sounds a bit tricky (in the general case without the container being canonical/synchronized), and I guess can be a bit confusing for the user if the user right clicks on an element in a diagram and selects to create a new child, e.g. a new attribute of a class. If the attribute then does not appear in the diagram, then I think that can be kind of confusing. I think that the user will expect such a menu to behave more like the diagram assistant. If you use the diagram assistant to create a new attribute for example, then the attribute is shown directly in the compartment (even if the compartment is not synchronized). Actually, when checking a bit closer, it do look like the menu choices available on the "Add UML" menu (as it is named in the legacy tooling) seem to be aligned with the available choices in the modelling assistant (and hence also the reason why there a fewer menu choices compared to what is available in the model explorer). There are also a few menu choices, like adding a new "Comment" or "Constraint", which both creates the comment element but also adds the link in the diagram. So considering all this, the question is in which direction to go really. I think that it is probably better to align this menu with the modeling assistant and go for the consistency (both behavior and which menu choices) since the user probably expects more direct diagram manipulation using such a menu, e.g. with respect to the direct visualization in the diagram.
thanks for this comment. I will see what I can do with the modeling assistants contributions in this case. A new question about this implementation: from Young soo implementation of the transition kind in RTRegionEditHelperAdvice, transition kind is set to local when creating a new transition, from a composite state to itself. In my case, I want to create specifically an internal transition (by only setting the kind to internal). Are there specific rules about the region containing the transitions in the legacy tooling? For example, if an internal transition is created in a state, what is the container of that transition? same question for a local transition, what will be the container ? I only wanted to check;: if the container of the local transition and internal transition is the same, then i will have to take this case into account in RTRegionEditHelperAdvice. If not, this will be a way to distinguish between the 2 cases.
(In reply to Remi Schnekenburger from comment #7) > thanks for this comment. I will see what I can do with the modeling > assistants contributions in this case. > > A new question about this implementation: from Young soo implementation of > the transition kind in RTRegionEditHelperAdvice, transition kind is set to > local when creating a new transition, from a composite state to itself. In > my case, I want to create specifically an internal transition (by only > setting the kind to internal). Are there specific rules about the region > containing the transitions in the legacy tooling? For example, if an > internal transition is created in a state, what is the container of that > transition? same question for a local transition, what will be the container > ? > I only wanted to check;: if the container of the local transition and > internal transition is the same, then i will have to take this case into > account in RTRegionEditHelperAdvice. If not, this will be a way to > distinguish between the 2 cases. As indicated in the Comment 1, the creation of an internal transition for a simple state, will in practice convert it into a composite state, to ensure that the internal transition is owned by the region of the composite state. Local transition are also owned by the region of the composite state for which the transition is being local to. So yes, both local and internal transitions are owned by the region of the composite state. We also have some aspects to consider regarding refactoring, i.e. changing the kind of a transition, e.g. changing an external self transition to a local self transition shall also change the ownership of the transition. This aspect we started to discuss in Bug 506808, but it was unclear if the different refactoring scenarios should be included in the scope of that bug, or if we should extract that into separate bug(s).
New Gerrit change created: https://git.eclipse.org/r/85782
Gerrit change https://git.eclipse.org/r/85782 was merged to [master]. Commit: http://git.eclipse.org/c/papyrus-rt/org.eclipse.papyrus-rt.git/commit/?id=d1f9358740227d4f585af140228f6012a14fc751
New Gerrit change created: https://git.eclipse.org/r/86254
Gerrit change https://git.eclipse.org/r/86254 was merged to [master]. Commit: http://git.eclipse.org/c/papyrus-rt/org.eclipse.papyrus-rt.git/commit/?id=3b6544dac945d172eec23e31e6e3fbff7b235605
I have tried to test the changes merged so far, but I am not fully sure what to expect and it was a bit hard to test (I guess there is still not explicit support for creating internal transitions?). But from what I can see so far it looks like a good start. The annotation when a composite state has internal transitions looks okay, and the internal transitions compartment looks okay. Some findings though: * I am not sure if it is related to changes for this feature or if it is some other regression, but the synchronization/automatic layout of the initial pseudostate, the initial transition, and the first state, is not working. The diagram is now empty. The elements are created (and visible in the model explorer) as expected, but I have to manually drag-and-drop the elements into the diagram to get them to be shown. * The contents of the internal transitions compartment does not seem to be updated automatically, i.e. being synchronized. If I add additional internal transitions, e.g. by simply copy-paste an existing internal transition then it does not appear in the compartment. As a work-around I tried dragging-and-dropping the internal transition onto the state, but then they ended up in the name compartment (which seem to be similar to the issue in Bug 493802). * I tried this about double-clicking the internal transition annotation, but could not get it to work. The question is if this is a "good thing" or not. I know that we have had quite some issues where users have been annoyed that the double-click navigation does not work always, due to that you have have had to double-click at specific places, e.g. in the legacy tooling the double-click once did not work if you double-clicked on the name label itself, which lots of users found confusing/annoying. I just see a risk that the same will happen here, i.e. users expect to navigate into the composite state, but instead they double-click by mistake on the internal transition annotation and the expected navigation does not happen (instead the compartment is hidden/shown, if I understand how it is supposed to work, but since I don't get it to work I am not sure). It would be good with some more information about what to expect, or maybe I should simply wait with testing an providing feedback until it has reached a little bit more mature state?
(In reply to Peter Cigehn from comment #13) > I have tried to test the changes merged so far, but I am not fully sure what > to expect and it was a bit hard to test (I guess there is still not explicit > support for creating internal transitions?). But from what I can see so far > it looks like a good start. The annotation when a composite state has > internal transitions looks okay, and the internal transitions compartment > looks okay. > > Some findings though: > > * I am not sure if it is related to changes for this feature or if it is > some other regression, but the synchronization/automatic layout of the > initial pseudostate, the initial transition, and the first state, is not > working. The diagram is now empty. The elements are created (and visible in > the model explorer) as expected, but I have to manually drag-and-drop the > elements into the diagram to get them to be shown. > > * The contents of the internal transitions compartment does not seem to be > updated automatically, i.e. being synchronized. If I add additional internal > transitions, e.g. by simply copy-paste an existing internal transition then > it does not appear in the compartment. As a work-around I tried > dragging-and-dropping the internal transition onto the state, but then they > ended up in the name compartment (which seem to be similar to the issue in > Bug 493802). > > * I tried this about double-clicking the internal transition annotation, but > could not get it to work. The question is if this is a "good thing" or not. > I know that we have had quite some issues where users have been annoyed that > the double-click navigation does not work always, due to that you have have > had to double-click at specific places, e.g. in the legacy tooling the > double-click once did not work if you double-clicked on the name label > itself, which lots of users found confusing/annoying. I just see a risk that > the same will happen here, i.e. users expect to navigate into the composite > state, but instead they double-click by mistake on the internal transition > annotation and the expected navigation does not happen (instead the > compartment is hidden/shown, if I understand how it is supposed to work, but > since I don't get it to work I am not sure). > > It would be good with some more information about what to expect, or maybe I > should simply wait with testing an providing feedback until it has reached a > little bit more mature state? Thanks for this feedback. I may post a video on the current state, so I can demo what is the current workflow and what are the current parts developed. 1. I will check the canonical mode for the internal transition compartment and the state compartment. I had to change some of the css, there may be some side effects. 2. The canonical mode is also supposed to be applied on the internal transition compartment. I only tried to create them through the modeling assistant (which is the current way to deal with the internal transition creation), so I did not get this kind of issues. I will also look at that point for the next patch (and also check the tests to verify that compartment gets updated iwhen the semantic model only is changed) 3. I tried to mimic the legacy tooling workflow for internal transition compartment behavior. The double click action on the decorator only add the named style displayInternalTransition. There could be any other way to apply that style. Where would you see such command? In the modeling assistant popup for example?
(In reply to Remi Schnekenburger from comment #14) > 2. The canonical mode is also supposed to be applied on the internal > transition compartment. I only tried to create them through the modeling > assistant (which is the current way to deal with the internal transition > creation), so I did not get this kind of issues. I will also look at that > point for the next patch (and also check the tests to verify that > compartment gets updated iwhen the semantic model only is changed) Okay, I tried the modeling assistant to create an internal transition (since we discussed this about aligning a possible "new child" context menu when right clicking on the state with the contents of the modeling assistant), but I could not find a tool for that in the modeling assistant. Only the possibility of creating entry and exit point (which makes sense), and region, (which does not really make sense in the UML-RT context, unless you for some reason have removed the only region manually or by "mistake"), but I could not find the possibility of creating a transition. Is that an issue in itself that I lack the choice of creating a transition using the modeling assistant? > > 3. I tried to mimic the legacy tooling workflow for internal transition > compartment behavior. The double click action on the decorator only add the > named style displayInternalTransition. There could be any other way to apply > that style. Where would you see such command? In the modeling assistant > popup for example? Okay, I did not even know that you were able to show/hide the internal transition compartment in the legacy tooling, by double-clicking the internal transition annotation. But then I would suggest that we keep that as it is and align with the legacy tooling. If you by mistake double click that annotation, then you at least get a visual feedback (since the visibility of the compartment is toggled), compared to the cases we have had issues with regarding double-click navigation where the user felt that nothing happened when double-clicking on the name label for example. I assume that the natural behavior is that the compartment is shown automatically when the first internal transition is added, and that it is automatically hidden whenever the last internal transition is removed. Then the toggling scenario (with the double click) is only needed in the case the user wants to hide the compartment even if there exist internal transitions. One improvement though that I realized when toggling the visibility of the compartment "manually", was to have the compartments directly available on the context menu as you have in the legacy tooling. As it works now (in base Papyrus) is that you always have to bring up the Show/Hide Compartments dialog, flip the tree open, and then select/deselect the compartment you want to show/hide, and then press OK. Bringing up a dialog like this is only cumbersome for the case when you want to show/hide a single compartment. The only benefit of having such a dialog is of course if you want to show/hide multiple compartments at a time, which I feel is a less common use case, then showing/hiding a single compartment. I really think that (also) making the compartments directly available for toggling directly on the context menu, without the need for bringing up the dialog, would be good in base Papyrus to get a better workflow in general.
> Okay, I tried the modeling assistant to create an internal transition (since we discussed this about aligning a possible "new child" context menu when right clicking on the state with the contents of the modeling assistant), but I could not find a tool for that in the modeling assistant. Only the possibility of creating entry and exit point (which makes sense), and region, (which does not really make sense in the UML-RT context, unless you for some reason have removed the only region manually or by "mistake"), but I could not find the possibility of creating a transition. Is that an issue in itself that I lack the choice of creating a transition using the modeling assistant? I see, you were in the case that the state is a simple state. I do not have yet the way to switch to a composite state while creating the initial transition. I will add this to a next patch. It should be also impossible to add a region to a simple state, or another option would be to turn the simple state into a composite state when selecting 'add a region' > I assume that the natural behavior is that the compartment is shown automatically when the first internal transition is added, and that it is automatically hidden whenever the last internal transition is removed. Then the toggling scenario (with the double click) is only needed in the case the user wants to hide the compartment even if there exist internal transitions. That may be possible, of course. > One improvement though that I realized when toggling the visibility of the compartment "manually", was to have the compartments directly available on the context menu as you have in the legacy tooling. As it works now (in base Papyrus) is that you always have to bring up the Show/Hide Compartments dialog, flip the tree open, and then select/deselect the compartment you want to show/hide, and then press OK. Bringing up a dialog like this is only cumbersome for the case when you want to show/hide a single compartment. The only benefit of having such a dialog is of course if you want to show/hide multiple compartments at a time, which I feel is a less common use case, then showing/hiding a single compartment. I really think that (also) making the compartments directly available for toggling directly on the context menu, without the need for bringing up the dialog, would be good in base Papyrus to get a better workflow in general. That would be a great improvement, I do agree. The same as displaying all available named styles in contextual menu after create stype / edit style. This could be a menu where all compartments are, with a small 'V' indicating the compartment is visible. The last menu entry could be "Manage Compartments...." with the existing open/hide compartment dialog
(In reply to Remi Schnekenburger from comment #16) > I see, you were in the case that the state is a simple state. I do not have > yet the way to switch to a composite state while creating the initial > transition. I will add this to a next patch. It should be also impossible to > add a region to a simple state, or another option would be to turn the > simple state into a composite state when selecting 'add a region' Ah, yes I tried to add an internal transition to a simple state (which would be the normal case). When I now tried it again on a state that I had made into a composite state first, then the internal transition tool appeared in the modeling assistant! Regarding this about (manually) adding a region to a simple state, I am not sure what to think. In general I think that we should try to hide this about the region as much as possible (as examplified in Bug 494180) and a user of Papyrus-RT/UML-RT should (in normal cases) not have to bother or know about it. The two main scenarios we have discussed so far when it comes to making a simple state into a composite state is the double-click (navigation) action, and when adding the first internal transition to a simple state. Not sure what to think about this to "manually" add a region and thus "manually" make it a composite state. Keep in mind also that we consistently always want the diagram "inside" the composite state to be created when the region is created (which is not always created consistently in the legacy tooling). So if we should see this "add a region" to a simple state, then we must also ensure that the diagram also is created (in the same way as is done with the double-click action, and what should be done when adding the first internal transition). The problem is that it will be kind of confusing if you can 'add a region', but you don't see it in the model explorer (see Bug 494180). > > I assume that the natural behavior is that the compartment is shown > > automatically when the first internal transition is added, and that it is > > automatically hidden whenever the last internal transition is removed. Then the > > toggling scenario (with the double click) is only needed in the case the user > > wants to hide the compartment even if there exist internal transitions. > That may be possible, of course. This is (partly) how the legacy tooling works. At least that the compartment is automatically shown when the first internal transition is added. It currently don't handle the automatic hide, but that would be nice improvement and make it more consistent. Now when I was able to create a few internal transitions I have some more comments: * The annotation is not updated directly when the first internal transition is created. Seem to some refresh issue. Pressing F5 shows the annotation. * Automatic naming of transitions, not only internal transitions, is that they are named "t1", "t2" and so on, i.e. give it the next free number. This is how the legacy tooling names the transitions, and it feels like a reasonable default naming principle. * The internal transition compartment should be shown automatically when adding the first internal transition (as discussed above). Apart from that, this looks like good start! Next is also to get the list of internal transitions to be shown on the inside of the composite state as well. I can provide a screen shot of how this looks like in the legacy tooling.
New Gerrit change created: https://git.eclipse.org/r/86696
Created attachment 265769 [details] Screen shot showing list of internal transitions on the inside of a composite state The screen show shows how the internal transitions is shown in a list outside and below the frame of the state, on the inside of a composite state, in the legacy tooling. Something similar shall be used. It cannot be handled as a compartment inside the state frame itself, since that will cause layout issues, especially with entry/exit points on the lower border of the frame, something that was tried in earlier versions of this list in the legacy tooling. This solution with a list outside and below, was what finally became the solution to avoid such layout issues.
Work for the next patch: * implement a new listener that is aware of the first internal transition being created. This should be used for ** the decorator, ** the initial display of compartment ** hiding the compartment * add the internal transition compartment to composite state (see next attachment to show current status). My concern here is the size of the compartment compared to the region compartment. * naming of transitions: time ago, we had a work for Ericsson to remove names on all relationships, so they are not named anymore on creation. I can restore such a behavior for transitions, with "t" as prefix? * more tests, especially on canonical aspect.
Created attachment 265770 [details] initial version for internal transition for composite state Note the css theme applied to the state machine diagram! I myself find the look and feel more readable without the gradient ;)
(In reply to Remi Schnekenburger from comment #20) > * add the internal transition compartment to composite state (see next > attachment to show current status). My concern here is the size of the > compartment compared to the region compartment. Yes, what you have examplified is exactly what initially was the solution in the legacy tooling as well. But this did not work out at all. Consider the case where you want to have entry/exit points on the lower border (to match the position on the outside). Then you will have an extreme clutter with transitions going right through the internal transitions compartment. The only reasonable solution (which eventuellay was choosen in the legacy tooling) was to have the "compartment" (it probably should not be considered a compartment) outside and below the state frame itself, to avoid that transitions goiing to entry/exit points located at the bottom border of the state frame clutters the list of internal transitions. > * naming of transitions: time ago, we had a work for Ericsson to remove > names on all relationships, so they are not named anymore on creation. I can > restore such a behavior for transitions, with "t" as prefix? Okay, I did not know about such a request for removal of names for all relationships. Feels like a little bit too general to say *all* relationships. Transitions is a bit special also, and I do know that Bran has rather lengthy discussion in his user experience document regarding how to handle the naming of transitions. I do know that he proposes to leave the transition unnamed and let it pick up its name label from the protocol message for the trigger of the transitions (so that you avoid having to give it an explicit name and that the name label automatically picks up a changed name of the triggering protocol message). I have not yet given all this much thought, and maybe it *is* good to leave the name unset and maybe not even allow the user to name the transition. But I am not sure what is the best approach here. We probably need to consider any legacy models, which do have explicitly named transitions (and a default naming strategy of 't1', 't2' and so on, but those I expect always being changed to something matching/indicating the trigger(s) of the transition). So I am not sure in which direction to go: Change the naming strategy, e.g. to something that Bran proposes, or align more with legacy and allow/keep explicit naming (to make it easy to import legacy model). Or having some "mix" support, similar to what we have for the capsule structure diagram, i.e. if the name is left empty, then the name label is picked up from the capsule, otherwise the explicit name is used. Something similar could be used also for transitions, i.e. either the name from the triggering protocol message is uses, or the explicit name of the transition. Then we just have the question, which should the default name be? Maybe it is better to leave it unnamed initially. Because then you also get an indication (by the lack of name), that the transition is lacking a trigger.
I pushed a new patch, that I may merge soon (gerrit build is green, with the tests). Last additions: - add a new label manager for the internal transitions: The displayed label for an internal transition is now either the name if is set, or it is computed from the list of protocol messages referenced by the triggers. Each level of transition -> trigger -> call event -> operation (as a protocol message) are notifying the label that is should update when changed. So the name should react to any modificaiton (adding a new element, changing the name of the protocol message, etc.). Some Junit tests should be added, and I am wondering also what to indicate when '*' is selected. Currently nothing is displayed. - add a listener for internal transition additions / removal (and even change kind should work, even if not officially supported yet) to update the decorator visibility on states - change also the compartment visibility when the first internal transition is added to visible, and hide the compartment when the last transition is removed. The visibility can be enforced using the show / hide compartment. When no transition is there, you can still display the compartment, which will remain always visible, even if you add/remove transitions. The same when it is displayed, you can hide it, it wil lbe always hidden. Going to property view )=> style => Default style removes the forced behavior. Basic Junit tests are added to check compartment visiblity, they may be extended if some scenario is found to have a wrong behavior - It also updates some part of the code for quality => common classes in diagram.common plugin, remove usage of String replaced by constants, etc. - Removes a dependency introduced to e4.css in previous patches (incubation plugin, so it could cause some issues for getting out of incubation) Next steps before the bug could be considered as resolved: - evaluate the possibility and implement a static frame on the outside of the state frame, - creating an internal transition on a simple state should be possible, and turn the state into a composite state
Gerrit change https://git.eclipse.org/r/86696 was merged to [master]. Commit: http://git.eclipse.org/c/papyrus-rt/org.eclipse.papyrus-rt.git/commit/?id=459d7214108943f606eeb4046042dc9396395d2b
I tested the latest Papyrus-RT build a bit. The canonical display of the initial elements do seem to work as expected now (not mentioned below). (In reply to Remi Schnekenburger from comment #23) > - add a new label manager for the internal transitions: The displayed label > for an internal transition is now either the name if is set, or it is > computed from the list of protocol messages referenced by the triggers. Each > level of transition -> trigger -> call event -> operation (as a protocol > message) are notifying the label that is should update when changed. So the > name should react to any modificaiton (adding a new element, changing the > name of the protocol message, etc.). Some Junit tests should be added, and I > am wondering also what to indicate when '*' is selected. Currently nothing > is displayed. This naming principle looks like a nice feature. To be consistent I assume that we should have the same kind of principle also for the name label for other transition kinds (which have edges). In general we need to think about annotations for showing if the transitions actually have a trigger (in the case the user explicitly names the transition) and also effect and guard code. I do not think that we have a bug tracking this, which maybe we should have. I wonder about this comma separated list when you have multiple triggers. Bran proposed a notation with a + sign next to the name of the protocol message from the first trigger to avoid clutter when there are lots of triggers. But since we allow the user to explicitly name a transition, then the user can choose any ("aggregated") name for such transitions that gets a long comma separated list. So maybe it is good to have this comma separated list for the derived label as you have made it now. Regarding the label to use if the '*' AnyReceiveEvent is used for the trigger, I suggest to simply use '*' as the derived label. That would make the most sense. > - change also the compartment visibility when the first internal transition > is added to visible, and hide the compartment when the last transition is > removed. The visibility can be enforced using the show / hide compartment. > When no transition is there, you can still display the compartment, which > will remain always visible, even if you add/remove transitions. The same > when it is displayed, you can hide it, it wil lbe always hidden. Going to > property view )=> style => Default style removes the forced behavior. Basic > Junit tests are added to check compartment visiblity, they may be extended > if some scenario is found to have a wrong behavior The automatic display and hiding of the internal transitions compartment seem to work fine. It appears automatically when the first internal transitions is added, and hidden when the last internal transition is removed. But I am not fully sure I understand how the double-click on the internal transition annotation is supposed to work. If I add an internal transition and the compartment appears. If I then double click on the internal transition annotation in the top right corner, then the compartment is still visible. I can see though when having the Style tab visible in the properties view that a named style displayInternalTransitions gets applied/unapplied each time I double click. But it does not seem to have any effect on the diagram, the compartment is still always shown. So I am not fully sure what to expect here. Going to Filters > Show/Hide Compartments makes it possible to explicitly show and hide the compartment. But what about the double click? One thing that I wonder about, is the "embedded editor" for the transition (which in general causes some confusion with error markers in the model, as tracked by Bug 476365 (and where a similar effect can be seen in Bug 509268). I can see that when you add the first internal transition, this embedded editor (which I just find confusing, since the user expects to enter the transition name, but the grammar does not support only a name and thus you get the error marker) does not appear. It seem to only appear on the second (and following) internal transitions. As discussed in Bug 509268, should we replace this "embedded editor" from base Papyrus, with something else, e.g. the simple editor for a NamedElement so that you only allow the editing of the name. I am not sure in which direction we shall go here (and it probably should not be within the scope of this bug anyway). But this about it only being activated for the second (and following), and not the first internal transition feels a bit inconsistent. Maybe it has to do with the fact that the compartment is shown at this point, and thus the activation of the "embedded editor" comes to early (when the transition is not yet shown).
> This naming principle looks like a nice feature. To be consistent I assume that we should have the same kind of principle also for the name label for other transition kinds (which have edges). In general we need to think about annotations for showing if the transitions actually have a trigger (in the case the user explicitly names the transition) and also effect and guard code. I do not think that we have a bug tracking this, which maybe we should have. Can I let you write the bug around annotations for trigger / effect / guard? > I wonder about this comma separated list when you have multiple triggers. Bran proposed a notation with a + sign next to the name of the protocol message from the first trigger to avoid clutter when there are lots of triggers. But since we allow the user to explicitly name a transition, then the user can choose any ("aggregated") name for such transitions that gets a long comma separated list. So maybe it is good to have this comma separated list for the derived label as you have made it now. Any implementation (either each trigger or a '+' sign) is possible, and may be overriden with a specific label provider. I can for readability set the '+' sign at the end of the computer trigger name, and try to show in the tooltip the whole list of triggers. > Regarding the label to use if the '' AnyReceiveEvent is used for the trigger, I suggest to simply use '' as the derived label. That would make the most sense. The character seems missing between ''. Should we add a star '*'? > The automatic display and hiding of the internal transitions compartment seem to work fine. It appears automatically when the first internal transitions is added, and hidden when the last internal transition is removed. But I am not fully sure I understand how the double-click on the internal transition annotation is supposed to work. If I add an internal transition and the compartment appears. If I then double click on the internal transition annotation in the top right corner, then the compartment is still visible. I can see though when having the Style tab visible in the properties view that a named style displayInternalTransitions gets applied/unapplied each time I double click. But it does not seem to have any effect on the diagram, the compartment is still always shown. > So I am not fully sure what to expect here. Going to Filters > Show/Hide Compartments makes it possible to explicitly show and hide the compartment. But what about the double click? I missed the double click there, it is still relying on initial mechanism with a named style applied. I will propose a new patch for this one. > But this about it only being activated for the second (and following), and not the first internal transition feels a bit inconsistent. Maybe it has to do with the fact that the compartment is shown at this point, and thus the activation of the "embedded editor" comes to early (when the transition is not yet shown). This is the perfect analysis of the issue there ;) The compartment is displayed later on, after the transition is created. When first transition has been created => the compartment is shown => the label for the transition is created by canonical edit policy, not as a new created element. The direct edit policy is not activated here. I will try to find a solution here.
Created attachment 266090 [details] proposal for the label / tooltip for an internal transition with several triggers
Note that the tooltip may even be a bit more complex, with not only all protocol messages names, but also parameters, etc.
New Gerrit change created: https://git.eclipse.org/r/88145
Gerrit change https://git.eclipse.org/r/88145 was merged to [master]. Commit: http://git.eclipse.org/c/papyrus-rt/org.eclipse.papyrus-rt.git/commit/?id=d1cc24a419ea1dde28d7ba870c0e4c11b7f333e0
(In reply to Eclipse Genie from comment #30) > Gerrit change https://git.eclipse.org/r/88145 was merged to [master]. > Commit: > http://git.eclipse.org/c/papyrus-rt/org.eclipse.papyrus-rt.git/commit/ > ?id=d1cc24a419ea1dde28d7ba870c0e4c11b7f333e0 new patch added: - updating the double click mechanism on decoration annotation to handle the show/hide compartment properly - update label for internal transition (more than one trigger => <first name> +, and give a better tooltip - update label in Model explorer for triggers and events ('*' is displayed now for AnyReceivedEvent and trigger with an AnyReceivedEvent - tests updated accordingly - update tests to be using class rules rather than method rules for LabelProviderTest (faster tests, by avoiding to reload the editor after each test) Last issue is in the compartment initial display and the direct editor which is not triggered.
(In reply to Remi Schnekenburger from comment #26) > > This naming principle looks like a nice feature. To be consistent I > > assume that we should have the same kind of principle also for the name > > label for other transition kinds (which have edges). In general we need > > to think about annotations for showing if the transitions actually have > > a trigger (in the case the user explicitly names the transition) and > > also effect and guard code. I do not think that we have a bug tracking > > this, which maybe we should have. > > Can I let you write the bug around annotations for trigger / effect / guard? I can write such a more general bug. Bran have provided some input in his user experience document regarding this, and we always have the legacy tooling to align with. > > > I wonder about this comma separated list when you have multiple > > triggers. Bran proposed a notation with a + sign next to the name of the > > protocol message from the first trigger to avoid clutter when there are > > lots of triggers. But since we allow the user to explicitly name a > > transition, then the user can choose any ("aggregated") name for such > > transitions that gets a long comma separated list. So maybe it is good > > to have this comma separated list for the derived label as you have made > > it now. > > Any implementation (either each trigger or a '+' sign) is possible, and may > be overriden with a specific label provider. > I can for readability set the '+' sign at the end of the computer trigger > name, and try to show in the tooltip the whole list of triggers. When checking the latest merged change, I think that the '+' sign approach looks good. The user can always re-order the list of triggers to makes sure that the most relevant is the first. And the user can always give the transition an explicit name if an "aggregated" names is wanted. Let's go with that. Then the same principle shall of course be used for the name label for ordinary edge based transitions as well. I will add that to the general bug regarding annotations for trigger/effect/guard. > > > Regarding the label to use if the '' AnyReceiveEvent is used for the > > trigger, I suggest to simply use '' as the derived label. That would make > > the most sense. > The character seems missing between ''. Should we add a star '*'? Yes, that should have been '*'. Something seem to have happened when you quoted my response. If you check my original Comment 25 the * is there... I checked the latest merged change and the '*' is shown as expected. This is then also aligned with how '*' is being displayed in the trigger table as well as in the trigger selection dialog.
I've tested the latest merged change. (In reply to Remi Schnekenburger from comment #31) > new patch added: > > - updating the double click mechanism on decoration annotation to handle > the show/hide compartment properly Yes, this now works as one would expected! > - update label for internal transition (more than one trigger => <first > name> +, and give a better tooltip Looks good. Let's go for the '+' instead of the comma separated list. > - update label in Model explorer for triggers and events ('*' is > displayed now for AnyReceivedEvent and trigger with an AnyReceivedEvent > - tests updated accordingly > - update tests to be using class rules rather than method rules for > LabelProviderTest (faster tests, by avoiding to reload the editor after > each test) > > Last issue is in the compartment initial display and the direct editor which > is not triggered. Regarding the direct editor, I guess we at the same time should replace it with the simple "NamedElement" editor, to avoid the issues in Bug 476365. I also assume that this was not an exhaustive list of remaining issues, but just to recap what I can see is still missing: 1) Creating the first internal transition for a simple state. The user should not be required to (manually) make the state into a composite state. Creating the first internal transitions should automatically convert it into a composite state. This implies that the modeling assistant should have the "Add Internal Transition" tool also for a simple state (and the current "Add Region Shape" should be removed since the user should not really have the possibility of explicitly creating a region). 2) Moving the list of internal transitions on the inside of the composite state to be located below the state frame (to align with the latest feature in the legacy tooling and avoid the cluttering when entry/exit points are located at the bottom border of the state frame). 3) Making the tools on the modeling assistant also available as a "new child" menu on the right click context menu, so that you can create an internal transition by right clicking on the simple/composite state as well.
(In reply to Peter Cigehn from comment #33) > I've tested the latest merged change. > > (In reply to Remi Schnekenburger from comment #31) > > new patch added: > > > > - updating the double click mechanism on decoration annotation to handle > > the show/hide compartment properly > > Yes, this now works as one would expected! > > > - update label for internal transition (more than one trigger => <first > > name> +, and give a better tooltip > > Looks good. Let's go for the '+' instead of the comma separated list. > > > - update label in Model explorer for triggers and events ('*' is > > displayed now for AnyReceivedEvent and trigger with an AnyReceivedEvent > > - tests updated accordingly > > - update tests to be using class rules rather than method rules for > > LabelProviderTest (faster tests, by avoiding to reload the editor after > > each test) > > > > Last issue is in the compartment initial display and the direct editor which > > is not triggered. > > Regarding the direct editor, I guess we at the same time should replace it > with the simple "NamedElement" editor, to avoid the issues in Bug 476365. > > I also assume that this was not an exhaustive list of remaining issues, but > just to recap what I can see is still missing: > > 1) Creating the first internal transition for a simple state. The user > should not be required to (manually) make the state into a composite state. > Creating the first internal transitions should automatically convert it into > a composite state. This implies that the modeling assistant should have the > "Add Internal Transition" tool also for a simple state (and the current "Add > Region Shape" should be removed since the user should not really have the > possibility of explicitly creating a region). > > 2) Moving the list of internal transitions on the inside of the composite > state to be located below the state frame (to align with the latest feature > in the legacy tooling and avoid the cluttering when entry/exit points are > located at the bottom border of the state frame). > > 3) Making the tools on the modeling assistant also available as a "new > child" menu on the right click context menu, so that you can create an > internal transition by right clicking on the simple/composite state as well. Gerrit 88496 was merged to master, dealing with the creation and edition of the first internal transition (when the compartment has to be shown before creating internal transition) Next steps are the direct editor (which should be the simple one) and the creation of internal transition when the state is not a composite state yet (we should turn the state into composite, and then create the transition)
adding link to the commit of gerrit 88496: https://git.eclipse.org/c/papyrus-rt/org.eclipse.papyrus-rt.git/commit/?id=69faa0e599fc702e0d2f4d453ee81f0467ec4a27
(In reply to Remi Schnekenburger from comment #34) > (In reply to Peter Cigehn from comment #33) > > I've tested the latest merged change. > > > > (In reply to Remi Schnekenburger from comment #31) > > > new patch added: > > > > > > - updating the double click mechanism on decoration annotation to handle > > > the show/hide compartment properly > > > > Yes, this now works as one would expected! > > > > > - update label for internal transition (more than one trigger => <first > > > name> +, and give a better tooltip > > > > Looks good. Let's go for the '+' instead of the comma separated list. > > > > > - update label in Model explorer for triggers and events ('*' is > > > displayed now for AnyReceivedEvent and trigger with an AnyReceivedEvent > > > - tests updated accordingly > > > - update tests to be using class rules rather than method rules for > > > LabelProviderTest (faster tests, by avoiding to reload the editor after > > > each test) > > > > > > Last issue is in the compartment initial display and the direct editor which > > > is not triggered. > > > > Regarding the direct editor, I guess we at the same time should replace it > > with the simple "NamedElement" editor, to avoid the issues in Bug 476365. > > > > I also assume that this was not an exhaustive list of remaining issues, but > > just to recap what I can see is still missing: > > > > 1) Creating the first internal transition for a simple state. The user > > should not be required to (manually) make the state into a composite state. > > Creating the first internal transitions should automatically convert it into > > a composite state. This implies that the modeling assistant should have the > > "Add Internal Transition" tool also for a simple state (and the current "Add > > Region Shape" should be removed since the user should not really have the > > possibility of explicitly creating a region). > > > > 2) Moving the list of internal transitions on the inside of the composite > > state to be located below the state frame (to align with the latest feature > > in the legacy tooling and avoid the cluttering when entry/exit points are > > located at the bottom border of the state frame). > > > > 3) Making the tools on the modeling assistant also available as a "new > > child" menu on the right click context menu, so that you can create an > > internal transition by right clicking on the simple/composite state as well. > > Gerrit 88496 was merged to master, dealing with the creation and edition of > the first internal transition (when the compartment has to be shown before > creating internal transition) > Next steps are the direct editor (which should be the simple one) and the > creation of internal transition when the state is not a composite state yet > (we should turn the state into composite, and then create the transition) Gerrit 88541, merged on master, proposes a simple direct editor for transition, only for the edition of the name, single line. Warning when testing, preferences stored in the runtime environment may be still targeting the advanced transition editor => I reseted the preferences in my test workspace (delete entry in core.resources in metadata area...). This reset should not be necessary when running the tool in a fresh environment.
(In reply to Remi Schnekenburger from comment #36) > Gerrit 88541, merged on master, proposes a simple direct editor for > transition, only for the edition of the name, single line. Warning when > testing, preferences stored in the runtime environment may be still > targeting the advanced transition editor => I reseted the preferences in my > test workspace (delete entry in core.resources in metadata area...). This > reset should not be necessary when running the tool in a fresh environment. I tested the latest Papyrus-RT build (which I still have based on the latest Papyrus Neon nightly build) and the direct editor for internal transitions seem to work fine, including the first internal transition. There are though a few issues I can see: 1) Not sure if it is related at all to this work or not, but I just saw some new warnings in the error log . Each time I add a new state (including the case with the automatically created first state when you create a new state machine), generates two warnings in the error log (no stack trace): org.eclipse.papyrus.infra.gmfdiag.commands Warning Thu Jan 12 14:02:21 CET 2017 REMOVE MANY not yet implemented org.eclipse.papyrus.infra.gmfdiag.commands Warning Thu Jan 12 14:02:21 CET 2017 ADD MANY not yet implemented Seem to be coming from base Papyrus. 2) The simple name-based direct editor for ordinary (edge based) transitions, seem to misbehave a bit. Yes, you can now simply type a name (without getting the error marker as described in Bug 476365). But instead of assigning the name to the transition, there is now a Guard named t1 created instead. The transition did not get a name. And the transition label looks like '[t1 (no spec)]'. This also only happens for the first transition you create. For the second (and consecutive) transition, nothing happens at all when you type the name in the direct editor. It also looks like the name gets "cut off" (even a short name like 't1' is not even shown completely in the direct editor). It looks like the direct editor has gone into some strange state where it no longer works (only after restarting Papyrus-RT you get it to work again for one transition).
(In reply to Peter Cigehn from comment #37) > (In reply to Remi Schnekenburger from comment #36) > > Gerrit 88541, merged on master, proposes a simple direct editor for > > transition, only for the edition of the name, single line. Warning when > > testing, preferences stored in the runtime environment may be still > > targeting the advanced transition editor => I reseted the preferences in my > > test workspace (delete entry in core.resources in metadata area...). This > > reset should not be necessary when running the tool in a fresh environment. > > I tested the latest Papyrus-RT build (which I still have based on the latest > Papyrus Neon nightly build) and the direct editor for internal transitions > seem to work fine, including the first internal transition. > > There are though a few issues I can see: > > 1) Not sure if it is related at all to this work or not, but I just saw some > new warnings in the error log . Each time I add a new state (including the > case with the automatically created first state when you create a new state > machine), generates two warnings in the error log (no stack trace): > > org.eclipse.papyrus.infra.gmfdiag.commands > Warning > Thu Jan 12 14:02:21 CET 2017 > REMOVE MANY not yet implemented > > org.eclipse.papyrus.infra.gmfdiag.commands > Warning > Thu Jan 12 14:02:21 CET 2017 > ADD MANY not yet implemented > > Seem to be coming from base Papyrus. > > 2) The simple name-based direct editor for ordinary (edge based) > transitions, seem to misbehave a bit. Yes, you can now simply type a name > (without getting the error marker as described in Bug 476365). But instead > of assigning the name to the transition, there is now a Guard named t1 > created instead. The transition did not get a name. And the transition label > looks like '[t1 (no spec)]'. This also only happens for the first transition > you create. For the second (and consecutive) transition, nothing happens at > all when you type the name in the direct editor. It also looks like the name > gets "cut off" (even a short name like 't1' is not even shown completely in > the direct editor). It looks like the direct editor has gone into some > strange state where it no longer works (only after restarting Papyrus-RT you > get it to work again for one transition). Thanks for testing and feedback, Peter. I will have a look to those issues.
For point 1, it comes fortunately from Papyrus-RT. I was unsure this behavior would come, but this is the case ;) I will handle the remove/add many cases. That should be an easy fix. classes to modify: InternalTransitionsCompartmentEditPart.java & RTStateEditPart.java For point 2, it comes from the GMF parser from State Machine in Papyrus, more exactly from class org.eclipse.papyrus.uml.diagram.statemachine.custom.parsers.TransitionPropertiesParser. Goal of this class is to provide an advanced and complex display string and parser for the label of the transition. What would be the rules for the display of transition label in Papyrus-RT? The same as internal transitions, i.e. the name if set, or the protocol message associated to the call event link to the trigger? If yes, I will have to override the TransitionParser (which is rather simple, thanks to Parser extension point of GMF)
(In reply to Remi Schnekenburger from comment #39) > For point 2, it comes from the GMF parser from State Machine in Papyrus, > more exactly from class > org.eclipse.papyrus.uml.diagram.statemachine.custom.parsers. > TransitionPropertiesParser. Goal of this class is to provide an advanced and > complex display string and parser for the label of the transition. > What would be the rules for the display of transition label in Papyrus-RT? > The same as internal transitions, i.e. the name if set, or the protocol > message associated to the call event link to the trigger? If yes, I will > have to override the TransitionParser (which is rather simple, thanks to > Parser extension point of GMF) I was planning on detailing that in the general bug about annotations for trigger, guard and effect that was discussed in comment 32 and include also the aspect of the actual name label. But yes, I had foreseen that we should be consistent with the label for internal transitions, i.e. if the name is set use that, otherwise if the name of the transitions is not given, pick the name of the protocol message from the first trigger and add a '+' in case of multiple triggers. This is also what Bran has proposed in this user experience document. I have not yet had the time yet to write that bug though.
New Gerrit change created: https://git.eclipse.org/r/88572
Previous gerrit addresses comment 37 - first aspect about warning in the console. For 2nd aspect, and according to the comment, I should leave it to the resolution of the new bug, once it is written. You are really doing an impressive job of reviewing / documenting, Peter :) I can write the bug if that helps (especially as it is the same behavior as internal transitions)?
(In reply to Remi Schnekenburger from comment #42) > Previous gerrit addresses comment 37 - first aspect about warning in the > console. > For 2nd aspect, and according to the comment, I should leave it to the > resolution of the new bug, once it is written. > > You are really doing an impressive job of reviewing / documenting, Peter :) > I can write the bug if that helps (especially as it is the same behavior as > internal transitions)? This is even stranger than I thought. The transition can display 2 different labels, one for the name, another one for the guards. If the name of the transition is edited through property view, the name is correctly displayed in its own label, that can be edited when selected using F2. However, the target for the direct edit of the transition (i.e. when transition is selected and F2 is pressed) is the label for the guard/effects display. Interesting...
Gerrit change https://git.eclipse.org/r/88572 was merged to [master]. Commit: http://git.eclipse.org/c/papyrus-rt/org.eclipse.papyrus-rt.git/commit/?id=ceefdc56e603157a7792249ddc64ee5b3f0574fb
(In reply to Remi Schnekenburger from comment #42) > You are really doing an impressive job of reviewing / documenting, Peter :) > I can write the bug if that helps (especially as it is the same behavior as > internal transitions)? I can see if I can spend some time to write that bug tomorrow. I was basically planning on on simply try to align with the legacy tooling and the annotations used there. By default the legacy tooling only displays the transition name, and the guard and effect is not shown (this normally just makes things cluttered). The existence of effect and guard code is shown with graphical annotations on the transitions, over which you can hover and get the code displayed in a tool tip. The non-existence of triggers is shown with a separate graphical annotation. Since we have this approach of an automatic name label based on the trigger we could skip this one. But on the other hand, if the user explicitly names the transitions, then the missing trigger is "hidden" and maybe such a annotation is still useful. But I can hopefully put something together tomorrow.
(In reply to Remi Schnekenburger from comment #43) > This is even stranger than I thought. The transition can display 2 different > labels, one for the name, another one for the guards. If the name of the > transition is edited through property view, the name is correctly displayed > in its own label, that can be edited when selected using F2. However, the > target for the direct edit of the transition (i.e. when transition is > selected and F2 is pressed) is the label for the guard/effects display. > Interesting... Yes, I have seen that the transition have two separate labels, one for the name, and one for guard, and effect. In the legacy tooling there is only one label for the name, which optionally also can show the guard and effect according to the same principle with '[guard]/effect' as defined in the UML specification. But this is normally not used, and this preference to show guard and effect is by default off, and only the name is normally shown. To inspect the guard and effect code, as mentioned in previous comment, you can hover the graphical annotations on the transition and they appear in a tool tip. The of course we have the code snippet view for inspecting the code, as tracked by Bug 494288. But this then explains why a guard was created with what I would have expected to be the name of the transition. It would have been more natural if selecting the transition and then press F2 would activate the editor for the name label, and not the guard/effect label. Especially in the context of Papyrus-RT where we are expected to edit the guard and effect using the code snippet view as explained in Bug 494288.
New Gerrit change created: https://git.eclipse.org/r/88580
(In reply to comment #46) > (In reply to Remi Schnekenburger from comment #43) > > This is even stranger than I thought. The transition can display 2 different > > labels, one for the name, another one for the guards. If the name of the > > transition is edited through property view, the name is correctly displayed > > in its own label, that can be edited when selected using F2. However, the > > target for the direct edit of the transition (i.e. when transition is > > selected and F2 is pressed) is the label for the guard/effects display. > > Interesting... > > Yes, I have seen that the transition have two separate labels, one for the name, > and one for guard, and effect. In the legacy tooling there is only one label for > the name, which optionally also can show the guard and effect according to the > same principle with '[guard]/effect' as defined in the UML specification. But > this is normally not used, and this preference to show guard and effect is by > default off, and only the name is normally shown. To inspect the guard and > effect code, as mentioned in previous comment, you can hover the graphical > annotations on the transition and they appear in a tool tip. The of course we > have the code snippet view for inspecting the code, as tracked by Bug 494288. > > But this then explains why a guard was created with what I would have expected > to be the name of the transition. It would have been more natural if selecting > the transition and then press F2 would activate the editor for the name label, > and not the guard/effect label. Especially in the context of Papyrus-RT where we > are expected to edit the guard and effect using the code snippet view as > explained in Bug 494288. I did not find very user friendly to edit the gaurd/effect label rather the name. Maybe the rationale behind that is that the name is not the most important information about it? However, this may be possible to override that behavior in Papyrus-RT and select the name label when pressing F2 while transition is selected. And css may hide the guard/effect label by default.
A new patch has been pushed in gerrit, there were some inconsistencies in the internal transition creation in compartment, depending on its visiblity.
Gerrit change https://git.eclipse.org/r/88580 was merged to [master]. Commit: http://git.eclipse.org/c/papyrus-rt/org.eclipse.papyrus-rt.git/commit/?id=8b7b28070d48c4a4217c70460dc612eb46f761b0
(In reply to Peter Cigehn from comment #45) > (In reply to Remi Schnekenburger from comment #42) > > You are really doing an impressive job of reviewing / documenting, Peter :) > > I can write the bug if that helps (especially as it is the same behavior as > > internal transitions)? > > I can see if I can spend some time to write that bug tomorrow. I was > basically planning on on simply try to align with the legacy tooling and the > annotations used there. By default the legacy tooling only displays the > transition name, and the guard and effect is not shown (this normally just > makes things cluttered). The existence of effect and guard code is shown > with graphical annotations on the transitions, over which you can hover and > get the code displayed in a tool tip. The non-existence of triggers is shown > with a separate graphical annotation. Since we have this approach of an > automatic name label based on the trigger we could skip this one. But on the > other hand, if the user explicitly names the transitions, then the missing > trigger is "hidden" and maybe such a annotation is still useful. > > But I can hopefully put something together tomorrow. I wrote Bug 510411 to track these improvements for ordinary, edge based, transitions.
Not sure if it fits into this bugzilla but this breaks when extending a (UML-RT) StateMachine Diagram (though a viewpoint) since it seems that the functionality to create a composite state assumes that the diagram kind name is "UML-RT State Machine Diagram". See org.eclipse.papyrusrt.umlrt.tooling.diagram.common.utils.UMLRTStateMachineDiagramUtils.createStateMachineDiagram()
(In reply to Patrik Nandorf from comment #52) > Not sure if it fits into this bugzilla but this breaks when extending a > (UML-RT) StateMachine Diagram (though a viewpoint) since it seems that the > functionality to create a composite state assumes that the diagram kind name > is "UML-RT State Machine Diagram". > > See > org.eclipse.papyrusrt.umlrt.tooling.diagram.common.utils. > UMLRTStateMachineDiagramUtils.createStateMachineDiagram() Patrik, I am not sure to understand the relation here with the internal transition?
Sorry for not being clear. When adding a internal transition for the first time to a state it has to be made into a composite state right? When you do that you get the popup to ask you if you want to do that and if you select yes, the command that is created and eventually executed (CreateNestedStateMachineDiagramCommand I think) tries to create a diagram using Diagram result = UMLRTStateMachineDiagramUtils.createStateMachineDiagram(state, null); which fails since (if I get the code right) is assumes that the PapyrusView is a StateMachineUtils.UMLRT_STATE_MACHINE_DIAGRAM and not the custom/extended view that I have. Hopes this clarifies what I meant.
(In reply to Patrik Nandorf from comment #54) > Sorry for not being clear. > > When adding a internal transition for the first time to a state it has to be > made into a composite state right? > > When you do that you get the popup to ask you if you want to do that and if > you select yes, the command that is created and eventually executed > (CreateNestedStateMachineDiagramCommand I think) tries to create a diagram > using Diagram result = > UMLRTStateMachineDiagramUtils.createStateMachineDiagram(state, null); which > fails since (if I get the code right) is assumes that the PapyrusView is a > StateMachineUtils.UMLRT_STATE_MACHINE_DIAGRAM and not the custom/extended > view that I have. > > Hopes this clarifies what I meant. Oh, got it! Turning the state into composite state is not working yet in base Papyrus-RT also ;) I am currently working on it. But I wonder how we can handle the case of a specific view for composite states, that's a good point
(In reply to Remi Schnekenburger from comment #55) > (In reply to Patrik Nandorf from comment #54) > > When adding a internal transition for the first time to a state it has to be > > made into a composite state right? > > > > Oh, got it! Turning the state into composite state is not working yet in > base Papyrus-RT also ;) I am currently working on it. Just to clarify: The intention is that the when adding an internal transition to a simple state, the conversion to a composite state should be made automatically. See Comment 1 related to this. As Rémi, says, this is still "work in progress". So for now, it is just a "workaround" that you manually have to make the simple state into a composite state "manually" by double-clicking on it, before you can add an internal transition to it. But yes, in both cases, i.e. both the case when double clicking on a state and when creating an internal transition, the creation of the nested state machine diagram needs to consider that this diagram should be of the same view type as the diagram from where you "coming from". I guess it should not "hard code" the view type, but instead pick up the view type from current diagram, to be able to handle that it has been customized/extended to another view type (as Patrik is doing).
(In reply to Peter Cigehn from comment #56) > (In reply to Remi Schnekenburger from comment #55) > > (In reply to Patrik Nandorf from comment #54) > > > When adding a internal transition for the first time to a state it has to be > > > made into a composite state right? > > > > > > > Oh, got it! Turning the state into composite state is not working yet in > > base Papyrus-RT also ;) I am currently working on it. > > Just to clarify: The intention is that the when adding an internal > transition to a simple state, the conversion to a composite state should be > made automatically. See Comment 1 related to this. As Rémi, says, this is > still "work in progress". So for now, it is just a "workaround" that you > manually have to make the simple state into a composite state "manually" by > double-clicking on it, before you can add an internal transition to it. > > But yes, in both cases, i.e. both the case when double clicking on a state > and when creating an internal transition, the creation of the nested state > machine diagram needs to consider that this diagram should be of the same > view type as the diagram from where you "coming from". I guess it should not > "hard code" the view type, but instead pick up the view type from current > diagram, to be able to handle that it has been customized/extended to > another view type (as Patrik is doing). Thanks for clarification, again!
Ok, I can 'hack' this by calling my own SMD "UML-RT State Machine Diagram" but it is obviously not a viable solution but it allowed me to test further. The next problem is that the assistant doesn't show the internal transition popup. However, looking at the definition of the popup assistant I can't see why not.
New Gerrit change created: https://git.eclipse.org/r/88924
Gerrit change https://git.eclipse.org/r/88924 was merged to [master]. Commit: http://git.eclipse.org/c/papyrus-rt/org.eclipse.papyrus-rt.git/commit/?id=32ec72307807fa9b2e54a37f6edbd66cae039ee7
New Gerrit change created: https://git.eclipse.org/r/89329
Gerrit change https://git.eclipse.org/r/89329 was merged to [master]. Commit: http://git.eclipse.org/c/papyrus-rt/org.eclipse.papyrus-rt.git/commit/?id=350755b0aaad2cfeca0064396d852639981d7d9b
Bug on undo redo has been fixed on papyrus (master & maintenance/neon). Dev environment should be updated to latest nightly to get this fix. This caused some tests (18) to be failing, this has also been fixed. The latest patches on that stream fix several issues: - remove the modeling assistant for region on state. Only double click on state or creating an internal transition turns the state into a composite one - kind for diagram associated to a RTState is now chosen given the kind of the "parent" state machine or state, to allow for further customization - 2 failing tests have been annotated, while bug 510782 is not fixed.
New Gerrit change created: https://git.eclipse.org/r/89335
Gerrit change https://git.eclipse.org/r/89335 was merged to [master]. Commit: http://git.eclipse.org/c/papyrus-rt/org.eclipse.papyrus-rt.git/commit/?id=b4746a2ba923a2625c208a21ec467fbc6b35044c
New Gerrit change created: https://git.eclipse.org/r/89342
(In reply to Remi Schnekenburger from comment #63) > Bug on undo redo has been fixed on papyrus (master & maintenance/neon). Dev > environment should be updated to latest nightly to get this fix. > This caused some tests (18) to be failing, this has also been fixed. > > The latest patches on that stream fix several issues: > - remove the modeling assistant for region on state. Only double click on > state or creating an internal transition turns the state into a composite one > - kind for diagram associated to a RTState is now chosen given the kind of > the "parent" state machine or state, to allow for further customization > - 2 failing tests have been annotated, while bug 510782 is not fixed. I've tested the latest build of Papyrus-RT, with what I believe should be the latest build of Papyrus. The modeling assistant looks good, no possibility of creating a region. The director editor updates the name of the transitions when using F2. The size of the state shape resizes when the typing a long internal transition name. It all looks good. But I stumbled upon an issue with undo, which I am not sure if it is related to those changes made in Papyrus or not. But if you simply create a state and then undo the creation, then you get an error dialog popping up stating something about "index = 1, size = 0", and exceptions are logged in the error log. I can see other issue when testing some undo/redo scenarios, but since this one seem so basic, I don't know if they simple are consequences of this one or not.
New Gerrit change created: https://git.eclipse.org/r/89862
Gerrit change https://git.eclipse.org/r/89342 was merged to [master]. Commit: http://git.eclipse.org/c/papyrus-rt/org.eclipse.papyrus-rt.git/commit/?id=471a5b4d1f7f5cd43cd533152fc96b61bb928084
As shown in gerrit 89862, there is indeed an undo issue with creation of state. It took me some times to understand why the undo/redo was failing. This may be due to Expansion model and the way it adds elements on a GMF unsafe transaction. The new bug 511395 has been created for that issue, which may be a relay bug to the Papyrus bug 511394 on expansion, if the error comes really from this framework. As a temporary solution, I can provide a view provider that forces the creation of the compartment for internal transitions rather than depending on the InducedCreationEditPolicy. The fix will be made under the bug 511395. What is left for this bug is (from comment https://bugs.eclipse.org/bugs/show_bug.cgi?id=494287#c33) 2) Moving the list of internal transitions on the inside of the composite state to be located below the state frame (to align with the latest feature in the legacy tooling and avoid the cluttering when entry/exit points are located at the bottom border of the state frame). 3) Making the tools on the modeling assistant also available as a "new child" menu on the right click context menu, so that you can create an internal transition by right clicking on the simple/composite state as well.
(In reply to Remi Schnekenburger from comment #70) > What is left for this bug is (from comment > https://bugs.eclipse.org/bugs/show_bug.cgi?id=494287#c33) > 2) Moving the list of internal transitions on the inside of the composite > state to be located below the state frame (to align with the latest feature > in the legacy tooling and avoid the cluttering when entry/exit points are > located at the bottom border of the state frame). What is the current status of this one? I was just wondering if this maybe is something that we can improve later? The current list, as a compartment inside the state frame, might be "good enough" for the 0.9 release, and that we reduce the scope and write a separate bug for this improvement of placing the internal transition list outside, and below, the state frame itself which we either could plan for 1.0, or move to Future. If you already have started, and maybe feel that it is not much work left to be done, then maybe we keep in this bug and in 0.9. I just feel that we have some other aspects that probably are much more relevant to get into 0.9, e.g. Bug 494284 and the handling of automatically creating entry/exit points. > 3) Making the tools on the modeling assistant also available as a "new > child" menu on the right click context menu, so that you can create an > internal transition by right clicking on the simple/composite state as well. The same reasoning we could have for this one, even though I feel that it would be really nice to have this "new child" menu in place for 0.9. This one probably should be a generic feature, so that whenever we add/remove/update the modeling assistant, we have the corresponding "new child" menu updated correspondingly whenever we right-click an element in a diagram which have a corresponding modeling assistant defined. Any comments regarding "scope reduction" and writing separate bugs for these two remaining aspects?
(In reply to Peter Cigehn from comment #71) > (In reply to Remi Schnekenburger from comment #70) > > What is left for this bug is (from comment > > https://bugs.eclipse.org/bugs/show_bug.cgi?id=494287#c33) > > 2) Moving the list of internal transitions on the inside of the composite > > state to be located below the state frame (to align with the latest feature > > in the legacy tooling and avoid the cluttering when entry/exit points are > > located at the bottom border of the state frame). > > What is the current status of this one? I was just wondering if this maybe > is something that we can improve later? The current list, as a compartment > inside the state frame, might be "good enough" for the 0.9 release, and that > we reduce the scope and write a separate bug for this improvement of placing > the internal transition list outside, and below, the state frame itself > which we either could plan for 1.0, or move to Future. If you already have > started, and maybe feel that it is not much work left to be done, then maybe > we keep in this bug and in 0.9. I just feel that we have some other aspects > that probably are much more relevant to get into 0.9, e.g. Bug 494284 and > the handling of automatically creating entry/exit points. +1 It would also be interesting if the "internal transition list outside, and below" the state frame could be a dynamic drawer. > > > 3) Making the tools on the modeling assistant also available as a "new > > child" menu on the right click context menu, so that you can create an > > internal transition by right clicking on the simple/composite state as well. > > The same reasoning we could have for this one, even though I feel that it > would be really nice to have this "new child" menu in place for 0.9. This > one probably should be a generic feature, so that whenever we > add/remove/update the modeling assistant, we have the corresponding "new > child" menu updated correspondingly whenever we right-click an element in a > diagram which have a corresponding modeling assistant defined. I would prefer to have that one in 0.9. Right-click is such an intuitive way of doing such an action > > Any comments regarding "scope reduction" and writing separate bugs for these > two remaining aspects?
(In reply to Charles Rivet from comment #72) > (In reply to Peter Cigehn from comment #71) > > (In reply to Remi Schnekenburger from comment #70) > > > What is left for this bug is (from comment > > > https://bugs.eclipse.org/bugs/show_bug.cgi?id=494287#c33) > > > 2) Moving the list of internal transitions on the inside of the composite > > > state to be located below the state frame (to align with the latest feature > > > in the legacy tooling and avoid the cluttering when entry/exit points are > > > located at the bottom border of the state frame). > > > > What is the current status of this one? I was just wondering if this maybe > > is something that we can improve later? The current list, as a compartment > > inside the state frame, might be "good enough" for the 0.9 release, and that > > we reduce the scope and write a separate bug for this improvement of placing > > the internal transition list outside, and below, the state frame itself > > which we either could plan for 1.0, or move to Future. If you already have > > started, and maybe feel that it is not much work left to be done, then maybe > > we keep in this bug and in 0.9. I just feel that we have some other aspects > > that probably are much more relevant to get into 0.9, e.g. Bug 494284 and > > the handling of automatically creating entry/exit points. > > +1 > It would also be interesting if the "internal transition list outside, and > below" the state frame could be a dynamic drawer. Good that you bring that one in the discussion! I was indeed going to switch to 494284, as this seems to be much more critical than the compartment outside for internal transitions. I even not started to work on this, working for examples for labels on transition. Let's in this case postpone that to version 1.0. The feature for internal transition is already supported for 0.9, this should be possible to work on models, or import them from legacy tooling. The only thing I would ask here is the behavior of Papyrus-RT when importing models containing state representation with the outside compartment, and if something should be done on model import to make the intern compartment visible or not. What do you think on that point? > > > 3) Making the tools on the modeling assistant also available as a "new > > > child" menu on the right click context menu, so that you can create an > > > internal transition by right clicking on the simple/composite state as well. > > > > The same reasoning we could have for this one, even though I feel that it > > would be really nice to have this "new child" menu in place for 0.9. This > > one probably should be a generic feature, so that whenever we > > add/remove/update the modeling assistant, we have the corresponding "new > > child" menu updated correspondingly whenever we right-click an element in a > > diagram which have a corresponding modeling assistant defined. > > I would prefer to have that one in 0.9. Right-click is such an intuitive way > of doing such an action > > > > > Any comments regarding "scope reduction" and writing separate bugs for these > > two remaining aspects? Right clicking on the element and re-using the content of modeling assistants should be a possible generic feature for Papyrus(-RT). I will try to push it for 0.9, but again, I will prioritize it to a lower level than the other one on hierarchical state machines currently opened for 0.9. This task is also a good candidate for someone working on the tooling, as Young-soo or Asma?
(In reply to Remi Schnekenburger from comment #73) > (In reply to Charles Rivet from comment #72) > > (In reply to Peter Cigehn from comment #71) > > > (In reply to Remi Schnekenburger from comment #70) > > > > What is left for this bug is (from comment > > > > https://bugs.eclipse.org/bugs/show_bug.cgi?id=494287#c33) > > > > 2) Moving the list of internal transitions on the inside of the composite > > > > state to be located below the state frame (to align with the latest feature > > > > in the legacy tooling and avoid the cluttering when entry/exit points are > > > > located at the bottom border of the state frame). > > > > > > What is the current status of this one? I was just wondering if this maybe > > > is something that we can improve later? The current list, as a compartment > > > inside the state frame, might be "good enough" for the 0.9 release, and that > > > we reduce the scope and write a separate bug for this improvement of placing > > > the internal transition list outside, and below, the state frame itself > > > which we either could plan for 1.0, or move to Future. If you already have > > > started, and maybe feel that it is not much work left to be done, then maybe > > > we keep in this bug and in 0.9. I just feel that we have some other aspects > > > that probably are much more relevant to get into 0.9, e.g. Bug 494284 and > > > the handling of automatically creating entry/exit points. > > > > +1 > > It would also be interesting if the "internal transition list outside, and > > below" the state frame could be a dynamic drawer. > > Good that you bring that one in the discussion! I was indeed going to switch > to 494284, as this seems to be much more critical than the compartment > outside for internal transitions. I even not started to work on this, > working for examples for labels on transition. Let's in this case postpone > that to version 1.0. The feature for internal transition is already > supported for 0.9, this should be possible to work on models, or import them > from legacy tooling. The only thing I would ask here is the behavior of > Papyrus-RT when importing models containing state representation with the > outside compartment, and if something should be done on model import to make > the intern compartment visible or not. What do you think on that point? Agreed. +1 > > > > > 3) Making the tools on the modeling assistant also available as a "new > > > > child" menu on the right click context menu, so that you can create an > > > > internal transition by right clicking on the simple/composite state as well. > > > > > > The same reasoning we could have for this one, even though I feel that it > > > would be really nice to have this "new child" menu in place for 0.9. This > > > one probably should be a generic feature, so that whenever we > > > add/remove/update the modeling assistant, we have the corresponding "new > > > child" menu updated correspondingly whenever we right-click an element in a > > > diagram which have a corresponding modeling assistant defined. > > > > I would prefer to have that one in 0.9. Right-click is such an intuitive way > > of doing such an action > > > > > > > > Any comments regarding "scope reduction" and writing separate bugs for these > > > two remaining aspects? > > Right clicking on the element and re-using the content of modeling > assistants should be a possible generic feature for Papyrus(-RT). I will try > to push it for 0.9, but again, I will prioritize it to a lower level than > the other one on hierarchical state machines currently opened for 0.9. This > task is also a good candidate for someone working on the tooling, as > Young-soo or Asma? Agreed, +1
(In reply to Remi Schnekenburger from comment #73) > Good that you bring that one in the discussion! I was indeed going to switch > to 494284, as this seems to be much more critical than the compartment > outside for internal transitions. I even not started to work on this, > working for examples for labels on transition. Let's in this case postpone > that to version 1.0. The feature for internal transition is already > supported for 0.9, this should be possible to work on models, or import them > from legacy tooling. The only thing I would ask here is the behavior of > Papyrus-RT when importing models containing state representation with the > outside compartment, and if something should be done on model import to make > the intern compartment visible or not. What do you think on that point? Considering that there are so many other layout issues with importing state-machine diagrams (as discussed on the developer mailing list), I don't think that we need to do anything specific about this in the import tool for 0.9. I guess that the user can always manually hide the internal transition compartment, if they cause clutter. > > Right clicking on the element and re-using the content of modeling > assistants should be a possible generic feature for Papyrus(-RT). I will try > to push it for 0.9, but again, I will prioritize it to a lower level than > the other one on hierarchical state machines currently opened for 0.9. This > task is also a good candidate for someone working on the tooling, as > Young-soo or Asma? Okay, this sounds good. I think it is best if we then write two new bugs, one regarding improving the list of internal transitions to be located outside the state frame (possibly being a dynamic drawer) to align with legacy tooling, and one for making the contents of the modeling assistant available on a "new child" menu when right clicking on a element in a diagram (which then should tried to made generic, not only covering the creation of internal transitions). I will go ahead and do so, and then I guess that we can consider this bug (with the reduced scope) to be resolved fixed. Right?
> Considering that there are so many other layout issues with importing > state-machine diagrams (as discussed on the developer mailing list), I don't > think that we need to do anything specific about this in the import tool for > 0.9. I guess that the user can always manually hide the internal transition > compartment, if they cause clutter. > Agreed. > I think it is best if we then write two new bugs, one regarding improving > the list of internal transitions to be located outside the state frame > (possibly being a dynamic drawer) to align with legacy tooling, and one for > making the contents of the modeling assistant available on a "new child" > menu when right clicking on a element in a diagram (which then should tried > to made generic, not only covering the creation of internal transitions). > > I will go ahead and do so, and then I guess that we can consider this bug > (with the reduced scope) to be resolved fixed. Right? Agreed. Thanks!
I wrote Bug 511471 and Bug 511472 to track the two remaining issues separately. If you don't have unit tests still lacking, of if you know of anything else still missing, I suggest that we put this one into resolved fixed.
Closing this bug, as JUnit tests are already testing rendering and edition of internal transitions. Thanks for writing the 2 other bugs.
Closing - additional work identified as part of comments moved to separate bugs.
I really want to verify before putting into verified/closed fixed. I will do that tomorrow.
Verified to be fixed in the latest Papyrus-RT build. It is now possible to use the diagram assistant, both on simple and composite states, to create an internal transition. Creating the first internal transition for a simple state converts it into a composite state. The direct editor, directly after creation and pressing F2, renames the internal transition. Internal transitions with an explicit name, but without trigger defined get the warning indicator similar to as for edge based transitions. The internal transition compartment can simply be hidden by double-clicking the internal transition annotation. The aspect of being able to create internal transitions using a "new child" context menu when right-clicking on a, simple or composite, state is tracked separately in Bug 511472. The list of internal transitions on the "inside" of a composite state currently is shown as a compartment within the state frame which is known to have layout issues. Improvements to this is tracked in Bug 511471. Putting this bug into state verified fixed.
Closing as verified fixed.
