Bug 509206 - [Tooling] Improve or remove the UML-RT tab in properties view for a trigger
Summary: [Tooling] Improve or remove the UML-RT tab in properties view for a trigger
Status: NEW
Alias: None
Product: Papyrus-rt
Classification: Modeling
Component: tool (show other bugs)
Version: 0.8.0   Edit
Hardware: PC Windows 7
: P3 normal
Target Milestone: Future   Edit
Assignee: Project Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on: 477811
Blocks:
  Show dependency tree
 
Reported: 2016-12-14 04:56 EST by Peter Cigehn CLA
Modified: 2017-06-13 10:02 EDT (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Cigehn CLA 2016-12-14 04:56:09 EST
The current UML-RT tab for a trigger was introduced as a "work-around" before the combined trigger creating dialog as tracked by Bug 477811 existed. Thus the UML-RT tab for an individual trigger needs some improvement and clean-up. One possibility is to simply remove the UML-RT tab completely, since it is equally simple to work with the triggers from the trigger table on the UML-RT tab in the properties view for a transition.

Anyway, if it seen a need to keep the UML-RT tab in the properties view for a trigger then the following should be done:

1) Remove the name and the visibility fields. Normally it does not make sense to explicitly name a trigger and have different visibility so there is no need to have them on the UML-RT tab. These are not visible in the trigger table in the properties view for a transition, so there is no need to have them for individual triggers either.

2) Combine the pen edit button and the ... selection button for the ports table respectively the protocol message, into one single edit button, placed more centrally. This combined edit button should bring up the combined edit trigger dialog as tracked by Bug 477811. Currently it is rather confusing that these two separate edit/selection buttons brings up the same combined dialog as tracked by Bug 477811.

3) Possibly add a field for showing the guard code for the trigger (to be consistent with the contents of the trigger table). But since the editing of the guard code should be done using a separate code snippet view, as tracked by Bug 494288. Before the code snippet view is in place, we have inline editing of the guard code in the trigger table for a transition, so we probably don't want to have a second "work-around" for editing the guard code. 

The last bullet 3) is actually an argument for completely removing the UML-RT tab, since there is not much left in it, and from a work-flow perspective it is probably easier to simply manipulate triggers from the trigger table in the properties view for a transition.
Comment 1 Charles Rivet CLA 2016-12-14 08:36:13 EST
(In reply to Peter Cigehn from comment #0)
> The current UML-RT tab for a trigger was introduced as a "work-around"
> before the combined trigger creating dialog as tracked by Bug 477811
> existed. Thus the UML-RT tab for an individual trigger needs some
> improvement and clean-up. One possibility is to simply remove the UML-RT tab
> completely, since it is equally simple to work with the triggers from the
> trigger table on the UML-RT tab in the properties view for a transition.
> 
> Anyway, if it seen a need to keep the UML-RT tab in the properties view for
> a trigger then the following should be done:
> 
<...snip...>

I would like to highlight a few things to keep in mind when dealing with this bug.

My view is that if we have a property view for an element (trigger in this case), it should be possible for a modelling user to edit those properties.

This should be mitigated by the maintainability of supporting multiple views for the same information, e.g., having to modify two views if there is an element change.

From a workflow perspective, is the property view for a trigger needed if we tell the user to access the trigger from the transition's properties?
Comment 2 Peter Cigehn CLA 2016-12-14 09:03:27 EST
(In reply to Charles Rivet from comment #1)
> I would like to highlight a few things to keep in mind when dealing with
> this bug.
> 
> My view is that if we have a property view for an element (trigger in this
> case), it should be possible for a modelling user to edit those properties.

Yes, that is also one of the key points with this bug. If we (for some reason) keep the specific UML-RT tab for a trigger, then it should be improved and streamlined (since it was introduced as a work-around without really considering the work-flow that we actually wanted to have). We also have some specific timing issues, e.g. with current lack of the code snippet view and how editing of trigger guard code shall be done. I already had some discussion with Young-Soo regarding the current inline editing of the guard code as provided in the trigger table for a transition, which I eventually see should be replaced/provided by the code-snippet view (see Bug 507965) to get things like integration with CDT and content assist. We have the same kind of aspect to be considered here.

> 
> This should be mitigated by the maintainability of supporting multiple views
> for the same information, e.g., having to modify two views if there is an
> element change.
> 
> From a workflow perspective, is the property view for a trigger needed if we
> tell the user to access the trigger from the transition's properties?

Also my point. I really do not see the need for having a specific UML-RT tab for the trigger, since from a work flow perspective, it will equally easy (if not easier) to manipulate the triggers using the trigger table from the transition's properties, especially considering the new combined trigger dialog that is worked on within the scope of Bug 477811. 

This is also how the legacy tooling handles this. There is a properties view for triggers, but its tabs has not been adapted specifically for UML-RT and I assume it is in practice not used. Instead the trigger table available for the transition is what is used in practice (in combination with code snippet view for editing the effect, transition guard and trigger guard code).
Comment 3 Peter Cigehn CLA 2017-01-17 03:00:46 EST
Along with the changes related to the new combined create/edit trigger dialog, tracked by Bug 477811, changes were made also to the UML-RT tab in the properties view for a trigger, basically making the properties view to look exactly the same as the create/edit trigger dialog.

So improvements have already been made to the UML-RT tab in the properties view for a trigger. But the question is if this approach still will hold, e.g. when the buttons for creating ports and protocol messages will be added to the create/edit trigger dialog, as tracked by Bug 508994. Does it makes sense then to get these buttons also in the properties view for a trigger? So the question still holds if the specific UML-RT tab for a trigger should be removed completely, and the workflow for editing triggers always should be made through the trigger table in the properties view of a transition.
Comment 4 Peter Cigehn CLA 2017-01-17 06:40:37 EST
One thing that should be considered is that it is currently possible to specify an incomplete trigger via the properties view, since you can deselect the currently selected protocol message, causing the trigger to be only partly defined. In the create/edit dialog, this is prohibited by the fact that the OK button is disabled until a protocol message has been selected. Such a limitation does not exist in the properties view.
Comment 5 Peter Cigehn CLA 2017-06-13 10:02:00 EDT
Currently a null pointer exception is thrown if you try to edit an existing trigger via the properties view, e.g. when you de-select the currently selected protocol message (explicitly, or implicitly by selecting another port).

org.eclipse.jface
Error
Tue Jun 13 15:59:56 CEST 2017
Problems occurred when invoking code from plug-in: "org.eclipse.jface".

java.lang.NullPointerException
	at org.eclipse.jface.viewers.DecoratingStyledCellLabelProvider.getStyledText(DecoratingStyledCellLabelProvider.java:199)
	at org.eclipse.jface.viewers.DelegatingStyledCellLabelProvider.update(DelegatingStyledCellLabelProvider.java:106)
	at org.eclipse.jface.viewers.DecoratingStyledCellLabelProvider.update(DecoratingStyledCellLabelProvider.java:131)
	at org.eclipse.jface.viewers.ViewerColumn.refresh(ViewerColumn.java:141)
	at org.eclipse.jface.viewers.AbstractTreeViewer.doUpdateItem(AbstractTreeViewer.java:946)
	at org.eclipse.jface.viewers.AbstractTreeViewer$UpdateItemSafeRunnable.run(AbstractTreeViewer.java:117)
	at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:42)
	at org.eclipse.ui.internal.JFaceUtil.lambda$0(JFaceUtil.java:44)
	at org.eclipse.jface.util.SafeRunnable.run(SafeRunnable.java:173)
	at org.eclipse.jface.viewers.AbstractTreeViewer.doUpdateItem(AbstractTreeViewer.java:1025)
	at org.eclipse.jface.viewers.StructuredViewer$UpdateItemSafeRunnable.run(StructuredViewer.java:475)
	at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:42)
	at org.eclipse.ui.internal.JFaceUtil.lambda$0(JFaceUtil.java:44)
	at org.eclipse.jface.util.SafeRunnable.run(SafeRunnable.java:173)
	at org.eclipse.jface.viewers.StructuredViewer.updateItem(StructuredViewer.java:2159)
	at org.eclipse.jface.viewers.StructuredViewer.internalUpdate(StructuredViewer.java:2142)
	at org.eclipse.jface.viewers.StructuredViewer.update(StructuredViewer.java:2083)
	at org.eclipse.jface.viewers.ColumnViewer.update(ColumnViewer.java:542)
	at org.eclipse.ui.navigator.CommonViewer.update(CommonViewer.java:512)
	at org.eclipse.jface.viewers.StructuredViewer.update(StructuredViewer.java:2027)
	at org.eclipse.jface.viewers.StructuredViewer.handleLabelProviderChanged(StructuredViewer.java:1203)
	at org.eclipse.ui.navigator.CommonViewer.handleLabelProviderChanged(CommonViewer.java:237)
	at org.eclipse.jface.viewers.ContentViewer$1.labelProviderChanged(ContentViewer.java:99)
	at org.eclipse.jface.viewers.BaseLabelProvider$1.run(BaseLabelProvider.java:72)
	at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:42)
	at org.eclipse.ui.internal.JFaceUtil.lambda$0(JFaceUtil.java:44)
	at org.eclipse.jface.util.SafeRunnable.run(SafeRunnable.java:173)
	at org.eclipse.jface.viewers.BaseLabelProvider.fireLabelProviderChanged(BaseLabelProvider.java:69)
	at org.eclipse.jface.viewers.DecoratingStyledCellLabelProvider.lambda$0(DecoratingStyledCellLabelProvider.java:75)
	at org.eclipse.ui.internal.decorators.DecoratorManager$1.run(DecoratorManager.java:374)
	at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:42)
	at org.eclipse.ui.internal.decorators.DecoratorManager.fireListener(DecoratorManager.java:371)
	at org.eclipse.ui.internal.decorators.DecorationScheduler$3.runInUIThread(DecorationScheduler.java:485)
	at org.eclipse.ui.progress.UIJob.lambda$0(UIJob.java:95)
	at org.eclipse.swt.widgets.RunnableLock.run(RunnableLock.java:37)
	at org.eclipse.swt.widgets.Synchronizer.runAsyncMessages(Synchronizer.java:182)
	at org.eclipse.swt.widgets.Display.runAsyncMessages(Display.java:4213)
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3820)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$5.run(PartRenderingEngine.java:1155)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:336)
	at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:1044)
	at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:153)
	at org.eclipse.ui.internal.Workbench.lambda$3(Workbench.java:680)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:336)
	at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:594)
	at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:148)
	at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:151)
	at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:196)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:134)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:104)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:388)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:243)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
	at java.lang.reflect.Method.invoke(Unknown Source)
	at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:653)
	at org.eclipse.equinox.launcher.Main.basicRun(Main.java:590)
	at org.eclipse.equinox.launcher.Main.run(Main.java:1499)