[PATCH] Don't require a restart when a MapPaint color is changed

[taylor.smock]

All this patch does is call MapPaintStyles.readFromPreferences().

[taylor.smock]

Is there any interest for this patch?

[stoecker]

I don't think that patch is not right. It only cares for mappaint. There are other places in JOSM which will not switch color without restart. Thought the line may be additional helpful to reach the goal in most cases, which is probably enough.

OTOH I think this breaks the component separation efforts.

[taylor.smock]

Replying to stoecker:

I don't think that patch is not right. It only cares for mappaint. There are other places in JOSM which will not switch color without restart. Thought the line may be additional helpful to reach the goal in most cases, which is probably enough.

That is somewhat surprising. The current code only returns true if a mappaint color is changed ( https://josm.openstreetmap.de/browser/josm/trunk/src/org/openstreetmap/josm/gui/preferences/display/ColorPreference.java#L392 ). Specifically,

if (e.info.getValue() != null && e.toProperty().put(e.info.getValue())
    && NamedColorProperty.COLOR_CATEGORY_MAPPAINT.equals(e.info.getCategory()))

is the only place where the ret value is set to true, which means that only map paint colors require a restart (as far as the current code is concerned). I did some spot checking with other colors (non mappaint), and they worked without a restart.

OTOH I think this breaks the component separation efforts.

The only other thing I can think of is to add a preference change listener to Mappaint.

I presume you were talking about #15229?

[stoecker]

Replying to taylor.smock:

is the only place where the ret value is set to true, which means that only map paint colors require a restart (as far as the current code is concerned). I did some spot checking with other colors (non mappaint), and they worked without a restart.

Ah. So maybe this was improved already.

OTOH I think this breaks the component separation efforts.

The only other thing I can think of is to add a preference change listener to Mappaint.

I presume you were talking about #15229?

Yes.

[taylor.smock]

Use a listener to get preference change events

[taylor.smock]

I've added a preference change listener in MapPaintStyles, and the if statement in ColorPreference has been modified accordingly.

Disadvantages: MapPaintStyles.readFromPreferences may be called multiple times. Not a major issue, since this shouldn't be something that is called every minute.

[taylor.smock]

Fix an issue where the preference listener could be called twice in the same preference change invocation

[simon04]

In 16562/josm:

fix #19196 - Don't require a restart when a MapPaint color is changed (patch by taylor.smock)

[simon04]

This patch broke two/three unit tests:

• https://josm.openstreetmap.de/jenkins/job/JOSM/6497/jdk=JDK8/testReport/org.openstreetmap.josm.data.validation.tests/MapCSSTagCheckerTest/testTicket12627/
• https://josm.openstreetmap.de/jenkins/job/JOSM/6497/jdk=JDK8/testReport/org.openstreetmap.josm.data.validation.tests/MapCSSTagCheckerTest/testTicket14289/
• and maybe https://josm.openstreetmap.de/jenkins/job/JOSM/6497/jdk=JDK8/testReport/org.openstreetmap.josm.gui.dialogs/InspectPrimitiveDialogTest/testBuildMapPaintText/

[taylor.smock]

Fix unit tests

[simon04]

In 16578/josm:

see #19196 - Fix unit tests (patch by taylor.smock)