Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refresh issue in Model Explorer with custom Content Provider #101

Open
ymortier opened this issue Sep 14, 2023 · 2 comments · May be fixed by #105
Open

Refresh issue in Model Explorer with custom Content Provider #101

ymortier opened this issue Sep 14, 2023 · 2 comments · May be fixed by #105
Labels
component: core type: bug Something isn't working

Comments

@ymortier
Copy link

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

sirius-desktop/plugins/org.eclipse.sirius.ui/src/org/eclipse/sirius/ui/tools/internal/views/common/navigator/SiriusCommonContentProvider.java

Lines 1068 to 1074 in a0a0e0a

private boolean isSemanticChange(Resource resource) {
boolean result = false;
if (resource != null) {
allSemanticResources.contains(resource);
}
return result;
}

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 added a commit that referenced this issue Sep 18, 2023
@pcdavid
[101] Fix refresh issue in Model Explorer
85ba95e 
Bug: #101
Signed-off-by: Pierre-Charles David <[email protected]>
@pcdavid pcdavid linked a pull request Sep 18, 2023 that will close this issue
Open
@pcdavid
Copy link
Member

pcdavid commented Sep 18, 2023

Hi Yann! 👋

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?

@pcdavid pcdavid added type: bug Something isn't working component: core labels Sep 18, 2023
@pcdavid pcdavid linked a pull request Sep 18, 2023 that will close this issue
[101] Fix refresh issue in Model Explorer #105
Open
@ymortier
Copy link
Author

ymortier commented Oct 4, 2023

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:

  • EPackage
    • EClass
      • EStructuralFeature

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

  • EPackage
    • EStructuralFeature

The refresh limits the scope to impactedElements see

sirius-desktop/plugins/org.eclipse.sirius.ui/src/org/eclipse/sirius/ui/tools/internal/views/common/navigator/SiriusCommonContentProvider.java

Lines 986 to 994 in a0a0e0a

if (!impactedElements.isEmpty()) {
boolean needRefresh = shouldRefresh(notifications, impactedElements);
if (needRefresh) {
refreshViewer(impactedElements);
} else {
updateViewer(impactedElements);
}
}

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?

pcdavid added a commit that referenced this issue Oct 28, 2023
@pcdavid
[101] Fix refresh issue in Model Explorer
0c9335f 
Bug: #101
Signed-off-by: Pierre-Charles David <[email protected]>
@bedo2991 bedo2991 mentioned this issue Feb 29, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
component: core type: bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[101] Fix refresh issue in Model Explorer eclipse-sirius/sirius-desktop
2 participants
@pcdavid @ymortier