Ticket #3780: fix-shift-selection-2.patch

(a) core/src/org/openstreetmap/josm/actions/mapmode/DrawAction.java~fix-shift-selection vs. (b) core-dave/src/org/openstreetmap/josm/actions/mapmode/DrawAction.java

@@ -244 +244 @@
         // when exiting we let everybody know about the currently selected
         // primitives
         //
-        DataSet.fireSelectionChanged(getCurrentDataSet().getSelected());
+        getCurrentDataSet().fireSelectionChanged();
     }
 
     /**
@@ -271 +271 @@
 
     private void tryAgain(MouseEvent e) {
         getCurrentDataSet().setSelected();
-        DataSet.fireSelectionChanged(getCurrentDataSet().getSelected());
+        Main.main.getCurrentDataSet().fireSelectionChanged();
         mouseClicked(e);
     }
 
@@ -283 +283 @@
     private void finishDrawing() {
         // let everybody else know about the current selection
         //
-        DataSet.fireSelectionChanged(getCurrentDataSet().getSelected());
+        Main.main.getCurrentDataSet().fireSelectionChanged();
         lastUsedNode = null;
         wayIsFinished = true;
         Main.map.selectSelectTool(true);
@@ -584 +584 @@
                     (posn0 >= 1                             && targetNode.equals(selectedWay.getNode(posn0-1))) || // previous node
                     (posn0 < selectedWay.getNodesCount()-1) && targetNode.equals(selectedWay.getNode(posn0+1))) {  // next node
                 getCurrentDataSet().setSelected(targetNode);
-                DataSet.fireSelectionChanged(getCurrentDataSet().getSelected());
+                getCurrentDataSet().fireSelectionChanged();
                 lastUsedNode = targetNode;
                 return true;
             }

(a) core/src/org/openstreetmap/josm/actions/mapmode/SelectAction.java~fix-shift-selection vs. (b) core-dave/src/org/openstreetmap/josm/actions/mapmode/SelectAction.java

@@ -498 +498 @@
                         }
                     }
                 }
-                DataSet.fireSelectionChanged(selection);
+                getCurrentDataSet().fireSelectionChanged();
             }
         }
 
@@ -514 +514 @@
 
     public void selectPrims(Collection<OsmPrimitive> selectionList, boolean shift,
             boolean ctrl, boolean released, boolean area) {
+        DataSet ds = getCurrentDataSet();
         if ((shift && ctrl) || (ctrl && !released))
             return; // not allowed together
 
-        Collection<OsmPrimitive> curSel;
-        if (!ctrl && !shift) {
-            curSel = new LinkedList<OsmPrimitive>(); // new selection will replace the old.
+        // plain clicks with no modifiers clear the selection
+        if (!ctrl && !shift)
+            ds.clearSelection();
+
+        if (ctrl) {
+            // Ctrl on an item toggles its selection status,
+            // but Ctrl on an *area* just clears those items
+            // out of the selection.
+            if (area)
+                ds.clearSelection(selectionList);
+            else
+                ds.toggleSelected(selectionList);
         } else {
-            curSel = getCurrentDataSet().getSelected();
+            // This is either a plain click (which means we
+            // previously cleared the selection), or a
+            // shift-click where we are adding things to an
+            // existing selection.
+            ds.addSelected(selectionList);
         }
-
-        for (OsmPrimitive osm : selectionList)
-        {
-            if (ctrl)
-            {
-                if(curSel.contains(osm)) {
-                    curSel.remove(osm);
-                } else if(!area) {
-                    curSel.add(osm);
-                }
-            } else {
-                curSel.add(osm);
-            }
-        }
-        getCurrentDataSet().setSelected(curSel);
+        ds.fireSelectionChanged();
         Main.map.mapView.repaint();
     }
 

(a) core/src/org/openstreetmap/josm/actions/ReverseWayAction.java~fix-shift-selection vs. (b) core-dave/src/org/openstreetmap/josm/actions/ReverseWayAction.java

@@ -80 +80 @@
         }
         Main.main.undoRedo.add(new SequenceCommand(tr("Reverse ways"), c));
         if (propertiesUpdated) {
-            DataSet.fireSelectionChanged(getCurrentDataSet().getSelected());
+            getCurrentDataSet().fireSelectionChanged();
         }
         Main.map.repaint();
     }

(a) core/src/org/openstreetmap/josm/actions/UploadAction.java~fix-shift-selection vs. (b) core-dave/src/org/openstreetmap/josm/actions/UploadAction.java

@@ -687 +687 @@
             // partially uploaded
             //
             layer.cleanupAfterUpload(processedPrimitives);
-            DataSet.fireSelectionChanged(layer.data.getSelected());
+            layer.data.fireSelectionChanged();
             layer.fireDataChange();
             if (lastException != null) {
                 handleFailedUpload(lastException);

(a) core/src/org/openstreetmap/josm/data/osm/DataSet.java~fix-shift-selection vs. (b) core-dave/src/org/openstreetmap/josm/data/osm/DataSet.java

@@ -241 +241 @@
 
     LinkedHashSet<OsmPrimitive> selectedPrimitives = new LinkedHashSet<OsmPrimitive>();
 
-    public boolean toggleSelected(OsmPrimitive osm) {
+    public boolean toggleSelected(Collection<OsmPrimitive> osm) {
+        for (OsmPrimitive o : osm)
+            this.__toggleSelected(o);
+        fireSelectionChanged();
+        return true;
+    }
+    public boolean toggleSelected(OsmPrimitive... osm) {
+        return this.toggleSelected(Arrays.asList(osm));
+    }
+    private boolean __toggleSelected(OsmPrimitive osm) {
         if (!selectedPrimitives.remove(osm)) {
             selectedPrimitives.add(osm);
         }
@@ -275 +284 @@
     public void setSelected(Collection<? extends OsmPrimitive> selection, boolean fireSelectionChangeEvent) {
         selectedPrimitives = new LinkedHashSet<OsmPrimitive>(selection);
         if (fireSelectionChangeEvent) {
-            fireSelectionChanged(selection);
+            fireSelectionChanged();
         }
     }
 
@@ -313 +322 @@
     public void addSelected(Collection<? extends OsmPrimitive> selection, boolean fireSelectionChangeEvent) {
         selectedPrimitives.addAll(selection);
         if (fireSelectionChangeEvent) {
-            fireSelectionChanged(selection);
+            fireSelectionChanged();
         }
     }
 
@@ -325 +334 @@
         }
         List<OsmPrimitive> list = Arrays.asList(osm);
         setSelected(list);
-        fireSelectionChanged(list);
+        fireSelectionChanged();
     }
 
     /**
@@ -358 +367 @@
     public void clearSelection(OsmPrimitive... osm) {
         clearSelection(Arrays.asList(osm));
     }
-    private void clearSelection(Collection<? extends OsmPrimitive> list) {
+    public void clearSelection(Collection<? extends OsmPrimitive> list) {
         if (list == null)
             return;
         selectedPrimitives.removeAll(list);
     }
+    public void clearSelection() {
+        selectedPrimitives.clear();
+    }
 
     /**
      * Return all selected items in the collection.
@@ -379 +391 @@
         return sel;
     }
 
-    /**
-     * Remember to fire an selection changed event. A call to this will not fire the event
-     * immediately. For more,
-     * @see SelectionChangedListener
-     */
-    public static void fireSelectionChanged(Collection<? extends OsmPrimitive> sel) {
+    public void fireSelectionChanged()
+    {
+        __fireSelectionChanged(selectedPrimitives);
+    }
+
+    public static void __fireSelectionChanged(Collection<? extends OsmPrimitive> sel) {
         for (SelectionChangedListener l : selListeners) {
             l.selectionChanged(sel);
         }

(a) core/src/org/openstreetmap/josm/data/UndoRedoHandler.java~fix-shift-selection vs. (b) core-dave/src/org/openstreetmap/josm/data/UndoRedoHandler.java

@@ -49 +49 @@
         fireCommandsChanged();
 
         // the command may have changed the selection so tell the listeners about the current situation
-        DataSet.fireSelectionChanged(Main.main.getCurrentDataSet().getSelected());
+        Main.main.getCurrentDataSet().fireSelectionChanged();
     }
 
     /**

(a) core/src/org/openstreetmap/josm/gui/dialogs/PropertiesDialog.java~fix-shift-selection vs. (b) core-dave/src/org/openstreetmap/josm/gui/dialogs/PropertiesDialog.java

@@ -266 +266 @@
                             commands));
         }
 
-        DataSet.fireSelectionChanged(sel);
+        Main.main.getCurrentDataSet().fireSelectionChanged();
         selectionChanged(sel); // update whole table
         Main.parent.repaint(); // repaint all - drawing could have been changed
 
@@ -354 +354 @@
         if (value.equals(""))
             return;
         Main.main.undoRedo.add(new ChangePropertyCommand(sel, key, value));
-        DataSet.fireSelectionChanged(sel);
+        Main.main.getCurrentDataSet().fireSelectionChanged();
         selectionChanged(sel); // update table
         Main.parent.repaint(); // repaint all - drawing could have been changed
     }
@@ -825 +825 @@
             String key = propertyData.getValueAt(row, 0).toString();
             Collection<OsmPrimitive> sel = Main.main.getCurrentDataSet().getSelected();
             Main.main.undoRedo.add(new ChangePropertyCommand(sel, key, null));
-            DataSet.fireSelectionChanged(sel);
+            Main.main.getCurrentDataSet().fireSelectionChanged();
             selectionChanged(sel); // update table
 
             int rowCount = propertyTable.getRowCount();
@@ -851 +851 @@
                 rel.removeMembersFor(primitive);
             }
             Main.main.undoRedo.add(new ChangeCommand(cur, rel));
-            DataSet.fireSelectionChanged(sel);
+            Main.main.getCurrentDataSet().fireSelectionChanged();
             selectionChanged(sel); // update whole table
         }
 

(a) core/src/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditor.java~fix-shift-selection vs. (b) core-dave/src/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditor.java

@@ -882 +882 @@
 
         public void actionPerformed(ActionEvent e) {
             getLayer().data.setSelected(memberTableModel.getSelectedChildPrimitives());
-            DataSet.fireSelectionChanged(getLayer().data.getSelected());
         }
 
         public void valueChanged(ListSelectionEvent e) {
@@ -1005 +1004 @@
 
             // make sure everybody is notified about the changes
             //
-            DataSet.fireSelectionChanged(getLayer().data.getSelected());
+            getLayer().data.fireSelectionChanged();
             getLayer().fireDataChange();
             GenericRelationEditor.this.setRelation(newRelation);
             RelationDialogManager.getRelationDialogManager().updateContext(
@@ -1038 +1037 @@
             tagEditorPanel.getModel().applyToPrimitive(editedRelation);
             memberTableModel.applyToRelation(editedRelation);
             Main.main.undoRedo.add(new ChangeCommand(getRelation(), editedRelation));
-            DataSet.fireSelectionChanged(getLayer().data.getSelected());
+            getLayer().data.fireSelectionChanged();
             getLayer().fireDataChange();
             // this will refresh the snapshot and update the dialog title
             //

(a) core/src/org/openstreetmap/josm/gui/dialogs/RelationListDialog.java~fix-shift-selection vs. (b) core-dave/src/org/openstreetmap/josm/gui/dialogs/RelationListDialog.java

@@ -516 +516 @@
                 selection.add(model.getRelation(i));
             }
             Main.map.mapView.getEditLayer().data.setSelected(selection);
-            DataSet.fireSelectionChanged(selection);
+            Main.map.mapView.getEditLayer().data.fireSelectionChanged();
         }
 
         public void valueChanged(ListSelectionEvent e) {
@@ -544 +544 @@
                 members.addAll(r.getMemberPrimitives());
             }
             Main.map.mapView.getEditLayer().data.setSelected(members);
-            DataSet.fireSelectionChanged(members);
         }
 
         protected void updateEnabledState() {

(a) core/src/org/openstreetmap/josm/gui/dialogs/relation/MemberTable.java~fix-shift-selection vs. (b) core-dave/src/org/openstreetmap/josm/gui/dialogs/relation/MemberTable.java

@@ -203 +203 @@
             int row = rows[0];
             OsmPrimitive primitive = getMemberTableModel().getReferredPrimitive(row);
             layer.data.setSelected(primitive);
-            DataSet.fireSelectionChanged(layer.data.getSelected());
             AutoScaleAction action = new AutoScaleAction("selection");
             action.autoScale();
         }

(a) core/src/org/openstreetmap/josm/gui/io/UploadLayerTask.java~fix-shift-selection vs. (b) core-dave/src/org/openstreetmap/josm/gui/io/UploadLayerTask.java

@@ -143 +143 @@
         if (isCancelled())
             return;
         layer.cleanupAfterUpload(processedPrimitives);
-        DataSet.fireSelectionChanged(layer.data.getSelected());
+        layer.data.fireSelectionChanged();;
         layer.fireDataChange();
         layer.onPostUploadToServer();
 
@@ -158 +158 @@
             writer.cancel();
         }
     }
-}
- No newline at end of file
+}

(a) core/src/org/openstreetmap/josm/gui/MapStatus.java~fix-shift-selection vs. (b) core-dave/src/org/openstreetmap/josm/gui/MapStatus.java

@@ -332 +332 @@
                     ds.addSelected(nextSelected);
                 }
             }
-            DataSet.fireSelectionChanged(ds.getSelected());
+            ds.fireSelectionChanged();
         }
 
         /**
@@ -462 +462 @@
                     DataSet ds = Main.main.getCurrentDataSet();
                     // Let the user toggle the selection
                     ds.toggleSelected(osm);
-                    DataSet.fireSelectionChanged(ds.getSelected());
                     l.validate();
                 }
             });

(a) core/src/org/openstreetmap/josm/gui/MapView.java~fix-shift-selection vs. (b) core-dave/src/org/openstreetmap/josm/gui/MapView.java

@@ -502 +502 @@
         if (! (layer instanceof OsmDataLayer)) {
             if (getCurrentDataSet() != null) {
                 getCurrentDataSet().setSelected();
-                DataSet.fireSelectionChanged(getCurrentDataSet().getSelected());
+                getCurrentDataSet().fireSelectionChanged();
             }
         }
         Layer old = activeLayer;