Ticket #19859: 19859.patch

src/org/openstreetmap/josm/plugins/terracer/TerracerAction.java

@@ -63 +63 @@
  */
 public final class TerracerAction extends JosmAction {
 
-    private Collection<Command> commands;
-    private Collection<OsmPrimitive> primitives;
-    private TagCollection tagsInConflict;
-
+    private transient Collection<Command> commands;
+    private final transient List<Pair<TagCollection, List<OsmPrimitive>>> conflicts = new ArrayList<>();
+    
     public TerracerAction() {
         super(tr("Terrace a building"), "terrace",
                 tr("Creates individual buildings from a long building."),
@@ -75 +74 @@
                         Shortcut.SHIFT), true);
     }
 
-    protected static Set<Relation> findAssociatedStreets(Collection<OsmPrimitive> objects) {
+    private static Set<Relation> findAssociatedStreets(Collection<OsmPrimitive> objects) {
         Set<Relation> result = new HashSet<>();
         if (objects != null) {
             for (OsmPrimitive c : objects) {
@@ -180 +179 @@
 
         } catch (InvalidUserInputException ex) {
             Logging.warn("Terracer: "+ex.getMessage());
-            new ExtendedDialog(MainApplication.getMainFrame(), tr("Invalid selection"), new String[] {"OK"})
-                .setButtonIcons(new String[] {"ok"}).setIcon(JOptionPane.INFORMATION_MESSAGE)
+            new ExtendedDialog(MainApplication.getMainFrame(), tr("Invalid selection"), "OK")
+                .setButtonIcons("ok").setIcon(JOptionPane.INFORMATION_MESSAGE)
                 .setContent(tr("Select a single, closed way of at least four nodes. " +
                     "(Optionally you can also select a street for the addr:street tag " +
                     "and a node to mark the start of numbering.)"))
@@ -232 +231 @@
 
     private void cleanup() {
         commands = null;
-        primitives = null;
-        tagsInConflict = null;
+        conflicts.clear();
     }
 
     public Integer getNumber(String number) {
@@ -246 +244 @@
 
     /**
      * Sorts the house number nodes according their numbers only
-     *
-     * @param house
-     *            number nodes
      */
     static class HousenumberNodeComparator implements Comparator<Node> {
         private final Pattern pat = Pattern.compile("^(\\d+)\\s*(.*)");
@@ -314 +309 @@
                 boolean keepOutline, String buildingValue) throws UserCancelException {
         final int nb;
         Integer to = null, from = null;
-        if (housenumbers == null || housenumbers.isEmpty()) {
+        if (housenumbers.isEmpty()) {
             to = getNumber(end);
             from = getNumber(start);
             if (to != null && from != null) {
@@ -337 +332 @@
 
         final boolean swap = init != null && (interp.a.lastNode().equals(init) || interp.b.lastNode().equals(init));
 
-        final double frontLength = wayLength(interp.a);
-        final double backLength = wayLength(interp.b);
+        final double frontLength = interp.a.getLength();
+        final double backLength = interp.b.getLength();
 
         // new nodes array to hold all intermediate nodes
         // This set will contain at least 4 existing nodes from the original outline
@@ -430 +425 @@
             }
         }
 
-        UndoRedoHandler.getInstance().add(createTerracingCommand(outline));
+        UndoRedoHandler.getInstance().add(createTerracingCommand());
         if (nb <= 1 && street != null) {
             // Select the way (for quick selection of a new house (with the same way))
             MainApplication.getLayerManager().getEditDataSet().setSelected(street);
@@ -470 +465 @@
         this.commands.add(new AddCommand(getLayerManager().getEditDataSet(), associatedStreet));
     }
 
-    private Command createTerracingCommand(final Way outline) {
+    private Command createTerracingCommand() {
         return new SequenceCommand(tr("Terrace"), commands) {
             @Override
             public boolean executeCommand() {
                 boolean result = super.executeCommand();
-                if (result && tagsInConflict != null) {
+                if (result && !conflicts.isEmpty()) {
                     try {
-                        // Build conflicts commands only after all primitives have been added to dataset to fix #8942
-                        List<Command> conflictCommands = CombinePrimitiveResolverDialog.launchIfNecessary(
-                                tagsInConflict, primitives, Collections.singleton(outline));
-                        if (!conflictCommands.isEmpty()) {
-                            List<Command> newCommands = new ArrayList<>(commands);
-                            newCommands.addAll(conflictCommands);
-                            setSequence(newCommands.toArray(new Command[0]));
-                            // Run conflicts commands
-                            for (int i = 0; i < conflictCommands.size(); i++) {
-                                result = conflictCommands.get(i).executeCommand();
-                                if (!result && !continueOnError) {
-                                    setSequenceComplete(false);
-                                    undoCommands(commands.size()+i-1);
-                                    return false;
+                        for (Pair<TagCollection, List<OsmPrimitive>> conflict : conflicts) {
+                            // Build conflicts commands only after all primitives have been added to dataset to fix #8942
+                            OsmPrimitive target = getLayerManager().getEditDataSet().getPrimitiveById(conflict.b.get(1));
+                            if (target == null)
+                                continue;
+                            List<Command> conflictCommands = CombinePrimitiveResolverDialog.launchIfNecessary(
+                                    conflict.a, Collections.singleton(conflict.b.get(0)),
+                                    Collections.singleton(target));
+                            if (!conflictCommands.isEmpty()) {
+                                List<Command> newCommands = new ArrayList<>(commands);
+                                newCommands.addAll(conflictCommands);
+                                setSequence(newCommands.toArray(new Command[0]));
+                                // Run conflicts commands
+                                for (int i = 0; i < conflictCommands.size(); i++) {
+                                    result = conflictCommands.get(i).executeCommand();
+                                    if (!result && !continueOnError) {
+                                        setSequenceComplete(false);
+                                        undoCommands(commands.size()+i-1);
+                                        return false;
+                                    }
                                 }
                             }
                         }
@@ -511 +512 @@
      * @param streetName the name of a street (may be null). Used if not null and street is null.
      * @param associatedStreet The associated street. Used to determine if addr:street should be set or not.
      * @param buildingValue The value for {@code building} key to add
-     * @return {@code outline}
      * @throws UserCancelException if user cancels the operation
      */
     private void addressBuilding(Way outline, Way street, String streetName, Relation associatedStreet,
@@ -521 +521 @@
         boolean numberAdded = false;
         Map<String, String> tags = new HashMap<>();
         if (houseNum != null) {
-            primitives = Arrays.asList(new OsmPrimitive[]{houseNum, outline});
+            List<OsmPrimitive> primitives = Arrays.asList(houseNum, outline);
 
             TagCollection tagsToCopy = TagCollection.unionOfAllPrimitives(primitives).getTagsFor(houseNum.keySet());
-            tagsInConflict = tagsToCopy.getTagsFor(tagsToCopy.getKeysWithMultipleValues());
+            TagCollection tagsInConflict = tagsToCopy.getTagsFor(tagsToCopy.getKeysWithMultipleValues());
+            if (!tagsInConflict.isEmpty()) {
+                conflicts.add(Pair.create(tagsInConflict, primitives));
+            }
             tagsToCopy = tagsToCopy.minus(tagsInConflict).minus(TagCollection.from(outline));
 
             for (Tag tag : tagsToCopy) {
@@ -582 +585 @@
     }
 
     /**
-     * Calculates the great circle length of a way by summing the great circle
-     * distance of each pair of nodes.
-     *
-     * @param w The way to calculate length of.
-     * @return The length of the way.
-     */
-    private double wayLength(Way w) {
-        double length = 0.0;
-        for (Pair<Node, Node> p : w.getNodePairs(false)) {
-            length += p.a.getCoor().greatCircleDistance(p.b.getCoor());
-        }
-        return length;
-    }
-
-    /**
      * Given a way, try and find a definite front and back by looking at the
      * segments to find the "sides". Sides are assumed to be single segments
      * which cannot be contiguous.