Skip to content

Commit

Permalink
[82] Fix disconnected note attachment problem with edge and note/text
Browse files Browse the repository at this point in the history 
Fix the indirect deletion of note attachment, when
deleting note/text.

Fix the indirect hiding of hiding note attachment when hiding edges.

This commit also corrects when a note/text is deleted or edge is hide,
with the "remove/hide note when attached element is removed" preference
enabled, the note attached indirectly is now properly removed or hidden.
Preference now works the same way for text in corrected cases.

Bug: #82
Signed-off-by: Séraphin Costa <[email protected]>
  • Loading branch information
@scosta-obeo @mPorhel
scosta-obeo authored and mPorhel committed Sep 7, 2023
1 parent d8f67e0 commit a0a0e0a
Show file tree
Hide file tree
Showing 5 changed files with 227 additions and 212 deletions.
31 changes: 27 additions & 4 deletions ...i/src-diag/org/eclipse/sirius/diagram/ui/business/internal/command/ViewDeleteCommand.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2015, 2016 Obeo.
* Copyright (c) 2015, 2023 Obeo.
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
* which accompanies this distribution, and is available at
Expand All @@ -12,6 +12,10 @@
*******************************************************************************/
package org.eclipse.sirius.diagram.ui.business.internal.command;

import java.util.Collections;
import java.util.List;
import java.util.stream.Stream;

import org.eclipse.core.commands.ExecutionException;
import org.eclipse.core.runtime.IAdaptable;
import org.eclipse.core.runtime.IProgressMonitor;
Expand All @@ -21,9 +25,11 @@
import org.eclipse.gmf.runtime.diagram.core.commands.DeleteCommand;
import org.eclipse.gmf.runtime.diagram.core.util.ViewType;
import org.eclipse.gmf.runtime.notation.Edge;
import org.eclipse.gmf.runtime.notation.Node;
import org.eclipse.gmf.runtime.notation.View;

import com.google.common.collect.Iterables;
import org.eclipse.sirius.diagram.ui.internal.refresh.GMFHelper;
import org.eclipse.sirius.diagram.ui.provider.DiagramUIPlugin;
import org.eclipse.sirius.diagram.ui.tools.internal.preferences.SiriusDiagramUiInternalPreferencesKeys;

/**
* Extends the GMF {@link DeleteCommand} to avoid creating
Expand Down Expand Up @@ -61,13 +67,30 @@ protected CommandResult doExecuteWithResult(IProgressMonitor progressMonitor, IA
// Prevents GMF to install its CrossReferencerAdapter by not calling
// ViewUtil.destroy(View)

boolean prefRemoveAttachedPGE = DiagramUIPlugin.getPlugin().getPreferenceStore()
.getBoolean(SiriusDiagramUiInternalPreferencesKeys.PREF_REMOVE_HIDE_NOTE_WHEN_ANNOTED_ELEMENT_HIDDEN_OR_REMOVE.name());

// Remove incoming or outgoing NoteAttachment links
for (Edge edge : Iterables.filter(Iterables.concat(getView().getSourceEdges(), getView().getTargetEdges()), Edge.class)) {
List<Edge> attachedEdge = GMFHelper.getAttachedEdgesRecursively(Collections.singleton(getView()));

// Get PGE attached to all element that are about to be deleted
List<Node> attachedPGE = null;
if (prefRemoveAttachedPGE) {
attachedPGE = Stream.concat(GMFHelper.getAttachedPGE(getView()).stream(), GMFHelper.getAttachedPGE(attachedEdge).stream()).toList();
}
// Delete all elements
for (Edge edge : attachedEdge) {
if (ViewType.NOTEATTACHMENT.equals(edge.getType())) {
EcoreUtil.remove(edge);
}
}
EcoreUtil.remove(getView());

if (prefRemoveAttachedPGE) {
// attachedPGE can't be null here
GMFHelper.deleteDetachedPGE(attachedPGE);
}

return CommandResult.newOKCommandResult();
}

Expand Down
108 changes: 10 additions & 98 deletions .../src-diag/org/eclipse/sirius/diagram/ui/internal/refresh/CanonicalSynchronizerResult.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,11 @@
package org.eclipse.sirius.diagram.ui.internal.refresh;

import java.util.ArrayList;
import java.util.Collection;
import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Stream;

import org.eclipse.emf.ecore.EObject;
import org.eclipse.emf.ecore.util.EcoreUtil;
Expand Down Expand Up @@ -431,85 +429,14 @@ public void deleteOrphanEdges() {
* Compute all edges adjacent to the nodes to be deleted and adds them to the list.
*/
public void collectAttachedEdgeToNodes() {
collectAttachedEdges(orphanNodes);
this.addOrphanEdges(GMFHelper.getAttachedEdges(orphanNodes));
}

/**
* Compute all edges adjacent to the edges to be deleted and adds them to the list (and compute attached edge).
*/
public void collectAttachedEdgeToEdges() {
collectAttachedEdges(orphanEdges);
}

/**
* Add all incoming and outgoing edges of all views to orphan edges list :
* <ul>
* <li>If view is container collect also edge of all children recursively</li>
* <li>If view is edge collect recursively attached edges</li>
* </ul>
*
* @param <T>
* type of collection (View, Edge or Node in general)
* @param views
* collection of views for which we want all incoming and outgoing edges
*/
private <T extends View> void collectAttachedEdges(Collection<T> views) {
var removed = new HashSet<T>();
while (!views.isEmpty()) {
// Here we take view (take = remove from list), we put it aside (in removed).
// Then, if the view has not already been processed:
// we collect incoming and outgoing edges and we add these edges to the orphan edges list.
T view = views.stream().findAny().orElseThrow(); // get an element
if (removed.add(view)) {
// Before removing this view, we must identify incoming or outgoing
// edges of this view or of one of its children to delete them just
// after. Indeed, an Edge without source (or target) must not exist.
List<Edge> edgesToDelete = getIncomingOutgoingEdges(view);
this.addOrphanEdges(edgesToDelete);
}
views.removeIf(v -> view == v);
}
views.addAll(removed);
}

/**
* Get all incoming and outgoing edges of this <code>view</code> or of all of its children.
*
* @param view
* the concern view
* @return list of edges
*/
private List<Edge> getIncomingOutgoingEdges(View view) {
List<Edge> edgesToDelete = new ArrayList<>();
edgesToDelete.addAll(view.getSourceEdges());
edgesToDelete.addAll(view.getTargetEdges());

List<View> children = view.getChildren();
for (View child : children) {
edgesToDelete.addAll(getIncomingOutgoingEdges(child));
}
return edgesToDelete;
}

/**
* Collect all attached notes/texts/representation links (i.e. PGE: pure graphical elements).
*/
private void collectAttachedPGE(View view) {
// get all attached notes
List<Edge> noteAttachments = getIncomingOutgoingEdges(view).stream() //
.filter(GMFNotationHelper::isNoteAttachment).toList();

noteAttachments.stream().flatMap(edge -> {
return Stream.of(edge.getSource(), edge.getTarget());
}).filter(attachedView -> { // all nodes linked to note attachment: filter notes/texts
if (attachedView instanceof Node attachedNode) {
return GMFNotationHelper.isNote(attachedNode) || GMFNotationHelper.isTextNote(attachedNode);
} else {
return false;
}
}).map(Node.class::cast).forEach(attachedNote -> { // for each note
partialOrphanPGE.add(attachedNote);
});
this.addOrphanEdges(GMFHelper.getAttachedEdgesRecursively(orphanEdges));
}

/**
Expand All @@ -530,7 +457,7 @@ public void collectDetachedPGEFromNode() {
if (prefRemoveAttachedPGE) {
// collect possibly detached PGE <=> collect PGE attached to orphan view
for (View orphanNode : orphanNodes) {
collectAttachedPGE(orphanNode);
partialOrphanPGE.addAll(GMFHelper.getAttachedPGE(orphanNode));
}
}
}
Expand All @@ -553,14 +480,17 @@ public void collectDetachedPGEFromEdge() {
if (prefRemoveAttachedPGE) {
// collect possibly detached PGE <=> collect PGE attached to orphan view
for (Edge orphanEdge : orphanEdges) {
collectAttachedPGE(orphanEdge);
partialOrphanPGE.addAll(GMFHelper.getAttachedPGE(orphanEdge));
}
}
}

/**
* Mark to remove all notes/texts/representation links (i.e. PGE: pure graphical elements) attached to nodes that
* will be removed.
* Remove all notes/texts/representation links (i.e. PGE: pure graphical elements) attached to removed nodes and
* without other attachment.
*
* The PGE is removed if all attached element are removed. The PGE is hidden if all visible attached element are
* removed. If the PGE has at least one remaining attached element, the PGE is not removed and note hidden.
*
* Before calling this function, you need to collect all PGE attached to view that will be removed (using method
* collectDetachedPGEFromNode and collectDetachedPGEFromEdge)
Expand All @@ -570,25 +500,7 @@ public void collectDetachedPGEFromEdge() {
*/
public void deleteDetachedPGE() {
if (prefRemoveAttachedPGE) {
for (Node pureGraphicalElement : partialOrphanPGE) {
List<Edge> sourceEdges = pureGraphicalElement.getSourceEdges();
List<Edge> targetEdges = pureGraphicalElement.getTargetEdges();

List<Edge> validEdges = Stream.concat(sourceEdges.stream(), targetEdges.stream()) //
.filter(edge -> edge.eContainer() != null).toList();

// remove unattached notes/texts
if (validEdges.size() == 0) {
EcoreUtil.remove(pureGraphicalElement);
} else {
Stream<Edge> visibleEdges = validEdges.stream().filter(edge -> edge.isVisible());

// hide notes/texts attached to invisible element
if (visibleEdges.count() == 0) {
pureGraphicalElement.setVisible(false);
}
}
}
GMFHelper.deleteDetachedPGE(partialOrphanPGE);
partialOrphanPGE.clear();
}
}
Expand Down
Loading

0 comments on commit a0a0e0a

Please sign in to comment.