Changeset 18260 in josm

Timestamp:

2021-10-09T15:05:34+02:00 (3 years ago)

Author:

Don-vip

Message:

fix #21408 - fix fill-default property, add tests (patch by marcello)

Location:

trunk

Files:

• src/org/openstreetmap/josm/gui/tagging/presets/TaggingPresetItemGuiSupport.java (modified) (1 diff)
• src/org/openstreetmap/josm/gui/tagging/presets/items/Combo.java (modified) (1 diff)
• src/org/openstreetmap/josm/gui/tagging/presets/items/ComboMultiSelect.java (modified) (5 diffs)
• src/org/openstreetmap/josm/gui/tagging/presets/items/MultiSelect.java (modified) (2 diffs)
• test/unit/org/openstreetmap/josm/gui/tagging/presets/items/ComboTest.java (modified) (4 diffs)

trunk/src/org/openstreetmap/josm/gui/tagging/presets/TaggingPresetItemGuiSupport.java

@@ -85 +85 @@
 
     /**
-     * Returns true if all selected primitives matched this preset (before opening the dialog)
+     * Returns true if all selected primitives matched this preset (before opening the dialog).
+     * <p>
+     * This usually means that the preset dialog was opened from the Tags / Memberships panel as
+     * opposed to being opened by selection from the menu or toolbar or the search.
      *
      * @return true if the preset initially matched

trunk/src/org/openstreetmap/josm/gui/tagging/presets/items/Combo.java

@@ -154 +154 @@
         }
 
-        String valueToSelect = getInitialValue(usage);
-        if (valueToSelect != null) {
-            PresetListEntry selItem = find(valueToSelect);
-            if (selItem != null) {
-                combobox.setSelectedItem(selItem);
-            } else {
-                combobox.setText(valueToSelect);
-            }
-        }
+        String initialValue = getInitialValue(usage, support);
+        PresetListEntry selItem = find(initialValue);
+        if (selItem != null) {
+            combobox.setSelectedItem(selItem);
+        } else {
+            combobox.setText(initialValue);
+        }
+
         combobox.addActionListener(l -> support.fireItemValueModified(this, key, getSelectedItem().value));
         combobox.addComponentListener(new ComponentListener());

trunk/src/org/openstreetmap/josm/gui/tagging/presets/items/ComboMultiSelect.java

@@ -23 +23 @@
 
 import org.openstreetmap.josm.data.osm.Tag;
+import org.openstreetmap.josm.gui.tagging.presets.TaggingPresetItemGuiSupport;
 import org.openstreetmap.josm.gui.tagging.presets.TaggingPresetSelector;
 import org.openstreetmap.josm.gui.tagging.presets.TaggingPresets;
@@ -94 +95 @@
     protected final Map<String, PresetListEntry> seenValues = new TreeMap<>();
     protected Usage usage;
-    /** Used to see if the user edited the value. May be null. */
+    /** Used to see if the user edited the value. */
     protected String originalValue;
 
@@ -318 +319 @@
      * Returns the initial value to use for this preset.
      * <p>
-     * The initial value is the value shown in the control when the preset dialogs opens.
+     * The initial value is the value shown in the control when the preset dialog opens. For a
+     * discussion of all the options see the enclosed tickets.
      *
      * @param usage The key Usage
+     * @param support The support
      * @return The initial value to use.
-     */
-    protected String getInitialValue(Usage usage) {
+     *
+     * @see "https://josm.openstreetmap.de/ticket/5564"
+     * @see "https://josm.openstreetmap.de/ticket/12733"
+     * @see "https://josm.openstreetmap.de/ticket/17324"
+     */
+    protected String getInitialValue(Usage usage, TaggingPresetItemGuiSupport support) {
         String initialValue = null;
-        originalValue = null;
+        originalValue = "";
 
         if (usage.hasUniqueValue()) {
@@ -335 +342 @@
             initialValue = DIFFERENT;
             originalValue = initialValue;
-        } else if (PROP_FILL_DEFAULT.get() || isForceUseLastAsDefault()) {
+        } else if (!usage.hadKeys() || isForceUseLastAsDefault() || PROP_FILL_DEFAULT.get()) {
             // at this point no primitive had any value for this key
-            // use the last value no matter what
-            initialValue = LAST_VALUES.get(key);
-        } else if (!usage.hadKeys() && isUseLastAsDefault()) {
-            // use the last value only on objects with no keys at all
-            initialValue = LAST_VALUES.get(key);
-        } else if (!usage.hadKeys()) {
-            // use the default only on objects with no keys at all
-            initialValue = default_;
-        }
-        return initialValue;
+            if (!support.isPresetInitiallyMatches() && isUseLastAsDefault() && LAST_VALUES.containsKey(key)) {
+                initialValue = LAST_VALUES.get(key);
+            } else {
+                initialValue = default_;
+            }
+        }
+        return initialValue != null ? initialValue : "";
     }
 
@@ -354 +358 @@
 
         // no change if same as before
-        if (value.isEmpty() && originalValue == null)
-            return;
         if (value.equals(originalValue))
             return;

trunk/src/org/openstreetmap/josm/gui/tagging/presets/items/MultiSelect.java

@@ -49 +49 @@
         // disable if the selected primitives have different values
         list.setEnabled(usage.hasUniqueValue() || usage.unused());
-        String initialValue = getInitialValue(usage);
+        String initialValue = getInitialValue(usage, support);
 
         // Add values from the preset.
@@ -62 +62 @@
 
         // Select the values in the initial value.
-        if (initialValue != null && !DIFFERENT.equals(initialValue)) {
+        if (!initialValue.isEmpty() && !DIFFERENT.equals(initialValue)) {
             for (String value : initialValue.split(String.valueOf(delimiter), -1)) {
                 PresetListEntry e = new PresetListEntry(value, this);

trunk/test/unit/org/openstreetmap/josm/gui/tagging/presets/items/ComboTest.java

@@ -28 +28 @@
     @RegisterExtension
     @SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD")
-    public JOSMTestRules test = new JOSMTestRules().main().i18n("de");
+    public JOSMTestRules test = new JOSMTestRules().preferences().main().i18n("de");
 
     /**
@@ -55 +55 @@
         KeyedItem.LAST_VALUES.clear();
         KeyedItem.LAST_VALUES.put("addr:country", "AT");
+        Combo.PROP_FILL_DEFAULT.put(false);
+        combo.use_last_as_default = 0;
 
-        combo.use_last_as_default = 0;
         combo.addToPanel(new JPanel(), TaggingPresetItemGuiSupport.create(false, way));
         assertEquals("", combo.getSelectedItem().value);
@@ -79 +80 @@
         combo.addToPanel(new JPanel(), TaggingPresetItemGuiSupport.create(false, wayAT, waySI));
         assertEquals(Combo.DIFFERENT, combo.getSelectedItem().value);
+
+        Combo.PROP_FILL_DEFAULT.put(true);
+        combo.addToPanel(new JPanel(), TaggingPresetItemGuiSupport.create(false, way));
+        assertEquals("AT", combo.getSelectedItem().value);
+        combo.addToPanel(new JPanel(), TaggingPresetItemGuiSupport.create(false, wayTagged));
+        assertEquals("AT", combo.getSelectedItem().value);
+        combo.addToPanel(new JPanel(), TaggingPresetItemGuiSupport.create(false, wayAT));
+        assertEquals("AT", combo.getSelectedItem().value);
+        combo.addToPanel(new JPanel(), TaggingPresetItemGuiSupport.create(false, waySI));
+        assertEquals("SI", combo.getSelectedItem().value);
+        combo.addToPanel(new JPanel(), TaggingPresetItemGuiSupport.create(false, wayAT, waySI));
+        assertEquals(Combo.DIFFERENT, combo.getSelectedItem().value);
+        Combo.PROP_FILL_DEFAULT.put(false);
         combo.default_ = null;
 
@@ -104 +118 @@
         combo.addToPanel(new JPanel(), TaggingPresetItemGuiSupport.create(false, wayAT, waySI));
         assertEquals(Combo.DIFFERENT, combo.getSelectedItem().value);
+        combo.use_last_as_default = 0;
 
         KeyedItem.LAST_VALUES.clear();