Ticket #21408: 21408-2.patch

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

@@ -30 +30 @@
     private final ListenerList<ChangeListener> listeners = ListenerList.create();
 
     /** whether to fire events or not */
-    private boolean enabled = false;
+    private boolean enabled;
 
     /**
      * Returns whether firing of events is enabled

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

@@ -12 +12 @@
 import java.awt.event.ComponentEvent;
 import java.util.Arrays;
 import java.util.Comparator;
-import java.util.TreeMap;
 
 import javax.swing.AbstractAction;
 import javax.swing.JButton;
@@ -88 +87 @@
     protected boolean addToPanel(JPanel p, TaggingPresetItemGuiSupport support) {
         initializeLocaleText(null);
         usage = determineTextUsage(support.getSelected(), key);
-        seenValues = new TreeMap<>();
+        seenValues.clear();
         // get the standard values from the preset definition
         initListEntries();
 
         // init the model
-        dropDownModel = new AutoCompComboBoxModel<PresetListEntry>(Comparator.naturalOrder());
+        dropDownModel = new AutoCompComboBoxModel<>(Comparator.<PresetListEntry>naturalOrder());
 
         if (!usage.hasUniqueValue() && !usage.unused()) {
             addEntry(PresetListEntry.ENTRY_DIFFERENT);
@@ -124 +123 @@
         combobox.setRenderer(new ComboMultiSelectListCellRenderer(combobox, combobox.getRenderer(), 200, key));
         combobox.setEditable(editable);
 
-        autoCompModel = new AutoCompComboBoxModel<AutoCompletionItem>(Comparator.naturalOrder());
+        autoCompModel = new AutoCompComboBoxModel<>(Comparator.<AutoCompletionItem>naturalOrder());
         getAllForKeys(Arrays.asList(key)).forEach(autoCompModel::addElement);
         getDisplayValues().forEach(s -> autoCompModel.addElement(new AutoCompletionItem(s, AutoCompletionPriority.IS_IN_STANDARD)));
 
@@ -154 +153 @@
             p.add(combobox, GBC.eol().fill(GBC.HORIZONTAL)); // NOSONAR
         }
 
-        String valueToSelect = getInitialValue(default_);
+        String valueToSelect = getInitialValue(usage);
         if (valueToSelect != null) {
             PresetListEntry selItem = find(valueToSelect);
             if (selItem != null) {

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

@@ -4 +4 @@
 import static org.openstreetmap.josm.tools.I18n.tr;
 
 import java.awt.Component;
+import java.awt.Font;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 import java.util.ArrayList;
@@ -90 +91 @@
      */
     protected final List<PresetListEntry> presetListEntries = new ArrayList<>();
     /** Helps avoid duplicate list entries */
-    protected Map<String, PresetListEntry> seenValues = new TreeMap<>();
+    protected final Map<String, PresetListEntry> seenValues = new TreeMap<>();
     protected Usage usage;
     /** Used to see if the user edited the value. May be null. */
     protected String originalValue;
@@ -130 +131 @@
             JList<? extends PresetListEntry> list, PresetListEntry value, int index, boolean isSelected, boolean cellHasFocus) {
 
             JLabel l = (JLabel) renderer.getListCellRendererComponent(list, value, index, isSelected, cellHasFocus);
+            l.setComponentOrientation(component.getComponentOrientation());
             if (index != -1) {
                 // index -1 is set when measuring the size of the cell and when painting the
                 // editor-ersatz of a readonly combobox. fixes #6157
                 l.setText(value.getListDisplay(width));
             }
+            if (value.getCount() > 0) {
+                l.setFont(l.getFont().deriveFont(Font.ITALIC + Font.BOLD));
+            }
+            l.setIcon(value.getIcon());
             l.setToolTipText(value.getToolTipText(key));
-            l.setIcon(value.getIcon());
             return l;
         }
     }
@@ -220 +225 @@
         }
     }
 
-    private List<String> getValuesFromCode(String values_from) {
+    private List<String> getValuesFromCode(String valuesFrom) {
         // get the values from a Java function
-        String[] classMethod = values_from.split("#", -1);
+        String[] classMethod = valuesFrom.split("#", -1);
         if (classMethod.length == 2) {
             try {
                 Method method = Class.forName(classMethod[0]).getMethod(classMethod[1]);
@@ -314 +319 @@
      * <p>
      * The initial value is the value shown in the control when the preset dialogs opens.
      *
-     * @param def The default value
+     * @param usage The key Usage
      * @return The initial value to use.
      */
-    protected String getInitialValue(String def) {
+    protected String getInitialValue(Usage usage) {
         String initialValue = null;
+        originalValue = null;
 
         if (usage.hasUniqueValue()) {
             // all selected primitives have the same not empty value for this key
             initialValue = usage.getFirst();
+            originalValue = initialValue;
         } else if (!usage.unused()) {
             // at least one primitive has a value for this key (but not all have the same one)
             initialValue = DIFFERENT;
+            originalValue = initialValue;
         } else if (PROP_FILL_DEFAULT.get() || isForceUseLastAsDefault()) {
             // at this point no primitive had any value for this key
             // use the last value no matter what
@@ -335 +343 @@
             initialValue = LAST_VALUES.get(key);
         } else if (!usage.hadKeys()) {
             // use the default only on objects with no keys at all
-            initialValue = def;
+            initialValue = default_;
         }
-        originalValue = initialValue;
         return initialValue;
     }
 

src/org/openstreetmap/josm/gui/tagging/presets/items/KeyedItem.java

@@ -106 +106 @@
         public final SortedMap<String, Integer> map = new TreeMap<>();
         private boolean hadKeys;
         private boolean hadEmpty;
+        private int selectedCount;
 
         /**
          * Check if there is exactly one value for this key.
@@ -139 +140 @@
         public boolean hadKeys() {
             return hadKeys;
         }
+
+        /**
+         * Returns the number of primitives selected.
+         * @return the number of primitives selected.
+         */
+        public int getSelectedCount() {
+            return selectedCount;
+        }
+
+        /**
+         * Splits multiple values and adds their usage counts as single value.
+         * <p>
+         * A value of {@code regional;pizza} will increment the count of {@code regional} and of
+         * {@code pizza}.
+         * @param delimiter The delimiter used for splitting.
+         * @return A new usage object with the new counts.
+         */
+        public Usage splitValues(String delimiter) {
+            Usage usage = new Usage();
+            usage.hadEmpty = hadEmpty;
+            usage.hadKeys = hadKeys;
+            usage.selectedCount = selectedCount;
+            map.forEach((value, count) -> {
+                for (String v : value.split(String.valueOf(delimiter), -1)) {
+                    usage.map.merge(v, count, Integer::sum);
+                }
+            });
+            return usage;
+        }
     }
 
     /**
@@ -149 +179 @@
      */
     public static Usage determineTextUsage(Collection<OsmPrimitive> sel, String key) {
         Usage returnValue = new Usage();
+        returnValue.selectedCount = sel.size();
         for (OsmPrimitive s : sel) {
             String v = s.get(key);
             if (v != null) {
@@ -165 +196 @@
 
     protected static Usage determineBooleanUsage(Collection<OsmPrimitive> sel, String key) {
         Usage returnValue = new Usage();
+        returnValue.selectedCount = sel.size();
         for (OsmPrimitive s : sel) {
             String booleanValue = OsmUtils.getNamedOsmBoolean(s.get(key));
             if (booleanValue != null) {

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

@@ -1 +1 @@
 // License: GPL. For details, see LICENSE file.
 package org.openstreetmap.josm.gui.tagging.presets.items;
 
-import java.util.TreeMap;
+import java.awt.Dimension;
+import java.awt.Insets;
+import java.awt.Rectangle;
 import java.util.stream.Collectors;
 
 import javax.swing.DefaultListModel;
@@ -40 +42 @@
     protected boolean addToPanel(JPanel p, TaggingPresetItemGuiSupport support) {
         initializeLocaleText(null);
         usage = determineTextUsage(support.getSelected(), key);
-        seenValues = new TreeMap<>();
+        seenValues.clear();
         initListEntries();
 
         model.clear();
-        if (!usage.hasUniqueValue() && !usage.unused()) {
-            addEntry(PresetListEntry.ENTRY_DIFFERENT);
+        // disable if the selected primitives have different values
+        list.setEnabled(usage.hasUniqueValue() || usage.unused());
+        String initialValue = getInitialValue(usage);
+
+        // Add values from the preset.
+        presetListEntries.forEach(this::addEntry);
+
+        // Add all values used in the selected primitives. This also adds custom values and makes
+        // sure we won't lose them.
+        usage = usage.splitValues(String.valueOf(delimiter));
+        for (String value: usage.map.keySet()) {
+            addEntry(new PresetListEntry(value, this));
         }
 
-        String initialValue = getInitialValue(default_);
-        if (initialValue != null) {
-            // add all values already present to the list, otherwise we would remove all
-            // custom entries unknown to the preset
+        // Select the values in the initial value.
+        if (initialValue != null && !DIFFERENT.equals(initialValue)) {
             for (String value : initialValue.split(String.valueOf(delimiter), -1)) {
                 PresetListEntry e = new PresetListEntry(value, this);
                 addEntry(e);
@@ -60 +70 @@
             }
         }
 
-        presetListEntries.forEach(this::addEntry);
-
         ComboMultiSelectListCellRenderer renderer = new ComboMultiSelectListCellRenderer(list, list.getCellRenderer(), 200, key);
         list.setCellRenderer(renderer);
+        JLabel label = addLabel(p);
+        label.setLabelFor(list);
+        JScrollPane sp = new JScrollPane(list);
 
         if (rows > 0) {
             list.setVisibleRowCount(rows);
+            // setVisibleRowCount() only works when all cells have the same height, but sometimes we
+            // have icons of different sizes. Calculate the size of the first {@code rows} entries
+            // and size the scrollpane accordingly.
+            Rectangle r = list.getCellBounds(0, Math.min(rows, model.size() - 1));
+            Insets insets = list.getInsets();
+            r.width += insets.left + insets.right;
+            r.height += insets.top + insets.bottom;
+            insets = sp.getInsets();
+            r.width += insets.left + insets.right;
+            r.height += insets.top + insets.bottom;
+            sp.setPreferredSize(new Dimension(r.width, r.height));
         }
-        JLabel label = addLabel(p);
-        p.add(new JScrollPane(list), GBC.eol().fill(GBC.HORIZONTAL)); // NOSONAR
-        label.setLabelFor(list);
+        p.add(sp, GBC.eol().fill(GBC.HORIZONTAL)); // NOSONAR
 
         list.addListSelectionListener(l -> support.fireItemValueModified(this, key, getSelectedItem().value));
         list.setToolTipText(getKeyTooltipText());
@@ -82 +102 @@
     @Override
     protected PresetListEntry getSelectedItem() {
         return new PresetListEntry(list.getSelectedValuesList()
-            .stream().map(e -> e.value).collect(Collectors.joining(String.valueOf(delimiter))), this);
+            .stream().map(e -> e.value).distinct().sorted().collect(Collectors.joining(String.valueOf(delimiter))), this);
     }
 }

src/org/openstreetmap/josm/gui/tagging/presets/items/PresetListEntry.java

@@ -47 +47 @@
     /** The localized version of {@link #short_description}. */
     public String locale_short_description; // NOSONAR
 
-    private String cachedDisplayValue = null;
-    private String cachedShortDescription = null;
-    private ImageIcon cachedIcon = null;
+    private String cachedDisplayValue;
+    private String cachedShortDescription;
+    private ImageIcon cachedIcon;
 
     /**
      * Constructs a new {@code PresetListEntry}, uninitialized.
@@ -82 +82 @@
      */
     public String getListDisplay(int width) {
         String displayValue = getDisplayValue();
-        Integer count = cms == null ? null : cms.usage.map.get(value);
+        Integer count = getCount();
 
-        if (count != null) {
-            displayValue = String.format("%s (%d)", displayValue, count);
+        if (count > 0 && cms.usage.getSelectedCount() > 1) {
+            displayValue = tr("{0} ({1})", displayValue, count);
         }
 
         if (this.equals(ENTRY_DIFFERENT)) {
@@ -183 +183 @@
         return Objects.hash(value);
     }
 
+    /**
+     * Returns how many selected primitives had this value set.
+     * @return see above
+     */
+    public int getCount() {
+        Integer count = cms == null ? null : cms.usage.map.get(value);
+        return count == null ? 0 : count;
+    }
+
     @Override
     public int compareTo(PresetListEntry o) {
         return AlphanumComparator.getInstance().compare(this.value, o.value);

src/org/openstreetmap/josm/gui/widgets/JosmComboBox.java

@@ -47 +47 @@
     public static final String PROP_MAXIMUM_ROW_COUNT = "gui.combobox.maximum-row-count";
 
     /** the configured maximum row count or null */
-    private Integer configMaximumRowCount = null;
+    private Integer configMaximumRowCount;
 
     /**
      * The preferred height of the combobox when closed.  Use if the items in the list dropdown are