From bfe94e705fc2636a1e57fec1b7015090c5e4b8f6 Mon Sep 17 00:00:00 2001 From: Taylor Smock Date: Mon, 7 Aug 2023 07:58:31 -0600 Subject: [PATCH] Fix #23104: `Mark Selected` should account for items not in the list Signed-off-by: Taylor Smock --- .../josm/plugins/todo/TodoListModel.java | 12 ++++++----- .../josm/plugins/todo/TodoDialogTest.java | 20 ++++++++++++++++++- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/src/org/openstreetmap/josm/plugins/todo/TodoListModel.java b/src/org/openstreetmap/josm/plugins/todo/TodoListModel.java index 160317e..01f8378 100644 --- a/src/org/openstreetmap/josm/plugins/todo/TodoListModel.java +++ b/src/org/openstreetmap/josm/plugins/todo/TodoListModel.java @@ -227,11 +227,13 @@ void markItems(Collection items) { final var tempDoneList = new ArrayList(indices.length); for (var i = indices.length - 1; i >= 0; i--) { final var index = indices[i]; - final var item = todoList.get(index); - todoList.remove(index); - tempDoneList.add(item); - if (sel > index) - sel--; + if (index >= 0) { + final var item = todoList.get(index); + todoList.remove(index); + tempDoneList.add(item); + if (sel > index) + sel--; + } } doneList.addAll(tempDoneList); if (sel >= getSize()) diff --git a/test/unit/org/openstreetmap/josm/plugins/todo/TodoDialogTest.java b/test/unit/org/openstreetmap/josm/plugins/todo/TodoDialogTest.java index 090677e..a1582d7 100644 --- a/test/unit/org/openstreetmap/josm/plugins/todo/TodoDialogTest.java +++ b/test/unit/org/openstreetmap/josm/plugins/todo/TodoDialogTest.java @@ -1,6 +1,7 @@ // License: GPL. For details, see LICENSE file. package org.openstreetmap.josm.plugins.todo; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertSame; @@ -107,7 +108,24 @@ void testClear() { assertTrue(this.model.getTodoList().isEmpty()); } + /** + * Non-regression test for #23104: `Mark Selected` should account for items not in the list + */ @Test - void testNonRegression23092() { + void testNonRegression23104() throws Exception { + // Sort the list for stability in tests + final var list = new ArrayList<>(this.ds.allPrimitives()); + list.sort(Comparator.comparing(prim -> prim.get("access"))); + this.ds.setSelected(list.get(0)); + this.actAdd.actionPerformed(null); + final var actMarkSelected = (JosmAction) tryToReadFieldValue("actMarkSelected"); + assertEquals(1, this.model.getSize()); + assertSame(list.get(0), this.model.getElementAt(0).primitive()); + this.ds.setSelected(list.get(1)); + assertDoesNotThrow(() -> actMarkSelected.actionPerformed(null)); + this.ds.setSelected(list.get(1), list.get(2)); + assertDoesNotThrow(() -> actMarkSelected.actionPerformed(null)); + this.ds.setSelected(list); + assertDoesNotThrow(() -> actMarkSelected.actionPerformed(null)); } }