Ticket #3780: fix-shift-selection.patch

(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/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(Collection<OsmPrimitive> osm) {
+        for (OsmPrimitive o : osm)
+            this.toggleSelected(o);
+        return true;
+    }
+    public boolean toggleSelected(OsmPrimitive... osm) {
+        return this.toggleSelected(Arrays.asList(osm));
+    }
     public boolean toggleSelected(OsmPrimitive osm) {
         if (!selectedPrimitives.remove(osm)) {
             selectedPrimitives.add(osm);
@@ -275 +283 @@
     public void setSelected(Collection<? extends OsmPrimitive> selection, boolean fireSelectionChangeEvent) {
         selectedPrimitives = new LinkedHashSet<OsmPrimitive>(selection);
         if (fireSelectionChangeEvent) {
-            fireSelectionChanged(selection);
+            fireSelectionChanged();
         }
     }
 
@@ -313 +321 @@
     public void addSelected(Collection<? extends OsmPrimitive> selection, boolean fireSelectionChangeEvent) {
         selectedPrimitives.addAll(selection);
         if (fireSelectionChangeEvent) {
-            fireSelectionChanged(selection);
+            fireSelectionChanged();
         }
     }
 
@@ -325 +333 @@
         }
         List<OsmPrimitive> list = Arrays.asList(osm);
         setSelected(list);
-        fireSelectionChanged(list);
+        fireSelectionChanged();
     }
 
     /**
@@ -358 +366 @@
     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 = new LinkedHashSet<OsmPrimitive>();
+    }
 
     /**
      * Return all selected items in the collection.
@@ -379 +390 @@
         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);
         }