diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/FunctionEditorDialog.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/FunctionEditorDialog.java index 4d79539db5..da0ecfab24 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/FunctionEditorDialog.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/FunctionEditorDialog.java @@ -27,8 +27,7 @@ import javax.swing.border.CompoundBorder; import javax.swing.event.*; import javax.swing.table.TableCellEditor; -import docking.DialogComponentProvider; -import docking.DockingUtils; +import docking.*; import docking.widgets.OptionDialog; import docking.widgets.checkbox.GCheckBox; import docking.widgets.combobox.GComboBox; @@ -73,7 +72,10 @@ public class FunctionEditorDialog extends DialogComponentProvider implements Mod private JCheckBox storageCheckBox; private JScrollPane scroll; private JPanel previewPanel; + private FunctionSignatureTextField signatureTextField; + private UndoRedoKeeper signatureFieldUndoRedoKeeper; + private MyGlassPane glassPane; private JPanel centerPanel; @@ -94,7 +96,6 @@ public class FunctionEditorDialog extends DialogComponentProvider implements Mod addCancelButton(); glassPane = new MyGlassPane(); dataChanged(); - setFocusComponent(nameField); } private static String createTitle(Function function) { @@ -122,6 +123,23 @@ public class FunctionEditorDialog extends DialogComponentProvider implements Mod return strBuilder.toString(); } + @Override + protected void dialogShown() { + + // put user focus in the signature field, ready to take keyboard input + signatureTextField.requestFocus(); + Swing.runLater(() -> { + int start = model.getFunctionNameStartPosition(); + int end = model.getNameString().length(); + signatureTextField.setCaretPosition(end); + signatureTextField.setSelectionStart(start); + signatureTextField.setSelectionEnd(start + end); + + // reset any edits that happened before the user interacted with the field + signatureFieldUndoRedoKeeper.clear(); + }); + } + @Override protected void okCallback() { if (model.isInParsingMode()) { @@ -223,6 +241,9 @@ public class FunctionEditorDialog extends DialogComponentProvider implements Mod private JComponent createSignatureTextPanel() { JPanel panel = new JPanel(new BorderLayout()); signatureTextField = new FunctionSignatureTextField(); + + signatureFieldUndoRedoKeeper = DockingUtils.installUndoRedo(signatureTextField); + Font font = signatureTextField.getFont(); signatureTextField.setFont(font.deriveFont(18.0f)); panel.add(signatureTextField); @@ -559,7 +580,13 @@ public class FunctionEditorDialog extends DialogComponentProvider implements Mod private void updatePreviewField() { String preview = model.getFunctionSignatureTextFromModel(); int caretPosition = signatureTextField.getCaretPosition(); - signatureTextField.setText(preview); + + // don't cause undo/redo updates if the text has not changed + String oldText = signatureTextField.getText(); + if (!preview.equals(oldText)) { + signatureTextField.setText(preview); + } + if (!model.hasValidName()) { signatureTextField.setError(model.getFunctionNameStartPosition(), model.getNameString().length()); diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/FunctionSignatureTextField.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/FunctionSignatureTextField.java index f2b0f271ac..d42c139764 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/FunctionSignatureTextField.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/editor/FunctionSignatureTextField.java @@ -28,6 +28,7 @@ import javax.swing.event.*; import javax.swing.text.*; import docking.actions.KeyBindingUtils; +import ghidra.util.Swing; class FunctionSignatureTextField extends JTextPane { private static final String ENTER_ACTION_NAME = "ENTER"; @@ -138,30 +139,22 @@ class FunctionSignatureTextField extends JTextPane { } private void updateColors() { - SwingUtilities.invokeLater(new Runnable() { - @Override - public void run() { - String text = getText(); - List computeColors = computeColors(text); - if (computeColors != null) { - doc.setCharacterAttributes(0, text.length(), defaultAttributes, true); - for (ColorField colorField : computeColors) { - doc.setCharacterAttributes(colorField.start, colorField.length(), - colorField.attributes, true); - } + Swing.runLater(() -> { + String text = getText(); + List computeColors = computeColors(text); + if (computeColors != null) { + doc.setCharacterAttributes(0, text.length(), defaultAttributes, true); + for (ColorField colorField : computeColors) { + doc.setCharacterAttributes(colorField.start, colorField.length(), + colorField.attributes, true); } - notifyChange(); } + notifyChange(); }); } void clearAttributes(final int start, final int length) { - SwingUtilities.invokeLater(new Runnable() { - @Override - public void run() { - doc.setCharacterAttributes(start, length, defaultAttributes, true); - } - }); + Swing.runLater(() -> doc.setCharacterAttributes(start, length, defaultAttributes, true)); } void notifyChange() { @@ -175,7 +168,7 @@ class FunctionSignatureTextField extends JTextPane { } List computeColors(String text) { - List list = new ArrayList(); + List list = new ArrayList<>(); int functionRightParenIndex = text.lastIndexOf(')'); int functionLeftParenIndex = findMatchingLeftParenIndex(text, functionRightParenIndex); if (functionLeftParenIndex < 0) { @@ -190,13 +183,12 @@ class FunctionSignatureTextField extends JTextPane { SubString substring = new SubString(text, 0, functionLeftParenIndex).trim(); SubString functionName = getLastWord(substring); -// SubString returnTypeString = getAllButLastWord(substring); if (functionName == null) { return null; } - list.add(new ColorField(functionName.getStart(), functionName.getEnd(), - functionNameAttributes)); + list.add( + new ColorField(functionName.getStart(), functionName.getEnd(), functionNameAttributes)); for (int i = 0; i < paramStartStopIndexes.size() - 1; i++) { int start = paramStartStopIndexes.get(i) + 1; int end = paramStartStopIndexes.get(i + 1); @@ -233,16 +225,8 @@ class FunctionSignatureTextField extends JTextPane { return string.substring(lastIndexOf + 1); } -// private SubString getAllButLastWord(SubString string) { -// int lastIndexOf = string.lastIndexOf(' '); -// if (lastIndexOf < 0) { -// return null; -// } -// return string.substring(0, lastIndexOf).trim(); -// } - private List findParamStartStopindexes(String text, int startIndex, int endIndex) { - List commaIndexes = new ArrayList(); + List commaIndexes = new ArrayList<>(); int templateCount = 0; commaIndexes.add(startIndex); for (int i = startIndex + 1; i < endIndex; i++) { @@ -299,7 +283,7 @@ class FunctionSignatureTextField extends JTextPane { public static void main(String[] args) { JFrame jFrame = new JFrame(); - jFrame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE); + jFrame.setDefaultCloseOperation(WindowConstants.EXIT_ON_CLOSE); FunctionSignatureTextField field = new FunctionSignatureTextField(); JPanel panel = new JPanel(new BorderLayout()); panel.setBorder(BorderFactory.createEmptyBorder(10, 10, 10, 10)); @@ -309,7 +293,7 @@ class FunctionSignatureTextField extends JTextPane { jFrame.setVisible(true); } - class SubString { + private class SubString { private String text; private int subStringStart; private int subStringEnd; @@ -340,24 +324,11 @@ class FunctionSignatureTextField extends JTextPane { return new SubString(text, subStringStart + start, subStringEnd); } - public SubString substring(int start, int end) { - return new SubString(text, subStringStart + start, subStringStart + end); - } - @Override public String toString() { return text.substring(subStringStart, subStringEnd); } - public int indexOf(char c) { - for (int i = subStringStart; i < subStringEnd; i++) { - if (text.charAt(i) == c) { - return i - subStringStart; - } - } - return -1; - } - public int lastIndexOf(char c) { for (int i = subStringEnd - 1; i >= subStringStart; i--) { if (text.charAt(i) == c) { @@ -385,12 +356,6 @@ class FunctionSignatureTextField extends JTextPane { } void setError(final int position, final int length) { - SwingUtilities.invokeLater(new Runnable() { - - @Override - public void run() { - doc.setCharacterAttributes(position, length, errorAttributes, true); - } - }); + Swing.runLater(() -> doc.setCharacterAttributes(position, length, errorAttributes, true)); } } diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/script/GhidraScriptComponentProvider.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/script/GhidraScriptComponentProvider.java index 8dc4a894a2..5618f802f9 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/script/GhidraScriptComponentProvider.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/script/GhidraScriptComponentProvider.java @@ -1059,6 +1059,12 @@ public class GhidraScriptComponentProvider extends ComponentProviderAdapter { } } + @Override + public void componentActivated() { + // put the user focus in the filter field, as often the user wishes to search for a script + tableFilterPanel.requestFocus(); + } + @Override public ActionContext getActionContext(MouseEvent event) { Object source = scriptTable; diff --git a/Ghidra/Framework/Docking/src/main/java/docking/DockingUtils.java b/Ghidra/Framework/Docking/src/main/java/docking/DockingUtils.java index 3d598ac547..ea54da70d4 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/DockingUtils.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/DockingUtils.java @@ -87,7 +87,7 @@ import resources.ResourceManager; * {@link JList}{@link GList} * {@link ListCellRenderer}
{@link DefaultListCellRenderer}{@link GListCellRenderer} * {@link TableCellRenderer}{@link GTableCellRenderer} - * {@link TreeCellRenderer}
{@link DefaultTreeCellRenderer}{@link GTreeRenderer}
{@link DnDTreeCellRenderer} + * {@link TreeCellRenderer}
{@link DefaultTreeCellRenderer}{@link GTreeRenderer}
DnDTreeCellRenderer * {@link JRadioButton}{@link GRadioButton} * {@link JButton}???tbd??? * @@ -179,15 +179,6 @@ public class DockingUtils { final UndoRedoKeeper undoRedoKeeper = new UndoRedoKeeper(); document.addUndoableEditListener(e -> { UndoableEdit edit = e.getEdit(); - -// TODO We are now handed a wrapper class and not the event for the 'edit'. It is not clear -// which use case caused this code to be added. If/when we find out, we can revisit how -// to filter these types of updates. -// DefaultDocumentEvent defaultDocumentEvent = (DefaultDocumentEvent) edit; -// if (defaultDocumentEvent.getType() == EventType.CHANGE) { -// return; // this happens for style updates -// } - undoRedoKeeper.addUndo(edit); }); diff --git a/Ghidra/Framework/Docking/src/main/java/docking/UndoRedoKeeper.java b/Ghidra/Framework/Docking/src/main/java/docking/UndoRedoKeeper.java index 9a5ec63f43..92e0d8a5ed 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/UndoRedoKeeper.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/UndoRedoKeeper.java @@ -1,6 +1,5 @@ /* ### * IP: GHIDRA - * REVIEWED: YES * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,31 +15,85 @@ */ package docking; -import ghidra.util.datastruct.FixedSizeStack; - +import javax.swing.JTextPane; +import javax.swing.undo.CompoundEdit; import javax.swing.undo.UndoableEdit; +import ghidra.util.datastruct.FixedSizeStack; + +/** + * Handles tracking undo and redo events. Clients may wish to hold on to this class in order + * to clear the undo/redo queue. + * + *

Style Edits
+ * {@link JTextPane}s allow for styles (color, bold, etc) to be applied to their text. The + * default undo/redo events may arrive singly, not in bulk. Thus, when the user presses undo, + * each style change is undo, one at a time. This is intuitive when the user controls the + * application of style. However, when style is applied programmatically, it can be odd to + * see that the user-type text does not change, but just the coloring applied to that text. + *

+ * To address this issue, this class takes the approach of combining all style edits into a + * single bulk edit. Then, as the user presses undo, all style edits can be removed together, as + * well as any neighboring text edits. Put simply, this class tracks style edits such + * that an undo operation will undo all style changes, as well as a single text edit. + */ public class UndoRedoKeeper { private static final int MAX_UNDO_REDO_SIZE = 50; + private static final String STYLE_EDIT_KEY = "style"; - private FixedSizeStack undoStack = new FixedSizeStack( - MAX_UNDO_REDO_SIZE); - private FixedSizeStack redoStack = new FixedSizeStack( - MAX_UNDO_REDO_SIZE); + private FixedSizeStack undoStack = new FixedSizeStack<>(MAX_UNDO_REDO_SIZE); + private FixedSizeStack redoStack = new FixedSizeStack<>(MAX_UNDO_REDO_SIZE); + + private StyleCompoundEdit lastStyleUndo; void addUndo(UndoableEdit edit) { + + String name = edit.getPresentationName(); + if (name.contains(STYLE_EDIT_KEY)) { + // (see header note about style edits) + addStyleEdit(edit); + return; + } + + endOutstandingStyleEdits(); + undoStack.push(edit); - redoStack.clear(); + redoStack.clear(); // new edit added; clear redo + } + + private void endOutstandingStyleEdits() { + if (lastStyleUndo != null) { + lastStyleUndo.end(); + lastStyleUndo = null; + } + } + + private void addStyleEdit(UndoableEdit edit) { + if (lastStyleUndo == null) { + lastStyleUndo = new StyleCompoundEdit(); + undoStack.push(lastStyleUndo); + } + + lastStyleUndo.addEdit(edit); + redoStack.clear(); // new edit added; clear redo } void undo() { if (undoStack.isEmpty()) { return; } + + endOutstandingStyleEdits(); + UndoableEdit item = undoStack.pop(); redoStack.push(item); item.undo(); + + // (see header note) + if (item instanceof StyleCompoundEdit) { + undo(); // call again to get a 'real' edit + } } void redo() { @@ -48,13 +101,24 @@ public class UndoRedoKeeper { return; } + endOutstandingStyleEdits(); + UndoableEdit item = redoStack.pop(); undoStack.push(item); item.redo(); + + // (see header note) + if (item instanceof StyleCompoundEdit) { + undo(); // call again to get a 'real' edit + } } public void clear() { undoStack.clear(); redoStack.clear(); } + + private static class StyleCompoundEdit extends CompoundEdit { + // simple class for us to track internally + } }