Merge remote-tracking branch 'origin/GP-1_bug_fixes_related_to_toggle_variables_feature' into Ghidra_12.0

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/codebrowser/CodeViewerProvider.java

@@ -481,22 +481,23 @@ public class CodeViewerProvider extends NavigatableComponentProviderAdapter
 		action = new GotoNextFunctionAction(tool, plugin.getName());
 		tool.addAction(action);
 
-		toggleVariablesAction = new ToggleActionBuilder("Show Function Variables", plugin.getName())
+		toggleVariablesAction =
-				.popupMenuPath("Show/Hide All Variables")
+			new ToggleActionBuilder("Show All Function Variables", plugin.getName())
-				.popupMenuGroup("Variables")
+					.popupMenuPath("Function", "Show/Hide All Variables")
-				.helpLocation(new HelpLocation("CodeBrowserPlugin", "Show_All_Variables"))
+					.popupMenuGroup("Variables")
-				.selected(true)
+					.helpLocation(new HelpLocation("CodeBrowserPlugin", "Show_All_Variables"))
-				.withContext(ProgramLocationActionContext.class)
+					.selected(true)
-				.enabledWhen(this::isInFunctionArea)
+					.withContext(ProgramLocationActionContext.class)
-				.onAction(c -> showVariablesForAllFunctions(toggleVariablesAction.isSelected()))
+					.onAction(c -> showVariablesForAllFunctions(toggleVariablesAction.isSelected()))
-				.buildAndInstallLocal(this);
+					.buildAndInstallLocal(this);
 
 		new ActionBuilder("Toggle Show Function Variables", plugin.getName())
-				.popupMenuPath("Show/Hide Variables")
+				.popupMenuPath("Function", "Show/Hide Variables")
 				.popupMenuGroup("Variables")
 				.helpLocation(new HelpLocation("CodeBrowserPlugin", "Show_Variables"))
 				.keyBinding("SPACE")
 				.withContext(ProgramLocationActionContext.class)
+				.validWhen(this::isInFunctionArea)
 				.enabledWhen(this::isInFunctionArea)
 				.onAction(c -> toggleShowVariables(c.getAddress()))
 				.buildAndInstallLocal(this);

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/codebrowser/actions/CollapseAllDataAction.java

@@ -42,7 +42,8 @@ public class CollapseAllDataAction extends ProgramLocationContextAction {
 		super("Collapse All Data", provider.getOwner());
 		this.provider = provider;
 
-		setPopupMenuData(new MenuData(new String[] { "Collapse All Data" }, null, "Structure"));
+		setPopupMenuData(
+			new MenuData(new String[] { "Data", "Collapse All Data" }, null, "BasicData2"));
 
 		setHelpLocation(new HelpLocation("CodeBrowserPlugin", "ExpandCollapseActions"));
 

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/codebrowser/actions/ExpandAllDataAction.java

@@ -39,7 +39,8 @@ public class ExpandAllDataAction extends ProgramLocationContextAction {
 		super("Expand All Data", provider.getOwner());
 		this.provider = provider;
 
-		setPopupMenuData(new MenuData(new String[] { "Expand All Data" }, null, "Structure"));
+		setPopupMenuData(
+			new MenuData(new String[] { "Data", "Expand All Data" }, null, "BasicData2"));
 		setDescription("Open all data recursively from the current location downward.");
 
 		setHelpLocation(new HelpLocation("CodeBrowserPlugin", "ExpandCollapseActions"));
@@ -83,11 +84,11 @@ public class ExpandAllDataAction extends ProgramLocationContextAction {
 
 	private void updatePopupMenuName(boolean hasSelection) {
 		if (hasSelection) {
-			getPopupMenuData().setMenuPath(new String[] { "Expand All Data In Selection" });
+			getPopupMenuData().setMenuPath(new String[] { "Data", "Expand All Data In Selection" });
 			setDescription("Open all data recursively in the current selection.");
 		}
 		else {
-			getPopupMenuData().setMenuPath(new String[] { "Expand All Data" });
+			getPopupMenuData().setMenuPath(new String[] { "Data", "Expand All Data" });
 			setDescription("Open all data recursively from the current location downward.");
 		}
 	}

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/codebrowser/actions/ToggleExpandCollapseDataAction.java

@@ -42,7 +42,8 @@ public class ToggleExpandCollapseDataAction extends ProgramLocationContextAction
 		this.provider = provider;
 
 		setPopupMenuData(
-			new MenuData(new String[] { "Toggle Expand/Collapse Data" }, null, "Structure"));
+			new MenuData(new String[] { "Data", "Toggle Expand/Collapse Data" }, null,
+				"BasicData2"));
 		setKeyBindingData(new KeyBindingData(' ', 0));
 
 		setHelpLocation(new HelpLocation("CodeBrowserPlugin", "ExpandCollapseActions"));

Ghidra/Features/Base/src/main/java/ghidra/app/util/viewer/format/FieldFormatModel.java

@@ -83,7 +83,7 @@ public class FieldFormatModel {
 	}
 
 	/**
-	 * Returns the formatMgr that is managing this model.
+	 * {@return the FormatManager managing this format.}
 	 */
 	public FormatManager getFormatManager() {
 		return formatMgr;
@@ -109,6 +109,7 @@ public class FieldFormatModel {
 	/**
 	 * Adds new empty row at the given position.  The position must be in the
 	 * interval [0,numRows].
+	 * @param index the index to add a new row
 	 * @exception IllegalArgumentException thrown if the position is outside the
 	 * interval [0,numRows].
 	 */
@@ -186,21 +187,23 @@ public class FieldFormatModel {
 	}
 
 	/**
-	 * Returns the number of rows in the model.
+	 * {@return the number of rows in this format}
 	 */
 	public int getNumRows() {
 		return rows.size();
 	}
 
 	/**
-	 * Returns the name of this format model.
+	 * {@return the name of this format model.}
 	 */
 	public String getName() {
 		return name;
 	}
 
 	/**
-	 * Returns the number of FieldFactorys on any given row.
+	 * Returns the number of factories on the given row.
+	 * @param row the row to get the number of factories for
+	 * @return the number of factories on the given row
 	 */
 	public int getNumFactorys(int row) {
 		if ((row < 0) || (row >= rows.size())) {
@@ -210,14 +213,16 @@ public class FieldFormatModel {
 	}
 
 	/**
-	 * Returns the FieldFactorys on a given row.
+	 * Returns the factories on the given row.
+	 * @param row the row to factories for
+	 * @return the FieldFactorys for a given row
 	 */
 	public FieldFactory[] getFactorys(int row) {
 		return (rows.get(row)).getFactorys();
 	}
 
 	/**
-	 * Returns the list factories valid for this format.
+	 * {@return the list of factories used in this format.}
 	 */
 	public FieldFactory[] getFactorys() {
 		return factories.clone();
@@ -251,7 +256,7 @@ public class FieldFormatModel {
 	}
 
 	/**
-	 * Returns the width of this model
+	 * {@return the width of this format.}
 	 */
 	public int getWidth() {
 		return width;
@@ -280,6 +285,7 @@ public class FieldFormatModel {
 
 	/**
 	 * Saves this format to XML.
+	 * @return the XML element for the saved format
 	 */
 	public Element saveToXml() {
 		Element root = new Element("FORMAT");

Ghidra/Features/Base/src/main/java/ghidra/app/util/viewer/format/FormatManager.java

@@ -959,6 +959,9 @@ public class FormatManager implements OptionsChangeListener {
 		for (int i = 0; i < NUM_MODELS; i++) {
 			if (saveState.hasValue(models[i].getName())) {
 				models[i].restoreFromXml(saveState.getXmlElement(models[i].getName()));
+				// hack to make sure the new open/close variables field is present
+				// If missing, we are just going to reset it to the default format
+				checkForMissingOpenCloseField(models[i]);
 			}
 			else {
 				models[i].restoreFromXml(getDefaultModel(i));
@@ -968,6 +971,27 @@ public class FormatManager implements OptionsChangeListener {
 		modelChanged(null);
 	}
 
+	// This is a hack to make sure the new variables open/close field is present.
+	// This was added in version 12.0 and can probably be removed in a few releases.
+	private void checkForMissingOpenCloseField(FieldFormatModel model) {
+		if (!model.getName().equals("Variable")) {
+			return;
+		}
+		if (!hasField(model, "+")) {
+			model.restoreFromXml(getDefaultVariableFormat());
+		}
+	}
+
+	private boolean hasField(FieldFormatModel model, String fieldName) {
+		FieldFactory[] unusedFactories = model.getUnusedFactories();
+		for (FieldFactory fieldFactory : unusedFactories) {
+			if (fieldFactory.getFieldName().equals("+")) {
+				return false;
+			}
+		}
+		return true;
+	}
+
 	public ServiceProvider getServiceProvider() {
 		return serviceProvider;
 	}