Index: trunk/src/org/openstreetmap/josm/actions/SplitWayAction.java =================================================================== --- trunk/src/org/openstreetmap/josm/actions/SplitWayAction.java (revision 3389) +++ trunk/src/org/openstreetmap/josm/actions/SplitWayAction.java (revision 3392) @@ -10,12 +10,9 @@ import java.util.ArrayList; import java.util.Collection; -import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; import java.util.LinkedList; import java.util.List; -import java.util.Map; import java.util.Set; -import java.util.Map.Entry; import javax.swing.JOptionPane; @@ -98,98 +95,68 @@ List selectedNodes = OsmPrimitive.getFilteredList(selection, Node.class); List selectedWays = OsmPrimitive.getFilteredList(selection, Way.class); - - if (!checkSelection(selection)) { + List selectedRelations = OsmPrimitive.getFilteredList(selection, Relation.class); + List applicableWays = getApplicableWays(selectedWays, selectedNodes); + + if (applicableWays == null) { JOptionPane.showMessageDialog( Main.parent, - tr("The current selection cannot be used for splitting."), + tr("The current selection cannot be used for splitting - no node is selected."), tr("Warning"), - JOptionPane.WARNING_MESSAGE - ); + JOptionPane.WARNING_MESSAGE); return; - } - - - Way selectedWay = null; - if (!selectedWays.isEmpty()){ - selectedWay = selectedWays.get(0); - } - - // If only nodes are selected, try to guess which way to split. This works if there - // is exactly one way that all nodes are part of. - if (selectedWay == null && !selectedNodes.isEmpty()) { - Map wayOccurenceCounter = new HashMap(); - for (Node n : selectedNodes) { - for (Way w : OsmPrimitive.getFilteredList(n.getReferrers(), Way.class)) { - if (!w.isUsable()) { - continue; - } + } else if (applicableWays.isEmpty()) { + JOptionPane.showMessageDialog(Main.parent, + tr("The selected nodes do not share the same way."), + tr("Warning"), + JOptionPane.WARNING_MESSAGE); + return; + } + + { // Remove ways that doesn't have selected node in the middle + Iterator it = applicableWays.iterator(); + WAY_LOOP: + while (it.hasNext()) { + Way w = it.next(); + assert w.isUsable(); // Way is referrer of selected node(s) so it must be usable int last = w.getNodesCount() - 1; - if (last <= 0) { - continue; // zero or one node ways - } boolean circular = w.isClosed(); - int i = 0; - for (Node wn : w.getNodes()) { - if ((circular || (i > 0 && i < last)) && n.equals(wn)) { - Integer old = wayOccurenceCounter.get(w); - wayOccurenceCounter.put(w, (old == null) ? 1 : old + 1); - break; + + for (Node n : selectedNodes) { + int i = w.getNodes().indexOf(n); + if (!(circular || (i > 0 && i < last))) { + it.remove(); + continue WAY_LOOP; } - i++; } } - } - if (wayOccurenceCounter.isEmpty()) { - JOptionPane.showMessageDialog(Main.parent, - trn("The selected node is not in the middle of any way.", - "The selected nodes are not in the middle of any way.", - selectedNodes.size()), - tr("Warning"), - JOptionPane.WARNING_MESSAGE); - return; - } - - for (Entry entry : wayOccurenceCounter.entrySet()) { - if (entry.getValue().equals(selectedNodes.size())) { - if (selectedWay != null) { - JOptionPane.showMessageDialog(Main.parent, - trn("There is more than one way using the node you selected. Please select the way also.", - "There is more than one way using the nodes you selected. Please select the way also.", - selectedNodes.size()), - tr("Warning"), - JOptionPane.WARNING_MESSAGE); - return; - } - selectedWay = entry.getKey(); - } - } - - if (selectedWay == null) { - JOptionPane.showMessageDialog(Main.parent, - tr("The selected nodes do not share the same way."), - tr("Warning"), - JOptionPane.WARNING_MESSAGE); - return; - } - - // If a way and nodes are selected, verify that the nodes are part of the way. - } else if (selectedWay != null && !selectedNodes.isEmpty()) { - - HashSet nds = new HashSet(selectedNodes); - nds.removeAll(selectedWay.getNodes()); - if (!nds.isEmpty()) { - JOptionPane.showMessageDialog(Main.parent, - trn("The selected way does not contain the selected node.", - "The selected way does not contain all the selected nodes.", - selectedNodes.size()), - tr("Warning"), - JOptionPane.WARNING_MESSAGE); - return; - } - } + } + + if (applicableWays.isEmpty()) { + JOptionPane.showMessageDialog(Main.parent, + trn("The selected node is not in the middle of any way.", + "The selected nodes are not in the middle of any way.", + selectedNodes.size()), + tr("Warning"), + JOptionPane.WARNING_MESSAGE); + return; + } else if (applicableWays.size() > 1) { + JOptionPane.showMessageDialog(Main.parent, + trn("There is more than one way using the node you selected. Please select the way also.", + "There is more than one way using the nodes you selected. Please select the way also.", + selectedNodes.size()), + tr("Warning"), + JOptionPane.WARNING_MESSAGE); + return; + } + + // Finally, applicableWays contains only one perfect way + Way selectedWay = applicableWays.get(0); List> wayChunks = buildSplitChunks(selectedWay, selectedNodes); if (wayChunks != null) { - SplitWayResult result = splitWay(getEditLayer(),selectedWay, wayChunks); + List sel = new ArrayList(selectedWays.size() + selectedRelations.size()); + sel.addAll(selectedWays); + sel.addAll(selectedRelations); + SplitWayResult result = splitWay(getEditLayer(),selectedWay, wayChunks, sel); Main.main.undoRedo.add(result.getCommand()); getCurrentDataSet().setSelected(result.getNewSelection()); @@ -197,23 +164,42 @@ } - /** - * Checks if the selection consists of something we can work with. - * Checks only if the number and type of items selected looks good; - * does not check whether the selected items are really a valid - * input for splitting (this would be too expensive to be carried - * out from the selectionChanged listener). - */ - private boolean checkSelection(Collection selection) { - boolean way = false; - boolean node = false; - for (OsmPrimitive p : selection) { - if (p instanceof Way && !way) { - way = true; - } else if (p instanceof Node) { - node = true; - } else - return false; - } - return node; + private List getApplicableWays(List selectedWays, List selectedNodes) { + if (selectedNodes.isEmpty()) + return null; + + // List of ways shared by all nodes + List result = new ArrayList(OsmPrimitive.getFilteredList(selectedNodes.get(0).getReferrers(), Way.class)); + for (int i=1; i it = result.iterator(); + List ref = selectedNodes.get(i).getReferrers(); + while (it.hasNext()) { + if (!ref.contains(it.next())) { + it.remove(); + } + } + } + + { // Remove broken ways + Iterator it = result.iterator(); + while (it.hasNext()) { + if (it.next().getNodesCount() <= 2) { + it.remove(); + } + } + } + + if (selectedWays.isEmpty()) + return result; + else { + // Return only selected ways + Iterator it = result.iterator(); + while (it.hasNext()) { + if (!selectedWays.contains(it.next())) { + it.remove(); + } + } + return result; + } + } @@ -301,8 +287,9 @@ * @return */ - public static SplitWayResult splitWay(OsmDataLayer layer, Way way, List> wayChunks) { + public static SplitWayResult splitWay(OsmDataLayer layer, Way way, List> wayChunks, Collection selection) { // build a list of commands, and also a new selection list Collection commandList = new ArrayList(wayChunks.size()); - List newSelection = new ArrayList(wayChunks.size()); + List newSelection = new ArrayList(selection.size() + wayChunks.size()); + newSelection.addAll(selection); Iterator> chunkIt = wayChunks.iterator(); @@ -312,5 +299,7 @@ changedWay.setNodes(chunkIt.next()); commandList.add(new ChangeCommand(way, changedWay)); - newSelection.add(way); + if (!newSelection.contains(way)) { + newSelection.add(way); + } List newWays = new ArrayList(); @@ -495,8 +484,8 @@ * @return the result from the split operation */ - static public SplitWayResult split(OsmDataLayer layer, Way way, List atNodes){ + static public SplitWayResult split(OsmDataLayer layer, Way way, List atNodes, Collection selection){ List> chunks = buildSplitChunks(way, atNodes); if (chunks == null) return null; - return splitWay(layer,way, chunks); + return splitWay(layer,way, chunks, selection); } @@ -516,5 +505,11 @@ return; } - setEnabled(checkSelection(selection)); + for (OsmPrimitive primitive: selection) { + if (primitive instanceof Node) { + setEnabled(true); // Selection still can be wrong, but let SplitWayAction process and tell user what's wrong + return; + } + } + setEnabled(false); } } Index: trunk/src/org/openstreetmap/josm/command/DeleteCommand.java =================================================================== --- trunk/src/org/openstreetmap/josm/command/DeleteCommand.java (revision 3389) +++ trunk/src/org/openstreetmap/josm/command/DeleteCommand.java (revision 3392) @@ -23,6 +23,4 @@ import javax.swing.JOptionPane; import javax.swing.JPanel; -import javax.swing.tree.DefaultMutableTreeNode; -import javax.swing.tree.MutableTreeNode; import org.openstreetmap.josm.Main; @@ -185,7 +183,7 @@ @Override public Collection getChildren() { - if (toDelete.size() == 1) { + if (toDelete.size() == 1) return null; - } else { + else { List children = new ArrayList(); for (final OsmPrimitive osm : toDelete) { @@ -193,8 +191,8 @@ @Override public JLabel getDescription() { return new JLabel( - tr("Deleted ''{0}''", - osm.getDisplayName(DefaultNameFormatter.getInstance())), - ImageProvider.get(OsmPrimitiveType.from(osm)), JLabel.HORIZONTAL); - } + tr("Deleted ''{0}''", + osm.getDisplayName(DefaultNameFormatter.getInstance())), + ImageProvider.get(OsmPrimitiveType.from(osm)), JLabel.HORIZONTAL); + } @Override public Collection getParticipatingPrimitives() { return Collections.singleton(osm); @@ -425,5 +423,5 @@ chunks.add(n1); chunks.add(n2); - return SplitWayAction.splitWay(layer,ws.way, chunks).getCommand(); + return SplitWayAction.splitWay(layer,ws.way, chunks, Collections.emptyList()).getCommand(); } } @@ -443,9 +441,10 @@ if (a != null) { for (OsmPrimitive osm : primitivesToDelete) { - if (osm.isIncomplete()) + if (osm.isIncomplete()) { incomplete = true; - else if (osm instanceof Node && !osm.isNewOrUndeleted() - && !a.contains(((Node) osm).getCoor())) + } else if (osm instanceof Node && !osm.isNewOrUndeleted() + && !a.contains(((Node) osm).getCoor())) { outside = true; + } } } @@ -453,6 +452,7 @@ { for (OsmPrimitive osm : primitivesToDelete) - if (osm.isIncomplete()) + if (osm.isIncomplete()) { incomplete = true; + } } if(outside)