Index: src/org/openstreetmap/josm/plugins/terracer/TerracerAction.java =================================================================== --- src/org/openstreetmap/josm/plugins/terracer/TerracerAction.java (revision 35560) +++ src/org/openstreetmap/josm/plugins/terracer/TerracerAction.java (working copy) @@ -63,10 +63,9 @@ */ public final class TerracerAction extends JosmAction { - private Collection commands; - private Collection primitives; - private TagCollection tagsInConflict; - + private transient Collection commands; + private final transient List>> conflicts = new ArrayList<>(); + public TerracerAction() { super(tr("Terrace a building"), "terrace", tr("Creates individual buildings from a long building."), @@ -75,7 +74,7 @@ Shortcut.SHIFT), true); } - protected static Set findAssociatedStreets(Collection objects) { + private static Set findAssociatedStreets(Collection objects) { Set result = new HashSet<>(); if (objects != null) { for (OsmPrimitive c : objects) { @@ -180,8 +179,8 @@ } 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,8 +231,7 @@ private void cleanup() { commands = null; - primitives = null; - tagsInConflict = null; + conflicts.clear(); } public Integer getNumber(String number) { @@ -246,9 +244,6 @@ /** * Sorts the house number nodes according their numbers only - * - * @param house - * number nodes */ static class HousenumberNodeComparator implements Comparator { private final Pattern pat = Pattern.compile("^(\\d+)\\s*(.*)"); @@ -314,7 +309,7 @@ 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,8 +332,8 @@ 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,7 +425,7 @@ } } - 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,27 +465,33 @@ 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 conflictCommands = CombinePrimitiveResolverDialog.launchIfNecessary( - tagsInConflict, primitives, Collections.singleton(outline)); - if (!conflictCommands.isEmpty()) { - List 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> 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 conflictCommands = CombinePrimitiveResolverDialog.launchIfNecessary( + conflict.a, Collections.singleton(conflict.b.get(0)), + Collections.singleton(target)); + if (!conflictCommands.isEmpty()) { + List 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,7 +512,6 @@ * @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,10 +521,13 @@ boolean numberAdded = false; Map tags = new HashMap<>(); if (houseNum != null) { - primitives = Arrays.asList(new OsmPrimitive[]{houseNum, outline}); + List 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,21 +585,6 @@ } /** - * 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 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.