mirror of
https://github.com/NationalSecurityAgency/ghidra.git
synced 2025-10-05 10:49:34 +02:00
GT-2869 - Key Bindings - review fixes
This commit is contained in:
parent
2de5c40cd6
commit
f8bd49b4be
12 changed files with 84 additions and 204 deletions
|
@ -834,12 +834,6 @@ public class CallTreeProvider extends ComponentProviderAdapter implements Domain
|
|||
currentProgram = null;
|
||||
}
|
||||
|
||||
tool.removeLocalAction(this, recurseDepthAction);
|
||||
tool.removeLocalAction(this, refreshAction);
|
||||
tool.removeLocalAction(this, filterDuplicates);
|
||||
tool.removeLocalAction(this, navigationOutgoingAction);
|
||||
tool.removeLocalAction(this, navigateIncomingToggleAction);
|
||||
|
||||
recurseDepthAction.dispose();
|
||||
refreshAction.dispose();
|
||||
filterDuplicates.dispose();
|
||||
|
|
|
@ -206,11 +206,6 @@ public abstract class CompositeEditorProvider extends ComponentProviderAdapter
|
|||
|
||||
@Override
|
||||
public void dispose() {
|
||||
CompositeEditorTableAction[] allActions = actionMgr.getAllActions();
|
||||
for (CompositeEditorTableAction allAction : allActions) {
|
||||
tool.removeLocalAction(this, allAction);
|
||||
}
|
||||
tool.showComponentProvider(this, false);
|
||||
tool.removeComponentProvider(this);
|
||||
for (EditorListener el : listeners) {
|
||||
el.closed(this);
|
||||
|
|
|
@ -38,7 +38,8 @@ import ghidra.program.model.address.AddressSet;
|
|||
import ghidra.program.model.listing.Program;
|
||||
import ghidra.program.util.ProgramLocation;
|
||||
import ghidra.util.HelpLocation;
|
||||
import ghidra.util.table.*;
|
||||
import ghidra.util.table.GhidraTable;
|
||||
import ghidra.util.table.SelectionNavigationAction;
|
||||
import ghidra.util.table.actions.DeleteTableRowAction;
|
||||
import ghidra.util.task.SwingUpdateManager;
|
||||
import resources.Icons;
|
||||
|
@ -208,11 +209,6 @@ public class LocationReferencesProvider extends ComponentProviderAdapter
|
|||
|
||||
tool.removeComponentProvider(this);
|
||||
|
||||
tool.removeLocalAction(this, homeAction);
|
||||
tool.removeLocalAction(this, refreshAction);
|
||||
tool.removeLocalAction(this, selectionAction);
|
||||
tool.removeLocalAction(this, highlightAction);
|
||||
|
||||
homeAction.dispose();
|
||||
refreshAction.dispose();
|
||||
highlightAction.dispose();
|
||||
|
|
|
@ -93,7 +93,6 @@ public class PropertyManagerProvider extends ComponentProviderAdapter {
|
|||
}
|
||||
|
||||
void dispose() {
|
||||
tool.removeLocalAction(this, deleteAction);
|
||||
tool.removeComponentProvider(this);
|
||||
}
|
||||
|
||||
|
|
|
@ -15,7 +15,7 @@
|
|||
*/
|
||||
package docking;
|
||||
|
||||
import java.util.Set;
|
||||
import java.util.Iterator;
|
||||
|
||||
import docking.action.DockingActionIf;
|
||||
|
||||
|
@ -24,11 +24,11 @@ import docking.action.DockingActionIf;
|
|||
* {@link DockingWindowManager}. This allows the manager's interface to hide methods that
|
||||
* don't make sense for public consumption.
|
||||
*/
|
||||
public class DockingActionPackageHelper {
|
||||
public class ActionToGuiHelper {
|
||||
|
||||
private DockingWindowManager windowManager;
|
||||
|
||||
public DockingActionPackageHelper(DockingWindowManager windowManager) {
|
||||
public ActionToGuiHelper(DockingWindowManager windowManager) {
|
||||
this.windowManager = windowManager;
|
||||
}
|
||||
|
||||
|
@ -49,14 +49,6 @@ public class DockingActionPackageHelper {
|
|||
windowManager.removeToolAction(action);
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns all actions registered with this manager
|
||||
* @return the actions
|
||||
*/
|
||||
public Set<DockingActionIf> getAllActions() {
|
||||
return windowManager.getAllActions();
|
||||
}
|
||||
|
||||
/**
|
||||
* Adds an action that will be associated with the given provider. These actions will
|
||||
* appear in the local header for the component as a toolbar button or a drop-down menu
|
||||
|
@ -69,4 +61,21 @@ public class DockingActionPackageHelper {
|
|||
windowManager.addLocalAction(provider, action);
|
||||
}
|
||||
|
||||
/**
|
||||
* Get an iterator over the actions for the given provider
|
||||
* @param provider the component provider for which to iterate over all its owned actions
|
||||
* @return null if the provider does not exist in the window manager
|
||||
*/
|
||||
public Iterator<DockingActionIf> getComponentActions(ComponentProvider provider) {
|
||||
return windowManager.getComponentActions(provider);
|
||||
}
|
||||
|
||||
/**
|
||||
* Removes the action from the given provider's header bar.
|
||||
* @param provider the provider whose header bar from which the action should be removed.
|
||||
* @param action the action to be removed from the provider's header bar.
|
||||
*/
|
||||
public void removeProviderAction(ComponentProvider provider, DockingActionIf action) {
|
||||
windowManager.removeProviderAction(provider, action);
|
||||
}
|
||||
}
|
|
@ -60,8 +60,6 @@ public class DockingWindowManager implements PropertyChangeListener, Placeholder
|
|||
|
||||
final static String COMPONENT_MENU_NAME = "Window";
|
||||
|
||||
private final static List<DockingActionIf> EMPTY_LIST = Collections.emptyList();
|
||||
|
||||
private static DockingActionIf actionUnderMouse;
|
||||
private static Object objectUnderMouse;
|
||||
|
||||
|
@ -643,19 +641,6 @@ public class DockingWindowManager implements PropertyChangeListener, Placeholder
|
|||
placeholderManager.removeComponent(provider);
|
||||
}
|
||||
|
||||
/**
|
||||
* Get an iterator over the actions for the given provider.
|
||||
* @param provider the component provider for which to iterate over all its owned actions.
|
||||
* @return null if the provider does not exist in this window manager
|
||||
*/
|
||||
public Iterator<DockingActionIf> getComponentActions(ComponentProvider provider) {
|
||||
ComponentPlaceholder placeholder = getActivePlaceholder(provider);
|
||||
if (placeholder != null) {
|
||||
return placeholder.getActions();
|
||||
}
|
||||
return EMPTY_LIST.iterator();
|
||||
}
|
||||
|
||||
/**
|
||||
* Removes all components and actions associated with the given owner.
|
||||
* @param owner the name of the owner whose associated component and actions should be removed.
|
||||
|
@ -666,12 +651,21 @@ public class DockingWindowManager implements PropertyChangeListener, Placeholder
|
|||
scheduleUpdate();
|
||||
}
|
||||
|
||||
/**
|
||||
* Removes the action from the given provider's header bar.
|
||||
* @param provider the provider whose header bar from which the action should be removed.
|
||||
* @param action the action to be removed from the provider's header bar.
|
||||
*/
|
||||
public void removeProviderAction(ComponentProvider provider, DockingActionIf action) {
|
||||
//==================================================================================================
|
||||
// Package-level Action Methods
|
||||
//==================================================================================================
|
||||
|
||||
Iterator<DockingActionIf> getComponentActions(ComponentProvider provider) {
|
||||
ComponentPlaceholder placeholder = getActivePlaceholder(provider);
|
||||
if (placeholder != null) {
|
||||
return placeholder.getActions();
|
||||
}
|
||||
|
||||
List<DockingActionIf> emptyList = Collections.emptyList();
|
||||
return emptyList.iterator();
|
||||
}
|
||||
|
||||
void removeProviderAction(ComponentProvider provider, DockingActionIf action) {
|
||||
ComponentPlaceholder placeholder = getActivePlaceholder(provider);
|
||||
if (placeholder != null) {
|
||||
actionManager.removeLocalAction(action);
|
||||
|
@ -679,17 +673,6 @@ public class DockingWindowManager implements PropertyChangeListener, Placeholder
|
|||
}
|
||||
}
|
||||
|
||||
//==================================================================================================
|
||||
// Package-level Action Methods
|
||||
//==================================================================================================
|
||||
|
||||
/**
|
||||
* Adds an action that will be associated with the given provider. These actions will
|
||||
* appear in the local header for the component as a toolbar button or a drop-down menu
|
||||
* item if it has an icon and menu path respectively.
|
||||
* @param provider the provider whose header on which the action is to be placed.
|
||||
* @param action the action to add to the providers header bar.
|
||||
*/
|
||||
void addLocalAction(ComponentProvider provider, DockingActionIf action) {
|
||||
ComponentPlaceholder placeholder = getActivePlaceholder(provider);
|
||||
if (placeholder == null) {
|
||||
|
@ -699,34 +682,16 @@ public class DockingWindowManager implements PropertyChangeListener, Placeholder
|
|||
actionManager.addLocalAction(action, provider);
|
||||
}
|
||||
|
||||
/**
|
||||
* Adds an action to the global menu or toolbar which appear in the main frame. If
|
||||
* the action has a menu path, it will be in the menu. If it has an icon, it will
|
||||
* appear in the toolbar.
|
||||
* @param action the action to be added.
|
||||
*/
|
||||
void addToolAction(DockingActionIf action) {
|
||||
actionManager.addToolAction(action);
|
||||
scheduleUpdate();
|
||||
}
|
||||
|
||||
/**
|
||||
* Removes the given action from the global menu and toolbar.
|
||||
* @param action the action to be removed.
|
||||
*/
|
||||
void removeToolAction(DockingActionIf action) {
|
||||
actionManager.removeToolAction(action);
|
||||
scheduleUpdate();
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns all actions registered with this manager
|
||||
* @return the actions
|
||||
*/
|
||||
public Set<DockingActionIf> getAllActions() {
|
||||
return actionManager.getAllActions();
|
||||
}
|
||||
|
||||
//==================================================================================================
|
||||
// End Package-level Methods
|
||||
//==================================================================================================
|
||||
|
|
|
@ -336,7 +336,7 @@ public class KeyBindingUtils {
|
|||
* @param tool the tool containing the actions
|
||||
* @return the actions mapped by their full name (e.g., 'Name (OwnerName)')
|
||||
*/
|
||||
public static Map<String, DockingActionIf> getAllKeyBindingActionsByFullName(DockingTool tool) {
|
||||
public static Map<String, DockingActionIf> getAllActionsByFullName(DockingTool tool) {
|
||||
|
||||
Map<String, DockingActionIf> deduper = new HashMap<>();
|
||||
Set<DockingActionIf> actions = tool.getAllActions();
|
||||
|
|
|
@ -66,6 +66,10 @@ public class SharedStubKeyBindingAction extends DockingAction implements Options
|
|||
options.addOptionsChangeListener(this);
|
||||
}
|
||||
|
||||
void removeClientAction(DockingActionIf action) {
|
||||
clientActions.remove(action);
|
||||
}
|
||||
|
||||
void addClientAction(DockingActionIf action) {
|
||||
|
||||
// 1) Validate new action keystroke against existing actions
|
||||
|
|
|
@ -18,7 +18,6 @@ package docking.actions;
|
|||
import java.beans.PropertyChangeEvent;
|
||||
import java.beans.PropertyChangeListener;
|
||||
import java.util.*;
|
||||
import java.util.stream.Collectors;
|
||||
|
||||
import javax.swing.KeyStroke;
|
||||
|
||||
|
@ -29,6 +28,7 @@ import docking.action.*;
|
|||
import docking.tool.util.DockingToolConstants;
|
||||
import ghidra.framework.options.*;
|
||||
import ghidra.util.exception.AssertException;
|
||||
import util.CollectionUtils;
|
||||
|
||||
/**
|
||||
* An class to manage actions registered with the tool
|
||||
|
@ -36,7 +36,7 @@ import ghidra.util.exception.AssertException;
|
|||
public class ToolActions implements PropertyChangeListener {
|
||||
|
||||
private DockingWindowManager winMgr;
|
||||
private DockingActionPackageHelper actionGuiHelper;
|
||||
private ActionToGuiHelper actionGuiHelper;
|
||||
|
||||
/*
|
||||
Map of Maps of Sets
|
||||
|
@ -62,7 +62,7 @@ public class ToolActions implements PropertyChangeListener {
|
|||
public ToolActions(DockingTool tool, DockingWindowManager windowManager) {
|
||||
this.dockingTool = tool;
|
||||
this.winMgr = windowManager;
|
||||
this.actionGuiHelper = new DockingActionPackageHelper(winMgr);
|
||||
this.actionGuiHelper = new ActionToGuiHelper(winMgr);
|
||||
keyBindingOptions = tool.getOptions(DockingToolConstants.KEY_BINDINGS);
|
||||
}
|
||||
|
||||
|
@ -78,10 +78,6 @@ public class ToolActions implements PropertyChangeListener {
|
|||
actions.add(action);
|
||||
}
|
||||
|
||||
private void removeActionFromMap(DockingActionIf action) {
|
||||
getActionStorage(action).remove(action);
|
||||
}
|
||||
|
||||
/**
|
||||
* Add an action that works specifically with a component provider.
|
||||
* @param provider provider associated with the action
|
||||
|
@ -150,7 +146,7 @@ public class ToolActions implements PropertyChangeListener {
|
|||
*/
|
||||
public synchronized void removeToolAction(DockingActionIf action) {
|
||||
action.removePropertyChangeListener(this);
|
||||
removeActionFromMap(action);
|
||||
removeAction(action);
|
||||
actionGuiHelper.removeToolAction(action);
|
||||
}
|
||||
|
||||
|
@ -160,16 +156,19 @@ public class ToolActions implements PropertyChangeListener {
|
|||
*/
|
||||
public synchronized void removeToolActions(String owner) {
|
||||
|
||||
// remove from the outer map first, to prevent concurrent modification exceptions
|
||||
Map<String, Set<DockingActionIf>> toCleanup = actionsByNameByOwner.remove(owner);
|
||||
if (toCleanup == null) {
|
||||
return; // no actions registered for this owner
|
||||
}
|
||||
|
||||
//@formatter:off
|
||||
Set<DockingActionIf> toRemove = actionsByNameByOwner.get(owner).values()
|
||||
toCleanup.values()
|
||||
.stream()
|
||||
.flatMap(set -> set.stream())
|
||||
.collect(Collectors.toSet())
|
||||
.forEach(action -> removeToolAction(action))
|
||||
;
|
||||
//@formatter:on
|
||||
|
||||
// must do this later to avoid concurrent modification exceptions
|
||||
toRemove.forEach(action -> removeToolAction(action));
|
||||
}
|
||||
|
||||
private void checkForAlreadyAddedAction(ComponentProvider provider, DockingActionIf action) {
|
||||
|
@ -180,19 +179,7 @@ public class ToolActions implements PropertyChangeListener {
|
|||
}
|
||||
|
||||
/**
|
||||
* Remove an action that works specifically with a component provider.
|
||||
* @param provider provider associated with the action
|
||||
* @param action local action to the provider
|
||||
*/
|
||||
public synchronized void removeProviderAction(ComponentProvider provider,
|
||||
DockingActionIf action) {
|
||||
action.removePropertyChangeListener(this);
|
||||
removeActionFromMap(action);
|
||||
winMgr.removeProviderAction(provider, action);
|
||||
}
|
||||
|
||||
/**
|
||||
* Get all actions for the given owner.
|
||||
* Get all actions for the given owner
|
||||
* @param owner owner of the actions
|
||||
* @return array of actions; zero length array is returned if no
|
||||
* action exists with the given name
|
||||
|
@ -250,14 +237,37 @@ public class ToolActions implements PropertyChangeListener {
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Remove an action that works specifically with a component provider.
|
||||
* @param provider provider associated with the action
|
||||
* @param action local action to the provider
|
||||
*/
|
||||
public synchronized void removeProviderAction(ComponentProvider provider,
|
||||
DockingActionIf action) {
|
||||
action.removePropertyChangeListener(this);
|
||||
removeAction(action);
|
||||
actionGuiHelper.removeProviderAction(provider, action);
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the actions for the given provider and remove them from the action map
|
||||
* @param provider provider whose actions are to be removed
|
||||
*/
|
||||
public void removeComponentActions(ComponentProvider provider) {
|
||||
Iterator<DockingActionIf> iterator = winMgr.getComponentActions(provider);
|
||||
while (iterator.hasNext()) {
|
||||
removeActionFromMap(iterator.next());
|
||||
public synchronized void removeComponentActions(ComponentProvider provider) {
|
||||
Iterator<DockingActionIf> it = actionGuiHelper.getComponentActions(provider);
|
||||
Set<DockingActionIf> set = CollectionUtils.asSet(it);
|
||||
for (DockingActionIf action : set) {
|
||||
removeProviderAction(provider, action);
|
||||
}
|
||||
}
|
||||
|
||||
private void removeAction(DockingActionIf action) {
|
||||
getActionStorage(action).remove(action);
|
||||
if (action.usesSharedKeyBinding()) {
|
||||
SharedStubKeyBindingAction stub = sharedActionMap.get(action.getName());
|
||||
if (stub != null) {
|
||||
stub.removeClientAction(action);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -290,7 +290,6 @@ public class SharedKeyBindingDockingActionTest extends AbstractDockingTest {
|
|||
assertActionNotInTool(action1);
|
||||
assertActionNotInTool(action2);
|
||||
|
||||
String sharedName = action1.getFullName();
|
||||
assertNoSharedKeyBindingStubInstalled(action1);
|
||||
}
|
||||
|
||||
|
@ -357,8 +356,7 @@ public class SharedKeyBindingDockingActionTest extends AbstractDockingTest {
|
|||
//==================================================================================================
|
||||
|
||||
private void assertSharedStubInTool() {
|
||||
ToolActions actionManager =
|
||||
(ToolActions) getInstanceField("actionMgr", tool);
|
||||
ToolActions actionManager = (ToolActions) getInstanceField("actionMgr", tool);
|
||||
DockingActionIf action = actionManager.getSharedStubKeyBindingAction(SHARED_NAME);
|
||||
assertNotNull("Shared action stub is not in the tool", action);
|
||||
}
|
||||
|
|
|
@ -1,90 +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 docking;
|
||||
|
||||
import static org.junit.Assert.assertEquals;
|
||||
|
||||
import java.awt.event.InputEvent;
|
||||
import java.awt.event.KeyEvent;
|
||||
|
||||
import javax.swing.KeyStroke;
|
||||
|
||||
import org.junit.Test;
|
||||
|
||||
import generic.test.AbstractGenericTest;
|
||||
|
||||
public class DockingActionKeyBindingTest extends AbstractGenericTest {
|
||||
|
||||
@Test
|
||||
public void testKeybinding_Unmodified() {
|
||||
|
||||
KeyStroke javaKeyStroke = KeyStroke.getKeyStroke(KeyEvent.VK_C, 0);
|
||||
KeyStroke dockingKeyStroke = DockingKeyBindingAction.parseKeyStroke("C");
|
||||
assertEquals(javaKeyStroke, dockingKeyStroke);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testKeybinding_Unmodified_MixedCase() {
|
||||
|
||||
KeyStroke javaKeyStroke = KeyStroke.getKeyStroke(KeyEvent.VK_C, 0);
|
||||
KeyStroke dockingKeyStroke = DockingKeyBindingAction.parseKeyStroke("C");
|
||||
assertEquals(javaKeyStroke, dockingKeyStroke);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testKeybinding_Modifier() {
|
||||
|
||||
KeyStroke javaKeyStroke =
|
||||
KeyStroke.getKeyStroke(KeyEvent.VK_COMMA, InputEvent.CTRL_DOWN_MASK);
|
||||
KeyStroke dockingKeyStroke = DockingKeyBindingAction.parseKeyStroke("ctrl COMMA");
|
||||
assertEquals(javaKeyStroke, dockingKeyStroke);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testKeybinding_MultipleModifiers() {
|
||||
|
||||
KeyStroke javaKeyStroke = KeyStroke.getKeyStroke(KeyEvent.VK_COMMA,
|
||||
InputEvent.CTRL_DOWN_MASK | InputEvent.SHIFT_DOWN_MASK);
|
||||
KeyStroke dockingKeyStroke = DockingKeyBindingAction.parseKeyStroke("ctrl shift COMMA");
|
||||
assertEquals(javaKeyStroke, dockingKeyStroke);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testKeybinding_MultipleModifiersOutOfOrder() {
|
||||
|
||||
KeyStroke javaKeyStroke = KeyStroke.getKeyStroke(KeyEvent.VK_COMMA,
|
||||
InputEvent.CTRL_DOWN_MASK | InputEvent.SHIFT_DOWN_MASK);
|
||||
KeyStroke dockingKeyStroke = DockingKeyBindingAction.parseKeyStroke("ctrl COMMA shift");
|
||||
assertEquals(javaKeyStroke, dockingKeyStroke);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testKeybinding_MultipleModifiers_MixedCase() {
|
||||
|
||||
KeyStroke javaKeyStroke = KeyStroke.getKeyStroke(KeyEvent.VK_COMMA,
|
||||
InputEvent.CTRL_DOWN_MASK | InputEvent.SHIFT_DOWN_MASK);
|
||||
KeyStroke dockingKeyStroke = DockingKeyBindingAction.parseKeyStroke("ctrl SHIFT comma");
|
||||
assertEquals(javaKeyStroke, dockingKeyStroke);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testKeybinding_InvalidControl() {
|
||||
|
||||
KeyStroke javaKeyStroke = KeyStroke.getKeyStroke(KeyEvent.VK_C, InputEvent.CTRL_DOWN_MASK);
|
||||
KeyStroke dockingKeyStroke = DockingKeyBindingAction.parseKeyStroke("control C");
|
||||
assertEquals(javaKeyStroke, dockingKeyStroke);
|
||||
}
|
||||
}
|
|
@ -171,7 +171,7 @@ public class KeyBindingsPanel extends JPanel {
|
|||
originalValues = new HashMap<>();
|
||||
String longestName = "";
|
||||
|
||||
actionsByFullName = KeyBindingUtils.getAllKeyBindingActionsByFullName(tool);
|
||||
actionsByFullName = KeyBindingUtils.getAllActionsByFullName(tool);
|
||||
Set<Entry<String, DockingActionIf>> entries = actionsByFullName.entrySet();
|
||||
for (Entry<String, DockingActionIf> entry : entries) {
|
||||
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue