GP-6450 - Updated Alt key binding handling to add an option for combing

Alt and AltGraph

Ghidra/Features/Base/src/main/help/help/topics/Tool/ToolOptions_Dialog.htm

@@ -456,6 +456,15 @@
                 automations to provide visual feedback that something is happening, such as
                 launching a tool.</TD>
               </TR>
+			  
+			  <TR>
+			    <TD valign="top" width="200" align="left">Use Combined Alt Keys</TD>
+
+			    <TD valign="top" align="left">When selected, any keybding created that uses the Alt
+					key will work using the left and right Alt keys.  When unselected, key bindings
+					using the Alt key will only work with the left Alt on some systems, such as on
+					Windows.</TD>
+			  </TR>
               
             </TBODY>
           </TABLE>

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/analysis/AutoAnalysisPlugin.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,6 +31,7 @@ import ghidra.app.events.*;
 import ghidra.app.plugin.PluginCategoryNames;
 import ghidra.app.services.Analyzer;
 import ghidra.app.util.importer.MessageLog;
+import ghidra.framework.model.DomainFile;
 import ghidra.framework.options.OptionType;
 import ghidra.framework.options.Options;
 import ghidra.framework.plugintool.*;
@@ -133,13 +134,21 @@ public class AutoAnalysisPlugin extends Plugin implements AutoAnalysisManagerLis
 	}
 
 	private void updateActionName(ActionContext context) {
-		String programName = "";
-		if (context instanceof ListingActionContext) {
-			ListingActionContext listingContext = (ListingActionContext) context;
-			programName = listingContext.getProgram().getDomainFile().getName();
+		if (!(context instanceof ListingActionContext lac)) {
+			return;
 		}
+
+		Program p = lac.getProgram();
+		DomainFile df = p.getDomainFile();
+		String fileName = df.getName();
+		String programName = "'" + fileName + "'";
+
 		MenuData menuBarData = autoAnalyzeAction.getMenuBarData();
-		menuBarData.setMenuItemName("&Auto Analyze '" + programName + "'...");
+		String currentName = menuBarData.getMenuItemName();
+		String newName = "Auto Analyze " + programName + "...";
+		if (!currentName.equals(newName)) {
+			menuBarData.setMenuItemName("&" + newName);
+		}
 	}
 
 	private void analyzeCallback(ActionContext context) {

Ghidra/Framework/Docking/src/main/java/docking/DockingUtils.java

@@ -126,6 +126,8 @@ public class DockingUtils {
 
 	private static boolean globalTooltipsEnabled = true;
 
+	private static boolean useCombinedAltKeysEnabled;
+
 	public static JSeparator createToolbarSeparator() {
 		Dimension sepDim = new Dimension(2, ICON_SIZE + 2);
 		JSeparator separator = new JSeparator(SwingConstants.VERTICAL);
@@ -366,6 +368,16 @@ public class DockingUtils {
 		Swing.runLater(() -> ToolTipManager.sharedInstance().setEnabled(enabled));
 	}
 
+	/**
+	 * Not meant for public consumption.   This is for application code to control how key bindings
+	 * that use the Alt key get mapped.  When true, a key binding that uses the Alt key will get 
+	 * mapped to the left and right alt keys. 
+	 * @param enabled true if enabled
+	 */
+	public static void setCombinedAltKeysEnabled(boolean enabled) {
+		useCombinedAltKeysEnabled = enabled;
+	}
+
 	/** 
 	 * Note: calling this method has no effect
 	 * @param enabled true if enabled; false prevents all Java tooltips
@@ -389,6 +401,15 @@ public class DockingUtils {
 		return globalTooltipsEnabled;
 	}
 
+	/**
+	 * True if the application should map Alt key binding usage to the left and right key. 
+	 * @return true if the application should map Alt key binding usage to the left and right key.
+	 * @see #setCombinedAltKeysEnabled(boolean)
+	 */
+	public static boolean isCombineAltKeysEnabled() {
+		return useCombinedAltKeysEnabled;
+	}
+
 	/** Hides any open tooltip window */
 	public static void hideTipWindow() {
 

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

@@ -153,19 +153,34 @@ public class KeyBindingsManager implements PropertyChangeListener {
 	private void fixupAltGraphKeyStrokeMapping(ComponentProvider provider, DockingActionIf action,
 			KeyStroke keyStroke) {
 
+		KeyStroke altGraphKs = maybeGenerateAltGraphKeyStroke(keyStroke);
+		if (altGraphKs != null) {
+			doAddKeyBinding(provider, action, altGraphKs, keyStroke);
+		}
+	}
+
+	/**
+	 * Some operating systems allow the left and right Alt keys to be mapped separately.  Some users
+	 * find it convenient to be able to use both Alt keys for a single key binding.
+	 * @param keyStroke the key stroke
+	 * @return a new key stroke that uses the AltGraph key or null if not appropriate
+	 */
+	private KeyStroke maybeGenerateAltGraphKeyStroke(KeyStroke keyStroke) {
+		if (!DockingUtils.isCombineAltKeysEnabled()) {
+			return null;
+		}
+
 		// special case
 		int modifiers = keyStroke.getModifiers();
 		if ((modifiers & InputEvent.ALT_DOWN_MASK) == InputEvent.ALT_DOWN_MASK) {
-			//
-			// Also register the 'Alt' binding with the 'Alt Graph' mask.  This fixes the but
-			// on Windows (https://bugs.openjdk.java.net/browse/JDK-8194873)
-			// that have different key codes for the left and right Alt keys.
-			//
+			// Also register the Alt binding with the 'Alt Graph' mask.  Some operating systems 
+			// allow the left and right Alt keys to be mapped separately.  Some users find it 
+			// convenient to be able to use both Alt keys for a single key binding.
 			modifiers |= InputEvent.ALT_GRAPH_DOWN_MASK;
-			KeyStroke updateKeyStroke =
-				KeyStroke.getKeyStroke(keyStroke.getKeyCode(), modifiers, false);
-			doAddKeyBinding(provider, action, updateKeyStroke, keyStroke);
+			return KeyStroke.getKeyStroke(keyStroke.getKeyCode(), modifiers, false);
 		}
+
+		return null;
 	}
 
 	private void doAddKeyBinding(ComponentProvider provider, DockingActionIf action,
@@ -230,6 +245,17 @@ public class KeyBindingsManager implements PropertyChangeListener {
 			return;
 		}
 
+		removeKeyBindingFromCache(action, keyStroke);
+
+		// also remove any secondarily mapped Alt key stroke
+		KeyStroke altGraphKs = maybeGenerateAltGraphKeyStroke(keyStroke);
+		if (altGraphKs != null) {
+			removeKeyBindingFromCache(action, altGraphKs);
+		}
+	}
+
+	private void removeKeyBindingFromCache(DockingActionIf action, KeyStroke keyStroke) {
+
 		DockingKeyBindingAction existingAction = dockingKeyMap.get(keyStroke);
 		if (existingAction == null) {
 			return;
@@ -238,9 +264,7 @@ public class KeyBindingsManager implements PropertyChangeListener {
 		if (existingAction instanceof SystemKeyBindingAction) {
 			dockingKeyMap.remove(keyStroke);
 		}
-		else if (existingAction instanceof MultipleKeyAction) {
-
-			MultipleKeyAction mkAction = (MultipleKeyAction) existingAction;
+		else if (existingAction instanceof MultipleKeyAction mkAction) {
 			mkAction.removeAction(action);
 			if (mkAction.isEmpty()) {
 				dockingKeyMap.remove(keyStroke);

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

@@ -15,7 +15,7 @@
  */
 package docking.actions;
 
-import static org.apache.commons.lang3.StringUtils.*;
+import static org.apache.commons.lang3.Strings.*;
 
 import java.awt.Component;
 import java.awt.KeyboardFocusManager;
@@ -674,19 +674,19 @@ public class KeyBindingUtils {
 		StringBuilder buffy = new StringBuilder();
 		if (isShift(modifiers)) {
 			buffy.insert(0, SHIFT + MODIFIER_SEPARATOR);
-			keyString = removeIgnoreCase(keyString, SHIFT);
+			keyString = remove(keyString, SHIFT);
 		}
 		if (isAlt(modifiers)) {
 			buffy.insert(0, ALT + MODIFIER_SEPARATOR);
-			keyString = removeIgnoreCase(keyString, ALT);
+			keyString = remove(keyString, ALT);
 		}
 		if (isControl(modifiers)) {
 			buffy.insert(0, CTRL + MODIFIER_SEPARATOR);
-			keyString = removeIgnoreCase(keyString, CONTROL);
+			keyString = remove(keyString, CONTROL);
 		}
 		if (isMeta(modifiers)) {
 			buffy.insert(0, META + MODIFIER_SEPARATOR);
-			keyString = removeIgnoreCase(keyString, META);
+			keyString = remove(keyString, META);
 		}
 		buffy.append(keyString);
 
@@ -698,7 +698,15 @@ public class KeyBindingUtils {
 	}
 
 	private static int indexOf(String source, String search, int offset) {
-		return StringUtils.indexOfIgnoreCase(source, search, offset);
+		return CI.indexOf(source, search, offset);
+	}
+
+	private static int indexOf(String source, String search) {
+		return CI.indexOf(source, search);
+	}
+
+	private static String remove(String source, String toRemove) {
+		return CI.remove(source, toRemove);
 	}
 
 	// ignore the deprecated; remove when we are confident that all tool actions no longer use the
@@ -767,33 +775,33 @@ public class KeyBindingUtils {
 		StringBuilder buffy = new StringBuilder();
 		for (Iterator<String> iterator = pieces.iterator(); iterator.hasNext();) {
 			String piece = iterator.next();
-			if (indexOfIgnoreCase(piece, SHIFT) != -1) {
+			if (indexOf(piece, SHIFT) != -1) {
 				buffy.append("shift ");
 				iterator.remove();
 			}
-			else if (indexOfIgnoreCase(piece, CTRL) != -1) {
+			else if (indexOf(piece, CTRL) != -1) {
 				buffy.append("ctrl ");
 				iterator.remove();
 			}
-			else if (indexOfIgnoreCase(piece, CONTROL) != -1) {
+			else if (indexOf(piece, CONTROL) != -1) {
 				buffy.append("ctrl ");
 				iterator.remove();
 			}
-			else if (indexOfIgnoreCase(piece, ALT) != -1) {
+			else if (indexOf(piece, ALT) != -1) {
 				buffy.append("alt ");
 				iterator.remove();
 			}
-			else if (indexOfIgnoreCase(piece, META) != -1) {
+			else if (indexOf(piece, META) != -1) {
 				buffy.append("meta ");
 				iterator.remove();
 			}
-			else if (indexOfIgnoreCase(piece, PRESSED) != -1) {
+			else if (indexOf(piece, PRESSED) != -1) {
 				iterator.remove();
 			}
-			else if (indexOfIgnoreCase(piece, TYPED) != -1) {
+			else if (indexOf(piece, TYPED) != -1) {
 				iterator.remove();
 			}
-			else if (indexOfIgnoreCase(piece, RELEASED) != -1) {
+			else if (indexOf(piece, RELEASED) != -1) {
 				iterator.remove();
 			}
 

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

@@ -394,7 +394,7 @@ public class ToolActions implements DockingToolActions, PropertyChangeListener {
 	}
 
 	/*
-	 * An odd method that really shoulnd't be on the interface.  This is a call that allows the 
+	 * An odd method that really shouldn't be on the interface.  This is a call that allows the 
 	 * framework to signal that the ToolOptions have been rebuilt, such as when restoring from xml.
 	 * During a rebuild, ToolOptions does not send out events, so this class does not get any of the
 	 * values from the new options.  This method tells us to get the new version of the options from

Ghidra/Framework/Project/src/main/java/ghidra/framework/main/FrontEndTool.java

@@ -91,6 +91,7 @@ public class FrontEndTool extends PluginTool implements OptionsChangeListener {
 	public static final String DEFAULT_TOOL_LAUNCH_MODE = "Default Tool Launch Mode";
 	public static final String AUTOMATICALLY_SAVE_TOOLS = "Automatically Save Tools";
 	private static final String USE_ALERT_ANIMATION_OPTION_NAME = "Use Notification Animation";
+	private static final String USE_COMBINED_ALT_GRAPH_OPTION_NAME = "Use Combined Alt Keys";
 	private static final String SHOW_TOOLTIPS_OPTION_NAME = "Show Tooltips";
 	private static final String BLINKING_CURSORS_OPTION_NAME = "Allow Blinking Cursors";
 
@@ -216,7 +217,7 @@ public class FrontEndTool extends PluginTool implements OptionsChangeListener {
 			return;
 		}
 
-		GhidraToolTemplate template = new GhidraToolTemplate((Element) root.getChildren().get(0),
+		GhidraToolTemplate template = new GhidraToolTemplate(root.getChildren().get(0),
 			TOOL_FILE.getAbsolutePath());
 		refresh(template);
 	}
@@ -349,6 +350,10 @@ public class FrontEndTool extends PluginTool implements OptionsChangeListener {
 		options.registerOption(USE_ALERT_ANIMATION_OPTION_NAME, true, help,
 			"Signals that user notifications should be animated.  This makes notifications more " +
 				"distinguishable.");
+		options.registerOption(USE_COMBINED_ALT_GRAPH_OPTION_NAME, true, help,
+			"Signals to have both right and left Alt keys be usable for key bindings that use the " +
+				"Alt key.");
+
 		options.registerOption(SHOW_TOOLTIPS_OPTION_NAME, true, help,
 			"Controls the display of tooltip popup windows.");
 		options.registerOption(ENABLE_COMPRESSED_DATABUFFER_OUTPUT, false, help,
@@ -369,6 +374,9 @@ public class FrontEndTool extends PluginTool implements OptionsChangeListener {
 		boolean animationEnabled = options.getBoolean(USE_ALERT_ANIMATION_OPTION_NAME, true);
 		AnimationUtils.setAnimationEnabled(animationEnabled);
 
+		boolean combineAltKeys = options.getBoolean(USE_COMBINED_ALT_GRAPH_OPTION_NAME, true);
+		DockingUtils.setCombinedAltKeysEnabled(combineAltKeys);
+
 		boolean showToolTips = options.getBoolean(SHOW_TOOLTIPS_OPTION_NAME, true);
 		DockingUtils.setGlobalTooltipEnabledOption(showToolTips);
 
@@ -396,6 +404,9 @@ public class FrontEndTool extends PluginTool implements OptionsChangeListener {
 		else if (USE_ALERT_ANIMATION_OPTION_NAME.equals(optionName)) {
 			AnimationUtils.setAnimationEnabled((Boolean) newValue);
 		}
+		else if (USE_COMBINED_ALT_GRAPH_OPTION_NAME.equals(optionName)) {
+			DockingUtils.setCombinedAltKeysEnabled((Boolean) newValue);
+		}
 		else if (SHOW_TOOLTIPS_OPTION_NAME.equals(optionName)) {
 			DockingUtils.setGlobalTooltipEnabledOption((Boolean) newValue);
 		}