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
         */
_

_______________________________________________
josm-dev mailing list
[email protected]
http://lists.openstreetmap.org/cgi-bin/mailman/listinfo/josm-dev

Reply via email to