Merge remote-tracking branch 'origin/GT-3109-dragonmacher-structure-edit-bug'

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/compositeeditor/CompositeEditorPanel.java

@@ -578,6 +578,14 @@ public abstract class CompositeEditorPanel extends JPanel
 				return;
 			}
 			model.setSelection(table.getSelectedRows());
+
+		});
+
+		table.getColumnModel().getSelectionModel().addListSelectionListener(e -> {
+			if (e.getValueIsAdjusting()) {
+				return;
+			}
+			model.setColumn(e.getFirstIndex());
 		});
 
 		JPanel tablePanel = new JPanel(new BorderLayout());

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/compositeeditor/EditFieldAction.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.
@@ -18,7 +17,6 @@ package ghidra.app.plugin.core.compositeeditor;
 
 import java.awt.event.KeyEvent;
 
-import javax.swing.JTable;
 import javax.swing.KeyStroke;
 
 import docking.ActionContext;
@@ -49,9 +47,7 @@ public class EditFieldAction extends CompositeEditorTableAction {
 	public void actionPerformed(ActionContext context) {
 		if (model != null) {
 			int row = model.getRow();
-			JTable table = provider.getTable();
+			int column = model.getColumn();
-			int column = table.getSelectedColumn();
-
 			if (model.isCellEditable(row, column)) {
 				model.beginEditingField(row, column);
 				return;

Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/compositeeditor/AbstractStructureEditorTest.java

@@ -15,7 +15,9 @@
  */
 package ghidra.app.plugin.core.compositeeditor;
 
-import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.*;
+
+import javax.swing.JTextField;
 
 import org.junit.Assert;
 
@@ -178,4 +180,8 @@ public abstract class AbstractStructureEditorTest extends AbstractEditorTest {
 		return (String) model.getValueAt(index, model.getMnemonicColumn());
 	}
 
+	protected void setText(String s) {
+		JTextField tf = getActiveEditorTextField();
+		setText(tf, s);
+	}
 }

Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/compositeeditor/StructureEditorLockedCellEditTest.java

@@ -22,7 +22,6 @@ import java.awt.event.KeyEvent;
 import java.io.File;
 
 import javax.swing.JTable;
-import javax.swing.JTextField;
 
 import org.junit.*;
 
@@ -781,9 +780,4 @@ public class StructureEditorLockedCellEditTest extends AbstractStructureEditorTe
 		escape();
 		assertNotEditingField();
 	}
-
-	protected void setText(String s) {
-		JTextField tf = getActiveEditorTextField();
-		setText(tf, s);
-	}
 }

Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/compositeeditor/StructureEditorProviderTest.java

@@ -101,36 +101,6 @@ public class StructureEditorProviderTest extends AbstractStructureEditorTest {
 		}
 	}
 
-//	public void testReplaceDataType() throws Exception {
-//		try {
-//			txId = program.startTransaction("Replace DataType");
-//			assertEquals(87, complexStructure.getComponent(12).getDataType().getLength());
-//			assertEquals(29, complexStructure.getComponent(15).getDataType().getLength());
-//			assertEquals(29, complexStructure.getComponent(20).getDataType().getLength());
-//			assertEquals(87, complexStructure.getComponent(12).getLength());
-//			assertEquals(29, complexStructure.getComponent(15).getLength());
-//			assertEquals(29, complexStructure.getComponent(20).getLength());
-//			assertEquals(87, complexStructure.getLength());
-//			assertEquals(21, complexStructure.getNumComponents());
-//			final Structure newSimpleStructure = new StructureDataType(new CategoryPath("/aa/bb"), "simpleStructure", 10);
-//			newSimpleStructure.add(new PointerDataType(), 8);
-//			newSimpleStructure.replace(2, new AsciiDataType(), 1);
-//			int newStructLen = newSimpleStructure.getLength();
-//			// Change the struct.  simpleStructure was 29 bytes.
-//			programDTM.replaceDataType(simpleStructure, newSimpleStructure, true);
-//			assertEquals(54, complexStructure.getComponent(12).getDataType().getLength());
-//			assertEquals(18, complexStructure.getComponent(15).getDataType().getLength());
-//			assertEquals(18, complexStructure.getComponent(20).getDataType().getLength());
-//			assertEquals(54, complexStructure.getComponent(12).getLength());
-//			assertEquals(18, complexStructure.getComponent(15).getLength());
-//			assertEquals(18, complexStructure.getComponent(20).getLength());
-//			assertEquals(56, complexStructure.getLength());
-//			assertEquals(21, complexStructure.getNumComponents());
-//		} finally {
-//			program.endTransaction(txId, true);
-//		}
-//	}
-//	
 	// Test Undo / Redo of program.
 	@Test
 	public void testModifiedDtAndProgramRestored() throws Exception {
@@ -140,7 +110,7 @@ public class StructureEditorProviderTest extends AbstractStructureEditorTest {
 			init(complexStructure, pgmTestCat, false);
 			program.addListener(restoreListener);
 
-			// Change the union.
+			// Change the structure
 			runSwingLater(() -> {
 				getTable().requestFocus();
 				setSelection(new int[] { 4, 5 });
@@ -476,7 +446,7 @@ public class StructureEditorProviderTest extends AbstractStructureEditorTest {
 			init(complexStructure, pgmTestCat, false);
 			program.addListener(restoreListener);
 
-			// Change the union.
+			// Change the structure
 			runSwingLater(() -> {
 				getTable().requestFocus();
 				setSelection(new int[] { 4, 5 });
@@ -531,7 +501,7 @@ public class StructureEditorProviderTest extends AbstractStructureEditorTest {
 		init(complexStructure, pgmTestCat, false);
 		DataType oldDt = model.viewComposite.clone(null);
 
-		// Change the union.
+		// Change the structure
 		runSwingLater(() -> {
 			getTable().requestFocus();
 			setSelection(new int[] { 4, 5 });
@@ -567,7 +537,7 @@ public class StructureEditorProviderTest extends AbstractStructureEditorTest {
 		init(complexStructure, pgmTestCat, false);
 		DataType oldDt = model.viewComposite.clone(null);
 
-		// Change the union.
+		// Change the structure
 		runSwing(() -> {
 			getTable().requestFocus();
 			setSelection(new int[] { 4, 5 });
@@ -601,7 +571,7 @@ public class StructureEditorProviderTest extends AbstractStructureEditorTest {
 		Window dialog;
 		init(complexStructure, pgmTestCat, false);
 
-		// Change the union.
+		// Change the structure
 		runSwingLater(() -> {
 			getTable().requestFocus();
 			setSelection(new int[] { 4, 5 });
@@ -630,8 +600,37 @@ public class StructureEditorProviderTest extends AbstractStructureEditorTest {
 		assertTrue(newDt.isEquivalent(model.viewComposite));
 	}
 
-	private void runSwingLater(Runnable r) {
+	@Test
-		runSwing(r, false);
+	public void testEditWillReEditLastColumnWhenPressingKeyboardEditAction() throws Exception {
+		//
+		// This is a regression test.   The user would edit a field by pressing the keyboard edit
+		// action (F2).  The user would finish the edit by pressing Enter.   The UI would show the
+		// current table cell as the cell that was just edited (this is correct).  When the user
+		// again presses the edit actions, the first editable cell in the current row would start
+		// to be edited (this was incorrect).   This test ensures that the currently selected cell
+		// is edited in this case.
+		//
+
+		init(simpleStructure, pgmBbCat, false);
+
+		int row = 3;
+		int column = model.getNameColumn();
+		assertNull(getComment(3));
+		clickTableCell(getTable(), row, column, 1);
+		performAction(editFieldAction, provider, true);
+
+		assertIsEditingField(row, column);
+
+		setText("Wow");
+		enter();
+
+		assertNotEditingField();
+		assertEquals(1, model.getNumSelectedRows());
+		assertEquals(3, model.getMinIndexSelected());
+		assertEquals("Wow", getFieldName(3));
+
+		performAction(editFieldAction, provider, true);
+		assertIsEditingField(row, column);
 	}
 
 	@Test
@@ -729,4 +728,8 @@ public class StructureEditorProviderTest extends AbstractStructureEditorTest {
 		assertEquals("325", model.getLengthAsString());
 	}
 
+	private void runSwingLater(Runnable r) {
+		runSwing(r, false);
+	}
+
 }