Changeset 5196 in josm for trunk/src/org/openstreetmap

Timestamp:

2012-04-16T22:53:07+02:00 (12 years ago)

Author:

simon04

Message:

tagging presets: fix customized matching for combo elements (broken due to r5172), remove duplicated/inconsistent parsing/interpreting of preset values

Location:

trunk/src/org/openstreetmap/josm

Files:

• data/validation/tests/RelationChecker.java (modified) (1 diff)
• data/validation/tests/TagChecker.java (modified) (3 diffs)
• gui/tagging/TaggingPreset.java (modified) (22 diffs)
• gui/tagging/TaggingPresetSearchDialog.java (modified) (3 diffs)
• gui/tagging/ac/AutoCompletionManager.java (modified) (2 diffs)

trunk/src/org/openstreetmap/josm/data/validation/tests/RelationChecker.java

@@ -46 +46 @@
 
     @Override
-    public void initialize() throws Exception {
+    public void initialize() {
         initializePresets();
     }

trunk/src/org/openstreetmap/josm/data/validation/tests/TagChecker.java

@@ -155 +155 @@
 
     @Override
-    public void initialize() throws Exception {
+    public void initialize() throws IOException {
         initializeData();
         initializePresets();
@@ -286 +286 @@
      * Reads the presets data.
      *
-     * @throws Exception
      */
-    public static void initializePresets() throws Exception {
+    public static void initializePresets() {
 
         if (!Main.pref.getBoolean(PREF_CHECK_VALUES, true))
@@ -308 +307 @@
             }
             for (TaggingPreset p : presets) {
-                for(TaggingPreset.Item i : p.data) {
-                    if (i instanceof TaggingPreset.Combo) {
-                        TaggingPreset.Combo combo = (TaggingPreset.Combo) i;
-                        if (combo.values != null) {
-                            for(String value : combo.values.split(",")) {
-                                presetsValueData.put(combo.key, value);
-                            }
-                        }
-                    } else if (i instanceof TaggingPreset.Key) {
-                        TaggingPreset.Key k = (TaggingPreset.Key) i;
-                        presetsValueData.put(k.key, k.value);
-                    } else if (i instanceof TaggingPreset.Text) {
-                        TaggingPreset.Text k = (TaggingPreset.Text) i;
-                        presetsValueData.putVoid(k.key);
-                    } else if (i instanceof TaggingPreset.Check) {
-                        TaggingPreset.Check k = (TaggingPreset.Check) i;
-                        presetsValueData.put(k.key, "yes");
-                        presetsValueData.put(k.key, "no");
+                for (TaggingPreset.Item i : p.data) {
+                    if (i instanceof TaggingPreset.KeyedItem) {
+                        TaggingPreset.KeyedItem ky = (TaggingPreset.KeyedItem) i;
+                        presetsValueData.putAll(ky.key, ky.getValues());
                     }
                 }

trunk/src/org/openstreetmap/josm/gui/tagging/TaggingPreset.java

@@ -23 +23 @@
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.EnumSet;
 import java.util.HashMap;
@@ -210 +211 @@
          * @return {@code true} if matches (positive), {@code null} if neutral, {@code false} if mismatches (negative).
          */
-        abstract Boolean matches(Map<String, String> tags);
+        Boolean matches(Map<String, String> tags) {
+            return null;
+        }
+    }
+
+    public static abstract class KeyedItem extends Item {
+
+        public String key;
+        public String text;
+        public String text_context;
+        public String match = getDefaultMatch().getValue();
+
+        public abstract MatchType getDefaultMatch();
+        public abstract Collection<String> getValues();
+
+        @Override
+        Boolean matches(Map<String, String> tags) {
+            switch (MatchType.ofString(match)) {
+                case NONE:
+                    return null;
+                case KEY:
+                    return tags.containsKey(key) ? true : null;
+                case KEY_REQUIRED:
+                    return tags.containsKey(key);
+                case KEY_VALUE:
+                    return tags.containsKey(key) && (getValues().contains(tags.get(key)));
+                default:
+                    throw new IllegalStateException();
+            }
+        }
+
     }
 
@@ -325 +356 @@
     }
 
-    public static class Text extends Item {
-
-        public String key;
-        public String text;
+    public static class Text extends KeyedItem {
+
         public String locale_text;
-        public String text_context;
         public String default_;
         public String originalValue;
         public String use_last_as_default = "false";
-        public String match = MatchType.NONE.getValue();
 
         private JComponent value;
@@ -410 +437 @@
 
         @Override
-        Boolean matches(Map<String, String> tags) {
-            switch (MatchType.ofString(match)) {
-                case NONE:
-                    return null;
-                case KEY:
-                    return tags.containsKey(key) ? true : null;
-                case KEY_REQUIRED:
-                    return tags.containsKey(key);
-                default:
-                    throw new IllegalArgumentException("key_value matching not supported for <text>: " + text);
-            }
-        }
-    }
-
-    public static class Check extends Item {
-
-        public String key;
-        public String text;
-        public String text_context;
+        public MatchType getDefaultMatch() {
+            return MatchType.NONE;
+        }
+
+        @Override
+        public Collection<String> getValues() {
+            if (default_ == null || default_.isEmpty()) {
+                return Collections.emptyList();
+            }
+            return Collections.singleton(default_);
+        }
+    }
+
+    public static class Check extends KeyedItem {
+
         public String locale_text;
         public String value_on = OsmUtils.trueval;
         public String value_off = OsmUtils.falseval;
         public boolean default_ = false; // only used for tagless objects
-        public String match = MatchType.NONE.getValue();
 
         private QuadStateCheckBox check;
@@ -509 +531 @@
 
         @Override
-        Boolean matches(Map<String, String> tags) {
-            switch (MatchType.ofString(match)) {
-                case NONE:
-                    return null;
-                case KEY:
-                    return tags.containsKey(key) ? true : null;
-                case KEY_REQUIRED:
-                    return tags.containsKey(key);
-                case KEY_VALUE:
-                    return value_off.equals(tags.get(key)) || value_on.equals(tags.get(key));
-                default:
-                    throw new IllegalStateException();
-            }
-        }
-    }
-
-    public static abstract class ComboMultiSelect extends Item {
-
-        public String key;
-        public String text;
-        public String text_context;
+        public MatchType getDefaultMatch() {
+            return MatchType.NONE;
+        }
+
+        @Override
+        public Collection<String> getValues() {
+            return Arrays.asList(value_on, value_off);
+        }
+    }
+
+    public static abstract class ComboMultiSelect extends KeyedItem {
+
         public String locale_text;
         public String values;
@@ -540 +553 @@
         public String delimiter = ";";
         public String use_last_as_default = "false";
-        public String match = MatchType.NONE.getValue();
 
         protected JComponent component;
         protected Map<String, PresetListEntry> lhm = new LinkedHashMap<String, PresetListEntry>();
+        private boolean initialized = false;
         protected Usage usage;
         protected Object originalValue;
 
         protected abstract Object getSelectedItem();
-        protected abstract void addToPanelAnchor(JPanel p, String def, String[] display_array);
+        protected abstract void addToPanelAnchor(JPanel p, String def);
 
         protected char getDelChar() {
@@ -555 +568 @@
 
         @Override
+        public Collection<String> getValues() {
+            initListEntries();
+            return lhm.keySet();
+        }
+
+        public Collection<String> getDisplayValues() {
+            initListEntries();
+            return Utils.transform(lhm.values(), new Utils.Function<PresetListEntry, String>() {
+
+                @Override
+                public String apply(PresetListEntry x) {
+                    return x.getDisplayValue(true);
+                }
+            });
+        }
+
+        @Override
         public boolean addToPanel(JPanel p, Collection<OsmPrimitive> sel) {
+
+            initListEntries();
 
             // find out if our key is already used in the selection.
             usage = determineTextUsage(sel, key);
-
-            String[] display_array;
-            if (lhm.isEmpty()) {
-                display_array = initListEntriesFromAttributes();
+            if (!usage.hasUniqueValue() && !usage.unused()) {
+                lhm.put(DIFFERENT, new PresetListEntry(DIFFERENT));
+            }
+
+            p.add(new JLabel(tr("{0}:", locale_text)), GBC.std().insets(0, 0, 10, 0));
+            addToPanelAnchor(p, default_);
+
+            return true;
+
+        }
+
+        private void initListEntries() {
+            if (initialized) {
+                lhm.remove(DIFFERENT); // possibly added in #addToPanel
+                return;
+            } else if (lhm.isEmpty()) {
+                initListEntriesFromAttributes();
             } else {
                 if (values != null) {
@@ -579 +624 @@
                             key, text, "short_descriptions", "list_entry"));
                 }
-                display_array = new String[lhm.values().size()];
-                int i = 0;
                 for (PresetListEntry e : lhm.values()) {
                     if (e.value_context == null) {
                         e.value_context = values_context;
                     }
-                    display_array[i++] = e.getDisplayValue(true);
-                }
-            }
-
+                }
+            }
             if (locale_text == null) {
                 locale_text = trc(text_context, fixPresetString(text));
             }
-            p.add(new JLabel(tr("{0}:", locale_text)), GBC.std().insets(0, 0, 10, 0));
-
-            addToPanelAnchor(p, default_, display_array);
-
-            return true;
-
+            initialized = true;
         }
 
@@ -621 +657 @@
             }
 
-            if (!usage.hasUniqueValue() && !usage.unused()) {
-                lhm.put(DIFFERENT, new PresetListEntry(DIFFERENT));
-            }
             for (int i = 0; i < value_array.length; i++) {
                 final PresetListEntry e = new PresetListEntry(value_array[i]);
@@ -637 +670 @@
                 display_array[i] = e.getDisplayValue(true);
             }
-
-            // as addToPanel may be called several times, set String to null to avoid "Ignoring * attribute as * elements are given"
-            values = null;
-            display_values = null;
-            locale_display_values = null;
-            short_descriptions = null;
-            locale_short_descriptions = null;
 
             return display_array;
@@ -744 +770 @@
 
         @Override
-        Boolean matches(Map<String, String> tags) {
-            switch (MatchType.ofString(match)) {
-                case NONE:
-                    return null;
-                case KEY:
-                    return tags.containsKey(key) ? true : null;
-                case KEY_REQUIRED:
-                    return tags.containsKey(key);
-                case KEY_VALUE:
-                    return tags.containsKey(key)
-                            && Arrays.asList(splitEscaped(getDelChar(), values)).contains(tags.get(key));
-                default:
-                    throw new IllegalStateException();
-            }
+        public MatchType getDefaultMatch() {
+            return MatchType.NONE;
         }
     }
@@ -771 +785 @@
 
         @Override
-        protected void addToPanelAnchor(JPanel p, String def, String[] display_array) {
+        protected void addToPanelAnchor(JPanel p, String def) {
             if (!usage.unused()) {
                 for (String s : usage.values) {
@@ -791 +805 @@
             AutoCompletingTextField tf = new AutoCompletingTextField();
             initAutoCompletionField(tf, key);
-            tf.getAutoCompletionList().add(Arrays.asList(display_array), AutoCompletionItemPritority.IS_IN_STANDARD);
+            tf.getAutoCompletionList().add(getDisplayValues(), AutoCompletionItemPritority.IS_IN_STANDARD);
             combo.setEditor(tf);
 
@@ -861 +875 @@
                 }
                 setSelectedIndices(Arrays.copyOf(intParts, j));
-                // check if we have acutally managed to represent the full
+                // check if we have actually managed to represent the full
                 // value with our presets. if not, cop out; we will not offer
                 // a selection list that threatens to ruin the value.
@@ -887 +901 @@
 
         @Override
-        protected void addToPanelAnchor(JPanel p, String def, String[] display_array) {
+        protected void addToPanelAnchor(JPanel p, String def) {
             list = new ConcatenatingJList(delimiter, lhm.values().toArray());
             component = list;
@@ -979 +993 @@
         public void addCommands(List<Tag> changedTags) {
         }
-
-        @Override
-        Boolean matches(Map<String, String> tags) {
-            return null;
-        }
     }
 
@@ -1017 +1026 @@
         @Override
         public void addCommands(List<Tag> changedTags) {
-        }
-
-        @Override
-        Boolean matches(Map<String, String> tags) {
-            return null;
         }
     }
@@ -1117 +1121 @@
         public void addCommands(List<Tag> changedTags) {
         }
-
-        @Override
-        Boolean matches(Map<String, String> tags) {
-            return null;
-        }
     }
 
@@ -1138 +1137 @@
         public void addCommands(List<Tag> changedTags) {
         }
-
-        @Override
-        Boolean matches(Map<String, String> tags) {
-            return null;
-        }
     }
 
@@ -1156 +1150 @@
         public void addCommands(List<Tag> changedTags) {
         }
-
-        @Override
-        Boolean matches(Map<String, String> tags) {
-            return null;
-        }
-    }
-
-    public static class Key extends Item {
-
-        public String key;
+    }
+
+    public static class Key extends KeyedItem {
+
         public String value;
-        public String match = MatchType.KEY_VALUE.getValue();
 
         @Override
@@ -1180 +1167 @@
 
         @Override
-        Boolean matches(Map<String, String> tags) {
-            switch (MatchType.ofString(match)) {
-                case NONE:
-                    return null;
-                case KEY:
-                    return tags.containsKey(key) ? true : null;
-                case KEY_REQUIRED:
-                    return tags.containsKey(key);
-                case KEY_VALUE:
-                    return value.equals(tags.get(key));
-                default:
-                    throw new IllegalStateException();
-            }
+        public MatchType getDefaultMatch() {
+            return MatchType.KEY_VALUE;
+        }
+
+        @Override
+        public Collection<String> getValues() {
+            return Collections.singleton(value);
         }
     }
@@ -1615 +1596 @@
     /**
      * Removes all unsuitable OsmPrimitives from the given list
-     * @param participants List of possibile OsmPrimitives to tag
+     * @param participants List of possible OsmPrimitives to tag
      * @return Cleaned list with suitable OsmPrimitives only
      */

trunk/src/org/openstreetmap/josm/gui/tagging/TaggingPresetSearchDialog.java

@@ -45 +45 @@
 import org.openstreetmap.josm.gui.ExtendedDialog;
 import org.openstreetmap.josm.gui.preferences.map.TaggingPresetPreference;
-import org.openstreetmap.josm.gui.tagging.TaggingPreset.Check;
-import org.openstreetmap.josm.gui.tagging.TaggingPreset.Combo;
 import org.openstreetmap.josm.gui.tagging.TaggingPreset.Item;
 import org.openstreetmap.josm.gui.tagging.TaggingPreset.Key;
@@ -52 +50 @@
 import org.openstreetmap.josm.gui.tagging.TaggingPreset.Role;
 import org.openstreetmap.josm.gui.tagging.TaggingPreset.Roles;
-import org.openstreetmap.josm.gui.tagging.TaggingPreset.Text;
 
 public class TaggingPresetSearchDialog extends ExtendedDialog implements SelectionChangedListener {
@@ -122 +119 @@
             }
             for (Item item: preset.data) {
-                if (item instanceof Check) {
-                    tags.add(((Check)item).key.toLowerCase());
-                } else if (item instanceof Combo) {
+                if (item instanceof TaggingPreset.KeyedItem) {
+                    tags.add(((TaggingPreset.KeyedItem) item).key);
                     // Should combo values also be added?
-                    tags.add(((Combo)item).key);
-                } else if (item instanceof Key) {
-                    tags.add(((Key) item).key);
-                    String value = ((Key) item).value;
-                    if (value != null) {
-                        tags.add(value);
-                    }
-                } else if (item instanceof Text) {
-                    tags.add(((Text) item).key);
+                    if (item instanceof Key && ((Key) item).value != null) {
+                        tags.add(((Key) item).value);
+                    }
                 } else if (item instanceof Roles) {
-                    for (Role role: ((Roles) item).roles) {
+                    for (Role role : ((Roles) item).roles) {
                         tags.add(role.key);
                     }

trunk/src/org/openstreetmap/josm/gui/tagging/ac/AutoCompletionManager.java

@@ -149 +149 @@
         for (final TaggingPreset p : presets) {
             for (TaggingPreset.Item item : p.data) {
-                if (item instanceof TaggingPreset.Check) {
-                    TaggingPreset.Check ch = (TaggingPreset.Check) item;
-                    if (ch.key == null) {
+                if (item instanceof TaggingPreset.KeyedItem) {
+                    TaggingPreset.KeyedItem ki = (TaggingPreset.KeyedItem) item;
+                    if (ki.key == null) {
                         continue;
                     }
-                    presetTagCache.put(ch.key, OsmUtils.falseval);
-                    presetTagCache.put(ch.key, OsmUtils.trueval);
-                } else if (item instanceof TaggingPreset.Combo) {
-                    TaggingPreset.Combo co = (TaggingPreset.Combo) item;
-                    if (co.key == null || co.values == null) {
-                        continue;
-                    }
-                    for (String value : co.values.split(",")) {
-                        presetTagCache.put(co.key, value);
-                    }
-                } else if (item instanceof TaggingPreset.Key) {
-                    TaggingPreset.Key ky = (TaggingPreset.Key) item;
-                    if (ky.key == null || ky.value == null) {
-                        continue;
-                    }
-                    presetTagCache.put(ky.key, ky.value);
-                } else if (item instanceof TaggingPreset.Text) {
-                    TaggingPreset.Text tt = (TaggingPreset.Text) item;
-                    if (tt.key == null) {
-                        continue;
-                    }
-                    presetTagCache.putVoid(tt.key);
-                    if (tt.default_ != null && !tt.default_.equals("")) {
-                        presetTagCache.put(tt.key, tt.default_);
-                    }
+                    presetTagCache.putAll(ki.key, ki.getValues());
                 } else if (item instanceof TaggingPreset.Roles) {
                     TaggingPreset.Roles r = (TaggingPreset.Roles) item;
@@ -266 +242 @@
      *
      * @param list the list to populate
-     * @param key the tag keys
+     * @param keys the tag keys
      */
     public void populateWithTagValues(AutoCompletionList list, List<String> keys) {