search

eclipse-sirius / sirius-desktop

Sirius Desktop: desktop-based graphical modelers for dedicated DSLs
https://eclipse.dev/sirius/
Eclipse Public License 2.0
14 stars 11 forks source link

Refresh issue in Model Explorer with custom Content Provider #101

Open ymortier opened 1 year ago

ymortier commented 1 year ago

The variable result in org.eclipse.sirius.ui.tools.internal.views.common.navigator.SiriusCommonContentProvider.RefreshViewerTriggerScope.isSemanticChange is never set (see line 1071):

https://github.com/eclipse-sirius/sirius-desktop/blob/a0a0e0a68c5c132f2ba7f07a535425f876b4f0ce/plugins/org.eclipse.sirius.ui/src/org/eclipse/sirius/ui/tools/internal/views/common/navigator/SiriusCommonContentProvider.java#L1068-L1074

A consequence is the model explorer is not refreshed by RefreshViewerTrigger on semantic change.

The Model Explorer still works with raw Sirius because contributions to the Model Explorer in Sirius use the AdapterFactoryContentProvider which is able to refresh the viewer on semantic changes.

If another plugin (outside of Sirius) contributes new content providers to the Model Explorer without using AdapterFactoryContentProvider then it has to manage the refresh on semantic changes (by copying the whole refresh process which can be error prone).

pcdavid commented 1 year ago

Hi Yann! :wave:

It's difficult to test if it requires a custom content provider, but can you verify if the PR above fixes the issue for you?

ymortier commented 1 year ago

Hello @pcdavid

The refresh is invoked with your changes. We still need to add some custom refresh because Sirius refreshes only impacted elements and we want to use a custom CNF Content Provider to filter out some intermediary elements.

For example, using ecore, the Model Explorer shows:

We want to hide EClass elements (only in Model Explorer):

The refresh limits the scope to impactedElements see https://github.com/eclipse-sirius/sirius-desktop/blob/a0a0e0a68c5c132f2ba7f07a535425f876b4f0ce/plugins/org.eclipse.sirius.ui/src/org/eclipse/sirius/ui/tools/internal/views/common/navigator/SiriusCommonContentProvider.java#L986-L994

We can not rely on this mechanism because if an EStructuralFeature is added, the refresh is called on the parent EClass which, in our case, is not shown in the Model Explorer. I dont know if this PR should be merged or if the semantic refresh management should be removed because it was inactive for years. It seems the refresh works well thanks to EMF Content/Label providers.

I think we should manage our custom refresh in our custom providers like EMF does. What are your thoughts?