Index: /trunk/src/org/openstreetmap/josm/data/Preferences.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/Preferences.java (revision 10823) +++ /trunk/src/org/openstreetmap/josm/data/Preferences.java (revision 10824) @@ -37,8 +37,8 @@ import java.util.SortedMap; import java.util.TreeMap; -import java.util.concurrent.CopyOnWriteArrayList; import java.util.function.Predicate; import java.util.regex.Matcher; import java.util.regex.Pattern; +import java.util.stream.Stream; import javax.json.Json; @@ -55,7 +55,11 @@ import org.openstreetmap.josm.Main; +import org.openstreetmap.josm.data.preferences.BooleanProperty; import org.openstreetmap.josm.data.preferences.ColorProperty; +import org.openstreetmap.josm.data.preferences.DoubleProperty; +import org.openstreetmap.josm.data.preferences.IntegerProperty; import org.openstreetmap.josm.data.preferences.ListListSetting; import org.openstreetmap.josm.data.preferences.ListSetting; +import org.openstreetmap.josm.data.preferences.LongProperty; import org.openstreetmap.josm.data.preferences.MapListSetting; import org.openstreetmap.josm.data.preferences.PreferencesReader; @@ -68,6 +72,6 @@ import org.openstreetmap.josm.tools.ColorHelper; import org.openstreetmap.josm.tools.I18n; +import org.openstreetmap.josm.tools.ListenerList; import org.openstreetmap.josm.tools.MultiMap; -import org.openstreetmap.josm.tools.SubclassFilteredCollection; import org.openstreetmap.josm.tools.Utils; import org.xml.sax.SAXException; @@ -215,4 +219,11 @@ } + /** + * Old color interface + *

+ * To be removed: end of 2016 + * @deprecated Use a {@link ColorProperty} instead. + */ + @Deprecated public interface ColorKey { String getColorName(); @@ -223,5 +234,7 @@ } - private final CopyOnWriteArrayList listeners = new CopyOnWriteArrayList<>(); + private final ListenerList listeners = ListenerList.create(); + + private final HashMap> keyListeners = new HashMap<>(); /** @@ -231,5 +244,5 @@ public void addPreferenceChangeListener(PreferenceChangedListener listener) { if (listener != null) { - listeners.addIfAbsent(listener); + listeners.addListener(listener); } } @@ -240,11 +253,56 @@ */ public void removePreferenceChangeListener(PreferenceChangedListener listener) { - listeners.remove(listener); + listeners.removeListener(listener); + } + + /** + * Adds a listener that only listens to changes in one preference + * @param key The preference key to listen to + * @param listener The listener to add. + * @since 10824 + */ + public void addKeyPreferenceChangeListener(String key, PreferenceChangedListener listener) { + listenersForKey(key).addListener(listener); + } + + /** + * Adds a weak listener that only listens to changes in one preference + * @param key The preference key to listen to + * @param listener The listener to add. + * @since 10824 + */ + public void addWeakKeyPreferenceChangeListener(String key, PreferenceChangedListener listener) { + listenersForKey(key).addWeakListener(listener); + } + + private ListenerList listenersForKey(String key) { + ListenerList keyListener = keyListeners.get(key); + if (keyListener == null) { + keyListener = ListenerList.create(); + keyListeners.put(key, keyListener); + } + return keyListener; + } + + /** + * Removes a listener that only listens to changes in one preference + * @param key The preference key to listen to + * @param listener The listener to add. + */ + public void removeKeyPreferenceChangeListener(String key, PreferenceChangedListener listener) { + ListenerList keyListener = keyListeners.get(key); + if (keyListener == null) { + throw new IllegalArgumentException("There are no listeners registered for " + key); + } + keyListener.removeListener(listener); } protected void firePreferenceChanged(String key, Setting oldValue, Setting newValue) { - PreferenceChangeEvent evt = new DefaultPreferenceChangeEvent(key, oldValue, newValue); - for (PreferenceChangedListener l : listeners) { - l.preferenceChanged(evt); + final PreferenceChangeEvent evt = new DefaultPreferenceChangeEvent(key, oldValue, newValue); + listeners.fireEvent(listener -> listener.preferenceChanged(evt)); + + ListenerList forKey = keyListeners.get(key); + if (forKey != null) { + forKey.fireEvent(listener -> listener.preferenceChanged(evt)); } } @@ -479,22 +537,47 @@ */ public boolean put(final String key, String value) { - if (value != null && value.isEmpty()) { - value = null; - } - return putSetting(key, value == null ? null : new StringSetting(value)); - } - + return putSetting(key, value == null || value.isEmpty() ? null : new StringSetting(value)); + } + + /** + * Set a boolean value for a certain setting. + * @param key the unique identifier for the setting + * @param value The new value + * @return {@code true}, if something has changed (i.e. value is different than before) + * @see BooleanProperty + */ public boolean put(final String key, final boolean value) { return put(key, Boolean.toString(value)); } + /** + * Set a boolean value for a certain setting. + * @param key the unique identifier for the setting + * @param value The new value + * @return {@code true}, if something has changed (i.e. value is different than before) + * @see IntegerProperty + */ public boolean putInteger(final String key, final Integer value) { return put(key, Integer.toString(value)); } + /** + * Set a boolean value for a certain setting. + * @param key the unique identifier for the setting + * @param value The new value + * @return {@code true}, if something has changed (i.e. value is different than before) + * @see DoubleProperty + */ public boolean putDouble(final String key, final Double value) { return put(key, Double.toString(value)); } + /** + * Set a boolean value for a certain setting. + * @param key the unique identifier for the setting + * @param value The new value + * @return {@code true}, if something has changed (i.e. value is different than before) + * @see LongProperty + */ public boolean putLong(final String key, final Long value) { return put(key, Long.toString(value)); @@ -506,14 +589,12 @@ */ public synchronized void save() throws IOException { - save(getPreferenceFile(), - new SubclassFilteredCollection<>(settingsMap.entrySet(), NO_DEFAULT_SETTINGS_ENTRY), false); + save(getPreferenceFile(), settingsMap.entrySet().stream().filter(NO_DEFAULT_SETTINGS_ENTRY), false); } public synchronized void saveDefaults() throws IOException { - save(getDefaultsCacheFile(), defaultsMap.entrySet(), true); - } - - protected void save(File prefFile, Collection>> settings, boolean defaults) throws IOException { - + save(getDefaultsCacheFile(), defaultsMap.entrySet().stream(), true); + } + + protected void save(File prefFile, Stream>> settings, boolean defaults) throws IOException { if (!defaults) { /* currently unused, but may help to fix configuration issues in future */ @@ -719,9 +800,13 @@ /** * Convenience method for accessing colour preferences. + *

+ * To be removed: end of 2016 * * @param colName name of the colour * @param def default value * @return a Color object for the configured colour, or the default value if none configured. - */ + * @deprecated Use a {@link ColorProperty} instead. + */ + @Deprecated public synchronized Color getColor(String colName, Color def) { return getColor(colName, null, def); @@ -743,7 +828,11 @@ /** * Returns the color for the given key. + *

+ * To be removed: end of 2016 * @param key The color key * @return the color - */ + * @deprecated Use a {@link ColorProperty} instead. + */ + @Deprecated public Color getColor(ColorKey key) { return getColor(key.getColorName(), key.getSpecialName(), key.getDefaultValue()); @@ -752,15 +841,17 @@ /** * Convenience method for accessing colour preferences. - * + *

+ * To be removed: end of 2016 * @param colName name of the colour * @param specName name of the special colour settings * @param def default value * @return a Color object for the configured colour, or the default value if none configured. - */ + * @deprecated Use a {@link ColorProperty} instead. + * You can replace this by: new ColorProperty(colName, def).getChildColor(specName) + */ + @Deprecated public synchronized Color getColor(String colName, String specName, Color def) { String colKey = ColorProperty.getColorKey(colName); - if (!colKey.equals(colName)) { - colornames.put(colKey, colName); - } + registerColor(colKey, colName); String colStr = specName != null ? get("color."+specName) : ""; if (colStr.isEmpty()) { @@ -771,4 +862,16 @@ } else { return def; + } + } + + /** + * Registers a color name conversion for the global color registry. + * @param colKey The key + * @param colName The name of the color. + * @since 10824 + */ + public void registerColor(String colKey, String colName) { + if (!colKey.equals(colName)) { + colornames.put(colKey, colName); } } Index: /trunk/src/org/openstreetmap/josm/data/osm/visitor/paint/PaintColors.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/osm/visitor/paint/PaintColors.java (revision 10823) +++ /trunk/src/org/openstreetmap/josm/data/osm/visitor/paint/PaintColors.java (revision 10824) @@ -7,11 +7,11 @@ import java.util.List; -import org.openstreetmap.josm.Main; -import org.openstreetmap.josm.data.Preferences.ColorKey; +import org.openstreetmap.josm.data.preferences.CachingProperty; +import org.openstreetmap.josm.data.preferences.ColorProperty; import org.openstreetmap.josm.gui.mappaint.MapPaintStyles; import org.openstreetmap.josm.gui.mappaint.MapPaintStyles.MapPaintSylesUpdateListener; import org.openstreetmap.josm.gui.mappaint.StyleSource; -public enum PaintColors implements ColorKey { +public enum PaintColors { INACTIVE(marktr("inactive"), Color.darkGray), @@ -34,9 +34,10 @@ private final String name; private final Color defaultColor; + private final CachingProperty property; private static volatile Color backgroundColorCache; private static final MapPaintSylesUpdateListener styleOverrideListener = new MapPaintSylesUpdateListener() { - + //TODO: Listen to wireframe map mode changes. @Override public void mapPaintStylesUpdated() { @@ -55,31 +56,15 @@ PaintColors(String name, Color defaultColor) { + property = new ColorProperty(name, defaultColor).cached(); this.name = name; this.defaultColor = defaultColor; } - @Override - public String getColorName() { - return name; - } - - @Override public Color getDefaultValue() { - return defaultColor; - } - - @Override - public String getSpecialName() { - return null; + return property.getDefaultValue(); } public Color get() { - return Main.pref.getColor(this); - } - - public static void getColors() { - for (PaintColors c:values()) { - c.get(); - } + return property.get(); } @@ -98,7 +83,8 @@ } if (backgroundColorCache == null) { - backgroundColorCache = BACKGROUND.get(); + return BACKGROUND.get(); + } else { + return backgroundColorCache; } - return backgroundColorCache; } } Index: /trunk/src/org/openstreetmap/josm/data/preferences/AbstractProperty.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/preferences/AbstractProperty.java (revision 10823) +++ /trunk/src/org/openstreetmap/josm/data/preferences/AbstractProperty.java (revision 10824) @@ -3,4 +3,7 @@ import org.openstreetmap.josm.Main; +import org.openstreetmap.josm.data.Preferences; +import org.openstreetmap.josm.data.Preferences.PreferenceChangeEvent; +import org.openstreetmap.josm.data.Preferences.PreferenceChangedListener; /** @@ -9,4 +12,136 @@ */ public abstract class AbstractProperty { + + private final class PreferenceChangedListenerAdapter implements PreferenceChangedListener { + private ValueChangeListener listener; + + PreferenceChangedListenerAdapter(ValueChangeListener listener) { + this.listener = listener; + } + + @Override + public void preferenceChanged(PreferenceChangeEvent e) { + listener.valueChanged(new ValueChangeEvent<>(e, AbstractProperty.this)); + } + + @Override + public int hashCode() { + final int prime = 31; + int result = 1; + result = prime * result + getOuterType().hashCode(); + result = prime * result + ((listener == null) ? 0 : listener.hashCode()); + return result; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) + return true; + if (obj == null) + return false; + if (getClass() != obj.getClass()) + return false; + @SuppressWarnings("unchecked") + PreferenceChangedListenerAdapter other = (PreferenceChangedListenerAdapter) obj; + if (!getOuterType().equals(other.getOuterType())) + return false; + if (listener == null) { + if (other.listener != null) + return false; + } else if (!listener.equals(other.listener)) + return false; + return true; + } + + private AbstractProperty getOuterType() { + return AbstractProperty.this; + } + + @Override + public String toString() { + return "PreferenceChangedListenerAdapter [listener=" + listener + ']'; + } + } + + /** + * A listener that listens to changes in the properties value. + * @author michael + * @param property type + * @since 10824 + */ + public interface ValueChangeListener { + /** + * Method called when a property value has changed. + * @param e property change event + */ + void valueChanged(ValueChangeEvent e); + } + + /** + * An event that is triggered if the value of a property changes. + * @author Michael Zangl + * @param property type + * @since 10824 + */ + public static class ValueChangeEvent { + private final PreferenceChangeEvent base; + + private final AbstractProperty source; + + ValueChangeEvent(PreferenceChangeEvent base, AbstractProperty source) { + this.base = base; + this.source = source; + } + + /** + * Get the property that was changed + * @return The property. + */ + public AbstractProperty getProperty() { + return source; + } + } + + /** + * An exception that is thrown if a preference value is invalid. + * @author Michael Zangl + * @since 10824 + */ + public static class InvalidPreferenceValueException extends RuntimeException { + + /** + * Constructs a new {@code InvalidPreferenceValueException} with the specified detail message and cause. + * @param message the detail message (which is saved for later retrieval by the {@link #getMessage()} method). + * @param cause the cause (which is saved for later retrieval by the {@link #getCause()} method). + */ + public InvalidPreferenceValueException(String message, Throwable cause) { + super(message, cause); + } + + /** + * Constructs a new {@code InvalidPreferenceValueException} with the specified detail message. + * The cause is not initialized, and may subsequently be initialized by a call to {@link #initCause}. + * + * @param message the detail message. The detail message is saved for later retrieval by the {@link #getMessage()} method. + */ + public InvalidPreferenceValueException(String message) { + super(message); + } + + /** + * Constructs a new {@code InvalidPreferenceValueException} with the specified cause and a detail message of + * (cause==null ? null : cause.toString()) (which typically contains the class and detail message of cause). + * + * @param cause the cause (which is saved for later retrieval by the {@link #getCause()} method). + */ + public InvalidPreferenceValueException(Throwable cause) { + super(cause); + } + } + + /** + * The preferences object this property is for. + */ + protected final Preferences preferences; protected final String key; protected final T defaultValue; @@ -19,4 +154,6 @@ */ public AbstractProperty(String key, T defaultValue) { + // Main.pref should not change in production but may change during tests. + preferences = Main.pref; this.key = key; this.defaultValue = defaultValue; @@ -24,4 +161,13 @@ /** + * Store the default value to {@link Preferences}. + */ + protected void storeDefaultValue() { + if (getPreferences() != null) { + get(); + } + } + + /** * Replies the property key. * @return The property key @@ -36,5 +182,5 @@ */ public boolean isSet() { - return !Main.pref.get(key).isEmpty(); + return !getPreferences().get(key).isEmpty(); } @@ -51,5 +197,5 @@ */ public void remove() { - Main.pref.put(getKey(), String.valueOf(getDefaultValue())); + put(getDefaultValue()); } @@ -68,3 +214,82 @@ */ public abstract boolean put(T value); + + /** + * Gets the preferences used for this property. + * @return The preferences for this property. + * @since 10824 + */ + protected Preferences getPreferences() { + return preferences; + } + + /** + * Adds a listener that listens only for changes to this preference key. + * @param listener The listener to add. + * @since 10824 + */ + public void addListener(ValueChangeListener listener) { + addListenerImpl(new PreferenceChangedListenerAdapter(listener)); + } + + protected void addListenerImpl(PreferenceChangedListener adapter) { + getPreferences().addKeyPreferenceChangeListener(getKey(), adapter); + } + + /** + * Adds a weak listener that listens only for changes to this preference key. + * @param listener The listener to add. + * @since 10824 + */ + public void addWeakListener(ValueChangeListener listener) { + addWeakListenerImpl(new PreferenceChangedListenerAdapter(listener)); + } + + protected void addWeakListenerImpl(PreferenceChangedListener adapter) { + getPreferences().addWeakKeyPreferenceChangeListener(getKey(), adapter); + } + + /** + * Removes a listener that listens only for changes to this preference key. + * @param listener The listener to add. + * @since 10824 + */ + public void removeListener(ValueChangeListener listener) { + removeListenerImpl(new PreferenceChangedListenerAdapter(listener)); + } + + protected void removeListenerImpl(PreferenceChangedListener adapter) { + getPreferences().removeKeyPreferenceChangeListener(getKey(), adapter); + } + + @Override + public int hashCode() { + final int prime = 31; + int result = 1; + result = prime * result + ((key == null) ? 0 : key.hashCode()); + result = prime * result + ((preferences == null) ? 0 : preferences.hashCode()); + return result; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) + return true; + if (obj == null) + return false; + if (getClass() != obj.getClass()) + return false; + AbstractProperty other = (AbstractProperty) obj; + if (key == null) { + if (other.key != null) + return false; + } else if (!key.equals(other.key)) + return false; + if (preferences == null) { + if (other.preferences != null) + return false; + } else if (!preferences.equals(other.preferences)) + return false; + return true; + } } Index: /trunk/src/org/openstreetmap/josm/data/preferences/AbstractToStringProperty.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/preferences/AbstractToStringProperty.java (revision 10824) +++ /trunk/src/org/openstreetmap/josm/data/preferences/AbstractToStringProperty.java (revision 10824) @@ -0,0 +1,161 @@ +// License: GPL. For details, see LICENSE file. +package org.openstreetmap.josm.data.preferences; + +import org.openstreetmap.josm.Main; +import org.openstreetmap.josm.data.Preferences.PreferenceChangedListener; +import org.openstreetmap.josm.tools.CheckParameterUtil; +import org.openstreetmap.josm.tools.bugreport.BugReport; + +/** + * This class represents a property that can be represented as String. + * + * @author Michael Zangl + * + * @param The property content type. + * @since 10824 + */ +public abstract class AbstractToStringProperty extends AbstractProperty { + + /** + * This is a version of this property that attempts to get the property with a more specialized key and - if that fails - uses the property + * value as default. + * + * @author Michael Zangl + * @param The content type + */ + public static class ChildProperty extends AbstractToStringProperty { + private AbstractToStringProperty parent; + + ChildProperty(AbstractToStringProperty parent, String key) { + super(key, null); + CheckParameterUtil.ensureParameterNotNull(parent, "parent"); + this.parent = parent; + } + + @Override + protected void storeDefaultValue() { + // Default value hidden in preferences. + } + + @Override + public T getDefaultValue() { + return parent.get(); + } + + @Override + protected T fromString(String string) { + return parent.fromString(string); + } + + @Override + protected String toString(T t) { + return parent.toString(t); + } + + @Override + protected void addListenerImpl(PreferenceChangedListener adapter) { + super.addListenerImpl(adapter); + parent.addListenerImpl(adapter); + } + + @Override + protected void addWeakListenerImpl(PreferenceChangedListener adapter) { + super.addWeakListenerImpl(adapter); + parent.addWeakListenerImpl(adapter); + } + + @Override + protected void removeListenerImpl(PreferenceChangedListener adapter) { + super.removeListenerImpl(adapter); + parent.removeListenerImpl(adapter); + } + + @Override + public CachingProperty cached() { + throw new UnsupportedOperationException("Not implemented yet."); + } + } + + /** + * Create a new property and store the default value. + * @param key The key + * @param defaultValue The default value. + * @see AbstractProperty#AbstractProperty(String, Object) + */ + public AbstractToStringProperty(String key, T defaultValue) { + super(key, defaultValue); + storeDefaultValue(); + } + + @Override + public T get() { + String string = getAsString(); + if (!string.isEmpty()) { + try { + return fromString(string); + } catch (InvalidPreferenceValueException e) { + Main.warn(BugReport.intercept(e).put("key", key).put("value", string)); + } + } + return getDefaultValue(); + } + + /** + * Converts the string to an object of the given type. + * @param string The string + * @return The object. + * @throws InvalidPreferenceValueException If the value could not be converted. + */ + protected abstract T fromString(String string); + + @Override + public boolean put(T value) { + String string = value == null ? null : toString(value); + return getPreferences().put(getKey(), string); + } + + /** + * Converts the string to an object of the given type. + * @param t The object. + * @return The string representing the object + * @throws InvalidPreferenceValueException If the value could not be converted. + */ + protected abstract String toString(T t); + + /** + * Gets the preference value as String. + * @return The string preference value. + */ + protected String getAsString() { + T def = getDefaultValue(); + return getPreferences().get(key, def == null ? "" : toString(def)); + } + + /** + * Gets a specialized setting value that has the current value as default + *

+ * The key will be getKey().spec + * @param spec The key specialization + * @return The property + */ + public AbstractToStringProperty getSpecialized(String spec) { + return getChildProperty(getKey() + "." + spec); + } + + /** + * Gets a setting that defaults to this setting if the key is not set. + * @param key The more specialized key. + * @return The new setting. + */ + protected AbstractToStringProperty getChildProperty(String key) { + return new ChildProperty<>(this, key); + } + + /** + * Creates a new {@link CachingProperty} instance for this property. + * @return The new caching property instance. + */ + public CachingProperty cached() { + return new CachingProperty<>(this); + } +} Index: /trunk/src/org/openstreetmap/josm/data/preferences/BooleanProperty.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/preferences/BooleanProperty.java (revision 10823) +++ /trunk/src/org/openstreetmap/josm/data/preferences/BooleanProperty.java (revision 10824) @@ -1,11 +1,9 @@ // License: GPL. For details, see LICENSE file. package org.openstreetmap.josm.data.preferences; - -import org.openstreetmap.josm.Main; /** * A property containing a {@code Boolean} value. */ -public class BooleanProperty extends AbstractProperty { +public class BooleanProperty extends AbstractToStringProperty { /** @@ -16,17 +14,26 @@ public BooleanProperty(String key, boolean defaultValue) { super(key, defaultValue); - if (Main.pref != null) { - get(); - } } @Override public Boolean get() { - return Main.pref.getBoolean(getKey(), defaultValue); + // Removing this implementation breaks binary compatibility + return super.get(); } @Override public boolean put(Boolean value) { - return Main.pref.put(getKey(), value); + // Removing this implementation breaks binary compatibility + return super.put(value); + } + + @Override + protected Boolean fromString(String string) { + return Boolean.valueOf(string); + } + + @Override + protected String toString(Boolean t) { + return t.toString(); } } Index: /trunk/src/org/openstreetmap/josm/data/preferences/CachedProperty.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/preferences/CachedProperty.java (revision 10823) +++ /trunk/src/org/openstreetmap/josm/data/preferences/CachedProperty.java (revision 10824) @@ -14,5 +14,5 @@ protected CachedProperty(String key, String defaultValueAsString) { super(key, null); - Main.pref.addPreferenceChangeListener(this); + Main.pref.addKeyPreferenceChangeListener(key, this); this.defaultValueAsString = defaultValueAsString; updateValue(); @@ -61,5 +61,5 @@ public String getAsString() { - return Main.pref.get(getKey(), getDefaultValueAsString()); + return getPreferences().get(getKey(), getDefaultValueAsString()); } Index: /trunk/src/org/openstreetmap/josm/data/preferences/CachingProperty.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/preferences/CachingProperty.java (revision 10824) +++ /trunk/src/org/openstreetmap/josm/data/preferences/CachingProperty.java (revision 10824) @@ -0,0 +1,48 @@ +// License: GPL. For details, see LICENSE file. +package org.openstreetmap.josm.data.preferences; + +import org.openstreetmap.josm.data.preferences.AbstractProperty.ValueChangeListener; + +/** + * This is a special wrapper of {@link AbstractProperty}. + * The current preference value is cached. The value is invalidated if the preference was changed. + * @author Michael Zangl + * + * @param property type + * @since 10824 + */ +public class CachingProperty extends AbstractProperty implements ValueChangeListener { + + private T cache; + private boolean cacheActive; + private final AbstractProperty toCache; + + /** + * Create a new caching property. + * @param toCache The property to cache. + */ + CachingProperty(AbstractProperty toCache) { + super(toCache.getKey(), toCache.getDefaultValue()); + this.toCache = toCache; + addWeakListener(this); + } + + @Override + public synchronized T get() { + if (!cacheActive) { + cache = toCache.get(); + cacheActive = true; + } + return cache; + } + + @Override + public boolean put(T value) { + return toCache.put(cache); + } + + @Override + public synchronized void valueChanged(ValueChangeEvent e) { + cacheActive = false; + } +} Index: /trunk/src/org/openstreetmap/josm/data/preferences/CollectionProperty.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/preferences/CollectionProperty.java (revision 10823) +++ /trunk/src/org/openstreetmap/josm/data/preferences/CollectionProperty.java (revision 10824) @@ -25,10 +25,10 @@ @Override public Collection get() { - return Main.pref.getCollection(getKey(), getDefaultValue()); + return getPreferences().getCollection(getKey(), getDefaultValue()); } @Override public boolean put(Collection value) { - return Main.pref.putCollection(getKey(), value); + return getPreferences().putCollection(getKey(), value); } } Index: /trunk/src/org/openstreetmap/josm/data/preferences/ColorProperty.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/preferences/ColorProperty.java (revision 10823) +++ /trunk/src/org/openstreetmap/josm/data/preferences/ColorProperty.java (revision 10824) @@ -5,6 +5,5 @@ import java.util.Locale; -import org.openstreetmap.josm.Main; -import org.openstreetmap.josm.data.Preferences.ColorKey; +import org.openstreetmap.josm.tools.ColorHelper; /** @@ -12,7 +11,16 @@ * @since 5464 */ -public class ColorProperty extends AbstractProperty implements ColorKey { +public class ColorProperty extends AbstractToStringProperty { private final String name; + + /** + * Constructs a new {@code ColorProperty}. + * @param colName The color name + * @param defaultValue The default value as HTML string + */ + public ColorProperty(String colName, String defaultValue) { + this(colName, ColorHelper.html2color(defaultValue)); + } /** @@ -24,17 +32,44 @@ super(getColorKey(colName), defaultValue); this.name = colName; - if (Main.pref != null) { - get(); - } + getPreferences().registerColor(getColorKey(colName), colName); } @Override public Color get() { - return Main.pref.getColor(this); + // Removing this implementation breaks binary compatibility due to the way generics work + return super.get(); } @Override public boolean put(Color value) { - return Main.pref.putColor(getColorKey(name), value); + // Removing this implementation breaks binary compatibility due to the way generics work + return super.put(value); + } + + @Override + protected Color fromString(String string) { + return ColorHelper.html2color(string); + } + + @Override + protected String toString(Color t) { + return ColorHelper.color2html(t, true); + } + + /** + * Gets a color of which the value can be set. + * @param colorName the name of the color. + * @return The child property that inherits this value if it is not set. + */ + public AbstractToStringProperty getChildColor(String colorName) { + return getChildProperty(getColorKey(colorName)); + } + + /** + * Gets the name this color was registered with. + * @return The name. + */ + public String getName() { + return name; } @@ -45,15 +80,10 @@ */ public static String getColorKey(String colName) { - return colName == null ? null : colName.toLowerCase(Locale.ENGLISH).replaceAll("[^a-z0-9]+", "."); + return colName == null ? null : "color." + colName.toLowerCase(Locale.ENGLISH).replaceAll("[^a-z0-9]+", "."); } @Override - public String getColorName() { - return name; - } - - @Override - public String getSpecialName() { - return null; + public String toString() { + return "ColorProperty [name=" + name + ", defaultValue=" + getDefaultValue() + "]"; } } Index: /trunk/src/org/openstreetmap/josm/data/preferences/DoubleProperty.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/preferences/DoubleProperty.java (revision 10823) +++ /trunk/src/org/openstreetmap/josm/data/preferences/DoubleProperty.java (revision 10824) @@ -1,6 +1,4 @@ // License: GPL. For details, see LICENSE file. package org.openstreetmap.josm.data.preferences; - -import org.openstreetmap.josm.Main; /** @@ -8,5 +6,5 @@ * @since 3246 */ -public class DoubleProperty extends AbstractProperty { +public class DoubleProperty extends AbstractToStringProperty { /** @@ -21,10 +19,26 @@ @Override public Double get() { - return Main.pref.getDouble(getKey(), getDefaultValue()); + // Removing this implementation breaks binary compatibility + return super.get(); } @Override public boolean put(Double value) { - return Main.pref.putDouble(getKey(), value); + // Removing this implementation breaks binary compatibility + return super.put(value); + } + + @Override + protected Double fromString(String string) { + try { + return Double.valueOf(string); + } catch (NumberFormatException e) { + throw new InvalidPreferenceValueException(e); + } + } + + @Override + protected String toString(Double t) { + return t.toString(); } Index: /trunk/src/org/openstreetmap/josm/data/preferences/IntegerProperty.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/preferences/IntegerProperty.java (revision 10823) +++ /trunk/src/org/openstreetmap/josm/data/preferences/IntegerProperty.java (revision 10824) @@ -8,5 +8,5 @@ * @since 3246 */ -public class IntegerProperty extends AbstractProperty { +public class IntegerProperty extends AbstractToStringProperty { /** @@ -24,10 +24,26 @@ @Override public Integer get() { - return Main.pref.getInteger(getKey(), getDefaultValue()); + // Removing this implementation breaks binary compatibility + return super.get(); } @Override public boolean put(Integer value) { - return Main.pref.putInteger(getKey(), value); + // Removing this implementation breaks binary compatibility + return super.put(value); + } + + @Override + protected Integer fromString(String string) { + try { + return Integer.valueOf(string); + } catch (NumberFormatException e) { + throw new InvalidPreferenceValueException(e); + } + } + + @Override + protected String toString(Integer t) { + return t.toString(); } Index: /trunk/src/org/openstreetmap/josm/data/preferences/LongProperty.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/preferences/LongProperty.java (revision 10823) +++ /trunk/src/org/openstreetmap/josm/data/preferences/LongProperty.java (revision 10824) @@ -1,6 +1,4 @@ // License: GPL. For details, see LICENSE file. package org.openstreetmap.josm.data.preferences; - -import org.openstreetmap.josm.Main; /** @@ -9,5 +7,5 @@ * */ -public class LongProperty extends AbstractProperty { +public class LongProperty extends AbstractToStringProperty { /** @@ -18,18 +16,30 @@ public LongProperty(String key, long defaultValue) { super(key, defaultValue); - if (Main.pref != null) { - get(); + } + + @Override + public Long get() { + // Removing this implementation breaks binary compatibility + return super.get(); + } + + @Override + public boolean put(Long value) { + // Removing this implementation breaks binary compatibility + return super.put(value); + } + + @Override + protected Long fromString(String string) { + try { + return Long.valueOf(string); + } catch (NumberFormatException e) { + throw new InvalidPreferenceValueException(e); } } @Override - public Long get() { - return Main.pref.getLong(getKey(), getDefaultValue()); + protected String toString(Long t) { + return t.toString(); } - - @Override - public boolean put(Long value) { - return Main.pref.putLong(getKey(), value); - } - } Index: /trunk/src/org/openstreetmap/josm/data/preferences/PreferencesWriter.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/preferences/PreferencesWriter.java (revision 10823) +++ /trunk/src/org/openstreetmap/josm/data/preferences/PreferencesWriter.java (revision 10824) @@ -6,4 +6,5 @@ import java.util.List; import java.util.Map; +import java.util.stream.Stream; import org.openstreetmap.josm.Main; @@ -38,4 +39,13 @@ */ public void write(Collection>> settings) { + write(settings.stream()); + } + + /** + * Write preferences. + * + * @param settings preferences settings to write as stream. + */ + public void write(Stream>> settings) { out.write(String.format("%n")); String rootElement = defaults ? "preferences-defaults" : "preferences"; @@ -45,8 +55,8 @@ } out.write(String.format(" version='%d'>%n", Version.getInstance().getVersion())); - for (Map.Entry> e : settings) { + settings.forEachOrdered(e -> { setKey(e.getKey()); e.getValue().visit(this); - } + }); out.write(String.format("%n", rootElement)); } Index: /trunk/src/org/openstreetmap/josm/data/preferences/StringProperty.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/preferences/StringProperty.java (revision 10823) +++ /trunk/src/org/openstreetmap/josm/data/preferences/StringProperty.java (revision 10824) @@ -1,11 +1,9 @@ // License: GPL. For details, see LICENSE file. package org.openstreetmap.josm.data.preferences; - -import org.openstreetmap.josm.Main; /** * A property containing an {@code String} value. */ -public class StringProperty extends AbstractProperty { +public class StringProperty extends AbstractToStringProperty { /** @@ -16,17 +14,26 @@ public StringProperty(String key, String defaultValue) { super(key, defaultValue); - if (Main.pref != null) { - get(); - } } @Override public String get() { - return Main.pref.get(getKey(), getDefaultValue()); + // Removing this implementation breaks binary compatibility + return super.get(); } @Override public boolean put(String value) { - return Main.pref.put(getKey(), value); + // Removing this implementation breaks binary compatibility + return super.put(value); + } + + @Override + protected String fromString(String string) { + return string; + } + + @Override + protected String toString(String string) { + return string; } } Index: /trunk/src/org/openstreetmap/josm/gui/MainApplication.java =================================================================== --- /trunk/src/org/openstreetmap/josm/gui/MainApplication.java (revision 10823) +++ /trunk/src/org/openstreetmap/josm/gui/MainApplication.java (revision 10824) @@ -314,47 +314,4 @@ } - initApplicationPreferences(); - - Policy.setPolicy(new Policy() { - // Permissions for plug-ins loaded when josm is started via webstart - private PermissionCollection pc; - - { - pc = new Permissions(); - pc.add(new AllPermission()); - } - - @Override - public PermissionCollection getPermissions(CodeSource codesource) { - return pc; - } - }); - - Thread.setDefaultUncaughtExceptionHandler(new BugReportExceptionHandler()); - - // initialize the platform hook, and - Main.determinePlatformHook(); - // call the really early hook before we do anything else - Main.platform.preStartupHook(); - - Main.COMMAND_LINE_ARGS.addAll(Arrays.asList(argArray)); - - if (args.containsKey(Option.VERSION)) { - System.out.println(Version.getInstance().getAgentString()); - System.exit(0); - } - - if (args.containsKey(Option.DEBUG) || args.containsKey(Option.TRACE)) { - // Enable JOSM debug level - logLevel = 4; - Main.info(tr("Printing debugging messages to console")); - } - - boolean skipLoadingPlugins = false; - if (args.containsKey(Option.SKIP_PLUGINS)) { - skipLoadingPlugins = true; - Main.info(tr("Plugin loading skipped")); - } - if (args.containsKey(Option.TRACE)) { // Enable JOSM debug level @@ -363,4 +320,45 @@ Utils.updateSystemProperty("debug", "true"); Main.info(tr("Enabled detailed debug level (trace)")); + } else if (args.containsKey(Option.DEBUG)) { + // Enable JOSM debug level + logLevel = 4; + Main.info(tr("Printing debugging messages to console")); + } + + initApplicationPreferences(); + + Policy.setPolicy(new Policy() { + // Permissions for plug-ins loaded when josm is started via webstart + private PermissionCollection pc; + + { + pc = new Permissions(); + pc.add(new AllPermission()); + } + + @Override + public PermissionCollection getPermissions(CodeSource codesource) { + return pc; + } + }); + + Thread.setDefaultUncaughtExceptionHandler(new BugReportExceptionHandler()); + + // initialize the platform hook, and + Main.determinePlatformHook(); + // call the really early hook before we do anything else + Main.platform.preStartupHook(); + + Main.COMMAND_LINE_ARGS.addAll(Arrays.asList(argArray)); + + if (args.containsKey(Option.VERSION)) { + System.out.println(Version.getInstance().getAgentString()); + System.exit(0); + } + + boolean skipLoadingPlugins = false; + if (args.containsKey(Option.SKIP_PLUGINS)) { + skipLoadingPlugins = true; + Main.info(tr("Plugin loading skipped")); } Index: /trunk/src/org/openstreetmap/josm/gui/MapFrame.java =================================================================== --- /trunk/src/org/openstreetmap/josm/gui/MapFrame.java (revision 10823) +++ /trunk/src/org/openstreetmap/josm/gui/MapFrame.java (revision 10824) @@ -760,4 +760,5 @@ } else if (mapMode != null) { mapMode.exitMode(); // if new mode is null - simply exit from previous mode + mapMode = null; } } Index: /trunk/src/org/openstreetmap/josm/gui/MapStatus.java =================================================================== --- /trunk/src/org/openstreetmap/josm/gui/MapStatus.java (revision 10823) +++ /trunk/src/org/openstreetmap/josm/gui/MapStatus.java (revision 10824) @@ -65,5 +65,8 @@ import org.openstreetmap.josm.data.osm.OsmPrimitive; import org.openstreetmap.josm.data.osm.Way; +import org.openstreetmap.josm.data.preferences.AbstractProperty; +import org.openstreetmap.josm.data.preferences.BooleanProperty; import org.openstreetmap.josm.data.preferences.ColorProperty; +import org.openstreetmap.josm.data.preferences.DoubleProperty; import org.openstreetmap.josm.gui.help.Helpful; import org.openstreetmap.josm.gui.preferences.projection.ProjectionPreference; @@ -93,5 +96,7 @@ private final DecimalFormat DECIMAL_FORMAT = new DecimalFormat(Main.pref.get("statusbar.decimal-format", "0.0")); - private final double DISTANCE_THRESHOLD = Main.pref.getDouble("statusbar.distance-threshold", 0.01); + private static final AbstractProperty DISTANCE_THRESHOLD = new DoubleProperty("statusbar.distance-threshold", 0.01).cached(); + + private static final AbstractProperty SHOW_ID = new BooleanProperty("osm-primitives.showid", false); /** @@ -100,5 +105,5 @@ */ public static final ColorProperty PROP_BACKGROUND_COLOR = new ColorProperty( - marktr("Status bar background"), Color.decode("#b8cfe5")); + marktr("Status bar background"), "#b8cfe5"); /** @@ -107,5 +112,5 @@ */ public static final ColorProperty PROP_ACTIVE_BACKGROUND_COLOR = new ColorProperty( - marktr("Status bar background: active"), Color.decode("#aaff5e")); + marktr("Status bar background: active"), "#aaff5e"); /** @@ -568,5 +573,5 @@ text.append(name); - boolean idShown = Main.pref.getBoolean("osm-primitives.showid"); + boolean idShown = SHOW_ID.get(); // fix #7557 - do not show ID twice @@ -1021,5 +1026,5 @@ public void setDist(double dist) { distValue = dist; - distText.setText(dist < 0 ? "--" : NavigatableComponent.getDistText(dist, DECIMAL_FORMAT, DISTANCE_THRESHOLD)); + distText.setText(dist < 0 ? "--" : NavigatableComponent.getDistText(dist, DECIMAL_FORMAT, DISTANCE_THRESHOLD.get())); } Index: /trunk/src/org/openstreetmap/josm/gui/conflict/ConflictColors.java =================================================================== --- /trunk/src/org/openstreetmap/josm/gui/conflict/ConflictColors.java (revision 10823) +++ /trunk/src/org/openstreetmap/josm/gui/conflict/ConflictColors.java (revision 10824) @@ -6,6 +6,5 @@ import java.awt.Color; -import org.openstreetmap.josm.Main; -import org.openstreetmap.josm.data.Preferences.ColorKey; +import org.openstreetmap.josm.data.preferences.ColorProperty; /** @@ -13,5 +12,5 @@ * @since 4162 */ -public enum ConflictColors implements ColorKey { +public enum ConflictColors { /** Conflict background: no conflict */ @@ -83,25 +82,8 @@ FGCOLOR_MEMBER_REMOVE(marktr("Conflict foreground: remove member"), Color.black); - private final String name; - private final Color defaultColor; + private final ColorProperty property; ConflictColors(String name, Color defaultColor) { - this.name = name; - this.defaultColor = defaultColor; - } - - @Override - public String getColorName() { - return name; - } - - @Override - public Color getDefaultValue() { - return defaultColor; - } - - @Override - public String getSpecialName() { - return null; + property = new ColorProperty(name, defaultColor); } @@ -111,5 +93,5 @@ */ public Color get() { - return Main.pref.getColor(this); + return property.get(); } Index: /trunk/src/org/openstreetmap/josm/gui/dialogs/LayerListDialog.java =================================================================== --- /trunk/src/org/openstreetmap/josm/gui/dialogs/LayerListDialog.java (revision 10823) +++ /trunk/src/org/openstreetmap/josm/gui/dialogs/LayerListDialog.java (revision 10824) @@ -21,4 +21,5 @@ import java.util.Collections; import java.util.List; +import java.util.Objects; import java.util.concurrent.CopyOnWriteArrayList; @@ -45,4 +46,5 @@ import org.openstreetmap.josm.Main; import org.openstreetmap.josm.actions.MergeLayerAction; +import org.openstreetmap.josm.data.preferences.AbstractProperty; import org.openstreetmap.josm.gui.MapFrame; import org.openstreetmap.josm.gui.MapView; @@ -546,26 +548,16 @@ } if (Main.pref.getBoolean("dialog.layer.colorname", true)) { - Color c = layer.getColor(false); - if (c != null) { - Color oc = null; - for (Layer l : model.getLayers()) { - oc = l.getColor(false); - if (oc != null) { - if (oc.equals(c)) { - oc = null; - } else { - break; - } - } - } + AbstractProperty prop = layer.getColorProperty(); + Color c = prop == null ? null : prop.get(); + if (c == null || !model.getLayers().stream() + .map(Layer::getColorProperty) + .filter(Objects::nonNull) + .map(AbstractProperty::get) + .anyMatch(oc -> oc != null && !oc.equals(c))) { /* not more than one color, don't use coloring */ - if (oc == null) { - c = null; - } + label.setForeground(UIManager.getColor(isSelected ? "Table.selectionForeground" : "Table.foreground")); + } else { + label.setForeground(c); } - if (c == null) { - c = UIManager.getColor(isSelected ? "Table.selectionForeground" : "Table.foreground"); - } - label.setForeground(c); } label.setIcon(layer.getIcon()); Index: /trunk/src/org/openstreetmap/josm/gui/dialogs/properties/PropertiesDialog.java =================================================================== --- /trunk/src/org/openstreetmap/josm/gui/dialogs/properties/PropertiesDialog.java (revision 10823) +++ /trunk/src/org/openstreetmap/josm/gui/dialogs/properties/PropertiesDialog.java (revision 10824) @@ -64,5 +64,5 @@ import org.openstreetmap.josm.command.ChangePropertyCommand; import org.openstreetmap.josm.command.Command; -import org.openstreetmap.josm.data.Preferences.PreferenceChangeEvent; +import org.openstreetmap.josm.data.Preferences.PreferenceChangedListener; import org.openstreetmap.josm.data.SelectionChangedListener; import org.openstreetmap.josm.data.osm.IRelation; @@ -226,4 +226,10 @@ + tr("Select objects for which to change tags.") + "

"); + private final PreferenceChangedListener preferenceListener = e -> { + if (Main.getLayerManager().getEditDataSet() != null) { + // Re-load data when display preference change + updateSelection(); + }}; + private final transient TaggingPresetHandler presetHandler = new TaggingPresetHandler() { @Override @@ -298,5 +304,5 @@ editHelper.loadTagsIfNeeded(); - Main.pref.addPreferenceChangeListener(this); + Main.pref.addKeyPreferenceChangeListener("display.discardable-keys", preferenceListener); } @@ -602,5 +608,5 @@ public void destroy() { super.destroy(); - Main.pref.removePreferenceChangeListener(this); + Main.pref.removeKeyPreferenceChangeListener("display.discardable-keys", preferenceListener); Container parent = pluginHook.getParent(); if (parent != null) { @@ -1390,13 +1396,4 @@ } - @Override - public void preferenceChanged(PreferenceChangeEvent e) { - super.preferenceChanged(e); - if ("display.discardable-keys".equals(e.getKey()) && Main.getLayerManager().getEditDataSet() != null) { - // Re-load data when display preference change - updateSelection(); - } - } - /** * Clears the row selection when it is filtered away by the row sorter. Index: /trunk/src/org/openstreetmap/josm/gui/layer/CustomizeColor.java =================================================================== --- /trunk/src/org/openstreetmap/josm/gui/layer/CustomizeColor.java (revision 10823) +++ /trunk/src/org/openstreetmap/josm/gui/layer/CustomizeColor.java (revision 10824) @@ -8,6 +8,8 @@ import java.awt.Component; import java.awt.event.ActionEvent; -import java.util.LinkedList; +import java.util.Collections; import java.util.List; +import java.util.Objects; +import java.util.stream.Collectors; import javax.swing.AbstractAction; @@ -18,10 +20,13 @@ import org.openstreetmap.josm.Main; +import org.openstreetmap.josm.data.preferences.AbstractProperty; +import org.openstreetmap.josm.gui.dialogs.LayerListDialog; import org.openstreetmap.josm.gui.layer.Layer.LayerAction; import org.openstreetmap.josm.gui.layer.Layer.MultiLayerAction; +import org.openstreetmap.josm.tools.CheckParameterUtil; import org.openstreetmap.josm.tools.ImageProvider; public class CustomizeColor extends AbstractAction implements LayerAction, MultiLayerAction { - private final transient List layers; + private final transient List> colors; /** @@ -31,6 +36,7 @@ public CustomizeColor(List l) { super(tr("Customize Color"), ImageProvider.get("colorchooser")); + colors = l.stream().map(Layer::getColorProperty).collect(Collectors.toList()); + CheckParameterUtil.ensureThat(colors.stream().allMatch(Objects::nonNull), "All layers must have colors."); putValue("help", ht("/Action/LayerCustomizeColor")); - layers = l; } @@ -40,15 +46,10 @@ */ public CustomizeColor(Layer l) { - this(new LinkedList()); - layers.add(l); + this(Collections.singletonList(l)); } @Override public boolean supportLayers(List layers) { - for (Layer layer: layers) { - if (layer.getColor(false) == null) - return false; - } - return true; + return layers.stream().allMatch(l -> l.getColorProperty() != null); } @@ -65,7 +66,5 @@ @Override public void actionPerformed(ActionEvent e) { - Color cl = layers.get(0).getColor(false); - if (cl == null) - cl = Color.gray; + Color cl = colors.stream().map(c -> c.get()).filter(Objects::nonNull).findAny().orElse(Color.GRAY); JColorChooser c = new JColorChooser(cl); Object[] options = new Object[]{tr("OK"), tr("Cancel"), tr("Default")}; @@ -82,17 +81,14 @@ switch (answer) { case 0: - for (Layer layer : layers) { - Main.pref.putColor("layer "+layer.getName(), c.getColor()); - } + colors.stream().forEach(prop -> prop.put(c.getColor())); break; case 1: return; case 2: - for (Layer layer : layers) { - Main.pref.putColor("layer "+layer.getName(), null); - } + colors.stream().forEach(prop -> prop.put(null)); break; } - Main.map.repaint(); + // TODO: Make the layer dialog listen to property change events so that this is not needed any more. + LayerListDialog.getInstance().repaint(); } } Index: /trunk/src/org/openstreetmap/josm/gui/layer/GpxLayer.java =================================================================== --- /trunk/src/org/openstreetmap/josm/gui/layer/GpxLayer.java (revision 10823) +++ /trunk/src/org/openstreetmap/josm/gui/layer/GpxLayer.java (revision 10824) @@ -5,5 +5,4 @@ import static org.openstreetmap.josm.tools.I18n.trn; -import java.awt.Color; import java.awt.Dimension; import java.awt.Graphics2D; @@ -32,4 +31,5 @@ import org.openstreetmap.josm.data.gpx.WayPoint; import org.openstreetmap.josm.data.osm.visitor.BoundingXYVisitor; +import org.openstreetmap.josm.data.preferences.ColorProperty; import org.openstreetmap.josm.data.projection.Projection; import org.openstreetmap.josm.gui.MapView; @@ -89,5 +89,5 @@ super(d.getString(GpxConstants.META_NAME)); data = d; - drawHelper = new GpxDrawHelper(data); + drawHelper = new GpxDrawHelper(data, getColorProperty()); SystemOfMeasurement.addSoMChangeListener(drawHelper); ensureTrackVisibilityLength(); @@ -97,6 +97,6 @@ @Override - public Color getColor(boolean ignoreCustom) { - return drawHelper.getColor(getName(), ignoreCustom); + protected ColorProperty getBaseColorProperty() { + return GpxDrawHelper.DEFAULT_COLOR; } Index: /trunk/src/org/openstreetmap/josm/gui/layer/Layer.java =================================================================== --- /trunk/src/org/openstreetmap/josm/gui/layer/Layer.java (revision 10823) +++ /trunk/src/org/openstreetmap/josm/gui/layer/Layer.java (revision 10824) @@ -26,4 +26,7 @@ import org.openstreetmap.josm.data.ProjectionBounds; import org.openstreetmap.josm.data.osm.visitor.BoundingXYVisitor; +import org.openstreetmap.josm.data.preferences.AbstractProperty; +import org.openstreetmap.josm.data.preferences.AbstractProperty.ValueChangeListener; +import org.openstreetmap.josm.data.preferences.ColorProperty; import org.openstreetmap.josm.data.projection.Projection; import org.openstreetmap.josm.data.projection.ProjectionChangeListener; @@ -146,4 +149,6 @@ private File associatedFile; + private final ValueChangeListener invalidateListener = change -> invalidate(); + /** * Create the layer and fill in the necessary components. @@ -182,7 +187,47 @@ * color is returned - mainly for layer internal use. * @return layer color - */ + * @deprecated Use the new {@link #getColorProperty()}. To be removed end of 2016. + */ + @Deprecated public Color getColor(boolean ignoreCustom) { return null; + } + + /** + * Gets the color property to use for this layer. + * @return The color property. + * @since 10824 + */ + public AbstractProperty getColorProperty() { + ColorProperty base = getBaseColorProperty(); + if (base != null) { + // cannot cache this - name may change. + return base.getChildColor("layer " + getName()); + } else { + return null; + } + } + + /** + * Gets the color property that stores the default color for this layer. + * @return The property or null if this layer is not colored. + * @since 10824 + */ + protected ColorProperty getBaseColorProperty() { + return null; + } + + private void addColorPropertyListener() { + AbstractProperty colorProperty = getColorProperty(); + if (colorProperty != null) { + colorProperty.addWeakListener(invalidateListener); + } + } + + private void removeColorPropertyListener() { + AbstractProperty colorProperty = getColorProperty(); + if (colorProperty != null) { + colorProperty.removeListener(invalidateListener); + } } @@ -265,7 +310,11 @@ */ public final void setName(String name) { + if (this.name != null) { + removeColorPropertyListener(); + } if (name == null) { name = ""; } + String oldValue = this.name; this.name = name; @@ -273,4 +322,8 @@ propertyChangeSupport.firePropertyChange(NAME_PROP, oldValue, this.name); } + + // re-add listener + addColorPropertyListener(); + invalidate(); } Index: /trunk/src/org/openstreetmap/josm/gui/layer/gpx/GpxDrawHelper.java =================================================================== --- /trunk/src/org/openstreetmap/josm/gui/layer/gpx/GpxDrawHelper.java (revision 10823) +++ /trunk/src/org/openstreetmap/josm/gui/layer/gpx/GpxDrawHelper.java (revision 10824) @@ -25,4 +25,6 @@ import org.openstreetmap.josm.data.gpx.GpxData; import org.openstreetmap.josm.data.gpx.WayPoint; +import org.openstreetmap.josm.data.preferences.AbstractProperty; +import org.openstreetmap.josm.data.preferences.ColorProperty; import org.openstreetmap.josm.gui.MapView; import org.openstreetmap.josm.tools.ColorScale; @@ -33,4 +35,11 @@ */ public class GpxDrawHelper implements SoMChangeListener { + + /** + * The color that is used for drawing GPX points. + * @since 10824 + */ + public static final ColorProperty DEFAULT_COLOR = new ColorProperty(marktr("gps point"), Color.magenta); + private final GpxData data; @@ -84,6 +93,4 @@ private int hdopAlpha; - private static final Color DEFAULT_COLOR = Color.magenta; - // lookup array to draw arrows without doing any math private static final int ll0 = 9; @@ -134,6 +141,8 @@ * Constructs a new {@code GpxDrawHelper}. * @param gpxData GPX data + * @param abstractProperty The color to draw with + * @since 10824 */ - public GpxDrawHelper(GpxData gpxData) { + public GpxDrawHelper(GpxData gpxData, AbstractProperty abstractProperty) { data = gpxData; setupColors(); @@ -151,6 +160,9 @@ */ public Color getColor(String layerName, boolean ignoreCustom) { - Color c = Main.pref.getColor(marktr("gps point"), specName(layerName), DEFAULT_COLOR); - return ignoreCustom || getColorMode(layerName) == ColorMode.NONE ? c : null; + if (ignoreCustom || getColorMode(layerName) == ColorMode.NONE) { + return DEFAULT_COLOR.getChildColor(specName(layerName)).get(); + } else { + return null; + } } @@ -174,5 +186,5 @@ **/ public static Color getGenericColor() { - return Main.pref.getColor(marktr("gps point"), DEFAULT_COLOR); + return DEFAULT_COLOR.get(); } Index: /trunk/src/org/openstreetmap/josm/gui/layer/markerlayer/MarkerLayer.java =================================================================== --- /trunk/src/org/openstreetmap/josm/gui/layer/markerlayer/MarkerLayer.java (revision 10823) +++ /trunk/src/org/openstreetmap/josm/gui/layer/markerlayer/MarkerLayer.java (revision 10824) @@ -38,4 +38,5 @@ import org.openstreetmap.josm.data.gpx.WayPoint; import org.openstreetmap.josm.data.osm.visitor.BoundingXYVisitor; +import org.openstreetmap.josm.data.preferences.ColorProperty; import org.openstreetmap.josm.gui.MapView; import org.openstreetmap.josm.gui.dialogs.LayerListDialog; @@ -74,4 +75,5 @@ private static final Color DEFAULT_COLOR = Color.magenta; + private static final ColorProperty COLOR_PROPERTY = new ColorProperty(marktr("gps marker"), DEFAULT_COLOR); /** @@ -195,11 +197,11 @@ @Override - public Color getColor(boolean ignoreCustom) { - return Main.pref.getColor(marktr("gps marker"), "layer "+getName(), DEFAULT_COLOR); + protected ColorProperty getBaseColorProperty() { + return COLOR_PROPERTY; } /* for preferences */ public static Color getGenericColor() { - return Main.pref.getColor(marktr("gps marker"), DEFAULT_COLOR); + return COLOR_PROPERTY.get(); } @@ -207,5 +209,5 @@ public void paint(Graphics2D g, MapView mv, Bounds box) { boolean showTextOrIcon = isTextOrIconShown(); - g.setColor(getColor(true)); + g.setColor(getColorProperty().get()); if (mousePressed) { Index: /trunk/src/org/openstreetmap/josm/gui/preferences/display/ColorPreference.java =================================================================== --- /trunk/src/org/openstreetmap/josm/gui/preferences/display/ColorPreference.java (revision 10823) +++ /trunk/src/org/openstreetmap/josm/gui/preferences/display/ColorPreference.java (revision 10824) @@ -257,5 +257,5 @@ */ private static void fixColorPrefixes() { - PaintColors.getColors(); + PaintColors.values(); ConflictColors.getColors(); Severity.getColors(); Index: /trunk/src/org/openstreetmap/josm/tools/ListenerList.java =================================================================== --- /trunk/src/org/openstreetmap/josm/tools/ListenerList.java (revision 10824) +++ /trunk/src/org/openstreetmap/josm/tools/ListenerList.java (revision 10824) @@ -0,0 +1,231 @@ +// License: GPL. For details, see LICENSE file. +package org.openstreetmap.josm.tools; + +import java.lang.ref.WeakReference; +import java.text.MessageFormat; +import java.util.HashMap; +import java.util.Iterator; +import java.util.Objects; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.stream.Stream; + +import org.openstreetmap.josm.Main; + +/** + * This is a list of listeners. It does error checking and allows you to fire all listeners. + * + * @author Michael Zangl + * @param The type of listener contained in this list. + * @since 10824 + */ +public class ListenerList { + /** + * This is a function that can be invoked for every listener. + * @param the listener type. + */ + @FunctionalInterface + public interface EventFirerer { + /** + * Should fire the event for the given listener. + * @param listener The listener to fire the event for. + */ + void fire(T listener); + } + + private static final class WeakListener { + + private WeakReference listener; + + WeakListener(T listener) { + this.listener = new WeakReference<>(listener); + } + + @Override + public boolean equals(Object obj) { + if (obj != null && obj.getClass() == WeakListener.class) { + return Objects.equals(listener.get(), ((WeakListener) obj).listener.get()); + } else { + return false; + } + } + + @Override + public int hashCode() { + T l = listener.get(); + if (l == null) { + return 0; + } else { + return l.hashCode(); + } + } + + @Override + public String toString() { + return "WeakListener [listener=" + listener + "]"; + } + } + + private final CopyOnWriteArrayList listeners = new CopyOnWriteArrayList<>(); + private final CopyOnWriteArrayList> weakListeners = new CopyOnWriteArrayList<>(); + + protected ListenerList() { + // hide + } + + /** + * Adds a listener. The listener will not prevent the object from being garbage collected. + * + * This should be used with care. It is better to add good cleanup code. + * @param listener The listener. + */ + public synchronized void addWeakListener(T listener) { + ensureNotInList(listener); + // clean the weak listeners, just to be sure... + while (weakListeners.remove(new WeakListener(null))) { + // continue + } + weakListeners.add(new WeakListener<>(listener)); + } + + /** + * Adds a listener. + * @param listener The listener to add. + */ + public synchronized void addListener(T listener) { + ensureNotInList(listener); + listeners.add(listener); + } + + private void ensureNotInList(T listener) { + CheckParameterUtil.ensureParameterNotNull(listener, "listener"); + if (containsListener(listener)) { + failAdd(listener); + } + } + + protected void failAdd(T listener) { + throw new IllegalArgumentException( + MessageFormat.format("Listener {0} (instance of {1}) was already registered.", listener.toString(), + listener.getClass().getName())); + } + + private boolean containsListener(T listener) { + return listeners.contains(listener) || weakListeners.contains(new WeakListener<>(listener)); + } + + /** + * Removes a listener. + * @param listener The listener to remove. + * @throws IllegalArgumentException if the listener was not registered before + */ + public synchronized void removeListener(T listener) { + if (!listeners.remove(listener) && !weakListeners.remove(new WeakListener<>(listener))) { + failRemove(listener); + } + } + + protected void failRemove(T listener) { + throw new IllegalArgumentException( + MessageFormat.format("Listener {0} (instance of {1}) was not registered before or already removed.", + listener.toString(), listener.getClass().getName())); + } + + /** + * Check if any listeners are registered. + * @return true if any are registered. + */ + public boolean hasListeners() { + return !listeners.isEmpty(); + } + + /** + * Fires an event to every listener. + * @param eventFirerer The firerer to invoke the event method of the listener. + */ + public void fireEvent(EventFirerer eventFirerer) { + for (T l : listeners) { + eventFirerer.fire(l); + } + for (Iterator> iterator = weakListeners.iterator(); iterator.hasNext();) { + WeakListener weakLink = iterator.next(); + T l = weakLink.listener.get(); + if (l == null) { + iterator.remove(); + } else { + eventFirerer.fire(l); + } + } + } + + /** + * This is a special {@link ListenerList} that traces calls to the add/remove methods. This may cause memory leaks. + * @author Michael Zangl + * + * @param The type of listener contained in this list + */ + public static class TracingListenerList extends ListenerList { + private HashMap listenersAdded = new HashMap<>(); + private HashMap listenersRemoved = new HashMap<>(); + + protected TracingListenerList() { + // hidden + } + + @Override + public synchronized void addListener(T listener) { + super.addListener(listener); + listenersRemoved.remove(listener); + listenersAdded.put(listener, Thread.currentThread().getStackTrace()); + } + + @Override + public synchronized void addWeakListener(T listener) { + super.addWeakListener(listener); + listenersRemoved.remove(listener); + listenersAdded.put(listener, Thread.currentThread().getStackTrace()); + } + + @Override + public synchronized void removeListener(T listener) { + super.removeListener(listener); + listenersAdded.remove(listener); + listenersRemoved.put(listener, Thread.currentThread().getStackTrace()); + } + + @Override + protected void failAdd(T listener) { + Main.trace("Previous addition of the listener"); + dumpStack(listenersAdded.get(listener)); + super.failAdd(listener); + } + + @Override + protected void failRemove(T listener) { + Main.trace("Previous removal of the listener"); + dumpStack(listenersRemoved.get(listener)); + super.failRemove(listener); + } + + private static void dumpStack(StackTraceElement[] stackTraceElements) { + if (stackTraceElements == null) { + Main.trace(" - (no trace recorded)"); + } else { + Stream.of(stackTraceElements).limit(20).forEach( + e -> Main.trace(e.getClassName() + "." + e.getMethodName() + " line " + e.getLineNumber())); + } + } + } + + /** + * Create a new listener list + * @param The listener type the list should hold. + * @return A new list. A tracing list is created if trace is enabled. + */ + public static ListenerList create() { + if (Main.isTraceEnabled()) { + return new TracingListenerList<>(); + } else { + return new ListenerList<>(); + } + } +} Index: /trunk/test/unit/org/openstreetmap/josm/data/preferences/ColorPropertyTest.java =================================================================== --- /trunk/test/unit/org/openstreetmap/josm/data/preferences/ColorPropertyTest.java (revision 10824) +++ /trunk/test/unit/org/openstreetmap/josm/data/preferences/ColorPropertyTest.java (revision 10824) @@ -0,0 +1,82 @@ +// License: GPL. For details, see LICENSE file. +package org.openstreetmap.josm.data.preferences; + +import static org.junit.Assert.assertEquals; + +import java.awt.Color; + +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.openstreetmap.josm.Main; +import org.openstreetmap.josm.testutils.JOSMTestRules; + +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + +/** + * Test {@link ColorProperty} + * @author Michael Zangl + */ +public class ColorPropertyTest { + /** + * This is a preference test. + */ + @Rule + @SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD") + public JOSMTestRules test = new JOSMTestRules().preferences(); + private ColorProperty base; + + /** + * Set up test case + */ + @Before + public void createTestProperty() { + base = new ColorProperty("test", Color.RED); + } + + /** + * Test {@link ColorProperty#get()} + */ + @Test + public void testGet() { + assertEquals(Color.RED, base.get()); + + Main.pref.put("color.test", "#00ff00"); + assertEquals(new Color(0xff00ff00), base.get()); + } + + /** + * Test {@link ColorProperty#put} + */ + @Test + public void testPut() { + assertEquals(Color.RED, base.get()); + + base.put(new Color(0xff00ff00)); + assertEquals(new Color(0xff00ff00), base.get()); + assertEquals("#00ff00", Main.pref.get("color.test").toLowerCase()); + + base.put(null); + assertEquals(Color.RED, base.get()); + } + + /** + * Test {@link ColorProperty#getChildColor(String)} + */ + @Test + public void testGetChildColor() { + AbstractToStringProperty child = base.getChildColor("test2"); + + assertEquals(Color.RED, child.get()); + + base.put(Color.GREEN); + assertEquals(Color.GREEN, child.get()); + + child.put(Color.YELLOW); + assertEquals(Color.YELLOW, child.get()); + assertEquals(Color.GREEN, base.get()); + + child.put(null); + assertEquals(Color.GREEN, child.get()); + } +} Index: /trunk/test/unit/org/openstreetmap/josm/gui/layer/GpxLayerTest.java =================================================================== --- /trunk/test/unit/org/openstreetmap/josm/gui/layer/GpxLayerTest.java (revision 10823) +++ /trunk/test/unit/org/openstreetmap/josm/gui/layer/GpxLayerTest.java (revision 10824) @@ -74,5 +74,5 @@ assertEquals("foo", layer.getName()); assertFalse(layer.isLocalFile()); - assertEquals(Color.MAGENTA, layer.getColor(false)); + assertEquals(Color.MAGENTA, layer.getColorProperty().get()); assertEquals("0 tracks, 0 routes, 0 waypoints
Length: < 0.01 m
", layer.getToolTipText()); @@ -80,5 +80,5 @@ assertEquals("bar", layer2.getName()); assertTrue(layer2.isLocalFile()); - assertEquals(Color.MAGENTA, layer2.getColor(true)); + assertEquals(Color.MAGENTA, layer2.getColorProperty().get()); assertEquals("0 tracks, 0 routes, 0 waypoints
Length: < 0.01 m
", layer2.getToolTipText()); Index: /trunk/test/unit/org/openstreetmap/josm/gui/layer/gpx/GpxDrawHelperTest.java =================================================================== --- /trunk/test/unit/org/openstreetmap/josm/gui/layer/gpx/GpxDrawHelperTest.java (revision 10823) +++ /trunk/test/unit/org/openstreetmap/josm/gui/layer/gpx/GpxDrawHelperTest.java (revision 10824) @@ -4,4 +4,5 @@ import static org.junit.Assert.assertEquals; +import java.awt.Color; import java.io.FileNotFoundException; import java.io.IOException; @@ -17,4 +18,5 @@ import org.openstreetmap.josm.data.gpx.GpxData; import org.openstreetmap.josm.data.gpx.WayPoint; +import org.openstreetmap.josm.data.preferences.ColorProperty; import org.openstreetmap.josm.io.GpxReaderTest; import org.openstreetmap.josm.tools.ColorHelper; @@ -125,5 +127,5 @@ static List calculateColors(String fileName, String layerName, int n) throws IOException, SAXException { final GpxData data = GpxReaderTest.parseGpxData(fileName); - final GpxDrawHelper gdh = new GpxDrawHelper(data); + final GpxDrawHelper gdh = new GpxDrawHelper(data, new ColorProperty("x", Color.MAGENTA)); gdh.readPreferences(layerName); gdh.calculateColors(); Index: /trunk/test/unit/org/openstreetmap/josm/gui/layer/markerlayer/MarkerLayerTest.java =================================================================== --- /trunk/test/unit/org/openstreetmap/josm/gui/layer/markerlayer/MarkerLayerTest.java (revision 10823) +++ /trunk/test/unit/org/openstreetmap/josm/gui/layer/markerlayer/MarkerLayerTest.java (revision 10824) @@ -42,5 +42,5 @@ assertEquals("foo", layer.getName()); - assertEquals(Color.magenta, layer.getColor(false)); + assertEquals(Color.magenta, layer.getColorProperty().get()); assertNotNull(layer.getIcon()); assertEquals("0 markers", layer.getToolTipText()); @@ -59,5 +59,5 @@ assertEquals("bar", layer.getName()); - assertEquals(Color.magenta, layer.getColor(false)); + assertEquals(Color.magenta, layer.getColorProperty().get()); assertNotNull(layer.getIcon()); assertEquals("3 markers", layer.getToolTipText());