GP-1981, javadocs, refactored ThemePropertFileReader, refactored

ThemeListener, fixed java property relationships for colors

Ghidra/Features/Base/src/main/java/ghidra/base/help/GhidraHelpService.java

@@ -122,8 +122,10 @@ public class GhidraHelpService extends HelpManager {
 
 	class HelpThemeListener implements ThemeListener {
 		@Override
-		public void themeChanged(GTheme newTheme) {
-			reload();
+		public void themeChanged(ThemeEvent event) {
+			if (event.isLookAndFeelChanged()) {
+				reload();
+			}
 		}
 	}
 }

Ghidra/Framework/Docking/src/main/java/docking/theme/gui/ExportThemeDialog.java

@@ -59,15 +59,21 @@ public class ExportThemeDialog extends DialogComponentProvider {
 
 	private boolean exportTheme() {
 		String themeName = nameField.getText();
+		GTheme activeTheme = Gui.getActiveTheme();
+		LafType laf = activeTheme.getLookAndFeelType();
+		boolean useDarkDefaults = activeTheme.useDarkDefaults();
+		File file = new File(fileTextField.getText());
+
 		if (themeName.isBlank()) {
 			setStatusText("Missing Theme Name", MessageType.ERROR, true);
 			return false;
 		}
 
 		try {
-			FileGTheme exportTheme = createExternalTheme(themeName);
+			GTheme exportTheme = new GTheme(file, themeName, laf, useDarkDefaults);
 			loadValues(exportTheme);
-			exportTheme.save();
+			ThemeWriter themeWriter = new ThemeWriter(exportTheme);
+			themeWriter.writeTheme(file, exportAsZip);
 			return true;
 		}
 		catch (IOException e) {
@@ -76,7 +82,7 @@ public class ExportThemeDialog extends DialogComponentProvider {
 		return false;
 	}
 
-	private void loadValues(FileGTheme exportTheme) {
+	private void loadValues(GTheme exportTheme) {
 		if (includeDefaultsCheckbox.isSelected()) {
 			exportTheme.load(Gui.getAllValues());
 		}
@@ -85,20 +91,6 @@ public class ExportThemeDialog extends DialogComponentProvider {
 		}
 	}
 
-	private FileGTheme createExternalTheme(String themeName) {
-		File file = new File(fileTextField.getText());
-
-		GTheme activeTheme = Gui.getActiveTheme();
-		LafType laf = activeTheme.getLookAndFeelType();
-		boolean useDarkDefaults = activeTheme.useDarkDefaults();
-
-		if (exportAsZip) {
-			return new ZipGTheme(file, themeName, laf, useDarkDefaults);
-		}
-		return new FileGTheme(file, themeName, laf, useDarkDefaults);
-
-	}
-
 	@Override
 	protected void cancelCallback() {
 		close();

Ghidra/Framework/Docking/src/main/java/docking/theme/gui/ThemeDialog.java

@@ -410,30 +410,20 @@ public class ThemeDialog extends DialogComponentProvider {
 
 	class DialogThemeListener implements ThemeListener {
 		@Override
-		public void themeChanged(GTheme newTheme) {
-			reset();
-		}
-
-		@Override
-		public void themeValuesRestored() {
-			reset();
-		}
-
-		@Override
-		public void fontChanged(String id) {
-			fontTableModel.reloadCurrent();
-			updateButtons();
-		}
-
-		@Override
-		public void colorChanged(String id) {
-			colorTableModel.reloadCurrent();
-			updateButtons();
-		}
-
-		@Override
-		public void iconChanged(String id) {
-			iconTableModel.reloadCurrent();
+		public void themeChanged(ThemeEvent event) {
+			if (event.haveAllValuesChanged()) {
+				reset();
+				return;
+			}
+			if (event.hasAnyColorChanged()) {
+				colorTableModel.reloadCurrent();
+			}
+			if (event.hasAnyFontChanged()) {
+				fontTableModel.reloadCurrent();
+			}
+			if (event.hasAnyIconChanged()) {
+				iconTableModel.reloadCurrent();
+			}
 			updateButtons();
 		}
 	}

Ghidra/Framework/Docking/src/main/java/docking/theme/gui/ThemeFontTableModel.java

@@ -23,7 +23,6 @@ import java.util.function.Supplier;
 
 import javax.swing.JLabel;
 
-import docking.theme.*;
 import docking.widgets.table.*;
 import generic.theme.*;
 import ghidra.docking.settings.Settings;
@@ -172,7 +171,7 @@ public class ThemeFontTableModel extends GDynamicColumnTableModel<FontValue, Obj
 				return "<No Value>";
 			}
 			Font font = resolvedFont.font();
-			String fontString = FileGTheme.fontToString(font);
+			String fontString = ThemeWriter.fontToString(font);
 
 			if (resolvedFont.refId() != null) {
 				return resolvedFont.refId() + "  [" + fontString + "]";

Ghidra/Framework/Docking/src/main/java/docking/theme/gui/ThemeIconTableModel.java

@@ -188,7 +188,7 @@ public class ThemeIconTableModel extends GDynamicColumnTableModel<IconValue, Obj
 			Icon icon = resolvedIcon.icon();
 			String sizeString = "[" + icon.getIconWidth() + "x" + icon.getIconHeight() + "] ";
 
-			String iconString = FileGTheme.JAVA_ICON;
+			String iconString = GTheme.JAVA_ICON;
 			if (icon instanceof UrlImageIcon urlIcon) {
 				iconString = urlIcon.getOriginalPath();
 			}

Ghidra/Framework/Docking/src/main/java/docking/theme/gui/ThemeUtils.java

@@ -92,19 +92,12 @@ public class ThemeUtils {
 		}
 		GTheme startingTheme = Gui.getActiveTheme();
 		try {
-			FileGTheme imported;
-			if (themeFile.getName().endsWith(GTheme.ZIP_FILE_EXTENSION)) {
-				imported = new ZipGTheme(themeFile);
-			}
-			else if (themeFile.getName().endsWith(GTheme.FILE_EXTENSION)) {
-				imported = new FileGTheme(themeFile);
-			}
-			else {
-				Msg.showError(ThemeUtils.class, null, "Error Importing Theme",
-					"Imported File must end in either " + GTheme.FILE_EXTENSION + " or " +
-						GTheme.ZIP_FILE_EXTENSION);
-				return;
-			}
+			GTheme imported = GTheme.loadTheme(themeFile);
+			// by setting the theme, we can let the normal save handle all the edge cases
+			// such as if a theme with that names exists and if so, should it be overwritten?
+			// Also, the imported theme may contain default values which we don't want to save. So
+			// by going through the usual save mechanism, only values that differ from defaults
+			// be saved.
 			Gui.setTheme(imported);
 			if (!ThemeUtils.saveThemeChanges()) {
 				Gui.setTheme(startingTheme);
@@ -143,7 +136,7 @@ public class ThemeUtils {
 
 	public static void deleteTheme() {
 		List<GTheme> savedThemes = new ArrayList<>(
-			Gui.getAllThemes().stream().filter(t -> t instanceof FileGTheme).toList());
+			Gui.getAllThemes().stream().filter(t -> t.getFile() != null).toList());
 		if (savedThemes.isEmpty()) {
 			Msg.showInfo(ThemeUtils.class, null, "Delete Theme", "There are no deletable themes");
 			return;
@@ -159,7 +152,7 @@ public class ThemeUtils {
 				"Can't delete the current theme.");
 			return;
 		}
-		FileGTheme fileTheme = (FileGTheme) selectedTheme;
+		GTheme fileTheme = selectedTheme;
 		int result = OptionDialog.showYesNoDialog(null, "Delete Theme: " + fileTheme.getName(),
 			"Are you sure you want to delete theme " + fileTheme.getName());
 		if (result == OptionDialog.YES_OPTION) {
@@ -175,24 +168,29 @@ public class ThemeUtils {
 
 	private static boolean canSaveToName(String name) {
 		GTheme existing = Gui.getTheme(name);
+		// if no theme exists with that name, then we are save to save it
 		if (existing == null) {
 			return true;
 		}
-		if (existing instanceof FileGTheme fileTheme) {
-			int result = OptionDialog.showYesNoDialog(null, "Overwrite Existing Theme?",
-				"Do you want to overwrite the existing theme file for \"" + name + "\"?");
-			if (result == OptionDialog.YES_OPTION) {
-				return true;
-			}
+		// if the existing theme is a built-in theme, then we definitely can't save to that name 
+		if (existing instanceof DiscoverableGTheme) {
+			return false;
 		}
-		return false;
+		// if the existing theme with that name has no associated file, we can save it without
+		// worrying about overwriting an existing file.
+		if (existing.getFile() == null) {
+			return true;
+		}
+		int result = OptionDialog.showYesNoDialog(null, "Overwrite Existing Theme?",
+			"Do you want to overwrite the existing theme file for \"" + name + "\"?");
+		return result == OptionDialog.YES_OPTION;
 	}
 
 	private static boolean saveCurrentValues(String themeName) {
 		GTheme activeTheme = Gui.getActiveTheme();
 		File file = getSaveFile(themeName);
 
-		FileGTheme newTheme = new FileGTheme(file, themeName, activeTheme.getLookAndFeelType(),
+		GTheme newTheme = new GTheme(file, themeName, activeTheme.getLookAndFeelType(),
 			activeTheme.useDarkDefaults());
 		newTheme.load(Gui.getNonDefaultValues());
 		try {

Ghidra/Framework/Docking/src/test/java/docking/theme/gui/ThemeUtilsTest.java

@@ -47,9 +47,9 @@ public class ThemeUtilsTest extends AbstractDockingTest {
 
 		// get rid of any leftover imported themes from previous tests
 		Set<GTheme> allThemes = Gui.getAllThemes();
-		for (GTheme gTheme : allThemes) {
-			if (gTheme instanceof FileGTheme fileTheme) {
-				Gui.deleteTheme(fileTheme);
+		for (GTheme theme : allThemes) {
+			if (!(theme instanceof DiscoverableGTheme)) {
+				Gui.deleteTheme(theme);
 			}
 		}
 	}
@@ -140,7 +140,7 @@ public class ThemeUtilsTest extends AbstractDockingTest {
 		pressButtonByText(exportDialog, "OK");
 		waitForSwing();
 		assertTrue(exportFile.exists());
-		ZipGTheme zipTheme = new ZipGTheme(exportFile);
+		GTheme zipTheme = GTheme.loadTheme(exportFile);
 		assertEquals("Nimbus Theme", zipTheme.getName());
 	}
 
@@ -155,7 +155,7 @@ public class ThemeUtilsTest extends AbstractDockingTest {
 		pressButtonByText(exportDialog, "OK");
 		waitForSwing();
 		assertTrue(exportFile.exists());
-		FileGTheme fileTheme = new FileGTheme(exportFile);
+		GTheme fileTheme = GTheme.loadTheme(exportFile);
 		assertEquals("Nimbus Theme", fileTheme.getName());
 	}
 
@@ -190,9 +190,9 @@ public class ThemeUtilsTest extends AbstractDockingTest {
 
 	private File createZipThemeFile(String themeName) throws IOException {
 		File file = createTempFile("Test_Theme", ".theme.zip");
-		ZipGTheme zipGTheme = new ZipGTheme(file, themeName, LafType.METAL, false);
-		zipGTheme.addColor(new ColorValue("Panel.Background", Color.RED));
-		zipGTheme.save();
+		GTheme outputTheme = new GTheme(file, themeName, LafType.METAL, false);
+		outputTheme.addColor(new ColorValue("Panel.Background", Color.RED));
+		outputTheme.saveToZip(file, false);
 		return file;
 	}
 

Ghidra/Framework/Generic/src/main/java/generic/theme/AbstractThemeReader.java

@@ -0,0 +1,291 @@
+/* ###
+ * IP: GHIDRA
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * 
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package generic.theme;
+
+import java.awt.Color;
+import java.awt.Font;
+import java.io.*;
+import java.util.*;
+
+import javax.swing.Icon;
+import javax.swing.plaf.FontUIResource;
+
+import ghidra.util.Msg;
+import ghidra.util.WebColors;
+import resources.ResourceManager;
+
+/**
+ * Abstract base class for reading theme values either in sections (theme property files) or no
+ * sections (theme files)
+ */
+public abstract class AbstractThemeReader {
+
+	private static final String NO_SECTION = "[No Section]";
+	private static final String DEFAULTS = "[Defaults]";
+	private static final String DARK_DEFAULTS = "[Dark Defaults]";
+
+	private List<String> errors = new ArrayList<>();
+	protected String source;
+
+	protected AbstractThemeReader(String source) {
+		this.source = source;
+	}
+
+	/**
+	 * Returns a list of errors found while parsing
+	 * @return a list of errors found while parsing
+	 */
+	public List<String> getErrors() {
+		return errors;
+	}
+
+	protected void read(Reader reader) throws IOException {
+		List<Section> sections = readSections(new LineNumberReader(reader));
+		for (Section section : sections) {
+			switch (section.getName()) {
+				case NO_SECTION:
+					processNoSection(section);
+					break;
+				case DEFAULTS:
+					processDefaultSection(section);
+					break;
+				case DARK_DEFAULTS:
+					processDarkDefaultSection(section);
+					break;
+				default:
+					error(section.getLineNumber(),
+						"Encounded unknown theme file section: " + section.getName());
+			}
+		}
+
+	}
+
+	protected abstract void processNoSection(Section section) throws IOException;
+
+	protected abstract void processDefaultSection(Section section) throws IOException;
+
+	protected abstract void processDarkDefaultSection(Section section) throws IOException;
+
+	protected void processValues(GThemeValueMap valueMap, Section section) {
+		for (String key : section.getKeys()) {
+			String value = section.getValue(key);
+			int lineNumber = section.getLineNumber(key);
+			if (ColorValue.isColorKey(key)) {
+				valueMap.addColor(parseColorProperty(key, value, lineNumber));
+			}
+			else if (FontValue.isFontKey(key)) {
+				valueMap.addFont(parseFontProperty(key, value, lineNumber));
+			}
+			else if (IconValue.isIconKey(key)) {
+				if (!GTheme.JAVA_ICON.equals(value)) {
+					valueMap.addIcon(parseIconProperty(key, value));
+				}
+			}
+			else {
+				error(lineNumber, "Can't process property: " + key + " = " + value);
+			}
+		}
+	}
+
+	private IconValue parseIconProperty(String key, String value) {
+		if (IconValue.isIconKey(value)) {
+			return new IconValue(key, value);
+		}
+		Icon icon = ResourceManager.loadImage(value);
+		return new IconValue(key, icon);
+	}
+
+	private FontValue parseFontProperty(String key, String value, int lineNumber) {
+		if (FontValue.isFontKey(value)) {
+			return new FontValue(key, value);
+		}
+		Font font = Font.decode(value);
+		if (font == null) {
+			error(lineNumber, "Could not parse Color: " + value);
+		}
+		return font == null ? null : new FontValue(key, new FontUIResource(font));
+	}
+
+	private ColorValue parseColorProperty(String key, String value, int lineNumber) {
+		if (ColorValue.isColorKey(value)) {
+			return new ColorValue(key, value);
+		}
+		Color color = WebColors.getColor(value);
+		if (color == null) {
+			error(lineNumber, "Could not parse Color: " + value);
+		}
+		return color == null ? null : new ColorValue(key, color);
+	}
+
+	private List<Section> readSections(LineNumberReader reader) throws IOException {
+
+		List<Section> sections = new ArrayList<>();
+		Section currentSection = new Section(NO_SECTION, 0);
+		sections.add(currentSection);
+
+		String line;
+		while ((line = reader.readLine()) != null) {
+			line = removeComments(line);
+
+			if (line.isBlank()) {
+				continue;
+			}
+
+			if (isSectionHeader(line)) {
+				currentSection = new Section(line, reader.getLineNumber());
+				sections.add(currentSection);
+			}
+			else {
+				currentSection.add(line, reader.getLineNumber());
+			}
+		}
+
+		return sections;
+	}
+
+	private String removeComments(String line) {
+		// remove any trailing comment on line
+		int commentIndex = line.indexOf("//");
+		if (commentIndex >= 0) {
+			line = line.substring(0, commentIndex);
+		}
+		line = line.trim();
+
+		// clear line if entire line is comment
+		if (line.startsWith("#")) {
+			return "";
+		}
+		return line;
+	}
+
+	private boolean isSectionHeader(String line) {
+		return line.startsWith("[") && line.endsWith("]");
+	}
+
+	protected void error(int lineNumber, String message) {
+		String msg =
+			"Error parsing file \"" + source + "\" at line: " + lineNumber + ", " + message;
+		errors.add(msg);
+		Msg.out(msg);
+	}
+
+	/**
+	 * Represents all the value found in a section of the theme properties file. Sections are 
+	 * defined by a line containing just "[section name]"
+	 */
+	protected class Section {
+
+		private String name;
+		private Map<String, String> properties = new HashMap<>();
+		private Map<String, Integer> lineNumbers = new HashMap<>();
+		private int startLineNumber;
+
+		/**
+		 * Constructor sectionName the section name
+		 * @param sectionName the name of this section
+		 * @param lineNumber the line number in the file where the section started
+		 */
+		public Section(String sectionName, int lineNumber) {
+			this.name = sectionName;
+			this.startLineNumber = lineNumber;
+		}
+
+		/**
+		 * Removes the value with the given key
+		 * @param key the key to remove
+		 */
+		public void remove(String key) {
+			properties.remove(key);
+		}
+
+		/**
+		 * Returns the value for the given key.
+		 * @param key the key to get a value for
+		 * @return the value for the given key
+		 */
+		public String getValue(String key) {
+			return properties.get(key);
+		}
+
+		/**
+		 * Returns a set of all keys in the section
+		 * @return a set of all keys in the section
+		 */
+		public Set<String> getKeys() {
+			return properties.keySet();
+		}
+
+		/**
+		 * Returns the line number in the original file where the key was parsed
+		 * @param key the key to get a line number for
+		 * @return the line number in the original file where the key was parsed
+		 */
+		public int getLineNumber(String key) {
+			return lineNumbers.get(key);
+		}
+
+		/**
+		 * Returns true if the section is empty.
+		 * @return true if the section is empty.
+		 */
+		public boolean isEmpty() {
+			return properties.isEmpty();
+		}
+
+		/**
+		 * Returns the line number in the file where this section began.
+		 * @return the line number in the file where this section began.
+		 */
+		public int getLineNumber() {
+			return startLineNumber;
+		}
+
+		/**
+		 * Returns the name of this section
+		 * @return the name of this section
+		 */
+		public String getName() {
+			return name;
+		}
+
+		/**
+		 * Adds a raw line from the file to this section. The line will be parsed into a a 
+		 * key-value pair.
+		 * @param line the line to be added/parsed
+		 * @param lineNumber the line number in the file for this line
+		 */
+		public void add(String line, int lineNumber) {
+			int splitIndex = line.indexOf('=');
+			if (splitIndex < 0) {
+				error(lineNumber, "Missing required \"=\" for propery line: \"" + line + "\"");
+				return;
+			}
+			String key = line.substring(0, splitIndex).trim();
+			String value = line.substring(splitIndex + 1, line.length()).trim();
+			if (key.isBlank()) {
+				error(lineNumber, "Missing key for propery line: \"" + line + "\"");
+				return;
+			}
+			if (key.isBlank()) {
+				error(lineNumber, "Missing value for propery line: \"" + line + "\"");
+				return;
+			}
+			properties.put(key, value);
+			lineNumbers.put(key, lineNumber);
+
+		}
+	}
+}

Ghidra/Framework/Generic/src/main/java/generic/theme/AllValuesChangedThemeEvent.java

@@ -0,0 +1,76 @@
+/* ###
+ * IP: GHIDRA
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * 
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package generic.theme;
+
+import javax.swing.LookAndFeel;
+
+/**
+ * {@link ThemeEvent} for when a new theme is set or the current theme is reset to its original 
+ * values.
+ */
+public class AllValuesChangedThemeEvent extends ThemeEvent {
+
+	private boolean lookAndFeelChanged;
+
+	/**
+	 * Constructor
+	 * @param lookAndFeelChanged true if the overall theme was changed which may have caused the
+	 * {@link LookAndFeel} to change
+	 */
+	public AllValuesChangedThemeEvent(boolean lookAndFeelChanged) {
+		this.lookAndFeelChanged = lookAndFeelChanged;
+	}
+
+	@Override
+	public boolean isColorChanged(String id) {
+		return true;
+	}
+
+	@Override
+	public boolean isFontChanged(String id) {
+		return true;
+	}
+
+	@Override
+	public boolean isIconChanged(String id) {
+		return true;
+	}
+
+	@Override
+	public boolean isLookAndFeelChanged() {
+		return lookAndFeelChanged;
+	}
+
+	@Override
+	public boolean haveAllValuesChanged() {
+		return true;
+	}
+
+	@Override
+	public boolean hasAnyColorChanged() {
+		return true;
+	}
+
+	@Override
+	public boolean hasAnyFontChanged() {
+		return true;
+	}
+
+	@Override
+	public boolean hasAnyIconChanged() {
+		return true;
+	}
+}

Ghidra/Framework/Generic/src/main/java/generic/theme/ColorChangedThemeEvent.java

@@ -0,0 +1,41 @@
+/* ###
+ * IP: GHIDRA
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * 
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package generic.theme;
+
+/**
+ * {@link ThemeEvent} for when a color changes for exactly one color id.
+ */
+public class ColorChangedThemeEvent extends ThemeEvent {
+	private final ColorValue color;
+
+	/**
+	 * Constructor
+	 * @param color the new {@link ColorValue} for the color id that changed
+	 */
+	public ColorChangedThemeEvent(ColorValue color) {
+		this.color = color;
+	}
+
+	@Override
+	public boolean isColorChanged(String id) {
+		return id.equals(color.getId());
+	}
+
+	@Override
+	public boolean hasAnyColorChanged() {
+		return true;
+	}
+}

Ghidra/Framework/Generic/src/main/java/generic/theme/ColorValue.java

@@ -20,16 +20,38 @@ import java.awt.Color;
 import ghidra.util.Msg;
 import utilities.util.reflection.ReflectionUtilities;
 
+/**
+ * A class for storing {@link Color} values that have a String id (e.g. color.bg.foo) and either
+ * a concrete color or a reference id which is the String id of another ColorValue that it
+ * will inherit its color from. So if this class's color value is non-null, the refId will be null
+ * and if the class's refId is non-null, then the color value will be null.
+ */
 public class ColorValue extends ThemeValue<Color> {
 	static final String COLOR_ID_PREFIX = "color.";
 	static final String EXTERNAL_PREFIX = "[color]";
 
 	public static final Color LAST_RESORT_DEFAULT = Color.GRAY;
 
+	/**
+	 * Constructor used when the ColorValue will have a direct {@link Color} value. The refId will
+	 * be null. Note: if a {@link GColor} is passed in as the value, then this will be an indirect
+	 * ColorValue that inherits its color from the id stored in the GColor.
+	 * @param id the id for this ColorValue
+	 * @param value the {@link Color} to associate with the given id
+	 */
 	public ColorValue(String id, Color value) {
 		super(id, getRefId(value), getRawColor(value));
+		if (value instanceof GColor) {
+			throw new IllegalArgumentException("Can't use GColor as the value!");
+		}
 	}
 
+	/**
+	 * Constructor used when the ColorValue will inherit its color from another ColorValue. The
+	 * color value field will be null.
+	 * @param id the id for this ColorValue
+	 * @param refId the id of another ColorValue that this ColorValue will inherit from
+	 */
 	public ColorValue(String id, String refId) {
 		super(id, refId, null);
 	}
@@ -54,11 +76,6 @@ public class ColorValue extends ThemeValue<Color> {
 		return LAST_RESORT_DEFAULT;
 	}
 
-	@Override
-	protected String getIdPrefix() {
-		return COLOR_ID_PREFIX;
-	}
-
 	@Override
 	public String toExternalId(String internalId) {
 		if (internalId.startsWith(COLOR_ID_PREFIX)) {
@@ -75,28 +92,15 @@ public class ColorValue extends ThemeValue<Color> {
 		return externalId;
 	}
 
+	/** 
+	 * Returns true if the given key string is a valid external key for a color value
+	 * @param key the key string to test
+	 * @return true if the given key string is a valid external key for a color value
+	 */
 	public static boolean isColorKey(String key) {
 		return key.startsWith(COLOR_ID_PREFIX) || key.startsWith(EXTERNAL_PREFIX);
 	}
 
-	@Override
-	protected int compareValues(Color v1, Color v2) {
-		int alpha1 = v1.getAlpha();
-		int alpha2 = v2.getAlpha();
-
-		if (alpha1 == alpha2) {
-			return getHsbCompareValue(v1) - getHsbCompareValue(v2);
-		}
-		return alpha1 - alpha2;
-	}
-
-	private int getHsbCompareValue(Color v) {
-		// compute a value the compares colors first by hue, then saturation, then brightness
-		// reduce noise by converting float values from 0-1 to integers 0 - 7
-		float[] hsb = Color.RGBtoHSB(v.getRed(), v.getGreen(), v.getBlue(), null);
-		return 100 * (int) (10 * hsb[0]) + 10 * (int) (10 * hsb[1]) + (int) (10 * hsb[2]);
-	}
-
 	private static Color getRawColor(Color value) {
 		if (value instanceof GColor) {
 			return null;

Ghidra/Framework/Generic/src/main/java/generic/theme/DiscoverableGTheme.java

@@ -17,6 +17,9 @@ package generic.theme;
 
 import ghidra.util.classfinder.ExtensionPoint;
 
+/**
+ * Abstract base class for built-in {@link GTheme}s.
+ */
 public abstract class DiscoverableGTheme extends GTheme implements ExtensionPoint {
 	static final String CLASS_PREFIX = "Class:";
 

Ghidra/Framework/Generic/src/main/java/generic/theme/ExternalThemeReader.java

@@ -1,63 +0,0 @@
-/* ###
- * IP: GHIDRA
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- * 
- *      http://www.apache.org/licenses/LICENSE-2.0
- * 
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package generic.theme;
-
-import java.io.*;
-import java.util.Enumeration;
-import java.util.zip.ZipEntry;
-import java.util.zip.ZipFile;
-
-import org.apache.commons.io.FileUtils;
-
-import ghidra.framework.Application;
-import ghidra.util.Msg;
-
-public class ExternalThemeReader extends ThemeReader {
-
-	public ExternalThemeReader(File file) throws IOException {
-		try (ZipFile zipFile = new ZipFile(file)) {
-			Enumeration<? extends ZipEntry> entries = zipFile.entries();
-			while (entries.hasMoreElements()) {
-				ZipEntry entry = entries.nextElement();
-				String name = entry.getName();
-				try (InputStream is = zipFile.getInputStream(entry)) {
-					if (name.endsWith(".theme")) {
-						processThemeData(name, is);
-					}
-					else {
-						processIconFile(name, is);
-					}
-				}
-			}
-		}
-	}
-
-	private void processIconFile(String path, InputStream is) throws IOException {
-		int indexOf = path.indexOf("images/");
-		if (indexOf < 0) {
-			Msg.error(this, "Unknown file: " + path);
-		}
-		String relativePath = path.substring(indexOf, path.length());
-		File dir = Application.getUserSettingsDirectory();
-		File iconFile = new File(dir, relativePath);
-		FileUtils.copyInputStreamToFile(is, iconFile);
-	}
-
-	private void processThemeData(String name, InputStream is) throws IOException {
-		InputStreamReader reader = new InputStreamReader(is);
-		read(reader);
-	}
-}

Ghidra/Framework/Generic/src/main/java/generic/theme/FontChangedThemeEvent.java

@@ -15,6 +15,27 @@
  */
 package generic.theme;
 
-public interface Refreshable {
-	public void refresh();
+/**
+ * {@link ThemeEvent} for when a font changes for exactly one font id.
+ */
+public class FontChangedThemeEvent extends ThemeEvent {
+	private final FontValue font;
+
+	/**
+	 * Constructor
+	 * @param font the new {@link FontValue} for the font id that changed
+	 */
+	public FontChangedThemeEvent(FontValue font) {
+		this.font = font;
+	}
+
+	@Override
+	public boolean isFontChanged(String id) {
+		return id.equals(font.getId());
+	}
+
+	@Override
+	public boolean hasAnyFontChanged() {
+		return true;
+	}
 }

Ghidra/Framework/Generic/src/main/java/generic/theme/FontValue.java

@@ -19,15 +19,33 @@ import java.awt.Font;
 
 import ghidra.util.Msg;
 
+/**
+ * A class for storing {@link Font} values that have a String id (e.g. font.foo.bar) and either
+ * a concrete font or a reference id which is the String id of another FontValue that it
+ * will inherit its font from. So if this class's font value is non-null, the refId will be null
+ * and if the class's refId is non-null, then the font value will be null.
+ */
 public class FontValue extends ThemeValue<Font> {
 	static final String FONT_ID_PREFIX = "font.";
 	public static final Font LAST_RESORT_DEFAULT = new Font("monospaced", Font.PLAIN, 12);
 	private static final String EXTERNAL_PREFIX = "[font]";
 
+	/**
+	 * Constructor used when the FontValue will have a direct {@link Font} value. The refId
+	 * will be null.
+	 * @param id the id for this FontValue
+	 * @param value the {@link Font} to associate with the given id
+	 */
 	public FontValue(String id, Font value) {
 		super(id, null, value);
 	}
 
+	/**
+	 * Constructor used when the FontValue will inherit its {@link Font} from another FontValue. The
+	 * font value field will be null.
+	 * @param id the id for this FontValue
+	 * @param refId the id of another FontValue that this FontValue will inherit from
+	 */
 	public FontValue(String id, String refId) {
 		super(id, refId, null);
 	}
@@ -43,11 +61,6 @@ public class FontValue extends ThemeValue<Font> {
 		return LAST_RESORT_DEFAULT;
 	}
 
-	@Override
-	protected String getIdPrefix() {
-		return FONT_ID_PREFIX;
-	}
-
 	@Override
 	public String toExternalId(String internalId) {
 		if (internalId.startsWith(FONT_ID_PREFIX)) {
@@ -64,12 +77,13 @@ public class FontValue extends ThemeValue<Font> {
 		return externalId;
 	}
 
+	/** 
+	 * Returns true if the given key string is a valid external key for a font value
+	 * @param key the key string to test
+	 * @return true if the given key string is a valid external key for a font value
+	 */
 	public static boolean isFontKey(String key) {
 		return key.startsWith(FONT_ID_PREFIX) || key.startsWith(EXTERNAL_PREFIX);
 	}
 
-	@Override
-	protected int compareValues(Font v1, Font v2) {
-		return v1.toString().compareTo(v2.toString());
-	}
 }

Ghidra/Framework/Generic/src/main/java/generic/theme/GColor.java

@@ -25,22 +25,41 @@ import java.util.Objects;
 import ghidra.util.WebColors;
 import ghidra.util.datastruct.WeakStore;
 
-public class GColor extends Color implements Refreshable {
+/**
+ * A {@link Color} whose value is dynamically determined by looking up its id into a global
+ * color table that is determined by the active {@link GTheme}. 
+ * <P>The idea is for developers to
+ * not use specific colors in their code, but to instead use a GColor with an id that hints at 
+ * its use. For example, instead of harding code a component's background color to white by coding
+ * "component.setBackground(Color.white)", you would do something like 
+ * "component.setBackground(new GColor("color.mywidget.bg"). Then in a 
+ * "[module name].theme.properties" file (located in the module's data directory), you would 
+ * set the default value by adding this line "color.mywidget.bg = white".
+ */
+public class GColor extends Color {
+	// keeps a weak reference to all uses of GColor, so their cached color value can be refreshed 
 	private static WeakStore<GColor> inUseColors = new WeakStore<>();
+
 	private String id;
 	private Color delegate;
 	private Short alpha;
 
-	public static void refreshAll() {
-		for (GColor gcolor : inUseColors.getValues()) {
-			gcolor.refresh();
-		}
-	}
-
+	/**
+	 * Construct a GColor with an id that will be used to look up the current color associated with 
+	 * that id, which can be changed at runtime.
+	 * @param id the id used to lookup the current value for this color
+	 */
 	public GColor(String id) {
 		this(id, true);
 	}
 
+	/**
+	 * Construct a GColor with an id that will be used to look up the current color associated with
+	 * that id, which can be changed at runtime.
+	 * @param id the id used to lookup the current value for this color
+	 * @param validate if true, an error will be generated if the id can't be resolved to a color
+	 * at this time
+	 */
 	public GColor(String id, boolean validate) {
 		super(0x808080);
 		this.id = id;
@@ -55,18 +74,24 @@ public class GColor extends Color implements Refreshable {
 		delegate = new Color(delegate.getRed(), delegate.getGreen(), delegate.getBlue(), alpha);
 	}
 
+	/**
+	 * Creates a transparent version of this GColor. If the underlying value of this GColor changes,
+	 * the transparent version will also change.
+	 * @param newAlpha the transparency level for the new color
+	 * @return a tranparent version of this GColor
+	 */
 	public GColor withAlpha(int newAlpha) {
 		return new GColor(id, newAlpha);
 	}
 
+	/**
+	 * Returns the id for this GColor.
+	 * @return the id for this GColor.
+	 */
 	public String getId() {
 		return id;
 	}
 
-	public boolean isEquivalent(Color color) {
-		return delegate.getRGB() == color.getRGB();
-	}
-
 	@Override
 	public int getRed() {
 		return delegate.getRed();
@@ -185,7 +210,9 @@ public class GColor extends Color implements Refreshable {
 		return delegate.getTransparency();
 	}
 
-	@Override
+	/**
+	 * Reloads the delegate.
+	 */
 	public void refresh() {
 		Color color = Gui.getRawColor(id, false);
 		if (color != null) {
@@ -197,4 +224,15 @@ public class GColor extends Color implements Refreshable {
 			}
 		}
 	}
+
+	/**
+	 * Static method for notifying all the existing GColors that colors have changed and they
+	 * should reload their cached indirect color. 
+	 */
+	public static void refreshAll() {
+		for (GColor gcolor : inUseColors.getValues()) {
+			gcolor.refresh();
+		}
+	}
+
 }

Ghidra/Framework/Generic/src/main/java/generic/theme/GColorUIResource.java

@@ -15,8 +15,15 @@
  */
 package generic.theme;
 
+import javax.swing.UIDefaults;
 import javax.swing.plaf.UIResource;
 
+/**
+ * Version of GColor that implements UIResource. It is important that when setting java defaults
+ * in the {@link UIDefaults} that it implements UIResource. Otherwise, java will think the color
+ * was set explicitly by client code and therefore can't update it generically when it goes to 
+ * update the default color in the UIs for each component.
+ */
 public class GColorUIResource extends GColor implements UIResource {
 
 	public GColorUIResource(String id) {

Ghidra/Framework/Generic/src/main/java/generic/theme/GIcon.java

@@ -20,31 +20,61 @@ import java.awt.Graphics;
 
 import javax.swing.Icon;
 
-import generic.theme.Refreshable;
 import ghidra.util.datastruct.WeakStore;
 
-public class GIcon implements Icon, Refreshable {
+/**
+ * An {@link Icon} whose value is dynamically determined by looking up its id into a global
+ * icon table that is determined by the active {@link GTheme}.
+ * <P> The idea is for developers to
+ * not use specific icons in their code, but to instead use a GIcon with an id that hints at 
+ * its use. For example, instead of harding code a label's icon by coding 
+ * "lable.setIcon(ResourceManager.loadImage("images/refresh.png", you would do something like 
+ * label.setIcon(new GIcon("icon.refresh"). Then in a "[module name].theme.properties" file 
+ * (located in the module's data directory), you would set the default value by adding this
+ * line "icon.refresh = images/refresh.png".
+ */
+public class GIcon implements Icon {
 	private static WeakStore<GIcon> inUseIcons = new WeakStore<>();
 
 	private String id;
 	private Icon delegate;
 
+	/**
+	 * Static method for notifying all the existing GIcon that icons have changed and they
+	 * should reload their cached indirect icon. 
+	 */
 	public static void refreshAll() {
 		for (GIcon gIcon : inUseIcons.getValues()) {
 			gIcon.refresh();
 		}
 	}
 
+	/**
+	 * Construct a GIcon with an id that will be used to look up the current icon associated with
+	 * that id, which can be changed at runtime.
+	 * @param id the id used to lookup the current value for this color
+	 */
 	public GIcon(String id) {
 		this(id, true);
 	}
 
+	/**
+	 * Construct a GIcon with an id that will be used to look up the current icon associated with
+	 * that id, which can be changed at runtime.
+	 * @param id the id used to lookup the current value for this icon
+	 * @param validate if true, an error will be generated if the id can't be resolved to a icon
+	 * at this time
+	 */
 	public GIcon(String id, boolean validate) {
 		this.id = id;
 		delegate = Gui.getRawIcon(id, validate);
 		inUseIcons.add(this);
 	}
 
+	/**
+	 * Returns the id for this GIcon.
+	 * @return the id for this GIcon.
+	 */
 	public String getId() {
 		return id;
 	}
@@ -64,7 +94,9 @@ public class GIcon implements Icon, Refreshable {
 		return delegate.getIconHeight();
 	}
 
-	@Override
+	/**
+	 * Reloads the delegate.
+	 */
 	public void refresh() {
 		Icon icon = Gui.getRawIcon(id, false);
 		if (icon != null) {

Ghidra/Framework/Generic/src/main/java/generic/theme/GIconUIResource.java

@@ -15,8 +15,15 @@
  */
 package generic.theme;
 
+import javax.swing.UIDefaults;
 import javax.swing.plaf.UIResource;
 
+/**
+ * Version of GIcon that implements UIResource. It is important that when setting java defaults
+ * in the {@link UIDefaults} that it implements UIResource. Otherwise, java will think the icon
+ * was set explicitly by client code and therefore can't update it generically when it goes to 
+ * update the default icon in the UIs for each component.
+ */
 public class GIconUIResource extends GIcon implements UIResource {
 
 	public GIconUIResource(String id) {

Ghidra/Framework/Generic/src/main/java/generic/theme/GTheme.java

@@ -22,38 +22,58 @@ import java.io.IOException;
 import java.util.Objects;
 
 import javax.swing.Icon;
+import javax.swing.LookAndFeel;
 
 /**
  * Class to store all the configurable appearance properties (Colors, Fonts, Icons, Look and Feel)
  * in an application.
  */
 public class GTheme extends GThemeValueMap {
+	public static final String FILE_PREFIX = "File:";
+	public static final String JAVA_ICON = "<JAVA ICON>";
+
 	public static String FILE_EXTENSION = "theme";
 	public static String ZIP_FILE_EXTENSION = "theme.zip";
 
-	static final String THEME_NAME_KEY = "name";
-	static final String THEME_LOOK_AND_FEEL_KEY = "lookAndFeel";
-	static final String THEME_USE_DARK_DEFAULTS = "useDarkDefaults";
-
 	private final String name;
 	private final LafType lookAndFeel;
 	private final boolean useDarkDefaults;
+	private final File file;
 
+	/**
+	 * Creates an new GTheme with the given name, the default {@link LookAndFeel} for the the 
+	 * platform and not using dark defaults. This theme will be using all the standard defaults
+	 * from the theme.property files and the defaults from the default LookAndFeel.
+	 * @param name the name for this GTheme
+	 */
 	public GTheme(String name) {
 		this(name, LafType.getDefaultLookAndFeel(), false);
 
 	}
 
 	/**
-	 * Creates a new empty GTheme with the given name
+	 * Creates a new empty GTheme with the given name, {@link LookAndFeel}, and whether or not to
+	 * use dark defaults.
 	 * @param name the name for the new GTheme
 	 * @param lookAndFeel the look and feel type used by this theme
 	 * @param useDarkDefaults determines whether or  
 	 */
 	public GTheme(String name, LafType lookAndFeel, boolean useDarkDefaults) {
+		this(null, name, lookAndFeel, useDarkDefaults);
+	}
+
+	/**
+	 * Constructor for creating a GTheme with an associated File. 
+	 * @param file the file that this theme will save to
+	 * @param name the name of the new theme
+	 * @param lookAndFeel the {@link LafType} for the new theme
+	 * @param useDarkDefaults true if this new theme uses dark defaults
+	 */
+	public GTheme(File file, String name, LafType lookAndFeel, boolean useDarkDefaults) {
 		this.name = name;
 		this.lookAndFeel = lookAndFeel;
 		this.useDarkDefaults = useDarkDefaults;
+		this.file = file;
 	}
 
 	/**
@@ -85,9 +105,20 @@ public class GTheme extends GThemeValueMap {
 	 * @return  a String that can be used to find and restore this theme.
 	 */
 	public String getThemeLocater() {
+		if (file != null) {
+			return FILE_PREFIX + file.getAbsolutePath();
+		}
 		return "Default";
 	}
 
+	/**
+	 * Returns the file associated with this theme.
+	 * @return the file associated with this theme.
+	 */
+	public File getFile() {
+		return file;
+	}
+
 	/**
 	 * Sets the Color for the given id
 	 * @param id the id to associate with the given Color
@@ -167,12 +198,36 @@ public class GTheme extends GThemeValueMap {
 		return Objects.equals(name, other.name) && Objects.equals(lookAndFeel, other.lookAndFeel);
 	}
 
+	/**
+	 * Returns true if this theme has a {@link LookAndFeel} that is supported by the current
+	 * platform.
+	 * @return true if this theme has a {@link LookAndFeel} that is supported by the current
+	 * platform.
+	 */
 	public boolean hasSupportedLookAndFeel() {
 		return lookAndFeel.isSupported();
 	}
 
-	public FileGTheme saveToFile(File file, boolean includeDefaults) throws IOException {
-		FileGTheme fileTheme = new FileGTheme(file, name, lookAndFeel, useDarkDefaults);
+	/**
+	 * Saves this theme to its associated file.
+	 * @throws IOException if an I/O error occurs when writing the file
+	 */
+	public void save() throws IOException {
+		ThemeWriter writer = new ThemeWriter(this);
+		writer.writeThemeToFile(file);
+	}
+
+	/**
+	 * Saves this theme to a new theme file. 
+	 * @param outputFile the file to save to
+	 * @param includeDefaults if true, write all values to the theme file including default values.
+	 * Otherwise, just values that are not the default values are written to the file.
+	 * @return a new FileGTheme that represents the new file/theme
+	 * @throws IOException if an I/O error occurs writing the theme file
+	 */
+	public GTheme saveToFile(File outputFile, boolean includeDefaults) throws IOException {
+
+		GTheme fileTheme = new GTheme(outputFile, name, lookAndFeel, useDarkDefaults);
 		if (includeDefaults) {
 			fileTheme.load(Gui.getDefaults());
 		}
@@ -181,4 +236,35 @@ public class GTheme extends GThemeValueMap {
 		return fileTheme;
 	}
 
+	/**
+	 * Saves this theme to a new theme file. 
+	 * @param outputFile the file to save to
+	 * @param includeDefaults if true, write all values to the theme file including default values.
+	 * Otherwise, just values that are not the default values are written to the file.
+	 * @return a new FileGTheme that represents the new file/theme
+	 * @throws IOException if an I/O error occurs writing the theme file
+	 */
+	public void saveToZip(File outputFile, boolean includeDefaults) throws IOException {
+
+		GTheme theme = new GTheme(name, lookAndFeel, useDarkDefaults);
+		if (includeDefaults) {
+			theme.load(Gui.getDefaults());
+		}
+		theme.load(this);
+		ThemeWriter writer = new ThemeWriter(theme);
+		writer.writeThemeToZipFile(outputFile);
+	}
+
+	/**
+	 * Reads a theme from a file. The file can be either a theme file or a zip file containing
+	 * a theme file and optionally a set of icon files.
+	 * @param file the file to read.
+	 * @return the theme that was read from the file
+	 * @throws IOException if an error occcured trying to read a theme from the file.
+	 */
+	public static GTheme loadTheme(File file) throws IOException {
+		ThemeReader reader = new ThemeReader(file);
+		return reader.readTheme();
+	}
+
 }

Ghidra/Framework/Generic/src/main/java/generic/theme/GThemeValueMap.java

@@ -24,48 +24,94 @@ import javax.swing.Icon;
 import resources.ResourceManager;
 import resources.icons.UrlImageIcon;
 
+/**
+ * Class for storing colors, fonts, and icons by id
+ */
 public class GThemeValueMap {
 	protected Map<String, ColorValue> colorMap = new HashMap<>();
 	protected Map<String, FontValue> fontMap = new HashMap<>();
 	protected Map<String, IconValue> iconMap = new HashMap<>();
 
+	/**
+	 * Constructs a new empty map.
+	 */
 	public GThemeValueMap() {
 	}
 
+	/**
+	 * Constructs a new value map, populated by all the values in the given map. Essentailly clones
+	 * the given map.
+	 * @param initial the set of values to initialize to
+	 */
 	public GThemeValueMap(GThemeValueMap initial) {
 		load(initial);
 	}
 
+	/**
+	 * Adds the {@link ColorValue} to the map. If a ColorValue already exists in the map with
+	 * the same id, it will be replaced
+	 * @param value the {@link ColorValue} to store in the map.
+	 */
 	public void addColor(ColorValue value) {
 		if (value != null) {
 			colorMap.put(value.getId(), value);
 		}
 	}
 
+	/**
+	 * Adds the {@link FontValue} to the map. If a FontValue already exists in the map with
+	 * the same id, it will be replaced
+	 * @param value the {@link FontValue} to store in the map.
+	 */
 	public void addFont(FontValue value) {
 		if (value != null) {
 			fontMap.put(value.getId(), value);
 		}
 	}
 
+	/**
+	 * Adds the {@link IconValue} to the map. If a IconValue already exists in the map with
+	 * the same id, it will be replaced
+	 * @param value the {@link IconValue} to store in the map.
+	 */
 	public void addIcon(IconValue value) {
 		if (value != null) {
 			iconMap.put(value.getId(), value);
 		}
 	}
 
+	/**
+	 * Returns the current {@link ColorValue} for the given id or null if none exists.
+	 * @param id the id to look up a color for
+	 * @return the current {@link ColorValue} for the given id or null if none exists.
+	 */
 	public ColorValue getColor(String id) {
 		return colorMap.get(id);
 	}
 
+	/**
+	 * Returns the current {@link FontValue} for the given id or null if none exists.
+	 * @param id the id to look up a font for
+	 * @return the current {@link FontValue} for the given id or null if none exists.
+	 */
 	public FontValue getFont(String id) {
 		return fontMap.get(id);
 	}
 
+	/**
+	 * Returns the current {@link IconValue} for the given id or null if none exists.
+	 * @param id the id to look up a icon for
+	 * @return the current {@link IconValue} for the given id or null if none exists.
+	 */
 	public IconValue getIcon(String id) {
 		return iconMap.get(id);
 	}
 
+	/**
+	 * Loads all the values from the given map into this map, replacing values with the 
+	 * same ids.
+	 * @param valueMap the map whose values are to be loaded into this map
+	 */
 	public void load(GThemeValueMap valueMap) {
 		valueMap.colorMap.values().forEach(v -> addColor(v));
 		valueMap.fontMap.values().forEach(v -> addFont(v));
@@ -73,48 +119,114 @@ public class GThemeValueMap {
 
 	}
 
+	/**
+	 * Returns a list of all the {@link ColorValue}s stored in this map.
+	 * @return a list of all the {@link ColorValue}s stored in this map.
+	 */
 	public List<ColorValue> getColors() {
 		return new ArrayList<>(colorMap.values());
 	}
 
+	/**
+	 * Returns a list of all the {@link FontValue}s stored in this map.
+	 * @return a list of all the {@link Fontvalue}s stored in this map.
+	 */
 	public List<FontValue> getFonts() {
 		return new ArrayList<>(fontMap.values());
 	}
 
+	/**
+	 * Returns a list of all the {@link IconValue}s stored in this map.
+	 * @return a list of all the {@link IconValue}s stored in this map.
+	 */
 	public List<IconValue> getIcons() {
 		return new ArrayList<>(iconMap.values());
 	}
 
+	/**
+	 * Returns true if a {@link ColorValue} exists in this map for the given id.
+	 * @param id the id to check
+	 * @return true if a {@link ColorValue} exists in this map for the given id
+	 */
 	public boolean containsColor(String id) {
 		return colorMap.containsKey(id);
 	}
 
+	/**
+	 * Returns true if a {@link FontValue} exists in this map for the given id.
+	 * @param id the id to check
+	 * @return true if a {@link FontValue} exists in this map for the given id
+	 */
 	public boolean containsFont(String id) {
 		return fontMap.containsKey(id);
 	}
 
+	/**
+	 * Returns true if an {@link IconValue} exists in this map for the given id.
+	 * @param id the id to check
+	 * @return true if an {@link IconValue} exists in this map for the given id
+	 */
 	public boolean containsIcon(String id) {
 		return iconMap.containsKey(id);
 	}
 
+	/**
+	 * Returns the total number of color, font, and icon values stored in this map
+	 * @return the total number of color, font, and icon values stored in this map
+	 */
 	public Object size() {
 		return colorMap.size() + fontMap.size() + iconMap.size();
 	}
 
+	/**
+	 * Clears all color, font, and icon values from this map
+	 */
 	public void clear() {
 		colorMap.clear();
 		fontMap.clear();
 		iconMap.clear();
 	}
 
+	/**
+	 * Returns true if there are not color, font, or icon values in this map
+	 * @return true if there are not color, font, or icon values in this map
+	 */
 	public boolean isEmpty() {
 		return colorMap.isEmpty() && fontMap.isEmpty() && iconMap.isEmpty();
 	}
 
+	/**
+	 * removes any {@link ColorValue} with the given id from this map.
+	 * @param id the id to remove
+	 */
 	public void removeColor(String id) {
 		colorMap.remove(id);
 	}
 
+	/**
+	 * removes any {@link FontValue} with the given id from this map.
+	 * @param id the id to remove
+	 */
+	public void removeFont(String id) {
+		fontMap.remove(id);
+	}
+
+	/**
+	 * removes any {@link IconValue} with the given id from this map.
+	 * @param id the id to remove
+	 */
+	public void removeIcon(String id) {
+		iconMap.remove(id);
+	}
+
+	/**
+	 * Returns a new {@link GThemeValueMap} that is only populated by values that don't exist
+	 * in the give map.
+	 * @param base the set of values (usually the default set) to compare against to determine 
+	 * what values are changed.
+	 * @return a new {@link GThemeValueMap} that is only populated by values that don't exist
+	 * in the give map
+	 */
 	public GThemeValueMap getChangedValues(GThemeValueMap base) {
 		GThemeValueMap map = new GThemeValueMap();
 		for (ColorValue color : colorMap.values()) {
@@ -135,14 +247,13 @@ public class GThemeValueMap {
 		return map;
 	}
 
-	public void removeFont(String id) {
-		fontMap.remove(id);
-	}
-
-	public void removeIcon(String id) {
-		iconMap.remove(id);
-	}
-
+	/**
+	 * Gets the set of icon (.png, .gif) files that are used by IconValues that came from files
+	 * versus resources in the classpath. These are the icon files that need to be included
+	 * when exporting this set of values to a zip file.
+	 * @return the set of icon (.png, .gif) files that are used by IconValues that came from files
+	 * versus resources in the classpath
+	 */
 	public Set<File> getExternalIconFiles() {
 		Set<File> files = new HashSet<>();
 		for (IconValue iconValue : iconMap.values()) {

Ghidra/Framework/Generic/src/main/java/generic/theme/IconChangedThemeEvent.java

@@ -0,0 +1,41 @@
+/* ###
+ * IP: GHIDRA
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * 
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package generic.theme;
+
+/**
+ * {@link ThemeEvent} for when an icon changes for exactly one icon id.
+ */
+public class IconChangedThemeEvent extends ThemeEvent {
+	private final IconValue icon;
+
+	/**
+	 * Constructor
+	 * @param icon the new {@link IconValue} for the icon id that changed
+	 */
+	public IconChangedThemeEvent(IconValue icon) {
+		this.icon = icon;
+	}
+
+	@Override
+	public boolean isIconChanged(String id) {
+		return id.equals(icon.getId());
+	}
+
+	@Override
+	public boolean hasAnyIconChanged() {
+		return true;
+	}
+}

Ghidra/Framework/Generic/src/main/java/generic/theme/IconValue.java

@@ -20,6 +20,12 @@ import javax.swing.Icon;
 import ghidra.util.Msg;
 import resources.ResourceManager;
 
+/**
+ * A class for storing {@link Icon} values that have a String id (e.g. icon.bg.foo) and either
+ * a concrete icon or a reference id which is the String id of another IconValue that it
+ * will inherit its icon from. So if this class's icon value is non-null, the refId will be null
+ * and if the class's refId is non-null, then the icon value will be null.
+ */
 public class IconValue extends ThemeValue<Icon> {
 	static final String ICON_ID_PREFIX = "icon.";
 
@@ -27,10 +33,27 @@ public class IconValue extends ThemeValue<Icon> {
 
 	private static final String EXTERNAL_PREFIX = "[icon]";
 
+	/**
+	 * Constructor used when the ColorValue will have a direct {@link Icon} value. The refId will
+	 * be null. Note: if a {@link GIcon} is passed in as the value, then this will be an indirect
+	 * IconValue that inherits its icon from the id stored in the GIcon.
+	 * @param id the id for this IconValue
+	 * @param icon the {@link Icon} to associate with the given id
+	 */
 	public IconValue(String id, Icon icon) {
-		super(id, null, icon);
+		super(id, getRefId(icon), getRawIcon(icon));
+		if (icon instanceof GIcon) {
+			throw new IllegalArgumentException("Can't use GIcon as the value!");
+		}
+
 	}
 
+	/**
+	 * Constructor used when the IconValue will inherit its {@link Icon} from another IconValue. The
+	 * icon value field will be null.
+	 * @param id the id for this IconValue
+	 * @param refId the id of another IconValue that this IconValue will inherit from
+	 */
 	public IconValue(String id, String refId) {
 		super(id, refId, null);
 	}
@@ -47,11 +70,6 @@ public class IconValue extends ThemeValue<Icon> {
 		return ResourceManager.getDefaultIcon();
 	}
 
-	@Override
-	protected String getIdPrefix() {
-		return ICON_ID_PREFIX;
-	}
-
 	@Override
 	public String toExternalId(String internalId) {
 		if (internalId.startsWith(ICON_ID_PREFIX)) {
@@ -68,12 +86,26 @@ public class IconValue extends ThemeValue<Icon> {
 		return externalId;
 	}
 
+	/** 
+	 * Returns true if the given key string is a valid external key for an icon value
+	 * @param key the key string to test
+	 * @return true if the given key string is a valid external key for an icon value
+	 */
 	public static boolean isIconKey(String key) {
 		return key.startsWith(ICON_ID_PREFIX) || key.startsWith(EXTERNAL_PREFIX);
 	}
 
-	@Override
-	protected int compareValues(Icon v1, Icon v2) {
-		return v1.toString().compareTo(v2.toString());
+	private static Icon getRawIcon(Icon value) {
+		if (value instanceof GIcon) {
+			return null;
+		}
+		return value;
+	}
+
+	private static String getRefId(Icon value) {
+		if (value instanceof GIcon) {
+			return ((GIcon) value).getId();
+		}
+		return null;
 	}
 }

Ghidra/Framework/Generic/src/main/java/generic/theme/LafType.java

@@ -15,6 +15,7 @@
  */
 package generic.theme;
 
+import javax.swing.LookAndFeel;
 import javax.swing.UIManager;
 import javax.swing.UIManager.LookAndFeelInfo;
 
@@ -23,6 +24,9 @@ import ghidra.framework.OperatingSystem;
 import ghidra.framework.Platform;
 import ghidra.util.exception.AssertException;
 
+/**
+ * An enumeration that represents the set of supported {@link LookAndFeel}s
+ */
 public enum LafType {
 	METAL("Metal"),
 	NIMBUS("Nimbus"),
@@ -41,10 +45,19 @@ public enum LafType {
 		this.name = name;
 	}
 
+	/**
+	 * Returns the name of this LafType.
+	 * @return the name of this LafType.
+	 */
 	public String getName() {
 		return name;
 	}
 
+	/**
+	 * Returns the LafType for the given name or null if the given name does not match any types
+	 * @param name the name to search a LafType for.
+	 * @return the LafType for the given name or null if the given name does not match any types
+	 */
 	public static LafType fromName(String name) {
 		for (LafType type : values()) {
 			if (type.getName().equals(name)) {
@@ -54,6 +67,12 @@ public enum LafType {
 		return null;
 	}
 
+	/**
+	 * Returns true if the {@link LookAndFeel} represented by this LafType is supported on the
+	 * current platform.
+	 * @return true if the {@link LookAndFeel} represented by this LafType is supported on the
+	 * current platform
+	 */
 	public boolean isSupported() {
 		LookAndFeelInfo[] installedLookAndFeels = UIManager.getInstalledLookAndFeels();
 		for (LookAndFeelInfo info : installedLookAndFeels) {
@@ -64,6 +83,12 @@ public enum LafType {
 		return false;
 	}
 
+	/**
+	 * Returns a LookAndFeelManager that can install and update the {@link LookAndFeel} associated
+	 * with this LafType.
+	 * @return a LookAndFeelManager that can install and update the {@link LookAndFeel} associated
+	 * with this LafType.
+	 */
 	public LookAndFeelManager getLookAndFeelManager() {
 		return getManager(this);
 	}
@@ -90,6 +115,10 @@ public enum LafType {
 		}
 	}
 
+	/**
+	 * Returns the default LafType for the current platform.
+	 * @return the default LafType for the current platform.
+	 */
 	public static LafType getDefaultLookAndFeel() {
 		OperatingSystem OS = Platform.CURRENT_PLATFORM.getOperatingSystem();
 		switch (OS) {

Ghidra/Framework/Generic/src/main/java/generic/theme/ThemeEvent.java

@@ -0,0 +1,93 @@
+/* ###
+ * IP: GHIDRA
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * 
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package generic.theme;
+
+import javax.swing.LookAndFeel;
+
+/**
+ * Event for when a theme value changes;
+ */
+public class ThemeEvent {
+
+	/**
+	 * Returns true if the color associated with the given id has changed.
+	 * @param id the color id to test if changed
+	 * @return true if the color associated with the given id has changed
+	 */
+	public boolean isColorChanged(String id) {
+		return false;
+	}
+
+	/**
+	 * Returns true if the font associated with the given id has changed.
+	 * @param id the font id to test if changed
+	 * @return true if the font associated with the given id has changed
+	 */
+	public boolean isFontChanged(String id) {
+		return false;
+	}
+
+	/**
+	 * Returns true if the icon associated with the given id has changed.
+	 * @param id the icon id to test if changed
+	 * @return true if the icon associated with the given id has changed
+	 */
+	public boolean isIconChanged(String id) {
+		return false;
+	}
+
+	/**
+	 * Returns true if the {@link LookAndFeel} has changed (theme changed).
+	 * @return true if the {@link LookAndFeel} has changed (theme changed).
+	 */
+	public boolean isLookAndFeelChanged() {
+		return false;
+	}
+
+	/**
+	 * Returns true if any color value changed.
+	 * @return true if any color value changed.
+	 */
+	public boolean hasAnyColorChanged() {
+		return false;
+	}
+
+	/**
+	 * Returns true if any font value changed.
+	 * @return true if any font value changed.
+	 */
+	public boolean hasAnyFontChanged() {
+		return false;
+	}
+
+	/**
+	 * Returns true if any icon value changed.
+	 * @return true if any icon value changed.
+	 */
+	public boolean hasAnyIconChanged() {
+		return false;
+	}
+
+	/**
+	 * Returns true if all colors, fonts, and icons may have changed. This doesn't guarantee that 
+	 * all the values have actually changed, just that they might have. In other words, a mass
+	 * change occurred (theme change, theme reset, etc.) and any or all values may have changed.
+	 * @return true if all colors, fonts, and icons may have changed.
+	 */
+	public boolean haveAllValuesChanged() {
+		return false;
+	}
+}

Ghidra/Framework/Generic/src/main/java/generic/theme/ThemeListener.java

@@ -15,24 +15,15 @@
  */
 package generic.theme;
 
+/**
+ * Listener interface for theme changes
+ */
 public interface ThemeListener {
-	public default void themeChanged(GTheme newTheme) {
-		// default do nothing
-	}
 
-	public default void colorChanged(String id) {
-		// default do nothing
-	}
+	/**
+	 * Called when the theme or any of its values change
+	 * @param event the {@link ThemeEvent} that describes what changed
+	 */
+	public void themeChanged(ThemeEvent event);
 
-	public default void fontChanged(String id) {
-		// default do nothing
-	}
-
-	public default void iconChanged(String id) {
-		// default do nothing
-	}
-
-	public default void themeValuesRestored() {
-		// default do nothing
-	}
 }

Ghidra/Framework/Generic/src/main/java/generic/theme/ThemePropertiesLoader.java

@@ -22,6 +22,10 @@ import generic.jar.ResourceFile;
 import ghidra.framework.Application;
 import ghidra.util.Msg;
 
+/**
+ * Loads all the system theme.property files that contain all the default color, font, and
+ * icon values.
+ */
 public class ThemePropertiesLoader {
 	GThemeValueMap defaults = new GThemeValueMap();
 	GThemeValueMap darkDefaults = new GThemeValueMap();
@@ -29,6 +33,10 @@ public class ThemePropertiesLoader {
 	public ThemePropertiesLoader() {
 	}
 
+	/**
+	 * Searches for all the theme.property files and loads them into either the standard
+	 * defaults (light) map or the dark defaults map.
+	 */
 	public void load() {
 		List<ResourceFile> themeDefaultFiles =
 			Application.findFilesByExtensionInApplication(".theme.properties");
@@ -49,10 +57,18 @@ public class ThemePropertiesLoader {
 		}
 	}
 
+	/**
+	 * Returns the standard defaults {@link GThemeValueMap}
+	 * @return the standard defaults {@link GThemeValueMap}
+	 */
 	public GThemeValueMap getDefaults() {
 		return defaults;
 	}
 
+	/**
+	 * Returns the dark defaults {@link GThemeValueMap}
+	 * @return the dark defaults {@link GThemeValueMap}
+	 */
 	public GThemeValueMap getDarkDefaults() {
 		return darkDefaults;
 	}

Ghidra/Framework/Generic/src/main/java/generic/theme/ThemePropertyFileReader.java

@@ -15,264 +15,75 @@
  */
 package generic.theme;
 
-import java.awt.Color;
-import java.awt.Font;
 import java.io.*;
-import java.util.*;
-
-import javax.swing.Icon;
-import javax.swing.plaf.FontUIResource;
-
-import org.apache.commons.collections4.map.HashedMap;
 
 import generic.jar.ResourceFile;
-import ghidra.util.Msg;
-import ghidra.util.WebColors;
-import resources.ResourceManager;
 
-public class ThemePropertyFileReader {
+/**
+ * Reads the values for a single theme.properities file
+ */
+public class ThemePropertyFileReader extends AbstractThemeReader {
 
-	private static final String NO_SECTION = "[No Section]";
-	private static final String DEFAULTS = "[Defaults]";
-	private static final String DARK_DEFAULTS = "[Dark Defaults]";
-
-	private GThemeValueMap defaults = new GThemeValueMap();
-	private GThemeValueMap darkDefaults = new GThemeValueMap();
-	private Map<String, List<String>> aliasMap = new HashedMap<>();
-	private List<String> errors = new ArrayList<>();
-	private String filePath;
-
-	public ThemePropertyFileReader(File file) throws IOException {
-		filePath = file.getAbsolutePath();
-		try (Reader reader = new FileReader(file)) {
-			read(reader);
-		}
-	}
+	private GThemeValueMap defaults;
+	private GThemeValueMap darkDefaults;
 
+	/**
+	 * Constructor for when the the theme.properties file is a {@link ResourceFile}
+	 * @param file the {@link ResourceFile} esourceFileto read
+	 * @throws IOException if an I/O error occurs reading the file
+	 */
 	public ThemePropertyFileReader(ResourceFile file) throws IOException {
-		filePath = file.getAbsolutePath();
+		super(file.getAbsolutePath());
+
 		try (Reader reader = new InputStreamReader(file.getInputStream())) {
 			read(reader);
 		}
-	}
-
-	protected ThemePropertyFileReader() {
 
 	}
 
-	ThemePropertyFileReader(String source, Reader reader) throws IOException {
-		filePath = source;
+	/**
+	 * Constructor using a Reader (needed for reading from zip files).
+	 * @param source the name or description of the Reader source
+	 * @param reader the {@link Reader} to parse as theme data
+	 * @throws IOException if an I/O error occurs while reading from the Reader
+	 */
+	protected ThemePropertyFileReader(String source, Reader reader) throws IOException {
+		super(source);
 		read(reader);
 	}
 
+	/**
+	 * Returns the map of standard defaults values. 
+	 * @return the map of standard defaults values.
+	 */
 	public GThemeValueMap getDefaultValues() {
-		return defaults;
+		return defaults == null ? new GThemeValueMap() : defaults;
 	}
 
+	/**
+	 * Returns the map of dark defaults values. 
+	 * @return the map of dark defaults values.
+	 */
 	public GThemeValueMap getDarkDefaultValues() {
-		return darkDefaults;
-	}
-
-	public Map<String, List<String>> getAliases() {
-		return aliasMap;
-	}
-
-	public List<String> getErrors() {
-		return errors;
-	}
-
-	protected void read(Reader reader) throws IOException {
-		List<Section> sections = readSections(new LineNumberReader(reader));
-		for (Section section : sections) {
-			switch (section.getName()) {
-				case NO_SECTION:
-					processNoSection(section);
-					break;
-				case DEFAULTS:
-					processValues(defaults, section);
-					break;
-				case DARK_DEFAULTS:
-					processValues(darkDefaults, section);
-					break;
-				default:
-					error(section.getLineNumber(),
-						"Encounded unknown theme file section: " + section.getName());
-			}
-		}
-
+		return darkDefaults == null ? new GThemeValueMap() : darkDefaults;
 	}
 
 	protected void processNoSection(Section section) throws IOException {
 		if (!section.isEmpty()) {
 			error(0, "Theme properties file has values defined outside of a defined section");
 		}
-
 	}
 
-	public void processValues(GThemeValueMap valueMap, Section section) {
-		for (String key : section.getKeys()) {
-			String value = section.getValue(key);
-			int lineNumber = section.getLineNumber(key);
-			if (ColorValue.isColorKey(key)) {
-				valueMap.addColor(parseColorProperty(key, value, lineNumber));
-			}
-			else if (FontValue.isFontKey(key)) {
-				valueMap.addFont(parseFontProperty(key, value, lineNumber));
-			}
-			else if (IconValue.isIconKey(key)) {
-				if (!FileGTheme.JAVA_ICON.equals(value)) {
-					valueMap.addIcon(parseIconProperty(key, value));
-				}
-			}
-			else {
-				error(lineNumber, "Can't process property: " + key + " = " + value);
-			}
-		}
+	@Override
+	protected void processDefaultSection(Section section) throws IOException {
+		defaults = new GThemeValueMap();
+		processValues(defaults, section);
 	}
 
-	private IconValue parseIconProperty(String key, String value) {
-		if (IconValue.isIconKey(value)) {
-			return new IconValue(key, value);
-		}
-		Icon icon = ResourceManager.loadImage(value);
-		return new IconValue(key, icon);
-	}
-
-	private FontValue parseFontProperty(String key, String value, int lineNumber) {
-		if (FontValue.isFontKey(value)) {
-			return new FontValue(key, value);
-		}
-		Font font = Font.decode(value);
-		if (font == null) {
-			error(lineNumber, "Could not parse Color: " + value);
-		}
-		return font == null ? null : new FontValue(key, new FontUIResource(font));
-	}
-
-	private ColorValue parseColorProperty(String key, String value, int lineNumber) {
-		if (ColorValue.isColorKey(value)) {
-			return new ColorValue(key, value);
-		}
-		Color color = WebColors.getColor(value);
-		if (color == null) {
-			error(lineNumber, "Could not parse Color: " + value);
-		}
-		return color == null ? null : new ColorValue(key, color);
-	}
-
-	private List<Section> readSections(LineNumberReader reader) throws IOException {
-
-		List<Section> sections = new ArrayList<>();
-		Section currentSection = new Section(NO_SECTION, 0);
-		sections.add(currentSection);
-
-		String line;
-		while ((line = reader.readLine()) != null) {
-			line = removeComments(line);
-
-			if (line.isBlank()) {
-				continue;
-			}
-
-			if (isSectionHeader(line)) {
-				currentSection = new Section(line, reader.getLineNumber());
-				sections.add(currentSection);
-			}
-			else {
-				currentSection.add(line, reader.getLineNumber());
-			}
-		}
-
-		return sections;
-	}
-
-	private String removeComments(String line) {
-		// remove any trailing comment on line
-		int commentIndex = line.indexOf("//");
-		if (commentIndex >= 0) {
-			line = line.substring(0, commentIndex);
-		}
-		line = line.trim();
-
-		// clear line if entire line is comment
-		if (line.startsWith("#")) {
-			return "";
-		}
-		return line;
-	}
-
-	private boolean isSectionHeader(String line) {
-		return line.startsWith("[") && line.endsWith("]");
-	}
-
-	protected void error(int lineNumber, String message) {
-		String msg =
-			"Error parsing file \"" + filePath + "\" at line: " + lineNumber + ", " + message;
-		errors.add(msg);
-		Msg.out(msg);
-	}
-
-	protected class Section {
-
-		private String name;
-		Map<String, String> properties = new HashMap<>();
-		Map<String, Integer> lineNumbers = new HashMap<>();
-		private int startLineNumber;
-
-		public Section(String sectionName, int lineNumber) {
-			this.name = sectionName;
-			this.startLineNumber = lineNumber;
-		}
-
-		public void remove(String key) {
-			properties.remove(key);
-		}
-
-		public String getValue(String key) {
-			return properties.get(key);
-		}
-
-		public Set<String> getKeys() {
-			return properties.keySet();
-		}
-
-		public int getLineNumber(String key) {
-			return lineNumbers.get(key);
-		}
-
-		public boolean isEmpty() {
-			return properties.isEmpty();
-		}
-
-		public int getLineNumber() {
-			return startLineNumber;
-		}
-
-		public String getName() {
-			return name;
-		}
-
-		public void add(String line, int lineNumber) {
-			int splitIndex = line.indexOf('=');
-			if (splitIndex < 0) {
-				error(lineNumber, "Missing required \"=\" for propery line: \"" + line + "\"");
-				return;
-			}
-			String key = line.substring(0, splitIndex).trim();
-			String value = line.substring(splitIndex + 1, line.length()).trim();
-			if (key.isBlank()) {
-				error(lineNumber, "Missing key for propery line: \"" + line + "\"");
-				return;
-			}
-			if (key.isBlank()) {
-				error(lineNumber, "Missing value for propery line: \"" + line + "\"");
-				return;
-			}
-			properties.put(key, value);
-			lineNumbers.put(key, lineNumber);
-
-		}
-
+	@Override
+	protected void processDarkDefaultSection(Section section) throws IOException {
+		darkDefaults = new GThemeValueMap();
+		processValues(darkDefaults, section);
 	}
 
 }

Ghidra/Framework/Generic/src/main/java/generic/theme/ThemeReader.java

@@ -15,59 +15,131 @@
  */
 package generic.theme;
 
-import java.io.File;
-import java.io.IOException;
+import java.io.*;
+import java.util.Enumeration;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipFile;
 
-public class ThemeReader extends ThemePropertyFileReader {
+import org.apache.commons.io.FileUtils;
 
-	private Section themeSection;
-	private String themeName;
-	private LafType lookAndFeel;
-	private boolean useDarkDefaults;
+import ghidra.framework.Application;
+import ghidra.util.Msg;
 
-	public ThemeReader(File file) throws IOException {
-		super(file);
+/**
+ * Reads Themes from a file or {@link Reader}
+ */
+class ThemeReader extends AbstractThemeReader {
+
+	private File file;
+	private GTheme theme;
+
+	/**
+	 * Constructor for reading a theme from a file.
+	 * @param file the file to read as a theme
+	 * @throws IOException if an I/O error occurs reading the theme file
+	 */
+	ThemeReader(File file) throws IOException {
+		super(file.getAbsolutePath());
+		this.file = file;
 	}
 
-	protected ThemeReader() {
+	public GTheme readTheme() throws IOException {
+		if (file.getName().endsWith(GTheme.FILE_EXTENSION)) {
+			return readFileTheme();
+		}
+		if (file.getName().endsWith(GTheme.ZIP_FILE_EXTENSION)) {
+			return readZipTheme();
+		}
 
+		throw new IOException("Imported File must end in either " + GTheme.FILE_EXTENSION + " or " +
+			GTheme.ZIP_FILE_EXTENSION);
+	}
+
+	/**
+	 * Assumes the file is a theme file and reads it.
+	 * @return
+	 * @throws IOException
+	 */
+	private GTheme readFileTheme() throws IOException {
+		try (Reader reader = new FileReader(file)) {
+			read(reader);
+		}
+		if (theme == null) {
+			throw new IOException("Invalid Theme file: " + file);
+		}
+		return theme;
+	}
+
+	private GTheme readZipTheme() throws IOException {
+		try (ZipFile zipFile = new ZipFile(file)) {
+			Enumeration<? extends ZipEntry> entries = zipFile.entries();
+			while (entries.hasMoreElements()) {
+				ZipEntry entry = entries.nextElement();
+				String name = entry.getName();
+				try (InputStream is = zipFile.getInputStream(entry)) {
+					if (name.endsWith(".theme")) {
+						processThemeData(name, is);
+					}
+					else {
+						processIconFile(name, is);
+					}
+				}
+			}
+		}
+		return theme;
+	}
+
+	// for testing
+	GTheme readTheme(Reader reader) throws IOException {
+		read(reader);
+		return theme;
 	}
 
 	@Override
 	protected void processNoSection(Section section) throws IOException {
-		themeSection = section;
-		themeName = section.getValue(GTheme.THEME_NAME_KEY);
+		String themeName = section.getValue(ThemeWriter.THEME_NAME_KEY);
 		if (themeName == null) {
 			throw new IOException("Missing theme name!");
 		}
-		String lookAndFeelName = section.getValue(GTheme.THEME_LOOK_AND_FEEL_KEY);
-		lookAndFeel = LafType.fromName(lookAndFeelName);
+		String lookAndFeelName = section.getValue(ThemeWriter.THEME_LOOK_AND_FEEL_KEY);
+		LafType lookAndFeel = LafType.fromName(lookAndFeelName);
 		if (lookAndFeel == null) {
 			throw new IOException(
 				"Invalid or missing lookAndFeel name: \"" + lookAndFeelName + "\"");
 		}
-		useDarkDefaults = Boolean.valueOf(section.getValue(GTheme.THEME_USE_DARK_DEFAULTS));
+		boolean isDark = Boolean.valueOf(section.getValue(ThemeWriter.THEME_USE_DARK_DEFAULTS));
+
+		theme = new GTheme(file, themeName, lookAndFeel, isDark);
+		section.remove(ThemeWriter.THEME_NAME_KEY);
+		section.remove(ThemeWriter.THEME_LOOK_AND_FEEL_KEY);
+		section.remove(ThemeWriter.THEME_USE_DARK_DEFAULTS);
+		processValues(theme, section);
 	}
 
-	void loadValues(GTheme theme) {
-
-		// processValues expects only colors, fonts, and icons
-		themeSection.remove(GTheme.THEME_NAME_KEY);
-		themeSection.remove(GTheme.THEME_LOOK_AND_FEEL_KEY);
-		themeSection.remove(GTheme.THEME_USE_DARK_DEFAULTS);
-
-		processValues(theme, themeSection);
+	@Override
+	protected void processDefaultSection(Section section) throws IOException {
+		error(section.getLineNumber(), "[Defaults] section not allowed in theme files!");
 	}
 
-	public String getThemeName() {
-		return themeName;
+	@Override
+	protected void processDarkDefaultSection(Section section) throws IOException {
+		error(section.getLineNumber(), "[Dark Defaults] section not allowed in theme files!");
 	}
 
-	public LafType getLookAndFeelType() {
-		return lookAndFeel;
+	private void processIconFile(String path, InputStream is) throws IOException {
+		int indexOf = path.indexOf("images/");
+		if (indexOf < 0) {
+			Msg.error(this, "Unknown file: " + path);
+		}
+		String relativePath = path.substring(indexOf, path.length());
+		File dir = Application.getUserSettingsDirectory();
+		File iconFile = new File(dir, relativePath);
+		FileUtils.copyInputStreamToFile(is, iconFile);
 	}
 
-	public boolean useDarkDefaults() {
-		return useDarkDefaults;
+	private void processThemeData(String name, InputStream is) throws IOException {
+		InputStreamReader reader = new InputStreamReader(is);
+		read(reader);
 	}
+
 }

Ghidra/Framework/Generic/src/main/java/generic/theme/ThemeValue.java

@@ -19,41 +19,64 @@ import java.util.*;
 
 import ghidra.util.Msg;
 
-// TODO doc why 'cachedValue' is lazy loaded
+/**
+ * A generic class for storing theme values that have a String id (e.g. color.bg.foo) and either
+ * a concrete value of type T or a reference id which is the String id of another ThemeValue. So
+ * if this class's value is non-null, the refId will be null and if the class's refId is non-null,
+ * then the value will be null.
+ *
+ * @param <T> the base type this ThemeValue works on (i.e., Colors, Fonts, Icons)
+ */
 public abstract class ThemeValue<T> implements Comparable<ThemeValue<T>> {
 	private final String id;
 	private final T value;
 	private final String refId;
-//	private T cachedValue;
 
 	protected ThemeValue(String id, String refId, T value) {
 		this.id = fromExternalId(id);
 		this.refId = (refId == null) ? null : fromExternalId(refId);
 		this.value = value;
-		if (value instanceof GColor) {
-			System.out.println("Whoa");
-		}
 	}
 
-	protected abstract String getIdPrefix();
-
+	/**
+	 * Returns the identifier for this ThemeValue.
+	 * @return the identifier for this ThemeValue.
+	 */
 	public String getId() {
 		return id;
 	}
 
+	/**
+	 * Returns the referencId of another ThemeValue that we inherit its value pr null if we have
+	 * a value
+	 * 
+	 * @return  the referencId of another ThemeValue that we inherit its value or null if we have
+	 * a value
+	 */
 	public String getReferenceId() {
 		return refId;
 	}
 
+	/**
+	 * Returns the stored value. Does not follow referenceIds. Will be null if this instance
+	 * has a referenceId.
+	 * 
+	 * @return the stored value. Does not follow referenceIds. Will be null if this instance
+	 * has a referenceId.
+	 */
 	public T getRawValue() {
 		return value;
 	}
 
+	/**
+	 * Returns the T value for this instance, following references as needed. Uses the given
+	 * preferredValues map to resolve references.
+	 * @param preferredValues the {@link GThemeValueMap} used to resolve references if this 
+	 * instance doesn't have an actual value.
+	 * @return the T value for this instance, following references as needed.
+	 */
 	public T get(GThemeValueMap preferredValues) {
-//		if (cachedValue == null) {
 		return doGetValue(preferredValues);
-//		}
-//		return cachedValue;
 	}
 
 	private T doGetValue(GThemeValueMap values) {
@@ -75,35 +98,44 @@ public abstract class ThemeValue<T> implements Comparable<ThemeValue<T>> {
 		return getUnresolvedReferenceValue(id);
 	}
 
-	abstract protected T getUnresolvedReferenceValue(String theId);
+	/**
+	 * Returns the T to be used if the indirect reference couldn't be resolved.
+	 * @param unresolvedId the id that couldn't be resolved 
+	 * @return the default value to be used if the indirect reference couldn't be resolved.
+	 */
+	abstract protected T getUnresolvedReferenceValue(String unresolvedId);
 
+	/**
+	 * Returns the id to be used when writing to a theme file. For ThemeValues whose id begins
+	 * with the expected prefix (e.g. "color" for ColorValues), it is just the id. Otherwise, the
+	 * id is prepended with an appropriate string to make parsing easier.
+	 * @param internalId the id of this ThemeValue
+	 * @return the id to be used when writing to a theme file
+	 */
 	abstract public String toExternalId(String internalId);
 
+	/**
+	 * Converts an external id to an internal id (the id stored in this object)
+	 * @param externalId the external form of the id
+	 * @return the id for the ThemeValue being read from a file
+	 */
 	abstract public String fromExternalId(String externalId);
 
+	/**
+	 * Returns the ThemeValue referred to by this ThemeValue. Needs to be overridden by
+	 * concrete classes as they know the correct method to call on the preferredValues map.
+	 * @param preferredValues the {@link GThemeValueMap} to be used to resolve the reference id
+	 * @param referenceId  the id of the reference ThemeValue
+	 * @return the ThemeValue referred to by this ThemeValue.
+	 */
 	abstract protected ThemeValue<T> getReferredValue(GThemeValueMap preferredValues,
-			String theRefId);
+			String referenceId);
 
 	@Override
 	public int compareTo(ThemeValue<T> o) {
 		return id.compareTo(o.id);
 	}
 
-	public int compareValue(ThemeValue<T> o) {
-		if (o == null) {
-			return -1;
-		}
-		if (refId != null) {
-			return o.refId != null ? refId.compareTo(o.refId) : -1;
-		}
-		if (o.refId != null) {
-			return 1;
-		}
-		return compareValues(value, o.value);
-	}
-
-	protected abstract int compareValues(T v1, T v2);
-
 	@Override
 	public int hashCode() {
 		return Objects.hash(id, refId, value);

Ghidra/Framework/Generic/src/main/java/generic/theme/ThemeWriter.java

@@ -18,75 +18,98 @@ package generic.theme;
 import java.awt.Color;
 import java.awt.Font;
 import java.io.*;
-import java.util.Collections;
-import java.util.List;
+import java.util.*;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipOutputStream;
 
 import javax.swing.Icon;
 
+import com.google.common.io.Files;
+
 import ghidra.util.WebColors;
 import resources.icons.UrlImageIcon;
 
-public class FileGTheme extends GTheme {
-	public static final String JAVA_ICON = "<JAVA ICON>";
-	public static final String FILE_PREFIX = "File:";
-	protected final File file;
+/**
+ * Writes a theme to a file either as a single theme file or as a zip file that contains the theme
+ * file and any external (from the file system, not the classpath) icons used by the theme.
+ */
+public class ThemeWriter {
+	static final String THEME_NAME_KEY = "name";
+	static final String THEME_LOOK_AND_FEEL_KEY = "lookAndFeel";
+	static final String THEME_USE_DARK_DEFAULTS = "useDarkDefaults";
+	protected GTheme theme;
 
-	public FileGTheme(File file) throws IOException {
-		this(file, new ThemeReader(file));
+	/**
+	 * Constructor
+	 * @param theme the theme to be written to a file
+	 */
+	public ThemeWriter(GTheme theme) {
+		this.theme = theme;
 	}
 
-	public FileGTheme(File file, String name, LafType laf, boolean useDarkDefaults) {
-		super(name, laf, useDarkDefaults);
-		this.file = file;
-	}
-
-	FileGTheme(File file, ThemeReader reader) {
-		super(reader.getThemeName(), reader.getLookAndFeelType(), reader.useDarkDefaults());
-		this.file = file;
-		reader.loadValues(this);
-	}
-
-	@Override
-	public String getThemeLocater() {
-		return FILE_PREFIX + file.getAbsolutePath();
-	}
-
-	public boolean canSave() {
-		if (file.exists()) {
-			return file.canWrite();
+	/**
+	 * Writes the theme to the given file with the option to output as a zip file.
+	 * @param file the file to write to
+	 * @param asZip if true, outputs in zip format
+	 * @throws FileNotFoundException i
+	 * @throws IOException if an I/O error occurs trying to write the file
+	 */
+	public void writeTheme(File file, boolean asZip) throws IOException {
+		if (asZip) {
+			writeThemeToZipFile(file);
+		}
+		else {
+			writeThemeToFile(file);
 		}
-		return file.getParentFile().canWrite();
 	}
 
-	public File getFile() {
-		return file;
-	}
-
-	public void save() throws IOException {
+	/**
+	 * Writes the theme to the given file.
+	 * @param file the file to write to
+	 * @throws FileNotFoundException i
+	 * @throws IOException if an I/O error occurs trying to write the file
+	 */
+	public void writeThemeToFile(File file) throws IOException {
 		try (BufferedWriter writer = new BufferedWriter(new FileWriter(file))) {
 			writeThemeValues(writer);
-
 		}
+	}
 
+	/**
+	 * Writes the theme to the given file in a zip format.
+	 * @param file the file to write to
+	 * @throws IOException if an I/O error occurs trying to write the file
+	 */
+	public void writeThemeToZipFile(File file) throws IOException {
+		String dir = theme.getName() + ".theme/";
+		try (FileOutputStream fos = new FileOutputStream(file)) {
+			ZipOutputStream zos = new ZipOutputStream(fos);
+			saveThemeFileToZip(dir, zos);
+			Set<File> iconFiles = theme.getExternalIconFiles();
+			for (File iconFile : iconFiles) {
+				copyToZipFile(dir, iconFile, zos);
+			}
+			zos.finish();
+		}
 	}
 
 	protected void writeThemeValues(BufferedWriter writer) throws IOException {
-		List<ColorValue> colors = getColors();
+		List<ColorValue> colors = theme.getColors();
 		Collections.sort(colors);
 
-		List<FontValue> fonts = getFonts();
+		List<FontValue> fonts = theme.getFonts();
 		Collections.sort(fonts);
 
-		List<IconValue> icons = getIcons();
+		List<IconValue> icons = theme.getIcons();
 		Collections.sort(icons);
 
-		writer.write(THEME_NAME_KEY + " = " + getName());
+		writer.write(THEME_NAME_KEY + " = " + theme.getName());
 		writer.newLine();
 
-		writer.write(THEME_LOOK_AND_FEEL_KEY + " = " + getLookAndFeelType().getName());
+		writer.write(THEME_LOOK_AND_FEEL_KEY + " = " + theme.getLookAndFeelType().getName());
 		writer.newLine();
 
-		writer.write(THEME_USE_DARK_DEFAULTS + " = " + useDarkDefaults());
+		writer.write(THEME_USE_DARK_DEFAULTS + " = " + theme.useDarkDefaults());
 		writer.newLine();
 
 		for (ColorValue colorValue : colors) {
@@ -129,13 +152,6 @@ public class FileGTheme extends GTheme {
 		return iconToString(icon);
 	}
 
-	public static String iconToString(Icon icon) {
-		if (icon instanceof UrlImageIcon urlIcon) {
-			return urlIcon.getOriginalPath();
-		}
-		return JAVA_ICON;
-	}
-
 	private String getValueOutput(FontValue fontValue) {
 		if (fontValue.getReferenceId() != null) {
 			return fontValue.toExternalId(fontValue.getReferenceId());
@@ -144,10 +160,6 @@ public class FileGTheme extends GTheme {
 		return fontToString(font);
 	}
 
-	public static String fontToString(Font font) {
-		return String.format("%s-%s-%s", font.getName(), getStyleString(font), font.getSize());
-	}
-
 	private static String getStyleString(Font font) {
 		boolean bold = font.isBold();
 		boolean italic = font.isItalic();
@@ -162,4 +174,39 @@ public class FileGTheme extends GTheme {
 		}
 		return "PLAIN";
 	}
+
+	/**
+	 * Converts a file to a string.
+	 * @param font the font to convert to a String
+	 * @return a String that represents the font
+	 */
+	public static String fontToString(Font font) {
+		return String.format("%s-%s-%s", font.getName(), getStyleString(font), font.getSize());
+	}
+
+	/**
+	 * Converts an icon to a string.
+	 * @param icon the icon to convert to a String
+	 * @return a String that represents the icon
+	 */
+	public static String iconToString(Icon icon) {
+		if (icon instanceof UrlImageIcon urlIcon) {
+			return urlIcon.getOriginalPath();
+		}
+		return GTheme.JAVA_ICON;
+	}
+
+	private void copyToZipFile(String dir, File iconFile, ZipOutputStream zos) throws IOException {
+		ZipEntry entry = new ZipEntry(dir + "images/" + iconFile.getName());
+		zos.putNextEntry(entry);
+		Files.copy(iconFile, zos);
+	}
+
+	private void saveThemeFileToZip(String dir, ZipOutputStream zos) throws IOException {
+		ZipEntry entry = new ZipEntry(dir + theme.getName() + ".theme");
+		zos.putNextEntry(entry);
+		BufferedWriter writer = new BufferedWriter(new OutputStreamWriter(zos));
+		writeThemeValues(writer);
+		writer.flush();
+	}
 }

Ghidra/Framework/Generic/src/main/java/generic/theme/ZipGTheme.java

@@ -1,68 +0,0 @@
-/* ###
- * IP: GHIDRA
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- * 
- *      http://www.apache.org/licenses/LICENSE-2.0
- * 
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package generic.theme;
-
-import java.io.*;
-import java.util.Set;
-import java.util.zip.ZipEntry;
-import java.util.zip.ZipOutputStream;
-
-import com.google.common.io.Files;
-
-public class ZipGTheme extends FileGTheme {
-
-	public ZipGTheme(File file, String name, LafType laf, boolean useDarkDefaults) {
-		super(file, name, laf, useDarkDefaults);
-	}
-
-	public ZipGTheme(File file) throws IOException {
-		this(file, new ExternalThemeReader(file));
-	}
-
-	public ZipGTheme(File file, ThemeReader reader) {
-		super(file, reader.getThemeName(), reader.getLookAndFeelType(), reader.useDarkDefaults());
-		reader.loadValues(this);
-	}
-
-	@Override
-	public void save() throws IOException {
-		String dir = getName() + ".theme/";
-		try (FileOutputStream fos = new FileOutputStream(file)) {
-			ZipOutputStream zos = new ZipOutputStream(fos);
-			saveThemeFileToZip(dir, zos);
-			Set<File> iconFiles = getExternalIconFiles();
-			for (File iconFile : iconFiles) {
-				copyToZipFile(dir, iconFile, zos);
-			}
-			zos.finish();
-		}
-	}
-
-	private void copyToZipFile(String dir, File iconFile, ZipOutputStream zos) throws IOException {
-		ZipEntry entry = new ZipEntry(dir + "images/" + iconFile.getName());
-		zos.putNextEntry(entry);
-		Files.copy(iconFile, zos);
-	}
-
-	private void saveThemeFileToZip(String dir, ZipOutputStream zos) throws IOException {
-		ZipEntry entry = new ZipEntry(dir + getName() + ".theme");
-		zos.putNextEntry(entry);
-		BufferedWriter writer = new BufferedWriter(new OutputStreamWriter(zos));
-		writeThemeValues(writer);
-		writer.flush();
-	}
-
-}

Ghidra/Framework/Generic/src/main/java/generic/theme/laf/NimbusLookAndFeelInstaller.java

@@ -36,7 +36,10 @@ public class NimbusLookAndFeelInstaller extends LookAndFeelInstaller {
 
 	@Override
 	protected void installJavaDefaults() {
-		// do nothing - already handled by installing extended NimbusLookAndFeel
+		// even though java defaults have been installed, we need to fix them up now
+		// that nimbus has finished initializing
+		GColor.refreshAll();
+		Gui.setJavaDefaults(Gui.getJavaDefaults());
 	}
 
 	@Override

Ghidra/Framework/Generic/src/test/java/generic/theme/GThemeTest.java

@@ -35,8 +35,8 @@ public class GThemeTest extends AbstractGenericTest {
 	private static final Font COURIER = new Font("Courier", Font.BOLD, 14);
 	private static final Font DIALOG = new Font("Dialog", Font.PLAIN, 16);
 	private static final Color COLOR_WITH_ALPHA = new Color(10, 20, 30, 40);
-	private static final String ICON_PATH_1 = "images/arrow.png";
-	private static final String ICON_PATH_2 = "images/disk.png";
+	private static final String ICON_PATH_1 = "images/error.png";
+	private static final String ICON_PATH_2 = "images/exec.png";
 	private static final Icon ICON1 = ResourceManager.loadImage(ICON_PATH_1);
 	private static final Icon ICON2 = ResourceManager.loadImage(ICON_PATH_2);
 
@@ -50,7 +50,7 @@ public class GThemeTest extends AbstractGenericTest {
 
 	@Test
 	public void testGetName() {
-		assertEquals("Default", theme.getName());
+		assertEquals(Gui.getDefaultTheme().getName(), theme.getName());
 	}
 
 	@Test
@@ -95,10 +95,10 @@ public class GThemeTest extends AbstractGenericTest {
 		theme.setIcon("t.u.v", ICON1);
 		theme.setIconRef("t.u.v.1", "t.u.v");
 
-		File file = createTempFile("themeTest.theme");
+		File file = createTempFile("themeTest", ".theme");
 
 		theme.saveToFile(file, false);  // saveToFile returns new theme instance
-		theme = new FileGTheme(file);
+		theme = new ThemeReader(file).readTheme();
 
 		assertEquals("abc", theme.getName());
 		assertEquals(LafType.getDefaultLookAndFeel(), theme.getLookAndFeelType());
@@ -116,10 +116,10 @@ public class GThemeTest extends AbstractGenericTest {
 		assertEquals(COURIER, theme.getFont("x.y.z").get(theme));
 		assertEquals(COURIER, theme.getFont("x.y.z.1").get(theme));
 
-		assertEquals(ICON_PATH_1, theme.getIcon("icon.a.1").get(theme));
-		assertEquals(ICON_PATH_2, theme.getIcon("icon.a.2").get(theme));
-		assertEquals(ICON_PATH_1, theme.getIcon("t.u.v").get(theme));
-		assertEquals(ICON_PATH_1, theme.getIcon("t.u.v.1").get(theme));
+		assertEquals(ICON1, theme.getIcon("icon.a.1").get(theme));
+		assertEquals(ICON2, theme.getIcon("icon.a.2").get(theme));
+		assertEquals(ICON1, theme.getIcon("t.u.v").get(theme));
+		assertEquals(ICON1, theme.getIcon("t.u.v.1").get(theme));
 	}
 
 }

Ghidra/Framework/Generic/src/test/java/generic/theme/ThemePropertyFileReaderTest.java

@@ -24,14 +24,11 @@ import java.io.IOException;
 import java.io.StringReader;
 import java.util.List;
 
-import org.junit.Before;
 import org.junit.Test;
 
-public class ThemePropertyFileReaderTest {
+import resources.ResourceManager;
 
-	@Before
-	public void setUp() {
-	}
+public class ThemePropertyFileReaderTest {
 
 	@Test
 	public void testDefaults() throws IOException {
@@ -47,7 +44,7 @@ public class ThemePropertyFileReaderTest {
 			"  color.b.7    = color.b.1",				// ref
 			"  font.a.8     = dialog-PLAIN-14",
 			"  font.a.9     = font.a.8",
-			"  icon.a.10     = foo.png",
+			"  icon.a.10     = core.png",
 			"  icon.a.11     = icon.a.10",
 			"")));
 		//@formatter:on
@@ -67,7 +64,7 @@ public class ThemePropertyFileReaderTest {
 		assertEquals(new Font("dialog", Font.PLAIN, 14), getFontOrRef(values, "font.a.8"));
 		assertEquals("font.a.8", getFontOrRef(values, "font.a.9"));
 
-		assertEquals("foo.png", getIconOrRef(values, "icon.a.10"));
+		assertEquals(ResourceManager.loadImage("core.png"), getIconOrRef(values, "icon.a.10"));
 		assertEquals("icon.a.10", getIconOrRef(values, "icon.a.11"));
 
 	}