Bug 462861 - Show conflicts marker on Papyrus model node if one contained resource has a conflict marker
Summary: Show conflicts marker on Papyrus model node if one contained resource has a c...
Status: ASSIGNED
Alias: None
Product: Papyrus
Classification: Modeling
Component: Core (show other bugs)
Version: 3.0.0   Edit
Hardware: PC Windows NT
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Stefan Dirix CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-03-23 11:41 EDT by Philip Langer CLA
Modified: 2016-11-11 05:16 EST (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Philip Langer CLA 2015-03-23 11:41:06 EDT
After a conflicting Git merge, resources are annotated with the conflicts marker. If a model resource within a papyrus model is part of a conflicting Git merge (e.g., .uml or .notation), also the Papyrus node which encompasses the .di, .notation, and .uml file should be annotated with a conflict in the Project Explorer.
Comment 1 Philip Langer CLA 2015-06-17 08:33:41 EDT
Ideally, we should support any markers by team (i.e., als staged and dirty).
Comment 2 Eclipse Genie CLA 2015-06-24 10:59:35 EDT
New Gerrit change created: https://git.eclipse.org/r/50814
Comment 3 Stefan Dirix CLA 2015-06-26 08:53:40 EDT
Two days ago I committed a Gerrit Change (50814#1) with which I wanted to solve this problem generically from the Papyrus side of things, since they already use a decorator for the Papyrus node anyway.

Short explanation of the Papyrus Decorator: It basically intersects the children nodes' decorations and uses the result as its own decoration. The Gerrit Change (50814#1) changed the intersection to an union of decorations so that the decoration of the Papyrus Node behaves like a folder decoration.

But I realized that this does not work as intended: The prefixes/postfixes work but the overlays do not. I looked more closely at the Papyrus Decorator and found that it was flawed from the start: The code used methods only meant for testing and can only access limited information. For example it can not determine where the children's overlays are positioned (top-left, bottom-right etc.) and always places them bottom-right.

Also there is a fundamental issue here: Should Papyrus really be the one who decides how the Papyrus node is decorated for non-Papyrus decorations? For example the eGit decorator at the moment decorates the Papyrus node the same way as the .di node. This means when the .di file is "dirty", eGit places a "<" before the .di node and the Papyrus node. When the .uml and .notation file are also dirty, then all three files have the same "<" decoration. The Papyrus decorator sees that and adds another "<" to the Papyrus node which now has two "<"s. With my change (50814#1) this is even more incisive: now the Papyrus node already has two "<" when the .di file is "dirty".

I currently see two possible fixes: 

1) The eGit code could chose to ignore the Papyrus node and not decorate it. The issue is then solved from the Papyrus side by using reflection to access the information it needs (for example the location of the overlays could be checked by accessing the currentDefinition inside the DecorationBuilder which has only package visibility) or propose a change to eclipse.ui to change the decorator visibilities.

2) eGit should be fixed to decorate the Papyrus node itself the way eGit wants it to behave (in our case similar to a folder). Currently eGit tries do adapt its input (a resource mapping) to a resource instead of handling the mapping itself. This is why eGit decorates the Papyrus node the same way as the contained .di node. To fix eGit the decorator should be refactored to generically handle resource mappings and decorate the "root" element like a folder if its traversals contain more than one resource. The Papyrus decorator should then be removed since it is basically overstepping its boundaries anyway.

Personally I think the second option is the cleaner one, although it probably takes more time to implement. I do not know exactly why this Papyrus decorator was introduced so there may be issues I can not foresee right now if it is removed.

What do you think?
Comment 4 Camille Letavernier CLA 2015-06-26 09:03:32 EDT
Hi,

The Papyrus "OneFile" adapts to IFile and returns the *.di IFile (So that it behaves as much as possible as the *.di file, which is the entry point for Papyrus models). Thus, all generic tools (Without any specific integration with Papyrus) will handle this element as a standard *.di file.

Maybe we should adapt it to IContainer instead, but then this would imply changing a lot of file-based operations (Such as "Open editor"). But at least this can be mostly done in Papyrus (I think we already provide the "Open" implementation to always open the Papyrus editor on the Papyrus OneFile)

Regarding the Papyrus OneFile decorator, I don't know all the history behind it, but we've had a lot of issues with it (OneFile representation in general), so "Flawed from the start" is probably accurate

HTH,
Camille
Comment 5 Stefan Dirix CLA 2016-11-11 05:16:16 EST
In the mean time the Papyrus Adaption to IFile and IResource was removed (See https://bugs.eclipse.org/bugs/show_bug.cgi?id=475110). Therefore eGit now adapts the Papyrus File to ResourceMapping and can determine a proper overlay this way 

This means the Papyrus nodes usually (some edge cases still need to be fixed: See here https://bugs.eclipse.org/bugs/show_bug.cgi?id=498546) now receives the conflict marker by eGit. However the conflict marker is still not always shown since the OneFileDecorator of Papyrus sometimes overwrites it with other decorations.

Patrick, did you make any progress regarding the problems described in https://bugs.eclipse.org/bugs/show_bug.cgi?id=498442 ? With the current state of the Papyrus Decorator I see no reliable way of supporting proper (conflict) decorations by eGit.

We would like to discuss this issue and possible fixes with you to find a proper solution everybody can agree on.