From 33ecf97be3d53e386972b2db59d34a59057e2fd1 Mon Sep 17 00:00:00 2001
From: dragonmacher <48328597+dragonmacher@users.noreply.github.com>
Date: Mon, 19 May 2025 14:21:59 -0400
Subject: [PATCH] Updated the Quick Edit Field to fix the menu name and to fix
 a data type edit bug

---
 .../main/help/help/topics/DataPlugin/Data.htm |  6 +-
 .../app/plugin/core/data/DataPlugin.java      |  4 +-
 .../plugin/core/data/EditDataFieldDialog.java | 58 +++++++++++++------
 .../plugin/core/data/EditFieldDialogTest.java | 18 ++++++
 4 files changed, 63 insertions(+), 23 deletions(-)

diff --git a/Ghidra/Features/Base/src/main/help/help/topics/DataPlugin/Data.htm b/Ghidra/Features/Base/src/main/help/help/topics/DataPlugin/Data.htm
index 43377f3973..413088eef9 100644
--- a/Ghidra/Features/Base/src/main/help/help/topics/DataPlugin/Data.htm
+++ b/Ghidra/Features/Base/src/main/help/help/topics/DataPlugin/Data.htm
@@ -972,7 +972,7 @@
         <OL>
           <LI>Right mouse click on a structure member in the Listing</LI>
 
-          <LI>Choose the <B>Data</B><IMG src="help/shared/arrow.gif"> <B>Quick Edit Field</B>
+          <LI>Choose the <B>Data</B><IMG src="help/shared/arrow.gif"> <B>Edit Field...</B>
           action to bring the up the <A href=
           "#Edit_Field_Dialog">Edit Field Dialog</A> </LI>
 
@@ -1370,8 +1370,8 @@
     <P>As a convenience, a structure or union field can be edited directly from the listing without
     bringing up the entire structure or union editor. To edit a field, click anywhere on the line
     displaying that field in the listing and then right click and select <B>Data</B><IMG 
-    src="help/shared/arrow.gif"> <B>Edit Field</B> from the popup context menu.</P>
-    <H3><A name="Edit_Field_Dialog"></A><A name="Quick_Edit_Field"></A>Edit Field Dialog</H3>
+    src="help/shared/arrow.gif"> <B>Edit Field...</B> from the popup context menu.</P>
+    <H3><A name="Edit_Field_Dialog"></A>Edit Field Dialog</H3>
           <P align="center"><IMG src="images/EditFieldDialog.png" alt=""> &nbsp;</P>
     <UL>
     	<LI><B>Field Name</B>: The name of the structure or union field can be changed here.</LI>
diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/data/DataPlugin.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/data/DataPlugin.java
index fd5037874b..e832c6e6de 100644
--- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/data/DataPlugin.java
+++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/data/DataPlugin.java
@@ -146,9 +146,9 @@ public class DataPlugin extends Plugin implements DataService {
 		pointerAction = new PointerDataAction(this);
 		tool.addAction(pointerAction);
 
-		new ActionBuilder("Quick Edit Field", getName())
+		new ActionBuilder("Edit Field", getName())
 				.helpLocation(new HelpLocation("DataPlugin", "Quick_Edit_Field"))
-				.popupMenuPath("Data", "Quick Edit Field...")
+				.popupMenuPath("Data", "Edit Field...")
 				.popupMenuGroup("BasicData")
 				.keyBinding("ctrl shift E")
 				.sharedKeyBinding()
diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/data/EditDataFieldDialog.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/data/EditDataFieldDialog.java
index ef1a96bd9a..52c1f39dda 100644
--- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/data/EditDataFieldDialog.java
+++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/data/EditDataFieldDialog.java
@@ -56,7 +56,6 @@ public class EditDataFieldDialog extends DialogComponentProvider {
 	private JCheckBox dateCheckBox;
 
 	private PluginTool tool;
-	private DataType newDataType;
 	private DataTypeManagerService dtmService;
 
 	private Composite composite;
@@ -108,6 +107,7 @@ public class EditDataFieldDialog extends DialogComponentProvider {
 	 * @return the pending new datatype to change to
 	 */
 	public DataType getNewDataType() {
+		DataType newDataType = dataTypeEditor.getCellEditorValueAsDataType();
 		return newDataType != null ? newDataType : new Undefined1DataType();
 	}
 
@@ -149,8 +149,7 @@ public class EditDataFieldDialog extends DialogComponentProvider {
 	 * @param dataType the new pending datatype
 	 */
 	public void setDataType(DataType dataType) {
-		newDataType = dataType;
-		updateDataTypeTextField();
+		dataTypeEditor.setCellEditorValue(dataType);
 	}
 
 	private void initializeFields() {
@@ -247,7 +246,8 @@ public class EditDataFieldDialog extends DialogComponentProvider {
 
 	boolean hasDataTypeChange() {
 		DataType oldDt = getComponentDataType();
-		return newDataType != null && !newDataType.equals(oldDt);
+		DataType newDt = dataTypeEditor.getCellEditorValueAsDataType();
+		return newDt != null && !newDt.equals(oldDt);
 	}
 
 	boolean hasNameChange() {
@@ -321,16 +321,6 @@ public class EditDataFieldDialog extends DialogComponentProvider {
 		return panel;
 	}
 
-	private void updateDataTypeTextField() {
-		if (newDataType != null) {
-			dataTypeEditor.setCellEditorValue(newDataType);
-		}
-		else {
-			DataType dt = getComponentDataType();
-			dataTypeEditor.setCellEditorValue(dt);
-		}
-	}
-
 	private String generateTitle() {
 		String compositeName = composite.getName();
 		return "Edit " + compositeName + ", Field " + ordinal;
@@ -394,6 +384,10 @@ public class EditDataFieldDialog extends DialogComponentProvider {
 		return dataTypeEditor.getCellEditorValueAsText();
 	}
 
+	public DataTypeSelectionEditor getDataTypeEditor() {
+		return dataTypeEditor;
+	}
+
 	private class UpdateDataComponentCommand implements Command<Program> {
 		private String statusMessage = null;
 
@@ -420,17 +414,43 @@ public class EditDataFieldDialog extends DialogComponentProvider {
 				return;
 			}
 
+			growStructureAsNeeded(struct);
+			ensureDataTypeComponentRecordExists(struct);
+		}
+
+		private void growStructureAsNeeded(Structure struct) {
+			// This should not happen when editing from the Listing, since you must click on a valid
+			// ordinal to start an edit.  It is not clear if the Decompiler has a use case where 
+			// this can happen.
 			int n = composite.getNumComponents();
 			if (ordinal >= n) {
 				int amount = ordinal - n;
 				struct.growStructure(amount);
 			}
+		}
 
+		private void ensureDataTypeComponentRecordExists(Structure struct) {
 			DataTypeComponent dtc = composite.getComponent(ordinal);
-			if (dtc.getDataType() == DataType.DEFAULT) { // remove placeholder type
-				DataType newtype = new Undefined1DataType();
+			if (dtc.getDataType() != DataType.DEFAULT) {
+				return; // the record exists 
+			}
+
+			// The default type does not have a record for the data type component.  We need to
+			// replace that default type with a type that will force a record to be get created.
+			// We need a record to exist in order to set a comment or name.
+			DataType newtype = new Undefined1DataType();
+			DataTypeComponent newDtc =
 				struct.replaceAtOffset(dtc.getOffset(), newtype, 1, "tempName",
 					"Created by Edit Data Field action");
+
+			DataType oldDt = dtc.getDataType();
+			DataType editorDt = dataTypeEditor.getCellEditorValueAsDataType();
+			if (oldDt.equals(editorDt)) {
+				// If the user has changed the type, we want to keep that change.  Otherwise, the
+				// editor should always track what is in the structure so we know if the user has
+				// made a change to the data type.  When the user changes the type, we need to apply
+				// that change.
+				dataTypeEditor.setCellEditorValue(newDtc.getDataType());
 			}
 		}
 
@@ -484,8 +504,9 @@ public class EditDataFieldDialog extends DialogComponentProvider {
 		}
 
 		private void updateStructure(Structure struct) {
+			DataType newDt = dataTypeEditor.getCellEditorValueAsDataType();
 			DataTypeComponent dtc = composite.getComponent(ordinal);
-			DataType resolvedDt = program.getDataTypeManager().resolve(newDataType, null);
+			DataType resolvedDt = program.getDataTypeManager().resolve(newDt, null);
 			if (resolvedDt == DataType.DEFAULT) {
 				struct.clearComponent(ordinal);
 				return;
@@ -501,8 +522,9 @@ public class EditDataFieldDialog extends DialogComponentProvider {
 		}
 
 		private void updateUnion(Union union) throws DuplicateNameException {
+			DataType newDt = dataTypeEditor.getCellEditorValueAsDataType();
 			DataTypeComponent dtc = composite.getComponent(ordinal);
-			DataType resolvedDt = program.getDataTypeManager().resolve(newDataType, null);
+			DataType resolvedDt = program.getDataTypeManager().resolve(newDt, null);
 			String comment = dtc.getComment();
 			String fieldName = dtc.getFieldName();
 			union.insert(ordinal, resolvedDt);
diff --git a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/data/EditFieldDialogTest.java b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/data/EditFieldDialogTest.java
index 3722388c42..4da206a366 100644
--- a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/data/EditFieldDialogTest.java
+++ b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/data/EditFieldDialogTest.java
@@ -22,7 +22,9 @@ import java.util.Date;
 import org.junit.*;
 
 import docking.action.DockingActionIf;
+import docking.widgets.DropDownSelectionTextField;
 import ghidra.app.plugin.core.codebrowser.CodeBrowserPlugin;
+import ghidra.app.util.datatype.DataTypeSelectionEditor;
 import ghidra.framework.plugintool.PluginTool;
 import ghidra.program.model.address.Address;
 import ghidra.program.model.data.*;
@@ -188,6 +190,22 @@ public class EditFieldDialogTest extends AbstractGhidraHeadedIntegrationTest {
 		assertEquals("byte", structure.getComponent(1).getDataType().getDisplayName());
 	}
 
+	@Test
+	public void testEditUndefinedDataType_ByTypingText() {
+		goTo(0x101);
+		showFieldEditDialog();
+		assertNull(structure.getComponent(1).getComment());
+		assertEquals("undefined", getDataTypeText());
+
+		DataTypeSelectionEditor editor = dialog.getDataTypeEditor();
+		DropDownSelectionTextField<DataType> textField = editor.getDropDownTextField();
+		setText(textField, "byte");
+
+		pressOk();
+		waitForTasks();
+		assertEquals("byte", structure.getComponent(1).getDataType().getDisplayName());
+	}
+
 	@Test
 	public void testAddAddressCheckbox() {
 		goTo(0x101);