Skip to content

Reset in colors preference page now respects theme-specific defaults #2421

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 19, 2024
+99 −6
Merged

Reset in colors preference page now respects theme-specific defaults #2421

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions ...eclipse.e4.ui.css.swt/src/org/eclipse/e4/ui/css/swt/helpers/EclipsePreferencesHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,11 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import org.eclipse.core.runtime.preferences.DefaultScope;
import org.eclipse.core.runtime.preferences.IEclipsePreferences;
import org.eclipse.core.runtime.preferences.IEclipsePreferences.IPreferenceChangeListener;
import org.eclipse.core.runtime.preferences.IEclipsePreferences.PreferenceChangeEvent;
import org.osgi.service.prefs.BackingStoreException;
import org.osgi.service.prefs.Preferences;

public class EclipsePreferencesHelper {
Expand All @@ -34,6 +36,8 @@ public class EclipsePreferencesHelper {

private static String currentThemeId;

private static final String PROPS_DEFAULT_VALUE_BEFORE_OVERIDDEN_FROM_CSS = "defaultValueBeforeOverriddenFromCSS";

public static void appendOverriddenPropertyName(
IEclipsePreferences preferences, String name) {
String value = preferences.get(PROPS_OVERRIDDEN_BY_CSS_PROP, SEPARATOR);
Expand Down Expand Up @@ -134,4 +138,46 @@ protected void removeOverriddenByCssProperty(PreferenceChangeEvent event) {
}
}
}

public static void overrideDefault(IEclipsePreferences preferences, String name, String value) {
String prefName = preferences.name();
if (prefName == null) {
return;
}
IEclipsePreferences defaultPrefs = DefaultScope.INSTANCE.getNode(prefName);
if (defaultPrefs == null) {
return;
}
String existing = defaultPrefs.get(name, null);
if (existing != null && value != null && existing.equals(value) == false) {
defaultPrefs.put(name, value);
preferences.put(name + SEPARATOR + PROPS_DEFAULT_VALUE_BEFORE_OVERIDDEN_FROM_CSS, existing);
}
}

public static void resetOverriddenDefaults(IEclipsePreferences preferences) {
try {
String[] keys = preferences.keys();
if (keys == null) {
return;
}
for (String key : keys) {
if (key != null && key.endsWith(SEPARATOR + PROPS_DEFAULT_VALUE_BEFORE_OVERIDDEN_FROM_CSS)) {
String overriddenDefault = preferences.get(key, null);
String originKey = key.substring(0,
key.lastIndexOf(SEPARATOR + PROPS_DEFAULT_VALUE_BEFORE_OVERIDDEN_FROM_CSS));
IEclipsePreferences defaultPrefs = DefaultScope.INSTANCE.getNode(preferences.name());
if (defaultPrefs != null) {
String currentDefault = defaultPrefs.get(originKey, null);
if (overriddenDefault != null && currentDefault != null
&& !currentDefault.equals(overriddenDefault)) {
defaultPrefs.put(originKey, overriddenDefault);
}
}
preferences.remove(key);
}
}
} catch (BackingStoreException e) { // silently ignored
}
}
}
1 change: 1 addition & 0 deletions ...ss.swt/src/org/eclipse/e4/ui/css/swt/properties/preference/EclipsePreferencesHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,5 +61,6 @@ protected void overrideProperty(IEclipsePreferences preferences, String name, St
preferences.put(name, value);
EclipsePreferencesHelper.appendOverriddenPropertyName(preferences, name);
}
EclipsePreferencesHelper.overrideDefault(preferences, name, value);
}
}
1 change: 1 addition & 0 deletions ...e4.ui.workbench.swt/src/org/eclipse/e4/ui/internal/workbench/swt/PartRenderingEngine.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1440,6 +1440,7 @@ protected void resetOverriddenPreferences() {
}

protected void resetOverriddenPreferences(IEclipsePreferences preferences) {
EclipsePreferencesHelper.resetOverriddenDefaults(preferences);
for (String name : getOverriddenPropertyNames(preferences)) {
preferences.remove(name);
}
Expand Down
57 changes: 51 additions & 6 deletions ...e.ui.workbench/eclipseui/org/eclipse/ui/internal/themes/ColorsAndFontsPreferencePage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,10 @@
import org.eclipse.core.runtime.IStatus;
import org.eclipse.e4.core.services.events.IEventBroker;
import org.eclipse.e4.ui.css.swt.theme.IThemeEngine;
import org.eclipse.jface.preference.IPreferenceStore;
import org.eclipse.jface.preference.PreferenceConverter;
import org.eclipse.jface.preference.PreferencePage;
import org.eclipse.jface.resource.DataFormatException;
import org.eclipse.jface.resource.JFaceResources;
import org.eclipse.jface.resource.StringConverter;
import org.eclipse.jface.util.IPropertyChangeListener;
Expand Down Expand Up @@ -87,6 +89,7 @@
import org.eclipse.ui.internal.WorkbenchPlugin;
import org.eclipse.ui.internal.misc.StatusUtil;
import org.eclipse.ui.internal.util.PrefUtil;
import org.eclipse.ui.themes.ColorUtil;
import org.eclipse.ui.themes.ITheme;
import org.eclipse.ui.themes.IThemeManager;
import org.eclipse.ui.themes.IThemePreview;
Expand Down Expand Up @@ -331,9 +334,20 @@ private class PresentationLabelProvider extends LabelProvider implements IFontPr

private int usableImageSize = -1;

private static final int REFRESH_INTERVAL_IN_MS = 300;

private final Runnable updateControlsAndRefreshTreeRunnable = () -> {
if (fontChangeButton == null || fontChangeButton.isDisposed()) {
return;
}
updateControls();
tree.getViewer().refresh();
};

private IPropertyChangeListener listener = event -> {
if (event.getNewValue() != null) {
fireLabelProviderChanged(new LabelProviderChangedEvent(PresentationLabelProvider.this));
Display.getDefault().timerExec(REFRESH_INTERVAL_IN_MS, updateControlsAndRefreshTreeRunnable);
} else {
// Some theme definition element has been modified and we
// need to refresh the viewer
Expand Down Expand Up @@ -1304,6 +1318,30 @@ private void updateThemeInfo(IThemeManager manager) {
labelProvider.hookListeners(); // rehook the listeners
}

private RGB getColorTakingPreferenceDefaultValueIntoAccount(ColorDefinition definition) {
final RGB valueFromExtension = definition.getValue();
IPreferenceStore store = getPreferenceStore();
if (store == null) {
return valueFromExtension;
}
String id = definition.getId();
if (id == null || id.isBlank()) {
return valueFromExtension;
}
String storeDefault = store.getDefaultString(id);
if (storeDefault == null) {
return valueFromExtension;
}
try {
RGB defaultRGB = ColorUtil.getColorValue(storeDefault);
if (defaultRGB != null && !defaultRGB.equals(valueFromExtension)) {
return defaultRGB;
}
} catch (DataFormatException e) { // silently ignored
}
return valueFromExtension;
}

/**
* Answers whether the definition is currently set to the default value.
*
Expand All @@ -1314,23 +1352,29 @@ private void updateThemeInfo(IThemeManager manager) {
*/
private boolean isDefault(ColorDefinition definition) {
String id = definition.getId();

if (colorPreferencesToSet.containsKey(definition)) {
if (definition.getValue() != null) { // value-based color
if (colorPreferencesToSet.get(definition).equals(definition.getValue()))
if (colorPreferencesToSet.get(definition)
.equals(getColorTakingPreferenceDefaultValueIntoAccount(definition)))
return true;
} else if (colorPreferencesToSet.get(definition).equals(getColorAncestorValue(definition)))
return true;
} else if (colorValuesToSet.containsKey(id)) {
if (definition.getValue() != null) { // value-based color
if (colorValuesToSet.get(id).equals(definition.getValue()))
if (colorValuesToSet.get(id).equals(getColorTakingPreferenceDefaultValueIntoAccount(definition)))
return true;
} else {
if (colorValuesToSet.get(id).equals(getColorAncestorValue(definition)))
return true;
}
} else if (definition.getValue() != null) { // value-based color
if (getPreferenceStore().isDefault(createPreferenceKey(definition)))
IPreferenceStore store = getPreferenceStore();
String defaultString = store.getDefaultString(id);
String string = store.getString(id);
if (defaultString != null && string != null && defaultString.equals(string)) {
return true;
}
if (store.isDefault(createPreferenceKey(definition)))
return true;
} else {
// a descendant is default if it's the same value as its ancestor
Expand Down Expand Up @@ -1516,8 +1560,9 @@ private void refreshCategory() {
private boolean resetColor(ColorDefinition definition, boolean force) {
if (force || !isDefault(definition)) {
RGB newRGB;
if (definition.getValue() != null)
newRGB = definition.getValue();
if (definition.getValue() != null) {
newRGB = getColorTakingPreferenceDefaultValueIntoAccount(definition);
}
else
newRGB = getColorAncestorValue(definition);

Expand Down
Loading