Merge remote-tracking branch 'origin/GP-792-dragonmacher-table-dialog-sort-column'

Ghidra/Features/Base/src/main/java/ghidra/app/tablechooser/TableChooserDialog.java

@@ -20,6 +20,7 @@ import java.util.ArrayList;
 import java.util.List;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.atomic.AtomicReference;
 
 import javax.swing.*;
 import javax.swing.table.TableCellRenderer;
@@ -38,7 +39,7 @@ import ghidra.program.model.listing.Program;
 import ghidra.program.util.ProgramLocation;
 import ghidra.program.util.ProgramSelection;
 import ghidra.util.HelpLocation;
-import ghidra.util.SystemUtilities;
+import ghidra.util.Swing;
 import ghidra.util.datastruct.WeakDataStructureFactory;
 import ghidra.util.datastruct.WeakSet;
 import ghidra.util.table.*;
@@ -113,8 +114,7 @@ public class TableChooserDialog extends DialogComponentProvider
 			table.installNavigation(goToService, navigatable);
 		}
 		table.getSelectionModel()
-				.addListSelectionListener(
+				.addListSelectionListener(e -> setOkEnabled(table.getSelectedRowCount() > 0));
-					e -> setOkEnabled(table.getSelectedRowCount() > 0));
 
 		GhidraTableFilterPanel<AddressableRowObject> filterPanel =
 			new GhidraTableFilterPanel<>(table, model);
@@ -128,7 +128,7 @@ public class TableChooserDialog extends DialogComponentProvider
 	 * @param callback the callback to notify
 	 */
 	public void setClosedListener(Callback callback) {
-		this.closedCallback = Callback.dummyIfNull(callback);
+		Swing.runNow(() -> closedCallback = Callback.dummyIfNull(callback));
 	}
 
 	/**
@@ -153,8 +153,7 @@ public class TableChooserDialog extends DialogComponentProvider
 	private void createTableModel() {
 
 		// note: the task monitor is installed later when this model is added to the threaded panel
-		SystemUtilities.runSwingNow(
+		Swing.runNow(() -> model = new TableChooserTableModel("Test", tool, program, null));
-			() -> model = new TableChooserTableModel("Test", tool, program, null));
 	}
 
 	private void createActions() {
@@ -175,8 +174,8 @@ public class TableChooserDialog extends DialogComponentProvider
 		};
 
 		DockingAction selectionNavigationAction = new SelectionNavigationAction(owner, table);
-		selectionNavigationAction.setHelpLocation(
+		selectionNavigationAction
-			new HelpLocation(HelpTopics.SEARCH, "Selection_Navigation"));
+				.setHelpLocation(new HelpLocation(HelpTopics.SEARCH, "Selection_Navigation"));
 
 		addAction(selectAction);
 		addAction(selectionNavigationAction);
@@ -296,7 +295,49 @@ public class TableChooserDialog extends DialogComponentProvider
 	}
 
 	public void addCustomColumn(ColumnDisplay<?> columnDisplay) {
-		model.addCustomColumn(columnDisplay);
+		Swing.runNow(() -> model.addCustomColumn(columnDisplay));
+	}
+
+	/**
+	 * Sets the default sorted column for this dialog.
+	 *
+	 * <P>This method should be called after all custom columns have been added via
+	 * {@link #addCustomColumn(ColumnDisplay)}.
+	 *
+	 * @param index the view's 0-based column index
+	 * @see #setSortState(TableSortState)
+	 * @throws IllegalArgumentException if an invalid column is requested for sorting
+	 */
+	public void setSortColumn(int index) {
+		setSortState(TableSortState.createDefaultSortState(index));
+	}
+
+	/**
+	 * Sets the column sort state for this dialog.   The {@link TableSortState} allows for
+	 * combinations of sorted columns in ascending or descending order.
+	 *
+	 * <P>This method should be called after all custom columns have been added via
+	 * {@link #addCustomColumn(ColumnDisplay)}.
+	 *
+	 * @param state the sort state
+	 * @see #setSortColumn(int)
+	 * @throws IllegalArgumentException if an invalid column is requested for sorting
+	 */
+	public void setSortState(TableSortState state) {
+		AtomicReference<IllegalArgumentException> ref = new AtomicReference<>();
+		Swing.runNow(() -> {
+			try {
+				model.setTableSortState(state);
+			}
+			catch (IllegalArgumentException e) {
+				ref.set(e);
+			}
+		});
+		IllegalArgumentException exception = ref.get();
+		if (exception != null) {
+			// use a new exception so the stack trace points to this class, not the runnable above
+			throw new IllegalArgumentException(exception);
+		}
 	}
 
 	@Override
@@ -313,15 +354,17 @@ public class TableChooserDialog extends DialogComponentProvider
 	}
 
 	public void clearSelection() {
-		table.clearSelection();
+		Swing.runNow(() -> table.clearSelection());
 	}
 
 	public void selectRows(int... rows) {
 
-		ListSelectionModel selectionModel = table.getSelectionModel();
+		Swing.runNow(() -> {
-		for (int row : rows) {
+			ListSelectionModel selectionModel = table.getSelectionModel();
-			selectionModel.addSelectionInterval(row, row);
+			for (int row : rows) {
-		}
+				selectionModel.addSelectionInterval(row, row);
+			}
+		});
 	}
 
 	public int[] getSelectedRows() {

Ghidra/Features/Base/src/test.slow/java/ghidra/app/tablechooser/TableChooserDialogTest.java

@@ -33,12 +33,15 @@ import docking.action.*;
 import docking.actions.KeyEntryDialog;
 import docking.actions.ToolActions;
 import docking.tool.util.DockingToolConstants;
+import docking.widgets.table.TableSortState;
 import ghidra.app.nav.Navigatable;
 import ghidra.framework.options.ToolOptions;
 import ghidra.framework.plugintool.DummyPluginTool;
+import ghidra.program.database.ProgramBuilder;
+import ghidra.program.database.ProgramDB;
 import ghidra.program.model.address.Address;
-import ghidra.program.model.address.TestAddress;
+import ghidra.program.model.data.DataType;
-import ghidra.program.model.listing.Program;
+import ghidra.program.model.listing.*;
 import ghidra.test.AbstractGhidraHeadedIntegrationTest;
 import ghidra.test.ToyProgramBuilder;
 import resources.Icons;
@@ -75,16 +78,48 @@ public class TableChooserDialogTest extends AbstractGhidraHeadedIntegrationTest
 
 		tool = new DummyPluginTool();
 		tool.setVisible(true);
-		Program program = new ToyProgramBuilder("Test", true).getProgram();
+
+		List<Address> addresses = new ArrayList<>();
+		ToyProgramBuilder builder = new ToyProgramBuilder("Test", true);
+		builder.createMemory(".text", "0x0", 0x110);
+		Function f = createFunction(builder, 0x00);
+		addresses.add(f.getEntryPoint());
+		f = createFunction(builder, 0x10);
+		addresses.add(f.getEntryPoint());
+		f = createFunction(builder, 0x20);
+		addresses.add(f.getEntryPoint());
+		f = createFunction(builder, 0x30);
+		addresses.add(f.getEntryPoint());
+		f = createFunction(builder, 0x40);
+		addresses.add(f.getEntryPoint());
+		f = createFunction(builder, 0x50);
+		addresses.add(f.getEntryPoint());
+
+		Program program = builder.getProgram();
 		Navigatable navigatable = null;
 		dialog = new TableChooserDialog(tool, executor, program, "Dialog Title", navigatable);
 
 		testAction = new TestAction();
 		dialog.addAction(testAction);
 
+		dialog.addCustomColumn(new OffsetTestColumn());
+		dialog.addCustomColumn(new SpaceTestColumn());
+
 		dialog.show();
 		waitForDialogComponent(TableChooserDialog.class);
-		loadData();
+		loadData(addresses);
+	}
+
+	private Function createFunction(ProgramBuilder builder, long addr) throws Exception {
+		ProgramDB p = builder.getProgram();
+		FunctionManager fm = p.getFunctionManager();
+		Function f = fm.getFunctionAt(builder.addr(addr));
+		if (f != null) {
+			return f;
+		}
+
+		String a = Long.toHexString(addr);
+		return builder.createEmptyFunction("Function_" + a, "0x" + a, 5, DataType.DEFAULT);
 	}
 
 	private void reCreateDialog(SpyTableChooserExecutor dialogExecutor) throws Exception {
@@ -92,9 +127,9 @@ public class TableChooserDialogTest extends AbstractGhidraHeadedIntegrationTest
 		createDialog(dialogExecutor);
 	}
 
-	private void loadData() {
+	private void loadData(List<Address> addresses) {
-		for (int i = 0; i < 7; i++) {
+		for (Address a : addresses) {
-			dialog.add(new TestStubRowObject());
+			dialog.add(new TestStubRowObject(a));
 		}
 
 		waitForDialog();
@@ -308,10 +343,40 @@ public class TableChooserDialogTest extends AbstractGhidraHeadedIntegrationTest
 		assertTrue(newToolTip.contains("(A)"));
 	}
 
+	@Test
+	public void testSetSortColumn() throws Exception {
+		assertSortedColumn(0);
+		dialog.setSortColumn(1);
+		assertSortedColumn(1);
+	}
+
+	@Test
+	public void testSetSortState() throws Exception {
+		assertSortedColumn(0);
+		dialog.setSortState(TableSortState.createDefaultSortState(2, false));
+		assertSortedColumn(2);
+	}
+
+	@Test(expected = IllegalArgumentException.class)
+	public void testSetSortState_Invalid() throws Exception {
+		assertSortedColumn(0);
+		dialog.setSortState(TableSortState.createDefaultSortState(100));
+	}
+
 //==================================================================================================
 // Private Methods
 //==================================================================================================
 
+	private void assertSortedColumn(int expectedColumn) {
+		waitForCondition(() -> expectedColumn == getSortColumn(),
+			"Incorrect sorted column; expected " + expectedColumn + ", found " + getSortColumn());
+	}
+
+	private int getSortColumn() {
+		TableChooserTableModel model = getModel();
+		return runSwing(() -> model.getPrimarySortColumnIndex());
+	}
+
 	private void setKeyBindingViaF4Dialog(DockingAction action, KeyStroke ks) {
 
 		// simulate the user mousing over the toolbar button
@@ -415,6 +480,7 @@ public class TableChooserDialogTest extends AbstractGhidraHeadedIntegrationTest
 
 	private void waitForDialog() {
 		waitForCondition(() -> !dialog.isBusy());
+		waitForSwing();
 	}
 
 	private void pressExecuteButton() {
@@ -485,16 +551,15 @@ public class TableChooserDialogTest extends AbstractGhidraHeadedIntegrationTest
 
 	private static class TestStubRowObject implements AddressableRowObject {
 
-		private static int counter;
+		private Address addr;
-		private long addr;
 
-		TestStubRowObject() {
+		TestStubRowObject(Address a) {
-			addr = ++counter;
+			this.addr = a;
 		}
 
 		@Override
 		public Address getAddress() {
-			return new TestAddress(addr);
+			return addr;
 		}
 
 		@Override
@@ -503,6 +568,42 @@ public class TableChooserDialogTest extends AbstractGhidraHeadedIntegrationTest
 		}
 	}
 
+	private static class OffsetTestColumn extends AbstractColumnDisplay<String> {
+
+		@Override
+		public String getColumnValue(AddressableRowObject rowObject) {
+			return Long.toString(rowObject.getAddress().getOffset());
+		}
+
+		@Override
+		public String getColumnName() {
+			return "Offset";
+		}
+
+		@Override
+		public int compare(AddressableRowObject o1, AddressableRowObject o2) {
+			return o1.getAddress().compareTo(o2.getAddress());
+		}
+	}
+
+	private static class SpaceTestColumn extends AbstractColumnDisplay<String> {
+
+		@Override
+		public String getColumnValue(AddressableRowObject rowObject) {
+			return rowObject.getAddress().getAddressSpace().toString();
+		}
+
+		@Override
+		public String getColumnName() {
+			return "Space";
+		}
+
+		@Override
+		public int compare(AddressableRowObject o1, AddressableRowObject o2) {
+			return o1.getAddress().compareTo(o2.getAddress());
+		}
+	}
+
 	private class TestAction extends DockingAction {
 
 		private int invoked;

Ghidra/Framework/Docking/src/main/java/docking/widgets/dialogs/TableChooserDialog.java

@@ -26,11 +26,14 @@ import docking.DialogComponentProvider;
 import docking.widgets.table.*;
 
 /**
- * Dialog for displaying table data in a dialog for the purpose of the user selecting one or
+ * @param <T> the type
- * more items from the table.
  *
- * @param <T> The type of row object in the table.
+ *
+ * @deprecated  This class has been replaced by {@link TableSelectionDialog}.   At the time of
+ * writing, both classes are identical.   This version introduced a naming conflict with another
+ * API.   Thus, the new version better matches the existing dialog choosing API.
  */
+@Deprecated(forRemoval = true, since = "9.3")
 public class TableChooserDialog<T> extends DialogComponentProvider {
 
 	private RowObjectTableModel<T> model;
@@ -44,7 +47,9 @@ public class TableChooserDialog<T> extends DialogComponentProvider {
 	 * @param model a {@link RowObjectTableModel} that has the tRable data
 	 * @param allowMultipleSelection if true, the dialog allows the user to select more
 	 * than one row; otherwise, only single selection is allowed
+	 * @deprecated see the class header
 	 */
+	@Deprecated(forRemoval = true, since = "9.3")
 	public TableChooserDialog(String title, RowObjectTableModel<T> model,
 			boolean allowMultipleSelection) {
 		super(title);
@@ -57,7 +62,9 @@ public class TableChooserDialog<T> extends DialogComponentProvider {
 	/**
 	 * Returns the list of selected items or null if the dialog was cancelled.
 	 * @return  the list of selected items or null if the dialog was cancelled.
+	 * @deprecated see the class header
 	 */
+	@Deprecated(forRemoval = true, since = "9.3")
 	public List<T> getSelectionItems() {
 		return selectedItems;
 	}

Ghidra/Framework/Docking/src/main/java/docking/widgets/dialogs/TableSelectionDialog.java

@@ -0,0 +1,130 @@
+/* ###
+ * IP: GHIDRA
+ *
+ * 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.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package docking.widgets.dialogs;
+
+import java.awt.event.MouseAdapter;
+import java.awt.event.MouseEvent;
+import java.util.Arrays;
+import java.util.List;
+
+import javax.swing.*;
+
+import docking.DialogComponentProvider;
+import docking.widgets.table.*;
+
+/**
+ * Dialog for displaying table data in a dialog for the purpose of the user selecting one or
+ * more items from the table.
+ *
+ * @param <T> The type of row object in the table.
+ */
+public class TableSelectionDialog<T> extends DialogComponentProvider {
+
+	private RowObjectTableModel<T> model;
+	private GFilterTable<T> gFilterTable;
+	private List<T> selectedItems;
+
+	/**
+	 * Create a new Dialog for displaying and choosing table row items
+	 * 
+	 * @param title The title for the dialog
+	 * @param model a {@link RowObjectTableModel} that has the tRable data
+	 * @param allowMultipleSelection if true, the dialog allows the user to select more
+	 * than one row; otherwise, only single selection is allowed
+	 */
+	public TableSelectionDialog(String title, RowObjectTableModel<T> model,
+			boolean allowMultipleSelection) {
+		super(title);
+		this.model = model;
+		addWorkPanel(buildTable(allowMultipleSelection));
+		addOKButton();
+		addCancelButton();
+	}
+
+	/**
+	 * Returns the list of selected items or null if the dialog was cancelled.
+	 * @return  the list of selected items or null if the dialog was cancelled.
+	 */
+	public List<T> getSelectionItems() {
+		return selectedItems;
+	}
+
+	private void initializeTable(boolean allowMultipleSelection) {
+		GTable table = gFilterTable.getTable();
+
+		table.setAutoResizeMode(JTable.AUTO_RESIZE_LAST_COLUMN);
+
+		int selectionMode = allowMultipleSelection ? ListSelectionModel.MULTIPLE_INTERVAL_SELECTION
+				: ListSelectionModel.SINGLE_SELECTION;
+		table.getSelectionModel().setSelectionMode(selectionMode);
+
+	}
+
+	protected void processMouseClicked(MouseEvent e) {
+
+		if (e.getClickCount() != 2) {
+			return;
+		}
+
+		int rowAtPoint = gFilterTable.getTable().rowAtPoint(e.getPoint());
+		if (rowAtPoint < 0) {
+			return;
+		}
+
+		T selectedRowObject = gFilterTable.getSelectedRowObject();
+		selectedItems = Arrays.asList(selectedRowObject);
+		close();
+	}
+
+	@Override
+	protected void okCallback() {
+		selectedItems = gFilterTable.getSelectedRowObjects();
+		close();
+		gFilterTable.dispose();
+	}
+
+	@Override
+	protected void cancelCallback() {
+		selectedItems = null;
+		close();
+		gFilterTable.dispose();
+	}
+
+	@Override
+	protected void dialogShown() {
+		gFilterTable.focusFilter();
+	}
+
+	private JComponent buildTable(boolean allowMultipleSelection) {
+		gFilterTable = new GFilterTable<>(model);
+		initializeTable(allowMultipleSelection);
+		gFilterTable.getTable().addMouseListener(new MouseAdapter() {
+			@Override
+			public void mouseClicked(MouseEvent e) {
+				if (!e.isShiftDown()) {
+					processMouseClicked(e);
+				}
+				updateOkEnabled();
+			}
+		});
+		setOkEnabled(false);
+		return gFilterTable;
+	}
+
+	protected void updateOkEnabled() {
+		setOkEnabled(gFilterTable.getSelectedRowObject() != null);
+	}
+}

Ghidra/Framework/Docking/src/main/java/docking/widgets/table/AbstractSortedTableModel.java

@@ -158,14 +158,15 @@ public abstract class AbstractSortedTableModel<T> extends AbstractGTableModel<T>
 		}
 
 		// verify the requested columns are sortable
-		for (int i = 0; i < columnCount; i++) {
+		for (ColumnSortState state : tableSortState) {
-			ColumnSortState state = tableSortState.getColumnSortState(i);
+
-			if (state == null) {
+			int index = state.getColumnModelIndex();
-				continue; // no sort state for this column--nothing to validate
+			if (!isSortable(index)) {
+				return false; // the state wants to sort on an unsortable column
 			}
 
-			if (!isSortable(i)) {
+			if (index >= columnCount) {
-				return false; // the state wants to sort on an unsortable column
+				return false; // requested a column that is larger than the number of columns
 			}
 		}
 

Ghidra/Framework/Docking/src/main/java/docking/widgets/table/ColumnSortState.java

@@ -55,6 +55,9 @@ public class ColumnSortState {
 	private int sortOrder_OneBased = -1;
 
 	ColumnSortState(int columnModelIndex, SortDirection sortDirection, int sortOrder) {
+		if (columnModelIndex < 0) {
+			throw new IllegalArgumentException("Column index cannot be negative");
+		}
 		this.columnModelIndex = columnModelIndex;
 		this.sortDirection = sortDirection;
 		this.sortOrder_OneBased = sortOrder;