Merge remote-tracking branch 'origin/GP-5650_Dan_moveDynamicUpdateToToolbar--SQUASHED'

Ghidra/Features/Base/src/main/help/help/topics/ShowInstructionInfoPlugin/ShowInstructionInfo.htm

@@ -115,7 +115,7 @@
       (Mnemonic, Number of Operands, Address, Flow Type, Delay Slot Depth, Prototype Hash, Input
       Objects, Result Objects, Constructor line numbers, Instruction Bytes, etc.).</P>
 
-      <P>The Operand columns (<I>Op1</I>, <I>Op2</I>, etc.) display information about a particular
+      <P>The Operand columns (<I>Op0</I>, <I>Op1</I>, etc.) display information about a particular
       operand.&nbsp; Each operand has a number of rows.&nbsp; At the end of the row is a
       descriptive name for the information displayed on that row.</P>
 
@@ -197,13 +197,13 @@
       <P align="center">&nbsp;</P>
 
       <BLOCKQUOTE>
-        <P>The <I>Dynamic Update</I> checkbox indicates whether the window should update when you
+        <P>The <img src="icon.navigate.in"> <I>Dynamic Update</I> toggle indicates whether the
-        change the location in the Code Browser.&nbsp; By default, the checkbox is selected. As you
+        window should update when you change the location in the Code Browser.&nbsp; By default, the
-        change your <A href="help/topics/CodeBrowserPlugin/CodeBrowser.htm#Location">location</A>
+        toggle is selected. As you change your
-        in the Code Browser, the window will be updated to show the info for the new
+        <A href="help/topics/CodeBrowserPlugin/CodeBrowser.htm#Location">location</A> in the Code
-        location.&nbsp; If you turn off the checkbox, the window does not update; the next time you
+        Browser, the window will be updated to show the info for the new location.&nbsp; If you turn
-        choose <I>Instruction Info</I>, a new tab is displayed in the <I>Instruction Info</I>
+        off the toggle, the window does not update; the next time you choose <I>Instruction
-        window.</P>
+        Info</I>, a new tab is displayed in the <I>Instruction Info</I> window.</P>
       </BLOCKQUOTE>
 
       <P class="providedbyplugin">&nbsp;</P>

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/processors/InstructionInfoProvider.java

@@ -15,12 +15,14 @@
  */
 package ghidra.app.plugin.core.processors;
 
-import java.awt.*;
+import java.awt.BorderLayout;
+import java.awt.Dimension;
 
 import javax.swing.*;
 import javax.swing.table.DefaultTableModel;
 
-import docking.widgets.checkbox.GCheckBox;
+import docking.action.ToggleDockingAction;
+import docking.action.builder.ToggleActionBuilder;
 import generic.theme.Gui;
 import ghidra.app.plugin.processors.sleigh.SleighDebugLogger;
 import ghidra.app.plugin.processors.sleigh.SleighDebugLogger.SleighDebugMode;
@@ -33,6 +35,7 @@ import ghidra.program.model.listing.*;
 import ghidra.program.util.InstructionUtils;
 import ghidra.util.HelpLocation;
 import ghidra.util.table.GhidraTable;
+import resources.Icons;
 
 /**
  * Component provider to show the instruction info.
@@ -48,7 +51,7 @@ class InstructionInfoProvider extends ComponentProviderAdapter implements Domain
 	private JTextArea instructionText;
 	private JTable opTable;
 
-	private JCheckBox dynamicUpdateCB;
+	private ToggleDockingAction dynamicUpdateAction;
 
 	private OperandModel operandModel;
 	private Address myAddr;
@@ -57,10 +60,11 @@ class InstructionInfoProvider extends ComponentProviderAdapter implements Domain
 		super(plugin.getTool(), "Instruction Info", plugin.getName());
 		this.plugin = plugin;
 
-		buildMainPanel(isDynamic);
+		buildMainPanel();
 		setTransient();
 		setWindowMenuGroup("Instruction Info");
 		addToTool();
+		createActions(isDynamic);
 	}
 
 	@Override
@@ -73,8 +77,18 @@ class InstructionInfoProvider extends ComponentProviderAdapter implements Domain
 		return new HelpLocation(plugin.getName(), "Show_Instruction_Info_Window");
 	}
 
+	private void createActions(boolean isDynamic) {
+		dynamicUpdateAction = new ToggleActionBuilder("Dynamic Update", plugin.getName())
+				.toolBarIcon(Icons.NAVIGATE_ON_INCOMING_EVENT_ICON)
+				.description("Update this panel with navigation")
+				.onAction(ctx -> dynamicStateChanged())
+				.selected(isDynamic)
+				.buildAndInstallLocal(this);
+		dynamicStateChanged();
+	}
+
 	boolean dynamicUpdateSelected() {
-		return dynamicUpdateCB.isSelected();
+		return dynamicUpdateAction.isSelected();
 	}
 
 	/**
@@ -101,7 +115,7 @@ class InstructionInfoProvider extends ComponentProviderAdapter implements Domain
 	 *
 	 * @return JPanel the completed <CODE>Main Panel</CODE>
 	 */
-	protected JPanel buildMainPanel(boolean isDynamic) {
+	protected JPanel buildMainPanel() {
 
 		mainPanel = new JPanel(new BorderLayout());
 
@@ -120,11 +134,6 @@ class InstructionInfoProvider extends ComponentProviderAdapter implements Domain
 		pane.setResizeWeight(.25);
 		mainPanel.add(pane, BorderLayout.CENTER);
 
-		dynamicUpdateCB = new GCheckBox("Dynamic Update", isDynamic);
-		dynamicUpdateCB.setAlignmentX(Component.CENTER_ALIGNMENT);
-		dynamicUpdateCB.addItemListener(e -> dynamicStateChanged());
-
-		mainPanel.add(dynamicUpdateCB, BorderLayout.SOUTH);
 		mainPanel.validate();
 
 		return mainPanel;
@@ -194,8 +203,9 @@ class InstructionInfoProvider extends ComponentProviderAdapter implements Domain
 		setAddress(myAddr);
 	}
 
-	public void setNonDynamic() {
+	public void setDynamic(boolean dynamic) {
-		dynamicUpdateCB.setSelected(false);
+		dynamicUpdateAction.setSelected(dynamic);
+		dynamicStateChanged();
 	}
 
 	public Program getProgram() {
@@ -225,6 +235,7 @@ class InstructionInfoProvider extends ComponentProviderAdapter implements Domain
 
 		/**
 		 * Returns the number of columns in this data table.
+		 * 
 		 * @return the number of columns in the model
 		 */
 		@Override
@@ -234,10 +245,10 @@ class InstructionInfoProvider extends ComponentProviderAdapter implements Domain
 
 		/**
 		 * Returns the column name.
-		 * @return a name for this column using the string value of the
+		 * 
-		 * appropriate member in <I>columnIdentfiers</I>. If <I>columnIdentfiers</I>
+		 * @return a name for this column using the string value of the appropriate member in
-		 * is null or does not have and entry for this index return the default
+		 *         <I>columnIdentfiers</I>. If <I>columnIdentfiers</I> is null or does not have and
-		 * name provided by the superclass.
+		 *         entry for this index return the default name provided by the superclass.
 		 */
 		@Override
 		public String getColumnName(int column) {
@@ -249,6 +260,7 @@ class InstructionInfoProvider extends ComponentProviderAdapter implements Domain
 
 		/**
 		 * Returns the number of rows in this data table.
+		 * 
 		 * @return the number of rows in the model
 		 */
 		@Override
@@ -257,14 +269,12 @@ class InstructionInfoProvider extends ComponentProviderAdapter implements Domain
 		}
 
 		/**
-		 * Returns an attribute value for the cell at <I>row</I>
+		 * Returns an attribute value for the cell at <I>row</I> and <I>column</I>.
-		 * and <I>column</I>.
 		 *
 		 * @param row the row whose value is to be looked up
 		 * @param column the column whose value is to be looked up
 		 * @return the value Object at the specified cell
-		 * @exception  ArrayIndexOutOfBoundsException  if an invalid row or
+		 * @exception ArrayIndexOutOfBoundsException if an invalid row or column was given.
-		 *               column was given.
 		 */
 		@Override
 		public Object getValueAt(int row, int column) {
@@ -272,56 +282,39 @@ class InstructionInfoProvider extends ComponentProviderAdapter implements Domain
 				return null;
 			}
 			if (column == 0) {
-				switch (row) {
+				return switch (row) {
-					case 0:
+					case 0 -> "Operand";
-						return "Operand";
+					case 1 -> "Labeled";
-					case 1:
+					case 2 -> "Type";
-						return "Labeled";
+					case 3 -> "Scalar";
-					case 2:
+					case 4 -> "Address";
-						return "Type";
+					case 5 -> "Register";
-					case 3:
+					case 6 -> "Op-Objects";
-						return "Scalar";
+					case 7 -> "Operand Mask";
-					case 4:
+					case 8 -> "Masked Value";
-						return "Address";
+					default -> "";
-					case 5:
+				};
-						return "Register";
-					case 6:
-						return "Op-Objects";
-					case 7:
-						return "Operand Mask";
-					case 8:
-						return "Masked Value";
-				}
 			}
 			int opIndex = column - 1;
 			if (opIndex >= instruction.getNumOperands()) {
 				return "";
 			}
-			switch (row) {
+			return switch (row) {
-				case 0:
+				case 0 -> instruction.getDefaultOperandRepresentation(opIndex);
-					return instruction.getDefaultOperandRepresentation(opIndex);
+				case 1 -> CodeUnitFormat.DEFAULT.getOperandRepresentationList(instruction, opIndex);
-				case 1:
+				case 2 -> OperandType.toString(instruction.getOperandType(opIndex));
-					return CodeUnitFormat.DEFAULT.getOperandRepresentationList(instruction,
+				case 3 -> instruction.getScalar(opIndex);
-						opIndex);
+				case 4 -> {
-				case 2:
-					return OperandType.toString(instruction.getOperandType(opIndex));
-				case 3:
-					return instruction.getScalar(opIndex);
-				case 4:
 					Address addr = instruction.getAddress(opIndex);
-					return addr != null ? addr.toString(true) : "";
+					yield addr != null ? addr.toString(true) : "";
-				case 5:
-					return instruction.getRegister(opIndex);
-				case 6:
-					return getString(
-						InstructionUtils.getFormatedOperandObjects(instruction, opIndex));
-				case 7:
-					return debug != null ? debug.getFormattedInstructionMask(opIndex) : null;
-				case 8:
-					return debug != null ? debug.getFormattedMaskedValue(opIndex) : null;
 				}
-
+				case 5 -> instruction.getRegister(opIndex);
-			return "";
+				case 6 -> getString(
+					InstructionUtils.getFormatedOperandObjects(instruction, opIndex));
+				case 7 -> debug != null ? debug.getFormattedInstructionMask(opIndex) : null;
+				case 8 -> debug != null ? debug.getFormattedMaskedValue(opIndex) : null;
+				default -> "";
+			};
 		}
 
 		@Override

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/processors/ShowInstructionInfoPlugin.java

@@ -344,8 +344,8 @@ public class ShowInstructionInfoPlugin extends ProgramPlugin {
 	}
 
 	/**
-	 * Subclass should override this method if it is interested in
+	 * Subclass should override this method if it is interested in program location events.
-	 * program location events.
+	 * 
 	 * @param loc location could be null
 	 */
 	@Override
@@ -463,7 +463,7 @@ public class ShowInstructionInfoPlugin extends ProgramPlugin {
 		}
 		else if (provider != connectedProvider && isDynamic) {
 			if (connectedProvider != null) {
-				connectedProvider.setNonDynamic();
+				connectedProvider.setDynamic(false);
 			}
 			disconnectedProviders.remove(provider);
 			connectedProvider = provider;

Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/processors/ShowInstructionInfoPluginTest.java

@@ -22,7 +22,8 @@ import java.util.List;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
-import javax.swing.*;
+import javax.swing.JTable;
+import javax.swing.JTextArea;
 
 import org.junit.*;
 
@@ -150,6 +151,7 @@ public class ShowInstructionInfoPluginTest extends AbstractGhidraHeadedIntegrati
 		DockingActionIf infoAction = getAction(plugin, "Show Instruction Info");
 		// show the window
 		performAction(infoAction, cb.getProvider(), true);
+		InstructionInfoProvider provider = waitForComponentProvider(InstructionInfoProvider.class);
 
 		// make sure we are at an invalid Instruction
 		ListingActionContext context = getCurrentContext();
@@ -176,14 +178,8 @@ public class ShowInstructionInfoPluginTest extends AbstractGhidraHeadedIntegrati
 			componentProviderTablesHaveData());
 
 		// verify dynamic update has changed the window's contents
-		ComponentProvider componentProvider = getCurrentComponentProviderFromPlugin();
-		JComponent comp = componentProvider.getComponent();
-
-		final JCheckBox dynamicCheckBox = findComponent(comp, JCheckBox.class);
 		// make sure dynamic update is enabled
-		if (!dynamicCheckBox.isSelected()) {
+		runSwing(() -> provider.setDynamic(true));
-			runSwing(() -> dynamicCheckBox.doClick());
-		}
 
 		// change to another valid Instruction
 		currentInstruction = changeLocationToAddress("01000006");
@@ -200,7 +196,7 @@ public class ShowInstructionInfoPluginTest extends AbstractGhidraHeadedIntegrati
 		verifyAddressWithTableModels(currentInstruction.getMinAddress(), true, true);
 
 		// turn off dynamic update
-		runSwing(() -> dynamicCheckBox.doClick());
+		runSwing(() -> provider.setDynamic(false));
 
 		// change to another valid Instruction
 		currentInstruction = changeLocationToAddress("01000009");
@@ -223,7 +219,7 @@ public class ShowInstructionInfoPluginTest extends AbstractGhidraHeadedIntegrati
 		// place
 
 		// turn dynamic update back on
-		runSwing(() -> dynamicCheckBox.doClick());
+		runSwing(() -> provider.setDynamic(true));
 
 		// move to a valid location that has yet to be disassembled
 		currentInstruction = changeLocationToAddress("01000ffe");
@@ -269,13 +265,9 @@ public class ShowInstructionInfoPluginTest extends AbstractGhidraHeadedIntegrati
 		DockingActionIf infoAction = getAction(plugin, "Show Instruction Info");
 		// show the window
 		performAction(infoAction, cb.getProvider(), true);
+		InstructionInfoProvider provider = waitForComponentProvider(InstructionInfoProvider.class);
 
-		ComponentProvider componentProvider = getCurrentComponentProviderFromPlugin();
+		runSwing(() -> provider.setDynamic(false));
-		JComponent comp = componentProvider.getComponent();
-
-		final JCheckBox dynamicCheckBox = findComponent(comp, JCheckBox.class);
-		// turn off the checkbox
-		runSwing(() -> dynamicCheckBox.setSelected(false));
 
 		changeLocationToAddress("01000006");
 		performAction(infoAction, cb.getProvider(), true);
@@ -363,12 +355,10 @@ public class ShowInstructionInfoPluginTest extends AbstractGhidraHeadedIntegrati
 	}
 
 	/**
-	 * Moves the program location to the given address and returns the 
+	 * Moves the program location to the given address and returns the instruction at that location.
-	 * instruction at that location.
 	 * 
 	 * @param addressString The address location to move to.
-	 * @return The instruction at the new location or null if there is no
+	 * @return The instruction at the new location or null if there is no instruction.
-	 *         instruction.
 	 */
 	private Instruction changeLocationToAddress(String addressString) throws Exception {
 		CodeBrowserPlugin cbp = env.getPlugin(CodeBrowserPlugin.class);
@@ -395,18 +385,15 @@ public class ShowInstructionInfoPluginTest extends AbstractGhidraHeadedIntegrati
 	}
 
 	/**
-	 * Tests the addresses of the table models of the "Instruction Info" dialog.
+	 * Tests the addresses of the table models of the "Instruction Info" dialog. The method will
-	 * The method will fail the current test if the result is not as 
+	 * fail the current test if the result is not as expected by the caller of this method. For
-	 * expected by the caller of this method.  For example, if 
+	 * example, if {@code expectedSame} is true, then the method expects the values to be the same
-	 * {@code expectedSame} is true, then the method expects the values to
+	 * when compared with the given address and will fail if they are not. If {@code expectedSame}
-	 * be the same when compared with the given address and will fail if 
+	 * is false, then the method will fail if the test values are the same.
-	 * they are not.  If {@code expectedSame} is false, then the method will
-	 * fail if the test values are the same.
 	 * 
-	 * @param instructionAddress The address to compare against the address
+	 * @param instructionAddress The address to compare against the address stored in the table
-	 *        stored in the table model of the dialog.
+	 *            model of the dialog.
-	 * @param expectedSame True means a match is expected; false means a 
+	 * @param expectedSame True means a match is expected; false means a match is not expected.
-	 *        match is not expected.
 	 */
 	private void verifyAddressWithTableModels(Address instructionAddress, boolean fromConnected,
 			boolean expectedSame) {
@@ -447,8 +434,7 @@ public class ShowInstructionInfoPluginTest extends AbstractGhidraHeadedIntegrati
 	}
 
 	/**
-	 * A simple method to test that the tables of the "Instruction Info"
+	 * A simple method to test that the tables of the "Instruction Info" dialog contain data.
-	 * dialog contain data.
 	 * 
 	 * @return True if either of the tables have data.
 	 */

Ghidra/Test/IntegrationTest/src/screen/java/help/screenshot/ShowInstructionInfoPluginScreenShots.java

@@ -112,7 +112,9 @@ public class ShowInstructionInfoPluginScreenShots extends GhidraScreenShotGenera
 
 		performAction("Show Instruction Info", plugin.getName(), true);
 
-		captureProviderWindow("Instruction Info", 1200, 500);
+		ComponentProvider provider =
+			tool.getWindowManager().getComponentProvider("Instruction Info");
+		captureIsolatedProvider(provider, 1200, 510);
 
 //		finished("ShowInstructionInfoPlugin", "ShowInstructionInfo.png");
 	}