GP-1361 - Updated auto comments to show user-defined repeatable comments

from the reference destination

Closes #2475

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/navigation/locationreferences/ReferenceUtils.java

@@ -855,7 +855,7 @@ public final class ReferenceUtils {
 		// }
 
 		//
-		// Using the reference, we can heck for the 'Extended Markup' style reference, such as:
+		// Using the reference, we can check for the 'Extended Markup' style reference, such as:
 		// 		instruction ...=>Foo.bar.baz
 		//                     -------------
 		// Note: these references are to labels (not sure why the reference isn't to a data

Ghidra/Features/Base/src/main/java/ghidra/app/util/DisplayableEol.java

@@ -17,6 +17,8 @@ package ghidra.app.util;
 
 import java.util.*;
 
+import org.apache.commons.lang3.StringUtils;
+
 import docking.widgets.fieldpanel.support.RowColLocation;
 import ghidra.program.model.address.*;
 import ghidra.program.model.data.*;
@@ -240,7 +242,10 @@ public class DisplayableEol {
 			}
 		}
 
-		set.add("= " + getDataValueRepresentation(dataAccessAddress, data));
+		String dataRepresentation = getDataValueRepresentation(dataAccessAddress, data);
+		if (!StringUtils.isBlank(dataRepresentation)) {
+			set.add("= " + dataRepresentation);
+		}
 	}
 
 	private String getDataValueRepresentation(Address dataAccessAddress, Data data) {
@@ -249,8 +254,7 @@ public class DisplayableEol {
 		}
 
 		if (isOffcut(dataAccessAddress, data)) {
-			String offcut = getOffcutDataString(dataAccessAddress, data);
+			return getOffcutDataString(dataAccessAddress, data);
-			return offcut;
 		}
 
 		return data.getDefaultValueRepresentation();
@@ -516,13 +520,17 @@ public class DisplayableEol {
 			}
 
 			Address address = memRefs[i].getToAddress();
+			String repeatableComment = listing.getComment(CodeUnit.REPEATABLE_COMMENT, address);
+			if (repeatableComment != null) {
+				set.add(new RefRepeatComment(address, new String[] { repeatableComment }));
+			}
+
 			CodeUnit cu = listing.getCodeUnitAt(address);
 			if (cu == null) {
 				continue;
 			}
 
 			String[] comment = new String[0];
-
 			Function func = listing.getFunctionAt(address);
 			if (func != null) {
 				comment = func.getRepeatableCommentAsArray();
@@ -643,6 +651,33 @@ public class DisplayableEol {
 		return (String[]) displayCommentArrays[MY_AUTOMATIC];
 	}
 
+	@Override
+	public String toString() {
+
+		StringBuilder buffy = new StringBuilder();
+		String[] eols = (String[]) displayCommentArrays[MY_EOLS];
+		if (eols.length != 0) {
+			buffy.append("EOLs: ").append(Arrays.toString(eols));
+		}
+
+		String[] myRepeatables = (String[]) displayCommentArrays[MY_REPEATABLES];
+		if (myRepeatables.length != 0) {
+			buffy.append("My Repeatables: ").append(Arrays.toString(myRepeatables));
+		}
+
+		Object[] refRepeatables = displayCommentArrays[REF_REPEATABLES];
+		if (refRepeatables.length != 0) {
+			buffy.append("Ref Repeatables: ").append(Arrays.toString(refRepeatables));
+		}
+
+		String[] myAutomatic = (String[]) displayCommentArrays[MY_AUTOMATIC];
+		if (myAutomatic.length != 0) {
+			buffy.append("My Automatic: ").append(Arrays.toString(myAutomatic));
+		}
+
+		return buffy.toString();
+	}
+
 	public int getCommentLineCount(int subType) {
 		switch (subType) {
 			case MY_EOLS:

Ghidra/Features/Base/src/main/java/ghidra/app/util/viewer/field/EolCommentFieldFactory.java

@@ -310,7 +310,8 @@ public class EolCommentFieldFactory extends FieldFactory {
 			new DisplayableEol(cu, alwaysShowRepeatable, alwaysShowRefRepeatables,
 				alwaysShowAutomatic, codeUnitFormatOptions.followReferencedPointers(),
 				maxDisplayLines, useAbbreviatedAutomatic, showAutomaticFunctions);
-		ArrayList<FieldElement> elementList = new ArrayList<>();
+
+		List<FieldElement> elementList = new ArrayList<>();
 
 		// This Code Unit's End of Line Comment
 		AttributedString myEolPrefixString =
@@ -335,7 +336,6 @@ public class EolCommentFieldFactory extends FieldFactory {
 		if (alwaysShowRefRepeatables || elementList.isEmpty()) {
 			AttributedString refRepeatPrefixString = new AttributedString(SEMICOLON_PREFIX,
 				refRepeatableCommentColor, getMetrics(refRepeatableCommentStyle), false, null);
-			//int refRepeatLinesSoFar = 0;
 			int refRepeatCount = displayableEol.getReferencedRepeatableCommentsCount();
 			for (int subTypeIndex = 0; subTypeIndex < refRepeatCount; subTypeIndex++) {
 				RefRepeatComment refRepeatComment =
@@ -345,7 +345,6 @@ public class EolCommentFieldFactory extends FieldFactory {
 					refRepeatComments, program, refRepeatPrefixString, showSemicolon, isWordWrap,
 					prependRefAddress, refRepeatComment.getAddress(), getNextRow(elementList));
 				elementList.addAll(refRepeatFieldElements);
-				//refRepeatLinesSoFar += refRepeatComments.length;
 			}
 		}
 
@@ -368,7 +367,7 @@ public class EolCommentFieldFactory extends FieldFactory {
 			maxDisplayLines, hlProvider);
 	}
 
-	private int getNextRow(ArrayList<FieldElement> elementList) {
+	private int getNextRow(List<FieldElement> elementList) {
 		int elementIndex = elementList.size() - 1;
 		if (elementIndex >= 0) {
 			FieldElement element = elementList.get(elementIndex);

Ghidra/Features/Base/src/test.slow/java/ghidra/app/util/viewer/field/EolCommentFieldFactoryTest.java

@@ -15,6 +15,7 @@
  */
 package ghidra.app.util.viewer.field;
 
+import static org.hamcrest.core.StringStartsWith.*;
 import static org.junit.Assert.*;
 
 import javax.swing.SwingUtilities;
@@ -22,21 +23,17 @@ import javax.swing.SwingUtilities;
 import org.junit.*;
 
 import docking.widgets.fieldpanel.field.FieldElement;
-import ghidra.app.plugin.core.blockmodel.BlockModelServicePlugin;
 import ghidra.app.plugin.core.codebrowser.CodeBrowserPlugin;
-import ghidra.app.plugin.core.navigation.NextPrevAddressPlugin;
 import ghidra.framework.options.Options;
-import ghidra.framework.plugintool.PluginTool;
-import ghidra.program.database.ProgramBuilder;
 import ghidra.program.database.ProgramDB;
+import ghidra.program.model.address.Address;
+import ghidra.program.model.address.AddressFactory;
 import ghidra.program.model.listing.*;
-import ghidra.test.AbstractGhidraHeadedIntegrationTest;
+import ghidra.test.*;
-import ghidra.test.TestEnv;
 
 public class EolCommentFieldFactoryTest extends AbstractGhidraHeadedIntegrationTest {
 
 	private TestEnv env;
-	private PluginTool tool;
 	private CodeBrowserPlugin cb;
 	private Options fieldOptions;
 	private Program program;
@@ -47,12 +44,8 @@ public class EolCommentFieldFactoryTest extends AbstractGhidraHeadedIntegrationT
 		program = buildProgram();
 
 		env = new TestEnv();
-		tool = env.showTool(program);
+		env.launchDefaultTool(program);
-		tool.addPlugin(CodeBrowserPlugin.class.getName());
-		tool.addPlugin(NextPrevAddressPlugin.class.getName());
 		cb = env.getPlugin(CodeBrowserPlugin.class);
-		tool.addPlugin(BlockModelServicePlugin.class.getName());
-
 		fieldOptions = cb.getFormatManager().getFieldOptions();
 	}
 
@@ -78,15 +71,50 @@ public class EolCommentFieldFactoryTest extends AbstractGhidraHeadedIntegrationT
 		assertEquals(4, tf.getNumRows());
 	}
 
+	@Test
+	public void testRepeatableComment_FunctionCall() throws Exception {
+
+		// check existing auto comment
+		ListingTextField tf = getFieldText(addr("0x010022e6"));
+		assertEquals(1, tf.getNumRows());
+		assertThat(tf.getText(), startsWith("undefined ghidra(undefined4 param_1,"));
+
+		// set repeatable comment at destination
+		Address destination = addr("0x01002cf5");
+		String repeatableComment = "My repeatable comment";
+		setRepeatableComment(destination, repeatableComment);
+
+		// check that the auto comment now matches the updated comment
+		tf = getFieldText(addr("0x010022e6"));
+		assertEquals(1, tf.getNumRows());
+		assertEquals(tf.getText(), repeatableComment);
+	}
+
+	@Test
+	public void testRepeatableComment_DataAccess() throws Exception {
+
+		// check existing auto comment
+		ListingTextField tf = getFieldText(addr("0x01002265"));
+		assertEquals(1, tf.getNumRows());
+		assertThat(tf.getText(), startsWith("= 01h"));
+
+		// set repeatable comment at destination
+		Address destination = addr("0x01002265");
+		String repeatableComment = "My repeatable comment";
+		setRepeatableComment(destination, repeatableComment);
+
+		// check that the auto comment now matches the updated comment
+		tf = getFieldText(addr("0x01002265"));
+		assertEquals(1, tf.getNumRows());
+		assertEquals(tf.getText(), repeatableComment);
+	}
+
 //==================================================================================================
 // Private Methods
 //==================================================================================================
 
 	private ProgramDB buildProgram() throws Exception {
-		ProgramBuilder builder = new ProgramBuilder("sample", ProgramBuilder._TOY, this);
+		ClassicSampleX86ProgramBuilder builder = new ClassicSampleX86ProgramBuilder();
-		builder.createMemory(".text", "0x1001000", 0x6600);
-		builder.createEmptyFunction(null, "0x1002000", 20, null);
-
 		return builder.getProgram();
 	}
 
@@ -111,29 +139,46 @@ public class EolCommentFieldFactoryTest extends AbstractGhidraHeadedIntegrationT
 
 	private void changeFieldWidthToHalfCommentLength(Function function) throws Exception {
 		ListingTextField tf = getFieldText(function);
-
 		FieldElement fieldElement = tf.getFieldElement(0, 0);
 		int stringWidth = fieldElement.getStringWidth();
-
 		setFieldWidth(tf.getFieldFactory(), stringWidth / 2);
 	}
 
 	private ListingTextField getFieldText(Function function) {
-		assertTrue(cb.goToField(function.getEntryPoint(), EolCommentFieldFactory.FIELD_NAME, 1, 1));
+		return getFieldText(function.getEntryPoint());
+	}
+
+	private ListingTextField getFieldText(Address address) {
+		assertTrue(cb.goToField(address, EolCommentFieldFactory.FIELD_NAME, 1, 1));
 		ListingTextField tf = (ListingTextField) cb.getCurrentField();
 		return tf;
 	}
 
 	private void setFieldWidth(final FieldFactory fieldFactory, final int width) throws Exception {
 		SwingUtilities.invokeAndWait(() -> fieldFactory.setWidth(width));
-		waitForPostedSwingRunnables();
+		waitForSwing();
 		cb.updateNow();
 	}
 
 	private void setBooleanOption(final String name, final boolean value) throws Exception {
 		SwingUtilities.invokeAndWait(() -> fieldOptions.setBoolean(name, value));
-		waitForPostedSwingRunnables();
+		waitForSwing();
 		cb.updateNow();
 	}
 
+	private Address addr(String address) {
+		AddressFactory addressFactory = program.getAddressFactory();
+		return addressFactory.getAddress(address);
+	}
+
+	private void setRepeatableComment(Address a, String comment) {
+		setComment(a, CodeUnit.REPEATABLE_COMMENT, comment);
+	}
+
+	private void setComment(Address a, int commentType, String comment) {
+		CodeUnit cu = program.getListing().getCodeUnitAt(a);
+		tx(program, () -> {
+			cu.setComment(commentType, comment);
+		});
+	}
 }

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/Composite.java

@@ -42,7 +42,7 @@ public interface Composite extends DataType {
 	 * For Unions and packed Structures this is equivalent to {@link #getNumComponents()}
 	 * since they do not contain undefined components.
 	 * This count will always exclude all undefined filler components which may be present
-	 * within a Structure whoose packing is disabled (see {@link #isPackingEnabled()}).
+	 * within a Structure whose packing is disabled (see {@link #isPackingEnabled()}).
 	 * @return  the number of explicitly defined components in this composite
 	 */
 	public abstract int getNumDefinedComponents();
@@ -281,7 +281,7 @@ public interface Composite extends DataType {
 	 * </ul>
 	 * In addition, when packing is disabled the default alignment is always 1 unless a
 	 * different minimum alignment has been set.  When packing is enabled the overall
-	 * composite length infleunced by the composite's minimum alignment setting.
+	 * composite length influenced by the composite's minimum alignment setting.
 	 * If a change in enablement occurs, the default alignment and packing behavior
 	 * will be used.
 	 * @param enabled true enables packing of components respecting component
@@ -318,7 +318,7 @@ public interface Composite extends DataType {
 	 * If packing was previously disabled, packing will be enabled.  This value will
 	 * establish the maximum effective alignment for this composite and each of the
 	 * components during the alignment computation (e.g., a value of 1 will eliminate
-	 * any padding).  The overall composite length may be infleunced by the composite's
+	 * any padding).  The overall composite length may be influenced by the composite's
 	 * minimum alignment setting.
 	 * @param packingValue the new positive packing value.
 	 * @throws IllegalArgumentException if a non-positive value is specified.
@@ -338,7 +338,7 @@ public interface Composite extends DataType {
 	 * Enables default packing behavior.
 	 * If packing was previously disabled, packing will be enabled.
 	 * Composite will automatically pack based upon the alignment requirements
-	 * of its components with overall composite length possibly infleunced by the composite's
+	 * of its components with overall composite length possibly influenced by the composite's
 	 * minimum alignment setting.
 	 */
 	public void setToDefaultPacking();
@@ -376,8 +376,9 @@ public interface Composite extends DataType {
 	}
 
 	/**
-	 * Determine if an explicit minimum alignment has been set (see {@link #getExplicitMinimumAlignment()}).
+	 * Determine if an explicit minimum alignment has been set (see
-	 * An undefined value is returned if default alignment or machine alignment is enabled.
+	 * {@link #getExplicitMinimumAlignment()}). An undefined value is returned if default alignment
+	 * or machine alignment is enabled.
 	 * @return true if an explicit minimum alignment has been set, else false
 	 */
 	public default boolean hasExplicitMinimumAlignment() {
@@ -385,7 +386,7 @@ public interface Composite extends DataType {
 	}
 
 	/**
-	 * Get the explicitminimum alignment setting for this Composite which contributes 
+	 * Get the explicit minimum alignment setting for this Composite which contributes
 	 * to the actual computed alignment value (see {@link #getAlignment()}.
 	 * @return the minimum alignment setting for this Composite or an undefined
 	 * non-positive value if an explicit minimum alignment has not been set.
@@ -398,7 +399,7 @@ public interface Composite extends DataType {
 	 * affect the actual computed alignment of this composite.
 	 * When packing is enabled, the alignment setting may also affect padding
 	 * at the end of the composite and its length.  When packing is disabled,
-	 * this setting will not affect the length of thhis composite. 
+	 * this setting will not affect the length of this composite.
 	 * @param minAlignment the minimum alignment for this Composite.
 	 * @throws IllegalArgumentException if a non-positive value is specified
 	 */