Merge remote-tracking branch

'origin/GP-4892-dragonmacher-enum-editor-fix' (Closes #6873)

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/datamgr/editor/EnumEditorPanel.java

@@ -17,7 +17,8 @@ package ghidra.app.plugin.core.datamgr.editor;
 
 import java.awt.*;
 import java.awt.event.*;
-import java.util.*;
+import java.util.Arrays;
+import java.util.EventObject;
 
 import javax.swing.*;
 import javax.swing.event.*;
@@ -99,20 +100,15 @@ class EnumEditorPanel extends JPanel {
 
 		// invoke later because the key press on the table causes the selection to change
 		Swing.runLater(() -> {
-			try {
+			if (table.isEditing()) {
-				if (table.isEditing()) {
+				return; // don't change the selection if a new edit is in progress
-					return; // don't change the selection if a new edit is in progress
-				}
-
-				int row = tableModel.getRow(name);
-				if (row >= 0 && row < tableModel.getRowCount()) {
-					table.setRowSelectionInterval(row, row);
-					Rectangle rect = table.getCellRect(row, 0, false);
-					table.scrollRectToVisible(rect);
-				}
 			}
-			catch (NoSuchElementException e) {
+
-				// ignore
+			int row = tableModel.getRow(name);
+			if (row >= 0 && row < tableModel.getRowCount()) {
+				table.setRowSelectionInterval(row, row);
+				Rectangle rect = table.getCellRect(row, 0, false);
+				table.scrollRectToVisible(rect);
 			}
 		});
 	}
@@ -500,6 +496,26 @@ class EnumEditorPanel extends JPanel {
 		EnumEntry enumEntry = tableModel.getRowObject(row);
 		return enumEntry.getName();
 	}
+
+	private void edit(int row, int col) {
+		scrollToCell(row, col);
+		table.setRowSelectionInterval(row, row);
+		table.editCellAt(row, col);
+	}
+
+	private void scrollToCell(int row, int col) {
+		if (table.getAutoscrolls()) {
+			Rectangle cellRect = table.getCellRect(row, col, false);
+			if (cellRect != null) {
+				table.scrollRectToVisible(cellRect);
+			}
+		}
+	}
+
+	private int getRow(EnumEntry entry) {
+		return tableModel.getRowIndex(entry);
+	}
+
 //==================================================================================================
 // Inner Classes
 //==================================================================================================
@@ -578,61 +594,113 @@ class EnumEditorPanel extends JPanel {
 					return;
 				}
 
-				int row = table.getEditingRow();
+				int code = e.getKeyCode();
-				int col = table.getEditingColumn();
+				boolean moveEdit =
-				int rowCount = table.getRowCount();
+					code == KeyEvent.VK_TAB || code == KeyEvent.VK_UP || code == KeyEvent.VK_DOWN;
-				int columnCount = table.getColumnCount();
+				if (!moveEdit) {
-				int keycode = e.getKeyCode();
+					return;
-				switch (keycode) {
-					case KeyEvent.VK_TAB:
-						if (e.isShiftDown()) {
-							if (--col < 0) {
-								col = columnCount - 1;
-								if (--row < 0) {
-									row = rowCount - 1;
-									col = columnCount - 1;
-								}
-							}
-						}
-						else {
-							if (++col == columnCount) {
-								col = 0;
-
-								if (++row == rowCount) {
-									row = 0;
-								}
-							}
-						}
-						break;
-					case KeyEvent.VK_DOWN:
-						if (++row == rowCount) {
-							row = 0;
-						}
-						break;
-					case KeyEvent.VK_UP:
-						if (--row < 0) {
-							row = rowCount - 1;
-						}
-						break;
-					default:
-						return;
 				}
 
 				e.consume();
 
-				scrollToCell(row, col);
+				int row = table.getEditingRow();
-				table.setRowSelectionInterval(row, row);
+				int col = table.getEditingColumn();
-				table.editCellAt(row, col);
+
+				// 
+				// The user has attempted to edit a new cell while there is an edit in progress. The
+				// table may get re-sorted when this happens, as the current edit may get committed, 
+				// which can affect the table's sort.  In this case, we need to find where the 
+				// currently edited cell is moved to so that we can correctly move to the user's 
+				// requested cell, which is relative to the current cell being edited.
+				//
+				EnumEntry editedEntry = tableModel.getRowObject(row);
+
+				TableCellEditor editor = table.getCellEditor();
+				editor.stopCellEditing();
+
+				CellEditRequest cellEditRequest =
+					new CellEditRequest(EnumEditorPanel.this, editedEntry, col, e);
+				Swing.runLater(cellEditRequest);
 			}
 		};
 
-		private void scrollToCell(int row, int col) {
+		private record CellEditRequest(EnumEditorPanel editorPanel, EnumEntry editedEntry,
-			if (table.getAutoscrolls()) {
+				int editCol,
-				Rectangle cellRect = table.getCellRect(row, col, false);
+				KeyEvent e)
-				if (cellRect != null) {
+				implements Runnable {
-					table.scrollRectToVisible(cellRect);
+
+			@Override
+			public void run() {
+
+				JTable table = editorPanel.table;
+
+				// note: this lookup works because equals() is *not* overridden and any edits are
+				// applied to the object in memory so that the default '==' lookup works.
+				int row = editorPanel.getRow(editedEntry);
+				int col = editCol;
+				int rowCount = table.getRowCount();
+				switch (e.getKeyCode()) {
+					case KeyEvent.VK_TAB:
+						boolean forward = !e.isShiftDown();
+						editNextCell(table, forward, row, col);
+						return;
+					case KeyEvent.VK_DOWN:
+						if (++row == rowCount) {
+							row = 0;
+						}
+						editorPanel.edit(row, col);
+						return;
+					case KeyEvent.VK_UP:
+						if (--row < 0) {
+							row = rowCount - 1;
+						}
+						editorPanel.edit(row, col);
+						return;
+					default:
+						return;
+				}
+
+			}
+
+			private void editNextCell(JTable table, boolean forward, int row, int col) {
+
+				int columnCount = table.getColumnCount();
+				int rowCount = table.getRowCount();
+				if (forward) {
+
+					int nextRow = row;
+					int nextCol = col + 1;
+					if (nextCol == columnCount) {
+
+						// wrap to the next row
+						nextCol = 0;
+						nextRow++;
+						if (nextRow == rowCount) {
+							// wrap to the first row
+							nextRow = 0;
+						}
+					}
+
+					editorPanel.edit(nextRow, nextCol);
+					return;
+				}
+
+				// going backward
+				int nextRow = row;
+				int nextCol = col - 1;
+				if (nextCol < 0) {
+					nextCol = columnCount - 1;
+
+					nextRow--;
+					if (nextRow < 0) {
+						nextRow = rowCount - 1;
+						nextCol = columnCount - 1;
+					}
+
+					editorPanel.edit(nextRow, nextCol);
 				}
 			}
+
 		}
 	}
 

Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/datamgr/editor/EnumEditor2Test.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.
@@ -18,6 +18,7 @@ package ghidra.app.plugin.core.datamgr.editor;
 import static org.junit.Assert.*;
 
 import java.awt.*;
+import java.awt.event.KeyEvent;
 
 import javax.swing.*;
 import javax.swing.table.*;
@@ -27,6 +28,8 @@ import org.junit.*;
 import docking.*;
 import docking.action.DockingActionIf;
 import docking.widgets.OptionDialog;
+import docking.widgets.table.*;
+import docking.widgets.table.ColumnSortState.SortDirection;
 import ghidra.app.plugin.core.datamgr.DataTypeManagerPlugin;
 import ghidra.framework.plugintool.PluginTool;
 import ghidra.program.model.data.*;
@@ -469,8 +472,8 @@ public class EnumEditor2Test extends AbstractGhidraHeadedIntegrationTest {
 
 	@Test
 	public void testEditName() throws Exception {
-		Enum enummDt = editSampleEnum();
 
+		Enum enummDt = editSampleEnum();
 		EnumEditorPanel panel = findEditorPanel(tool.getToolFrame());
 
 		runSwing(() -> {
@@ -482,6 +485,37 @@ public class EnumEditor2Test extends AbstractGhidraHeadedIntegrationTest {
 		assertEquals("MyColors", enummDt.getName());
 	}
 
+	@Test
+	public void testEditName_RowChanges_NavigationWithTab() throws Exception {
+
+		editSampleEnum();
+
+		EnumEditorPanel panel = findEditorPanel(tool.getToolFrame());
+		JTable table = panel.getTable();
+
+		sortOnNameColumn();
+
+		int row = 0;
+		int col = 0;
+		clickTableCell(table, row, col, 2);
+		assertTrue(runSwing(() -> table.isEditing()));
+
+		// note: this new name will cause the enum entry at row 0 to get moved to row 1
+		String newName = "MyColors";
+		JTextField editorField = getCellEditorTextField();
+		assertNotNull(editorField);
+		setText(editorField, newName);
+
+		pressTab(editorField);
+
+		// get the row after the table has been sorted
+		int newRow = findRowByName(newName);
+		assertNotEquals(row, newRow);
+
+		int newColumn = col + 1;
+		assertEditing(newRow, newColumn);
+	}
+
 	@Test
 	public void testDuplicateName() throws Exception {
 
@@ -672,6 +706,71 @@ public class EnumEditor2Test extends AbstractGhidraHeadedIntegrationTest {
 // Private Methods
 //==================================================================================================
 
+	private void sortOnNameColumn() {
+
+		JTable table = getTable();
+		SortedTableModel sortedModel = (SortedTableModel) table.getModel();
+		TableSortState sortState = getSortState(sortedModel);
+		ColumnSortState primarySortState = sortState.iterator().next();
+		SortDirection sortDirection = primarySortState.getSortDirection();
+		if (primarySortState.getColumnModelIndex() == EnumTableModel.NAME_COL) {
+			if (SortDirection.ASCENDING == sortDirection) {
+				return; // already sorted
+			}
+		}
+
+		TableSortState newSortState =
+			TableSortState.createDefaultSortState(EnumTableModel.NAME_COL, true);
+		runSwing(() -> sortedModel.setTableSortState(newSortState));
+	}
+
+	private TableSortState getSortState(SortedTableModel sortedModel) {
+		return runSwing(() -> sortedModel.getTableSortState());
+	}
+
+	private JTable getTable() {
+		EnumEditorPanel panel = findEditorPanel(tool.getToolFrame());
+		return panel.getTable();
+	}
+
+	protected JTextField getCellEditorTextField() {
+		Object editorComponent = getTable().getEditorComponent();
+		if (editorComponent instanceof JTextField) {
+			return (JTextField) editorComponent;
+		}
+
+		fail("Either not editing, or editing a field that is a custom editor (not a text field)");
+		return null;
+	}
+
+	private void assertEditing(int row, int column) {
+		JTable table = getTable();
+		assertTrue(runSwing(table::isEditing));
+		assertEquals(row, (int) runSwing(table::getEditingRow));
+		assertEquals(row, (int) runSwing(table::getEditingColumn));
+	}
+
+	private int findRowByName(String name) {
+
+		JTable table = getTable();
+		return runSwing(() -> {
+			int col = 0; // Name column defaults to 0
+			int n = table.getRowCount();
+			for (int i = 0; i < n; i++) {
+				String value = table.getValueAt(i, col).toString();
+				if (name.equals(value)) {
+					return i;
+				}
+			}
+			return -1;
+		});
+	}
+
+	private void pressTab(JComponent component) {
+		triggerActionKey(component, 0, KeyEvent.VK_TAB);
+		waitForSwing();
+	}
+
 	private EnumEditorPanel findEditorPanel(Window w) {
 		Window[] windows = w.getOwnedWindows();
 		for (Window window : windows) {