Merge remote-tracking branch 'origin/GP-5599-dragonmacher-shared-key-binding-fix--SQUASHED'

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/navigation/locationreferences/LocationReferencesPlugin.java

@@ -4,9 +4,9 @@
  * 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.
@@ -112,7 +112,7 @@ public class LocationReferencesPlugin extends Plugin
 		// only so the user can bind a key binding to it if they wish.
 		new ActionBuilder("Show Xrefs", getName())
 				.description("Show the Xrefs to the code unit containing the cursor")
-				.validContextWhen(context -> context instanceof ListingActionContext)
+				.withContext(ListingActionContext.class)
 				.helpLocation(new HelpLocation("CodeBrowserPlugin", "Show_Xrefs"))
 				.onAction(context -> showXrefs(context))
 				.buildAndInstall(tool);

Ghidra/Features/Base/src/main/java/ghidra/features/base/codecompare/listing/ListingCodeComparisonPanel.java

@@ -394,7 +394,7 @@ public class ListingCodeComparisonPanel
 				.toolBarIcon(NEXT_DIFF_ICON)
 				.toolBarGroup(DIFF_NAVIGATE_GROUP)
 				.keyBinding("ctrl alt N")
-				.validContextWhen(c -> isValidPanelContext(c))
+				.validWhen(c -> isValidPanelContext(c))
 				.enabledWhen(c -> isShowing() && listingDiff.hasCorrelation())
 				.onAction(c -> nextAreaDiff(true))
 				.build();
@@ -409,7 +409,7 @@ public class ListingCodeComparisonPanel
 				.toolBarIcon(PREVIOUS_DIFF_ICON)
 				.toolBarGroup(DIFF_NAVIGATE_GROUP)
 				.keyBinding("ctrl alt P")
-				.validContextWhen(c -> isValidPanelContext(c))
+				.validWhen(c -> isValidPanelContext(c))
 				.enabledWhen(c -> isShowing() && listingDiff.hasCorrelation())
 				.onAction(c -> nextAreaDiff(false))
 				.build();

Ghidra/Framework/Docking/src/main/java/docking/action/MultipleKeyAction.java

@@ -584,5 +584,10 @@ public class MultipleKeyAction extends DockingKeyBindingAction {
 			tool.setStatusInfo(message, true);
 			Toolkit.getDefaultToolkit().beep();
 		}
+
+		@Override
+		public String toString() {
+			return getClass().getSimpleName() + ": " + validActions;
+		}
 	}
 }

Ghidra/Framework/Docking/src/main/java/docking/action/builder/AbstractActionBuilder.java

@@ -570,6 +570,24 @@ public abstract class AbstractActionBuilder<T extends DockingActionIf, C extends
 		return self();
 	}
 
+	/**
+	 * @param predicate the predicate 
+	 * @return this builder (for chaining)
+	 * @deprecated use {@link #validWhen(Predicate)}
+	 */
+	@Deprecated(forRemoval = true, since = "11.4")
+	public B validContextWhen(Predicate<C> predicate) {
+		validContextPredicate = Objects.requireNonNull(predicate);
+
+		// automatic enablement management triggered, make sure there is a existing enablement 
+		// predicate. The default behavior of manual management interferes with automatic management.
+		if (enabledPredicate == null) {
+			enabledPredicate = ALWAYS_TRUE;
+		}
+
+		return self();
+	}
+
 	/**
 	 * Sets a predicate for dynamically determining if this action is valid for the current 
 	 * {@link ActionContext}.  See {@link DockingActionIf#isValidContext(ActionContext)}.
@@ -584,7 +602,7 @@ public abstract class AbstractActionBuilder<T extends DockingActionIf, C extends
 	 * validity for a given {@link ActionContext}
 	 * @return this builder (for chaining)
 	 */
-	public B validContextWhen(Predicate<C> predicate) {
+	public B validWhen(Predicate<C> predicate) {
 		validContextPredicate = Objects.requireNonNull(predicate);
 
 		// automatic enablement management triggered, make sure there is a existing enablement 

Ghidra/Framework/Docking/src/main/java/docking/actions/SharedStubKeyBindingAction.java

@@ -211,14 +211,20 @@ public class SharedStubKeyBindingAction extends DockingAction implements Options
 	}
 
 	private void setKeyBindingData(DockingActionIf action, ActionTrigger actionTrigger) {
-		KeyBindingData kbData = null;
+
+		KeyBindingData newKbData = null;
 		if (actionTrigger != null) {
-			kbData = new KeyBindingData(actionTrigger);
+			newKbData = new KeyBindingData(actionTrigger);
+		}
+
+		KeyBindingData currentKbData = action.getKeyBindingData();
+		if (Objects.equals(currentKbData, newKbData)) {
+			return;
 		}
 
 		// we use the 'unvalidated' call since this value is provided by the user--we assume
 		// that user input is correct; we only validate programmer input
-		action.setUnvalidatedKeyBindingData(kbData);
+		action.setUnvalidatedKeyBindingData(newKbData);
 	}
 
 	private ActionTrigger getActionTriggerFromOptions(ActionTrigger validatedTrigger) {
@@ -232,6 +238,27 @@ public class SharedStubKeyBindingAction extends DockingAction implements Options
 		return data.getActionTrigger();
 	}
 
+	/*
+	 * This can be called from ToolActions.optionsRebuilt().  In that case, the code only
+	 * loops over KeyBindingType.INDIVIDAL actions types (which this class is), but not 
+	 * KeyBindingType.SHARED actions, which the contained actions are.  In the case of options 
+	 * getting restored, 1) only this action gets updated, and 2),  there is no options changed 
+	 * event fired.  Thus, to handle options being restored, we have to override this method to make
+	 * sure we update out internal client actions.
+	 */
+	@Override
+	public void setUnvalidatedKeyBindingData(KeyBindingData newKeyBindingData) {
+
+		// update this shared key binding
+		super.setUnvalidatedKeyBindingData(newKeyBindingData);
+
+		// update the client keybindings
+		ActionTrigger newTrigger = getActionTrigger(newKeyBindingData);
+		for (DockingActionIf action : clientActions.keySet()) {
+			setKeyBindingData(action, newTrigger);
+		}
+	}
+
 	@Override
 	public void optionsChanged(ToolOptions options, String optionName, Object oldValue,
 			Object newValue) {
@@ -240,9 +267,11 @@ public class SharedStubKeyBindingAction extends DockingAction implements Options
 			return; // not my binding
 		}
 
+		// update this shared key binding
 		ActionTrigger newTrigger = (ActionTrigger) newValue;
 		setKeyBindingData(this, newTrigger);
 
+		// update the client keybindings
 		for (DockingActionIf action : clientActions.keySet()) {
 			setKeyBindingData(action, newTrigger);
 		}
@@ -275,6 +304,11 @@ public class SharedStubKeyBindingAction extends DockingAction implements Options
 		keyBindingOptions.removeOptionsChangeListener(this);
 	}
 
+	@Override
+	public String toString() {
+		return "Shared Stub Action: " + super.toString();
+	}
+
 	private static void logDifferentKeyBindingsWarnigMessage(DockingActionIf newAction,
 			DockingActionIf existingAction, ActionTrigger existingDefaultTrigger) {
 

Ghidra/Framework/Docking/src/test/java/docking/action/ActionBuilderTest.java

@@ -4,9 +4,9 @@
  * 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.
@@ -31,18 +31,18 @@ public class ActionBuilderTest {
 	@Test
 	public void testDescription() {
 		DockingAction action = new ActionBuilder("Test", "Test")
-			.description("foo")
-			.onAction(e -> actionCount++)
-			.build();
+				.description("foo")
+				.onAction(e -> actionCount++)
+				.build();
 		assertEquals("foo", action.getDescription());
 	}
 
 	@Test
 	public void testMenuPath() {
 		DockingAction action = new ActionBuilder("Test", "Test")
-			.menuPath("foo", "bar")
-			.onAction(e -> actionCount++)
-			.build();
+				.menuPath("foo", "bar")
+				.onAction(e -> actionCount++)
+				.build();
 
 		MenuData data = action.getMenuBarData();
 		assertEquals("foo->bar", data.getMenuPathAsString());
@@ -51,10 +51,10 @@ public class ActionBuilderTest {
 	@Test
 	public void testMenuGroup() {
 		DockingAction action = new ActionBuilder("Test", "Test")
-			.menuPath("foo", "bar")
-			.menuGroup("A", "B")
-			.onAction(e -> actionCount++)
-			.build();
+				.menuPath("foo", "bar")
+				.menuGroup("A", "B")
+				.onAction(e -> actionCount++)
+				.build();
 
 		MenuData data = action.getMenuBarData();
 		assertEquals("A", data.getMenuGroup());
@@ -64,10 +64,10 @@ public class ActionBuilderTest {
 	@Test
 	public void testMenuIcon() {
 		DockingAction action = new ActionBuilder("Test", "Test")
-			.menuPath("foo", "bar")
-			.menuIcon(Icons.ADD_ICON)
-			.onAction(e -> actionCount++)
-			.build();
+				.menuPath("foo", "bar")
+				.menuIcon(Icons.ADD_ICON)
+				.onAction(e -> actionCount++)
+				.build();
 
 		MenuData data = action.getMenuBarData();
 		assertEquals(Icons.ADD_ICON, data.getMenuIcon());
@@ -76,10 +76,10 @@ public class ActionBuilderTest {
 	@Test
 	public void testMenuMnemonic() {
 		DockingAction action = new ActionBuilder("Test", "Test")
-			.menuPath("foo", "bar")
-			.menuMnemonic(5)
-			.onAction(e -> actionCount++)
-			.build();
+				.menuPath("foo", "bar")
+				.menuMnemonic(5)
+				.onAction(e -> actionCount++)
+				.build();
 
 		MenuData data = action.getMenuBarData();
 		assertEquals(5, data.getMnemonic());
@@ -88,9 +88,9 @@ public class ActionBuilderTest {
 	@Test
 	public void testPopupPath() {
 		DockingAction action = new ActionBuilder("Test", "Test")
-			.popupMenuPath("foo", "bar")
-			.onAction(e -> actionCount++)
-			.build();
+				.popupMenuPath("foo", "bar")
+				.onAction(e -> actionCount++)
+				.build();
 
 		MenuData data = action.getPopupMenuData();
 		assertEquals("foo->bar", data.getMenuPathAsString());
@@ -99,10 +99,10 @@ public class ActionBuilderTest {
 	@Test
 	public void testPopupGroup() {
 		DockingAction action = new ActionBuilder("Test", "Test")
-			.popupMenuPath("foo", "bar")
-			.popupMenuGroup("A", "B")
-			.onAction(e -> actionCount++)
-			.build();
+				.popupMenuPath("foo", "bar")
+				.popupMenuGroup("A", "B")
+				.onAction(e -> actionCount++)
+				.build();
 
 		MenuData data = action.getPopupMenuData();
 		assertEquals("A", data.getMenuGroup());
@@ -112,10 +112,10 @@ public class ActionBuilderTest {
 	@Test
 	public void testPopupIcon() {
 		DockingAction action = new ActionBuilder("Test", "Test")
-			.popupMenuPath("foo", "bar")
-			.popupMenuIcon(Icons.ADD_ICON)
-			.onAction(e -> actionCount++)
-			.build();
+				.popupMenuPath("foo", "bar")
+				.popupMenuIcon(Icons.ADD_ICON)
+				.onAction(e -> actionCount++)
+				.build();
 
 		MenuData data = action.getPopupMenuData();
 		assertEquals(Icons.ADD_ICON, data.getMenuIcon());
@@ -124,9 +124,9 @@ public class ActionBuilderTest {
 	@Test
 	public void testToolbarIcon() {
 		DockingAction action = new ActionBuilder("Test", "Test")
-			.toolBarIcon(Icons.ADD_ICON)
-			.onAction(e -> actionCount++)
-			.build();
+				.toolBarIcon(Icons.ADD_ICON)
+				.onAction(e -> actionCount++)
+				.build();
 
 		ToolBarData data = action.getToolBarData();
 		assertEquals(Icons.ADD_ICON, data.getIcon());
@@ -135,10 +135,10 @@ public class ActionBuilderTest {
 	@Test
 	public void testToolbarGroup() {
 		DockingAction action = new ActionBuilder("Test", "Test")
-			.toolBarIcon(Icons.ADD_ICON)
-			.toolBarGroup("A", "B")
-			.onAction(e -> actionCount++)
-			.build();
+				.toolBarIcon(Icons.ADD_ICON)
+				.toolBarGroup("A", "B")
+				.onAction(e -> actionCount++)
+				.build();
 
 		ToolBarData data = action.getToolBarData();
 		assertEquals("A", data.getToolBarGroup());
@@ -148,9 +148,9 @@ public class ActionBuilderTest {
 	@Test
 	public void testKeyBindingKeyStroke() {
 		DockingAction action = new ActionBuilder("Test", "Test")
-			.keyBinding(KeyStroke.getKeyStroke("A"))
-			.onAction(e -> actionCount++)
-			.build();
+				.keyBinding(KeyStroke.getKeyStroke("A"))
+				.onAction(e -> actionCount++)
+				.build();
 
 		assertEquals(KeyStroke.getKeyStroke("A"), action.getKeyBinding());
 	}
@@ -158,9 +158,9 @@ public class ActionBuilderTest {
 	@Test
 	public void testKeyBindingKeyString() {
 		DockingAction action = new ActionBuilder("Test", "Test")
-			.keyBinding("ALT A")
-			.onAction(e -> actionCount++)
-			.build();
+				.keyBinding("ALT A")
+				.onAction(e -> actionCount++)
+				.build();
 
 		assertEquals(KeyStroke.getKeyStroke("alt pressed A"), action.getKeyBinding());
 	}
@@ -168,8 +168,8 @@ public class ActionBuilderTest {
 	@Test
 	public void testOnAction() {
 		DockingAction action = new ActionBuilder("Test", "Test")
-			.onAction(e -> actionCount = 6)
-			.build();
+				.onAction(e -> actionCount = 6)
+				.build();
 
 		assertEquals(0, actionCount);
 		action.actionPerformed(new DefaultActionContext());
@@ -179,15 +179,15 @@ public class ActionBuilderTest {
 	@Test
 	public void testEnabled() {
 		DockingAction action = new ActionBuilder("Test", "Test")
-			.enabled(true)
-			.onAction(e -> actionCount++)
-			.build();
+				.enabled(true)
+				.onAction(e -> actionCount++)
+				.build();
 		assertTrue(action.isEnabled());
 
 		action = new ActionBuilder("Test", "Test")
-			.enabled(false)
-			.onAction(e -> actionCount++)
-			.build();
+				.enabled(false)
+				.onAction(e -> actionCount++)
+				.build();
 		assertFalse(action.isEnabled());
 
 	}
@@ -195,9 +195,9 @@ public class ActionBuilderTest {
 	@Test
 	public void testEnabledWhen() {
 		DockingAction action = new ActionBuilder("Test", "Test")
-			.enabledWhen(c -> c.getContextObject() == this)
-			.onAction(e -> actionCount++)
-			.build();
+				.enabledWhen(c -> c.getContextObject() == this)
+				.onAction(e -> actionCount++)
+				.build();
 
 		assertTrue(action.isEnabledForContext(new DefaultActionContext(null, this, null)));
 		assertFalse(action.isEnabledForContext(new DefaultActionContext()));
@@ -206,9 +206,9 @@ public class ActionBuilderTest {
 	@Test
 	public void testValidContextWhen() {
 		DockingAction action = new ActionBuilder("Test", "Test")
-			.validContextWhen(c -> c.getContextObject() == this)
-			.onAction(e -> actionCount++)
-			.build();
+				.validWhen(c -> c.getContextObject() == this)
+				.onAction(e -> actionCount++)
+				.build();
 
 		assertTrue(action.isValidContext(new DefaultActionContext(null, this, null)));
 		assertFalse(action.isValidContext(new DefaultActionContext()));
@@ -217,9 +217,9 @@ public class ActionBuilderTest {
 	@Test
 	public void testPopupWhen() {
 		DockingAction action = new ActionBuilder("Test", "Test")
-			.popupWhen(c -> c.getContextObject() == this)
-			.onAction(e -> actionCount++)
-			.build();
+				.popupWhen(c -> c.getContextObject() == this)
+				.onAction(e -> actionCount++)
+				.build();
 
 		assertTrue(action.isAddToPopup(new DefaultActionContext(null, this, null)));
 		assertFalse(action.isAddToPopup(new DefaultActionContext()));
@@ -228,10 +228,10 @@ public class ActionBuilderTest {
 	@Test
 	public void testWithContext() {
 		DockingAction action = new ActionBuilder("Test", "Test")
-			.withContext(FooActionContext.class)
-			.enabledWhen(c -> c.foo())
-			.onAction(e -> actionCount++)
-			.build();
+				.withContext(FooActionContext.class)
+				.enabledWhen(c -> c.foo())
+				.onAction(e -> actionCount++)
+				.build();
 
 		assertFalse(action.isEnabledForContext(new DefaultActionContext()));
 		assertTrue(action.isEnabledForContext(new FooActionContext()));
@@ -240,9 +240,9 @@ public class ActionBuilderTest {
 	@Test
 	public void testManualEnablement() {
 		DockingAction action = new ActionBuilder("Test", "Test")
-			.onAction(e -> actionCount++)
-			.enabled(false)
-			.build();
+				.onAction(e -> actionCount++)
+				.enabled(false)
+				.build();
 
 		assertFalse(action.isEnabledForContext(new DefaultActionContext()));
 		action.setEnabled(true);

Ghidra/Framework/Project/src/main/java/ghidra/framework/main/datatree/ClearCutAction.java

@@ -4,9 +4,9 @@
  * 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.
@@ -17,6 +17,7 @@ package ghidra.framework.main.datatree;
 
 import java.awt.event.KeyEvent;
 
+import docking.ActionContext;
 import docking.action.KeyBindingData;
 import ghidra.framework.main.datatable.ProjectTreeAction;
 
@@ -30,10 +31,18 @@ public class ClearCutAction extends ProjectTreeAction {
 	}
 
 	@Override
-	public boolean isEnabledForContext(FrontEndProjectTreeContext context) {
+	public boolean isValidContext(ActionContext context) {
 		return DataTreeClipboardUtils.isCuttablePresent();
 	}
 
+	@Override
+	public boolean isEnabledForContext(FrontEndProjectTreeContext context) {
+		// If we are valid, then we are enabled (see isValidContext()).  Most actions are always 
+		// valid, but only sometimes enabled.  We use the valid check to remove ourselves completely
+		// from the workflow.  But, if we are valid, then we are also enabled.
+		return true;
+	}
+
 	@Override
 	public void actionPerformed(FrontEndProjectTreeContext context) {
 		DataTreeClipboardUtils.clearCuttables();