[josm-dev] [PATCH 24/26] clean up MergeNodeAction
Dave Hansen
dave at sr71.net
Tue Apr 29 03:03:06 BST 2008
Honestly, this patch is a mess. It does too many different things.
The main functional thing it does is replace the ChangeCommand()
with a series of ChangePropertyCommand()s in a sequence command.
---
core-dave/src/org/openstreetmap/josm/actions/MergeNodesAction.java | 138 ++++------
1 file changed, 68 insertions(+), 70 deletions(-)
diff -puN src/org/openstreetmap/josm/actions/MergeNodesAction.java~reversewaycommand src/org/openstreetmap/josm/actions/MergeNodesAction.java
--- core/src/org/openstreetmap/josm/actions/MergeNodesAction.java~reversewaycommand 2008-04-28 18:59:32.000000000 -0700
+++ core-dave/src/org/openstreetmap/josm/actions/MergeNodesAction.java 2008-04-28 18:59:32.000000000 -0700
@@ -11,6 +11,7 @@ import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedList;
+import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.TreeMap;
@@ -24,9 +25,11 @@ import javax.swing.JOptionPane;
import javax.swing.JPanel;
import org.openstreetmap.josm.Main;
-import org.openstreetmap.josm.command.ChangeCommand;
+import org.openstreetmap.josm.command.ChangePropertyCommand;
import org.openstreetmap.josm.command.Command;
import org.openstreetmap.josm.command.DeleteCommand;
+import org.openstreetmap.josm.command.RemoveNodeInWayCommand;
+import org.openstreetmap.josm.command.ReplaceNodeInWayCommand;
import org.openstreetmap.josm.command.SequenceCommand;
import org.openstreetmap.josm.data.SelectionChangedListener;
import org.openstreetmap.josm.data.osm.DataSet;
@@ -40,18 +43,18 @@ import org.openstreetmap.josm.data.osm.v
import org.openstreetmap.josm.tools.GBC;
import org.openstreetmap.josm.tools.Pair;
-
/**
- * Merge two or more nodes into one node.
- * (based on Combine ways)
+ * Merge two or more nodes into one node. (based on Combine ways)
*
* @author Matthew Newton
*
*/
-public class MergeNodesAction extends JosmAction implements SelectionChangedListener {
+public class MergeNodesAction extends JosmAction implements
+ SelectionChangedListener {
public MergeNodesAction() {
- super(tr("Merge Nodes"), "mergenodes", tr("Merge nodes into one."), KeyEvent.VK_M, 0, true);
+ super(tr("Merge Nodes"), "mergenodes", tr("Merge nodes into one."),
+ KeyEvent.VK_M, 0, true);
DataSet.selListeners.add(this);
}
@@ -67,7 +70,8 @@ public class MergeNodesAction extends Jo
selectedNodes.add((Node)osm);
if (selectedNodes.size() < 2) {
- JOptionPane.showMessageDialog(Main.parent, tr("Please select at least two nodes to merge."));
+ JOptionPane.showMessageDialog(Main.parent,
+ tr("Please select at least two nodes to merge."));
return;
}
@@ -99,8 +103,6 @@ public class MergeNodesAction extends Jo
* really do the merging - returns the node that is left
*/
public static Node mergeNodes(LinkedList<Node> allNodes, Node dest) {
- Node newNode = new Node(dest);
-
// Check whether all ways have identical relationship membership. More
// specifically: If one of the selected ways is a member of relation X
// in role Y, then all selected ways must be members of X in role Y.
@@ -111,16 +113,17 @@ public class MergeNodesAction extends Jo
// Step 1, iterate over all relations and figure out which of our
// selected ways are members of a relation.
- HashMap<Pair<Relation,String>, HashSet<Node>> backlinks =
- new HashMap<Pair<Relation,String>, HashSet<Node>>();
+ HashMap<Pair<Relation, String>, HashSet<Node>> backlinks = new HashMap<Pair<Relation, String>, HashSet<Node>>();
HashSet<Relation> relationsUsingNodes = new HashSet<Relation>();
for (Relation r : Main.ds.relations) {
- if (r.deleted || r.incomplete) continue;
+ if (r.deleted || r.incomplete)
+ continue;
for (RelationMember rm : r.members) {
if (rm.member instanceof Node) {
for (Node n : allNodes) {
if (rm.member == n) {
- Pair<Relation,String> pair = new Pair<Relation,String>(r, rm.role);
+ Pair<Relation, String> pair = new Pair<Relation, String>(
+ r, rm.role);
HashSet<Node> nodelinks = new HashSet<Node>();
if (backlinks.containsKey(pair)) {
nodelinks = backlinks.get(pair);
@@ -141,11 +144,13 @@ public class MergeNodesAction extends Jo
// Complain to the user if the ways don't have equal memberships.
for (HashSet<Node> nodelinks : backlinks.values()) {
if (!nodelinks.containsAll(allNodes)) {
- int option = JOptionPane.showConfirmDialog(Main.parent,
- tr("The selected nodes have differing relation memberships. "
- + "Do you still want to merge them?"),
- tr("Merge nodes with different memberships?"),
- JOptionPane.YES_NO_OPTION);
+ int option = JOptionPane
+ .showConfirmDialog(
+ Main.parent,
+ tr("The selected nodes have differing relation memberships. "
+ + "Do you still want to merge them?"),
+ tr("Merge nodes with different memberships?"),
+ JOptionPane.YES_NO_OPTION);
if (option == JOptionPane.YES_OPTION)
break;
return null;
@@ -155,20 +160,22 @@ public class MergeNodesAction extends Jo
// collect properties for later conflict resolving
Map<String, Set<String>> props = new TreeMap<String, Set<String>>();
for (Node n : allNodes) {
- for (Entry<String,String> e : n.entrySet()) {
+ for (Entry<String, String> e : n.entrySet()) {
if (!props.containsKey(e.getKey()))
props.put(e.getKey(), new TreeSet<String>());
props.get(e.getKey()).add(e.getValue());
}
}
+ LinkedList<Command> cmds = new LinkedList<Command>();
// display conflict dialog
Map<String, JComboBox> components = new HashMap<String, JComboBox>();
JPanel p = new JPanel(new GridBagLayout());
for (Entry<String, Set<String>> e : props.entrySet()) {
if (TigerUtils.isTigerTag(e.getKey())) {
- String combined = TigerUtils.combineTags(e.getKey(), e.getValue());
- newNode.put(e.getKey(), combined);
+ String combined = TigerUtils.combineTags(e.getKey(), e
+ .getValue());
+ cmds.add(new ChangePropertyCommand(dest, e.getKey(), combined));
} else if (e.getValue().size() > 1) {
JComboBox c = new JComboBox(e.getValue().toArray());
c.setEditable(true);
@@ -177,96 +184,87 @@ public class MergeNodesAction extends Jo
p.add(c, GBC.eol());
components.put(e.getKey(), c);
} else
- newNode.put(e.getKey(), e.getValue().iterator().next());
+ cmds.add(new ChangePropertyCommand(dest, e.getKey(), e
+ .getValue().iterator().next()));
}
if (!components.isEmpty()) {
- int answer = JOptionPane.showConfirmDialog(Main.parent, p, tr("Enter values for all conflicts."), JOptionPane.OK_CANCEL_OPTION);
+ int answer = JOptionPane.showConfirmDialog(Main.parent, p,
+ tr("Enter values for all conflicts."),
+ JOptionPane.OK_CANCEL_OPTION);
if (answer != JOptionPane.OK_OPTION)
return null;
- for (Entry<String, JComboBox> e : components.entrySet())
- newNode.put(e.getKey(), e.getValue().getEditor().getItem().toString());
+ for (Entry<String, JComboBox> e : components.entrySet()) {
+ String newValue = e.getValue().getEditor().getItem().toString();
+ cmds.add(new ChangePropertyCommand(dest, e.getKey(), newValue));
+ }
}
- LinkedList<Command> cmds = new LinkedList<Command>();
- cmds.add(new ChangeCommand(dest, newNode));
-
Collection<OsmPrimitive> del = new HashSet<OsmPrimitive>();
for (Way w : Main.ds.ways) {
- if (w.deleted || w.incomplete || w.nodes.size() < 1) continue;
+ if (w.deleted || w.incomplete || w.nodes.size() < 1)
+ continue;
boolean modify = false;
for (Node sn : allNodes) {
- if (sn == dest) continue;
+ if (sn == dest)
+ continue;
if (w.nodes.contains(sn)) {
modify = true;
}
}
- if (!modify) continue;
+ if (!modify)
+ continue;
// OK - this way contains one or more nodes to change
- ArrayList<Node> nn = new ArrayList<Node>();
+ int new_way_size = w.nodes.size();
Node lastNode = null;
for (int i = 0; i < w.nodes.size(); i++) {
Node pushNode = w.nodes.get(i);
- if (allNodes.contains(pushNode)) {
- pushNode = dest;
- }
- if (pushNode != lastNode) {
- nn.add(pushNode);
+ if (!allNodes.contains(pushNode))
+ continue;
+ if (pushNode == lastNode) {
+ new_way_size--;
+ cmds.add(new RemoveNodeInWayCommand(w, pushNode));
+ } else {
+ cmds.add(new ReplaceNodeInWayCommand(w, pushNode, dest));
+ lastNode = pushNode;
}
- lastNode = pushNode;
}
- if (nn.size() < 2) {
- CollectBackReferencesVisitor backRefs =
- new CollectBackReferencesVisitor(Main.ds, false);
+ if (new_way_size < 2) {
+ CollectBackReferencesVisitor backRefs = new CollectBackReferencesVisitor(
+ Main.ds, false);
w.visit(backRefs);
if (!backRefs.data.isEmpty()) {
- JOptionPane.showMessageDialog(Main.parent,
- tr("Cannot merge nodes: " +
- "Would have to delete a way that is still used."));
+ JOptionPane
+ .showMessageDialog(
+ Main.parent,
+ tr("Cannot merge nodes: "
+ + "Would have to delete a way that is still used."));
return null;
}
del.add(w);
- } else {
- Way newWay = new Way(w);
- newWay.nodes.clear();
- newWay.nodes.addAll(nn);
- cmds.add(new ChangeCommand(w, newWay));
}
}
// delete any merged nodes
del.addAll(allNodes);
del.remove(dest);
- if (!del.isEmpty()) cmds.add(new DeleteCommand(del));
+ if (!del.isEmpty())
+ cmds.add(new DeleteCommand(del));
// modify all relations containing the now-deleted nodes
- for (Relation r : relationsUsingNodes) {
- Relation newRel = new Relation(r);
- newRel.members.clear();
- HashSet<String> rolesToReAdd = new HashSet<String>();
- for (RelationMember rm : r.members) {
- // Don't copy the member if it points to one of our nodes,
- // just keep a note to re-add it later on.
- if (allNodes.contains(rm.member)) {
- rolesToReAdd.add(rm.role);
- } else {
- newRel.members.add(rm);
- }
- }
- for (String role : rolesToReAdd) {
- newRel.members.add(new RelationMember(role, dest));
- }
- cmds.add(new ChangeCommand(r, newRel));
- }
+ List<OsmPrimitive> allOsmPrimitives = new LinkedList<OsmPrimitive>();
+ allOsmPrimitives.addAll(allNodes);
+ for (Relation r : relationsUsingNodes)
+ cmds.add(r.updateMembers(allOsmPrimitives, dest));
- Main.main.undoRedo.add(new SequenceCommand(tr("Merge {0} nodes", allNodes.size()), cmds));
+ Main.main.undoRedo.add(new SequenceCommand(tr("Merge {0} nodes",
+ allNodes.size()), cmds));
Main.ds.setSelected(dest);
return dest;
}
-
/**
* Enable the "Merge Nodes" menu option if more then one node is selected
*/
_
More information about the josm-dev
mailing list