Ticket #7513: 7513.patch

src/org/openstreetmap/josm/actions/CombineWayAction.java

@@ -1 +1 @@
 // License: GPL. Copyright 2007 by Immanuel Scholz and others
 package org.openstreetmap.josm.actions;
 
-import static org.openstreetmap.josm.gui.conflict.tags.TagConflictResolutionUtil.combineTigerTags;
-import static org.openstreetmap.josm.gui.conflict.tags.TagConflictResolutionUtil.completeTagCollectionForEditing;
-import static org.openstreetmap.josm.gui.conflict.tags.TagConflictResolutionUtil.normalizeTagCollectionBeforeEditing;
 import static org.openstreetmap.josm.gui.help.HelpUtil.ht;
 import static org.openstreetmap.josm.tools.I18n.tr;
 
@@ -33 +30 @@
 import org.openstreetmap.josm.corrector.UserCancelException;
 import org.openstreetmap.josm.data.osm.Node;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
-import org.openstreetmap.josm.data.osm.Relation;
 import org.openstreetmap.josm.data.osm.TagCollection;
 import org.openstreetmap.josm.data.osm.Way;
 import org.openstreetmap.josm.data.preferences.BooleanProperty;
@@ -95 +91 @@
     }
 
     /**
-     * Replies the set of referring relations
-     *
-     * @return the set of referring relations
-     */
-    public static Set<Relation> getParentRelations(Collection<Way> ways) {
-        HashSet<Relation> ret = new HashSet<Relation>();
-        for (Way w: ways) {
-            ret.addAll(OsmPrimitive.getFilteredList(w.getReferrers(), Relation.class));
-        }
-        return ret;
-    }
-
-    /**
      * @param ways
      * @return null if ways cannot be combined. Otherwise returns the combined
      *              ways and the commands to combine
@@ -193 +176 @@
         Way modifiedTargetWay = new Way(targetWay);
         modifiedTargetWay.setNodes(path);
 
-        TagCollection completeWayTags = new TagCollection(wayTags);
-        combineTigerTags(completeWayTags);
-        normalizeTagCollectionBeforeEditing(completeWayTags, ways);
-        TagCollection tagsToEdit = new TagCollection(completeWayTags);
-        completeTagCollectionForEditing(tagsToEdit);
-
-        CombinePrimitiveResolverDialog dialog = CombinePrimitiveResolverDialog.getInstance();
-        dialog.getTagConflictResolverModel().populate(tagsToEdit, completeWayTags.getKeysWithMultipleValues());
-        dialog.setTargetPrimitive(targetWay);
-        Set<Relation> parentRelations = getParentRelations(ways);
-        dialog.getRelationMemberConflictResolverModel().populate(
-                parentRelations,
-                ways
-        );
-        dialog.prepareDefaultDecisions();
-
-        // resolve tag conflicts if necessary
-        //
-        if (!completeWayTags.isApplicableToPrimitive() || !parentRelations.isEmpty()) {
-            dialog.setVisible(true);
-            if (dialog.isCanceled())
-                throw new UserCancelException();
-        }
+        List<Command> resolution = CombinePrimitiveResolverDialog.launchIfNecessary(wayTags, ways, Collections.singleton(targetWay));
 
         LinkedList<Command> cmds = new LinkedList<Command>();
         LinkedList<Way> deletedWays = new LinkedList<Way>(ways);
         deletedWays.remove(targetWay);
 
         cmds.add(new ChangeCommand(targetWay, modifiedTargetWay));
-        cmds.addAll(dialog.buildResolutionCommands());
+        cmds.addAll(resolution);
         cmds.add(new DeleteCommand(deletedWays));
         final SequenceCommand sequenceCommand = new SequenceCommand(tr("Combine {0} ways", ways.size()), cmds);
 

src/org/openstreetmap/josm/actions/JoinAreasAction.java

@@ -42 +42 @@
 import org.openstreetmap.josm.data.osm.TagCollection;
 import org.openstreetmap.josm.data.osm.Way;
 import org.openstreetmap.josm.gui.conflict.tags.CombinePrimitiveResolverDialog;
-import org.openstreetmap.josm.gui.conflict.tags.TagConflictResolutionUtil;
 import org.openstreetmap.josm.tools.Geometry;
 import org.openstreetmap.josm.tools.Pair;
 import org.openstreetmap.josm.tools.Shortcut;
@@ -537 +536 @@
             ways.addAll(pol.innerWays);
         }
 
-        if (ways.size() < 2)
+        if (ways.size() < 2) {
             return true;
-
-        //mostly copied from CombineWayAction.java.
-        TagCollection wayTags = TagCollection.unionOfAllPrimitives(ways);
-        TagCollection completeWayTags = new TagCollection(wayTags);
-        TagConflictResolutionUtil.combineTigerTags(completeWayTags);
-        TagConflictResolutionUtil.normalizeTagCollectionBeforeEditing(completeWayTags, ways);
-        TagCollection tagsToEdit = new TagCollection(completeWayTags);
-        TagConflictResolutionUtil.completeTagCollectionForEditing(tagsToEdit);
-
-        CombinePrimitiveResolverDialog dialog = CombinePrimitiveResolverDialog.getInstance();
-        dialog.getTagConflictResolverModel().populate(tagsToEdit, completeWayTags.getKeysWithMultipleValues());
-        dialog.setTargetPrimitive(ways.get(0));
-        Collection<Relation> parentRelations = CombineWayAction.getParentRelations(ways);
-        parentRelations = filterOwnMultipolygonRelations(parentRelations, polygons);
-        dialog.getRelationMemberConflictResolverModel().populate(
-                parentRelations,
-                ways
-        );
-        dialog.prepareDefaultDecisions();
-
-        // resolve tag conflicts if necessary
-        //
-        if (!completeWayTags.isApplicableToPrimitive() || !parentRelations.isEmpty()) {
-            dialog.setVisible(true);
-            if (dialog.isCanceled())
-                return false;
         }
 
-        for (Way way : ways) {
-            dialog.setTargetPrimitive(way);
-            cmds.addAll(dialog.buildResolutionCommands());
+        TagCollection wayTags = TagCollection.unionOfAllPrimitives(ways);
+        try {
+            cmds.addAll(CombinePrimitiveResolverDialog.launchIfNecessary(wayTags, ways, ways));
+            commitCommands(marktr("Fix tag conflicts"));
+            return true;
+        } catch (UserCancelException ex) {
+            return false;
         }
-
-        commitCommands(marktr("Fix tag conflicts"));
-        return true;
     }
 
     /**
@@ -1208 +1182 @@
         Set<Way> processedOuterWays = new LinkedHashSet<Way>();
         Set<Way> processedInnerWays = new LinkedHashSet<Way>();
 
-        for (Relation r : CombineWayAction.getParentRelations(selectedWays)) {
+        for (Relation r : OsmPrimitive.getParentRelations(selectedWays)) {
             if (r.isDeleted() || !r.isMultipolygon()) {
                 continue;
             }

src/org/openstreetmap/josm/actions/MergeNodesAction.java

@@ -8 +8 @@
 import java.awt.event.KeyEvent;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.List;
@@ -21 +22 @@
 import org.openstreetmap.josm.command.Command;
 import org.openstreetmap.josm.command.DeleteCommand;
 import org.openstreetmap.josm.command.SequenceCommand;
+import org.openstreetmap.josm.corrector.UserCancelException;
 import org.openstreetmap.josm.data.coor.EastNorth;
 import org.openstreetmap.josm.data.coor.LatLon;
 import org.openstreetmap.josm.data.osm.Node;
@@ -32 +34 @@
 import org.openstreetmap.josm.gui.HelpAwareOptionPane;
 import org.openstreetmap.josm.gui.HelpAwareOptionPane.ButtonSpec;
 import org.openstreetmap.josm.gui.conflict.tags.CombinePrimitiveResolverDialog;
-import org.openstreetmap.josm.gui.conflict.tags.TagConflictResolutionUtil;
 import org.openstreetmap.josm.gui.layer.OsmDataLayer;
 import org.openstreetmap.josm.tools.CheckParameterUtil;
 import org.openstreetmap.josm.tools.ImageProvider;
@@ -255 +256 @@
     public static Command mergeNodes(OsmDataLayer layer, Collection<Node> nodes, Node targetNode, Node targetLocationNode) {
         CheckParameterUtil.ensureParameterNotNull(layer, "layer");
         CheckParameterUtil.ensureParameterNotNull(targetNode, "targetNode");
-        if (nodes == null)
+        if (nodes == null) {
             return null;
+        }
 
         Set<RelationToChildReference> relationToNodeReferences = RelationToChildReference.getRelationToChildReferences(nodes);
 
-        // build the tag collection
-        //
-        TagCollection nodeTags = TagCollection.unionOfAllPrimitives(nodes);
-        TagConflictResolutionUtil.combineTigerTags(nodeTags);
-        TagConflictResolutionUtil.normalizeTagCollectionBeforeEditing(nodeTags, nodes);
-        TagCollection nodeTagsToEdit = new TagCollection(nodeTags);
-        TagConflictResolutionUtil.completeTagCollectionForEditing(nodeTagsToEdit);
+        try {
+            TagCollection nodeTags = TagCollection.unionOfAllPrimitives(nodes);
+            List<Command> resultion = CombinePrimitiveResolverDialog.launchIfNecessary(nodeTags, nodes, Collections.singleton(targetNode));
+            LinkedList<Command> cmds = new LinkedList<Command>();
 
-        // launch a conflict resolution dialog, if necessary
-        //
-        CombinePrimitiveResolverDialog dialog = CombinePrimitiveResolverDialog.getInstance();
-        dialog.getTagConflictResolverModel().populate(nodeTagsToEdit, nodeTags.getKeysWithMultipleValues());
-        dialog.getRelationMemberConflictResolverModel().populate(relationToNodeReferences);
-        dialog.setTargetPrimitive(targetNode);
-        dialog.prepareDefaultDecisions();
-        // conflict resolution is necessary if there are conflicts in the merged tags
-        // or if at least one of the merged nodes is referred to by a relation
-        //
-        if (! nodeTags.isApplicableToPrimitive() || relationToNodeReferences.size() > 1) {
-            dialog.setVisible(true);
-            if (dialog.isCanceled())
-                return null;
-        }
-        LinkedList<Command> cmds = new LinkedList<Command>();
+            // the nodes we will have to delete
+            //
+            Collection<Node> nodesToDelete = new HashSet<Node>(nodes);
+            nodesToDelete.remove(targetNode);
 
-        // the nodes we will have to delete
-        //
-        Collection<Node> nodesToDelete = new HashSet<Node>(nodes);
-        nodesToDelete.remove(targetNode);
+            // fix the ways referring to at least one of the merged nodes
+            //
+            Collection<Way> waysToDelete = new HashSet<Way>();
+            List<Command> wayFixCommands = fixParentWays(
+                    nodesToDelete,
+                    targetNode);
+            if (wayFixCommands == null) {
+                return null;
+            }
+            cmds.addAll(wayFixCommands);
 
-        // fix the ways referring to at least one of the merged nodes
-        //
-        Collection<Way> waysToDelete= new HashSet<Way>();
-        List<Command> wayFixCommands = fixParentWays(
-                nodesToDelete,
-                targetNode
-        );
-        if (wayFixCommands == null)
+            // build the commands
+            //
+            if (targetNode != targetLocationNode) {
+                Node newTargetNode = new Node(targetNode);
+                newTargetNode.setCoor(targetLocationNode.getCoor());
+                cmds.add(new ChangeCommand(targetNode, newTargetNode));
+            }
+            cmds.addAll(resultion);
+            if (!nodesToDelete.isEmpty()) {
+                cmds.add(new DeleteCommand(nodesToDelete));
+            }
+            if (!waysToDelete.isEmpty()) {
+                cmds.add(new DeleteCommand(waysToDelete));
+            }
+            Command cmd = new SequenceCommand(tr("Merge {0} nodes", nodes.size()), cmds);
+            return cmd;
+        } catch (UserCancelException ex) {
             return null;
-        cmds.addAll(wayFixCommands);
-
-        // build the commands
-        //
-        if (targetNode != targetLocationNode) {
-            Node newTargetNode = new Node(targetNode);
-            newTargetNode.setCoor(targetLocationNode.getCoor());
-            cmds.add(new ChangeCommand(targetNode, newTargetNode));
-        }
-        cmds.addAll(dialog.buildResolutionCommands());
-        if (!nodesToDelete.isEmpty()) {
-            cmds.add(new DeleteCommand(nodesToDelete));
-        }
-        if (!waysToDelete.isEmpty()) {
-            cmds.add(new DeleteCommand(waysToDelete));
         }
-        Command cmd = new SequenceCommand(tr("Merge {0} nodes", nodes.size()), cmds);
-        return cmd;
     }
 
     @Override

src/org/openstreetmap/josm/data/osm/OsmPrimitive.java

@@ -1134 +1134 @@
         return condition.match(this);
     }
 
+    /**
+     * Replies the set of referring relations
+     *
+     * @return the set of referring relations
+     */
+    public static Set<Relation> getParentRelations(Collection<? extends OsmPrimitive> primitives) {
+        HashSet<Relation> ret = new HashSet<Relation>();
+        for (OsmPrimitive w : primitives) {
+            ret.addAll(OsmPrimitive.getFilteredList(w.getReferrers(), Relation.class));
+        }
+        return ret;
+    }
 }

src/org/openstreetmap/josm/gui/DefaultNameFormatter.java

@@ -36 +36 @@
 import org.openstreetmap.josm.tools.AlphanumComparator;
 import org.openstreetmap.josm.tools.I18n;
 import org.openstreetmap.josm.tools.TaggingPresetNameTemplateList;
+import org.openstreetmap.josm.tools.Utils;
+import org.openstreetmap.josm.tools.Utils.Function;
 
 /**
  * This is the default implementation of a {@see NameFormatter} for names of {@see OsmPrimitive}s.
@@ -685 +687 @@
     }
 
     public String formatAsHtmlUnorderedList(Collection<? extends OsmPrimitive> primitives) {
-        StringBuilder sb = new StringBuilder(1024);
-        sb.append("<ul>");
-        for (OsmPrimitive i : primitives) {
-            sb.append("<li>").append(i.getDisplayName(this)).append("</li>");
-        }
-        sb.append("</ul>");
-        return sb.toString();
+        return Utils.joinAsHtmlUnorderedList(Utils.transform(primitives, new Function<OsmPrimitive, String>() {
+
+            @Override
+            public String apply(OsmPrimitive x) {
+                return x.getDisplayName(DefaultNameFormatter.this);
+            }
+        }));
     }
 
     public String formatAsHtmlUnorderedList(OsmPrimitive... primitives) {

src/org/openstreetmap/josm/gui/conflict/tags/CombinePrimitiveResolverDialog.java

@@ -3 +3 @@
 
 import static org.openstreetmap.josm.gui.help.HelpUtil.ht;
 import static org.openstreetmap.josm.tools.I18n.tr;
+import static org.openstreetmap.josm.tools.I18n.trn;
 
 import java.awt.BorderLayout;
 import java.awt.Component;
@@ -15 +16 @@
 import java.awt.event.WindowEvent;
 import java.beans.PropertyChangeEvent;
 import java.beans.PropertyChangeListener;
+import java.util.Collection;
 import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.List;
@@ -29 +31 @@
 import javax.swing.JSplitPane;
 
 import org.openstreetmap.josm.Main;
+import org.openstreetmap.josm.actions.ExpertToggleAction;
 import org.openstreetmap.josm.command.ChangePropertyCommand;
 import org.openstreetmap.josm.command.Command;
+import org.openstreetmap.josm.corrector.UserCancelException;
+import org.openstreetmap.josm.data.osm.NameFormatter;
 import org.openstreetmap.josm.data.osm.Node;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
 import org.openstreetmap.josm.data.osm.Relation;
 import org.openstreetmap.josm.data.osm.TagCollection;
 import org.openstreetmap.josm.data.osm.Way;
+import org.openstreetmap.josm.gui.ConditionalOptionPaneUtil;
 import org.openstreetmap.josm.gui.DefaultNameFormatter;
 import org.openstreetmap.josm.gui.SideButton;
 import org.openstreetmap.josm.gui.help.ContextSensitiveHelpAction;
 import org.openstreetmap.josm.gui.help.HelpUtil;
 import org.openstreetmap.josm.tools.ImageProvider;
+import org.openstreetmap.josm.tools.Utils;
+import org.openstreetmap.josm.tools.Utils.Function;
 import org.openstreetmap.josm.tools.WindowGeometry;
 
 /**
  * This dialog helps to resolve conflicts occurring when ways are combined or
  * nodes are merged.
  *
+ * Usage: {@link #launchIfNecessary} followed by {@link #buildResolutionCommands}.
+ *
+ * Prior to {@link #launchIfNecessary}, the following usage sequence was needed:
+ *
  * There is a singleton instance of this dialog which can be retrieved using
  * {@see #getInstance()}.
  *
@@ -81 +93 @@
      * Replies the unique instance of the dialog
      *
      * @return the unique instance of the dialog
+     * @deprecated use {@link #launchIfNecessary} instead.
      */
+    @Deprecated
     public static CombinePrimitiveResolverDialog getInstance() {
         if (instance == null) {
             instance = new CombinePrimitiveResolverDialog(Main.parent);
@@ -420 +434 @@
             }
         }
     }
+
+    public static List<Command> launchIfNecessary(
+            final TagCollection tagsOfPrimitives,
+            final Collection<? extends OsmPrimitive> primitives,
+            final Collection<? extends OsmPrimitive> targetPrimitives) throws UserCancelException {
+
+        final TagCollection completeWayTags = new TagCollection(tagsOfPrimitives);
+        TagConflictResolutionUtil.combineTigerTags(completeWayTags);
+        TagConflictResolutionUtil.normalizeTagCollectionBeforeEditing(completeWayTags, primitives);
+        final TagCollection tagsToEdit = new TagCollection(completeWayTags);
+        TagConflictResolutionUtil.completeTagCollectionForEditing(tagsToEdit);
+
+        final CombinePrimitiveResolverDialog dialog = CombinePrimitiveResolverDialog.getInstance();
+
+        dialog.getTagConflictResolverModel().populate(tagsToEdit, completeWayTags.getKeysWithMultipleValues());
+
+        final Set<Relation> parentRelations = OsmPrimitive.getParentRelations(primitives);
+        dialog.getRelationMemberConflictResolverModel().populate(parentRelations, primitives);
+        dialog.prepareDefaultDecisions();
+
+        // show information dialog to non-experts
+        if (!completeWayTags.isApplicableToPrimitive() && !ExpertToggleAction.isExpert()) {
+            String conflicts = Utils.joinAsHtmlUnorderedList(Utils.transform(completeWayTags.getKeysWithMultipleValues(), new Function<String, String>() {
+
+                @Override
+                public String apply(String key) {
+                    return tr("{0} ({1})", key, Utils.join(tr(", "), Utils.transform(completeWayTags.getValues(key), new Function<String, String>() {
+
+                        @Override
+                        public String apply(String x) {
+                            return x == null || x.isEmpty() ? tr("<i>missing</i>") : x;
+                        }
+                    })));
+                }
+            }));
+            String msg = tr("You are about to combine {0} objects, "
+                    + "but the following tags are used conflictingly:<br/>{1}"
+                    + "If these objects are combined, the resulting object may have unwanted tags.<br/>"
+                    + "If you want to continue, you are shown a dialog to fix the conflicting tags.<br/><br/>"
+                    + "Do you want to continue?",
+                    primitives.size(), conflicts);
+            if (!ConditionalOptionPaneUtil.showConfirmationDialog(
+                    "combine_tags",
+                    Main.parent,
+                    "<html>" + msg + "</html>",
+                    tr("Combine confirmation"),
+                    JOptionPane.YES_NO_OPTION,
+                    JOptionPane.QUESTION_MESSAGE,
+                    JOptionPane.YES_OPTION)) {
+                throw new UserCancelException();
+            }
+        }
+
+        if (!parentRelations.isEmpty() && !ExpertToggleAction.isExpert()) {
+            String msg = trn("You are about to combine {1} objects, "
+                    + "which are part of {0} relation:<br/>{2}"
+                    + "Combining these objects may break this relation. If you are unsure, please cancel this operation.<br/>"
+                    + "If you want to continue, you are shown a dialog to decide how to adapt the relation.<br/><br/>"
+                    + "Do you want to continue?",
+                    "You are about to combine {1} objects, "
+                    + "which are part of {0} relations:<br/>{2}"
+                    + "Combining these objects may break these relations. If you are unsure, please cancel this operation.<br/>"
+                    + "If you want to continue, you are shown a dialog to decide how to adapt the relations.<br/><br/>"
+                    + "Do you want to continue?",
+                    parentRelations.size(), parentRelations.size(), primitives.size(),
+                    DefaultNameFormatter.getInstance().formatAsHtmlUnorderedList(parentRelations));
+            if (!ConditionalOptionPaneUtil.showConfirmationDialog(
+                    "combine_tags",
+                    Main.parent,
+                    "<html>" + msg + "</html>",
+                    tr("Combine confirmation"),
+                    JOptionPane.YES_NO_OPTION,
+                    JOptionPane.QUESTION_MESSAGE,
+                    JOptionPane.YES_OPTION)) {
+                throw new UserCancelException();
+            }
+        }
+
+        // resolve tag conflicts if necessary
+        if (!completeWayTags.isApplicableToPrimitive() || !parentRelations.isEmpty()) {
+            dialog.setVisible(true);
+            if (dialog.isCanceled()) {
+                throw new UserCancelException();
+            }
+        }
+        List<Command> cmds = new LinkedList<Command>();
+        for (OsmPrimitive i : targetPrimitives) {
+            dialog.setTargetPrimitive(i);
+            cmds.addAll(dialog.buildResolutionCommands());
+        }
+        return cmds;
+    }
 }

src/org/openstreetmap/josm/gui/conflict/tags/TagConflictResolver.java

@@ -27 +27 @@
 
     /** the model for the tag conflict resolver */
     private TagConflictResolverModel model;
-    /** selects wheter only tags with conflicts are displayed */
+    /** selects whether only tags with conflicts are displayed */
     private JCheckBox cbShowTagsWithConflictsOnly;
     private JCheckBox cbShowTagsWithMultiValuesOnly;
 
@@ -40 +40 @@
         gc.weighty = 1.0;
         gc.weightx = 1.0;
         gc.anchor = GridBagConstraints.LINE_START;
+        gc.gridwidth = 2;
         pnl.add(new JLabel(tr("<html>Please select the values to keep for the following tags.</html>")), gc);
 
+        gc.gridwidth = 1;
         gc.gridy = 1;
         gc.fill = GridBagConstraints.HORIZONTAL;
         gc.weighty = 0.0;

src/org/openstreetmap/josm/tools/Utils.java

@@ -22 +22 @@
 import java.util.Collection;
 import java.util.Iterator;
 import java.util.List;
+import org.openstreetmap.josm.data.osm.OsmPrimitive;
 
 /**
  * Basic utils, that can be useful in different parts of the program.
@@ -165 +166 @@
         return s.toString();
     }
 
+    public static String joinAsHtmlUnorderedList(Collection<?> values) {
+        StringBuilder sb = new StringBuilder(1024);
+        sb.append("<ul>");
+        for (Object i : values) {
+            sb.append("<li>").append(i).append("</li>");
+        }
+        sb.append("</ul>");
+        return sb.toString();
+    }
+
     /**
      * convert Color to String
      * (Color.toString() omits alpha value)
@@ -422 +433 @@
      * @param f a function that transforms objects of {@code A} to objects of {@code B}
      * @return the transformed unmodifiable collection
      */
-    public static <A, B> Collection<B> transform(final Collection<A> c, final Function<A, B> f) {
+    public static <A, B> Collection<B> transform(final Collection<? extends A> c, final Function<A, B> f) {
         return new Collection<B>() {
 
             @Override
@@ -459 +470 @@
             public Iterator<B> iterator() {
                 return new Iterator<B>() {
 
-                    private Iterator<A> it = c.iterator();
+                    private Iterator<? extends A> it = c.iterator();
 
                     @Override
                     public boolean hasNext() {