Fix JTemplateTree dnd and view-related bugs

Mostly fixes #485, except for the reset causing UUID duplication.

src/main/kotlin/com/fwdekker/randomness/template/TemplateJTree.kt

@@ -22,6 +22,7 @@ import com.intellij.ui.AnActionButton
 import com.intellij.ui.ColoredTreeCellRenderer
 import com.intellij.ui.CommonActionsPanel
 import com.intellij.ui.LayeredIcon
+import com.intellij.ui.RowsDnDSupport.RefinedDropSupport.Position
 import com.intellij.ui.SimpleTextAttributes
 import com.intellij.ui.ToolbarDecorator
 import com.intellij.ui.TreeSpeedSearch
@@ -144,6 +145,8 @@ class TemplateJTree(
         isRootVisible = false
         showsRootHandles = true
         selectionModel.selectionMode = TreeSelectionModel.SINGLE_TREE_SELECTION
+        myModel.viewIndexToModelIndex = { myModel.root.descendants.indexOf(getPathForRow(it).lastPathComponent) }
+        myModel.wrapDrop = ::runPreservingState
         setCellRenderer(CellRenderer())
 
         setSelectionRow(0)
@@ -250,7 +253,7 @@ class TemplateJTree(
         selectionPath =
             myModel.getPathToRoot(
                 if (myModel.isLeaf(parent)) parent
-                else parent.children[min(oldIndex, parent.children.count() - 1)]
+                else parent.children[min(oldIndex, parent.children.lastIndex)]
             )
     }
 
@@ -275,47 +278,65 @@ class TemplateJTree(
         }
     }
 
+    /**
+     * Returns `true` if and only if [moveSchemeByOnePosition] can be invoked with these parameters.
+     *
+     * @see TemplateJTreeModel.canMoveRow
+     */
+    fun canMoveSchemeByOnePosition(scheme: Scheme, moveDown: Boolean): Boolean {
+        val (fromIndex, toIndex, position) = getMoveDescriptor(scheme, moveDown)
+        return myModel.canMoveRow(fromIndex, toIndex, position)
+    }
+
     /**
      * Moves [scheme] by one position; down if [moveDown] is `true, and up otherwise.
      *
-     * If a non-[Template] is moved up or down outside its parent's boundaries, it is moved to the next parent in that
-     * direction.
+     * @see TemplateJTreeModel.moveRow
      */
     fun moveSchemeByOnePosition(scheme: Scheme, moveDown: Boolean) {
         val node = StateNode(scheme)
 
         runPreservingState {
-            myModel.exchangeRows(
-                myModel.root.descendants.indexOf(node),
-                getMoveTargetIndex(scheme, moveDown)
-            )
+            val (fromIndex, toIndex, position) = getMoveDescriptor(scheme, moveDown)
+            myModel.moveRow(fromIndex, toIndex, position)
         }
 
         makeVisible(myModel.getPathToRoot(node))
     }
 
     /**
-     * Returns `true` if and only if [moveSchemeByOnePosition] can be invoked with these parameters.
+     * Returns the arguments to pass to [TemplateJTreeModel.moveRow] or [TemplateJTreeModel.canMoveRow] to describe
+     * moving [scheme] up (if [moveDown] is `false`) or down (if [moveDown] is `true`).
+     *
+     * If an invalid move is returned, for example with negative indices, the move was not possible to begin with.
+     *
+     * @see canMoveSchemeByOnePosition
+     * @see moveSchemeByOnePosition
      */
-    fun canMoveSchemeByOnePosition(scheme: Scheme, moveDown: Boolean) =
-        myModel.canExchangeRows(
-            myModel.root.descendants.indexOf(StateNode(scheme)),
-            getMoveTargetIndex(scheme, moveDown)
-        )
+    @Suppress("detekt:CognitiveComplexMethod", "detekt:MagicNumber") // Complexity unavoidable, -2 is not magic
+    private fun getMoveDescriptor(scheme: Scheme, moveDown: Boolean): Triple<Int, Int, Position> {
+        val descendants = myModel.root.descendants
+        val templates = myModel.root.children
 
-    /**
-     * Returns the index to which [scheme] is moved (down if [moveDown] is `true`, or up otherwise) when
-     * [moveSchemeByOnePosition] is invoked, or an out-of-range index if [scheme] cannot be moved in that direction.
-     */
-    private fun getMoveTargetIndex(scheme: Scheme, moveDown: Boolean): Int {
-        val node = StateNode(scheme)
+        val fromNode = StateNode(scheme)
+        val (toNode, position) =
+            if (scheme is Template) {
+                val index = templates.indexOf(fromNode)
 
-        val children = myModel.root.children
-        val descendants = myModel.root.descendants
-        val diff = if (moveDown) 1 else -1
+                if (index == templates.lastIndex - 1) Pair(descendants.last(), Position.BELOW)
+                else Pair(templates.getOrNull(index + if (!moveDown) -1 else 2), Position.ABOVE)
+            } else {
+                val index = descendants.indexOf(fromNode)
+                val candidate = descendants.getOrNull(index + if (!moveDown) -2 else 1)
+
+                when {
+                    candidate == null || candidate.state !is Template -> Pair(candidate, Position.BELOW)
+                    candidate.children.isEmpty() -> Pair(candidate, Position.INTO)
+                    else -> Pair(candidate.children.first(), Position.ABOVE)
+                }
+            }
 
-        return if (scheme !is Template) descendants.indexOf(node) + diff
-        else descendants.indexOf(children.getOrNull(children.indexOf(node) + diff))
+        return Triple(descendants.indexOf(fromNode), descendants.indexOf(toNode), position)
     }
 
 
@@ -501,7 +522,7 @@ class TemplateJTree(
             override fun onChosen(value: Scheme?, finalChoice: Boolean) =
                 when (value) {
                     POPUP_STEP_SCHEMES[0] -> TemplatesPopupStep()
-                    POPUP_STEP_SCHEMES[POPUP_STEP_SCHEMES.size - 1] -> ReferencesPopupStep()
+                    POPUP_STEP_SCHEMES[POPUP_STEP_SCHEMES.lastIndex] -> ReferencesPopupStep()
                     else -> super.onChosen(value, finalChoice)
                 }
 
@@ -514,7 +535,7 @@ class TemplateJTree(
              * Returns `true` if and only if [value] equals the [Template] or [TemplateReference] entry.
              */
             override fun hasSubstep(value: Scheme?) =
-                value == POPUP_STEP_SCHEMES[0] || value == POPUP_STEP_SCHEMES[POPUP_STEP_SCHEMES.size - 1]
+                value == POPUP_STEP_SCHEMES[0] || value == POPUP_STEP_SCHEMES[POPUP_STEP_SCHEMES.lastIndex]
 
             /**
              * Returns a separator if [value] should be preceded by a separator, or `null` otherwise.

src/main/kotlin/com/fwdekker/randomness/template/TemplateJTreeModel.kt

@@ -4,7 +4,10 @@ import com.fwdekker.randomness.Bundle
 import com.fwdekker.randomness.Scheme
 import com.fwdekker.randomness.State
 import com.fwdekker.randomness.setAll
+import com.intellij.ui.RowsDnDSupport.RefinedDropSupport
+import com.intellij.ui.RowsDnDSupport.RefinedDropSupport.Position
 import com.intellij.util.ui.EditableModel
+import javax.swing.JComponent
 import javax.swing.JTree
 import javax.swing.event.TreeModelEvent
 import javax.swing.event.TreeModelListener
@@ -22,7 +25,9 @@ import javax.swing.tree.TreePath
  * @param list the list to be modeled
  */
 @Suppress("detekt:TooManyFunctions") // Normal for Swing implementations
-class TemplateJTreeModel(list: TemplateList = TemplateList(mutableListOf())) : TreeModel, EditableModel {
+class TemplateJTreeModel(
+    list: TemplateList = TemplateList(mutableListOf()),
+) : TreeModel, EditableModel, RefinedDropSupport {
     /**
      * The listeners that are informed when the state of the tree changes.
      */
@@ -38,6 +43,35 @@ class TemplateJTreeModel(list: TemplateList = TemplateList(mutableListOf())) : T
      */
     private val nodes get() = listOf(root) + root.descendants
 
+    /**
+     * Converts the row index in the view to the corresponding row index in the model.
+     *
+     * For example, if the model has rows `[0, 3]` with children `[1, 2]` and `[4]`, respectively, and row `0` is
+     * collapsed in the view, then the view has rows `[0, 3, 4]` while the model has rows `[0, 1, 2, 3, 4]`, and this
+     * method functions as the map `{0: 0, 1: 3, 2: 4}`. Inputs outside the view's valid row indices are not supported.
+     *
+     * This field is used only in the methods [canDrop] and [drop] in order to implement [RefinedDropSupport], which
+     * assumes that the model knows view-based indices.
+     *
+     * @see RefinedDropSupport
+     */
+    var viewIndexToModelIndex: (Int) -> Int = { it }
+
+    /**
+     * A wrapper provided by the view and used by [TemplateJTreeModel.drop] to ensure the view's selection and expansion
+     * state are retained.
+     *
+     * This wrapper is invoked in the method [drop] with a lambda that performs the dropping when invoked. By default,
+     * the wrapper does nothing special, but can be overridden to perform some actions before and after dropping the
+     * [StateNode].
+     *
+     * This field is used only in the method [drop] in order to implement [RefinedDropSupport], which assumes that the
+     * model can somehow retain the view's state.
+     *
+     * @see RefinedDropSupport
+     */
+    var wrapDrop: (() -> Unit) -> Unit = { it() }
+
 
     /**
      * Returns [root].
@@ -108,66 +142,122 @@ class TemplateJTreeModel(list: TemplateList = TemplateList(mutableListOf())) : T
     }
 
     /**
-     * Exchanges two consecutive rows, moving the node at row [oldIndex] to row [newIndex].
+     * Use [moveRow] instead.
+     *
+     * @throws UnsupportedOperationException always
+     * @see moveRow
+     */
+    @Throws(UnsupportedOperationException::class)
+    override fun exchangeRows(oldIndex: Int, newIndex: Int) = throw UnsupportedOperationException()
+
+    /**
+     * Use [canMoveRow] instead.
+     *
+     * @throws UnsupportedOperationException always
+     * @see canMoveRow
+     */
+    @Throws(UnsupportedOperationException::class)
+    override fun canExchangeRows(oldIndex: Int, newIndex: Int): Boolean = throw UnsupportedOperationException()
+
+    /**
+     * Returns `true` if and only if [moveRow] can be invoked with the given parameters.
      *
-     * If [oldIndex] and [newIndex] both refer to a template or both refer to a scheme, then the node at [oldIndex] will
-     * be inserted so that it has the same index in its parent as the node at [newIndex] had before invoking this
-     * method. Otherwise, if [oldIndex] refers to a non-template scheme and [newIndex] refers to a template, then the
-     * non-template scheme is moved into the template at [newIndex]; specifically, if [oldIndex] is less than [newIndex]
-     * then the non-template scheme becomes the template's first child, otherwise it becomes the template's last child.
+     * @see moveRow
      */
-    override fun exchangeRows(oldIndex: Int, newIndex: Int) {
-        require(canExchangeRows(oldIndex, newIndex)) {
-            Bundle("template_list.error.cannot_swap_rows", oldIndex, newIndex)
+    fun canMoveRow(fromIndex: Int, toIndex: Int, position: Position): Boolean {
+        val descendants = root.descendants
+        val fromNode = descendants.getOrNull(fromIndex)
+        val toNode = descendants.getOrNull(toIndex)
+
+        return when {
+            fromIndex == toIndex || fromNode == null || toNode == null -> false
+            fromNode.state !is Template -> (toNode.state !is Template) xor (position == Position.INTO)
+            position != Position.ABOVE -> position == Position.BELOW && toIndex == descendants.lastIndex
+            else -> toNode.state is Template && toNode != root.children.run { getOrNull(indexOf(fromNode) + 1) }
+        }
+    }
+
+    /**
+     * Moves the node at row [fromIndex] near the node at row [toIndex] as specified by [position].
+     *
+     * If [fromIndex] refers to a [Template], then the [Template] can either be moved [Position.ABOVE] another
+     * [Template], or [Position.BELOW] the very last [Scheme] of the [TemplateList].
+     *
+     * If [fromIndex] refers to a non-[Template] [Scheme], then the [Scheme] can either be moved [Position.INTO] a
+     * [Template] to append it to that [Template], or [Position.ABOVE] or [Position.BELOW] another non-[Template]
+     * [Scheme].
+     *
+     * @throws IllegalArgumentException if [canMoveRow] returns `false` for the given arguments
+     */
+    fun moveRow(fromIndex: Int, toIndex: Int, position: Position) {
+        require(canMoveRow(fromIndex, toIndex, position)) {
+            Bundle("template_list.error.cannot_move_row", fromIndex, position.name, toIndex)
         }
 
-        val nodeToMove = root.descendants[oldIndex]
-        val targetNode = root.descendants[newIndex]
+        val descendants = root.descendants
+        val fromNode = descendants[fromIndex]
+        val toNode = descendants[toIndex]
 
-        val targetNodeParent: StateNode
-        val targetIndexInParent: Int
-        if (nodeToMove.state !is Template && targetNode.state is Template) {
-            if (newIndex < oldIndex) {
-                targetNodeParent = root.children[root.children.indexOf(targetNode) - 1]
-                targetIndexInParent = targetNodeParent.children.count()
-            } else {
-                targetNodeParent = targetNode
-                targetIndexInParent = 0
+        removeNode(fromNode)
+
+        if (fromNode.state is Template) {
+            when (position) {
+                Position.ABOVE -> insertNode(root, fromNode, root.children.indexOf(toNode))
+                Position.BELOW -> insertNode(root, fromNode)
+                Position.INTO -> error("Bug: 'canMoveRow' should have caught this case.")
             }
         } else {
-            targetNodeParent = getParentOf(targetNode)!!
-            targetIndexInParent = targetNodeParent.children.indexOf(targetNode)
+            val toNodeParent = getParentOf(toNode)!!
+            when (position) {
+                Position.ABOVE -> insertNode(toNodeParent, fromNode, toNodeParent.children.indexOf(toNode))
+                Position.BELOW -> insertNodeAfter(toNodeParent, toNode, fromNode)
+                Position.INTO -> insertNode(toNode, fromNode)
+            }
         }
-
-        removeNode(nodeToMove)
-        insertNode(targetNodeParent, nodeToMove, targetIndexInParent)
     }
 
     /**
-     * Returns `true` if and only if the node at row [oldIndex] can be moved to row [newIndex].
+     * Invokes [canMoveRow] after converting [fromIndex] and [toIndex] using [viewIndexToModelIndex].
+     *
+     * @see canMoveRow
+     * @see RefinedDropSupport
      */
-    override fun canExchangeRows(oldIndex: Int, newIndex: Int): Boolean {
-        val oldNode = root.descendants.getOrNull(oldIndex)
-        val newNode = root.descendants.getOrNull(newIndex)
+    override fun canDrop(fromIndex: Int, toIndex: Int, position: Position) =
+        canMoveRow(viewIndexToModelIndex(fromIndex), viewIndexToModelIndex(toIndex), position)
 
-        return oldNode != null && newNode != null &&
-            if (oldNode.state is Template) newNode.state is Template
-            else newIndex != 0
-    }
+    /**
+     * Returns `true` if and only if the node at [fromIndex] can be moved [Position.INTO] the node at [toIndex].
+     *
+     * @see canMoveRow
+     * @see RefinedDropSupport
+     */
+    override fun isDropInto(component: JComponent, fromIndex: Int, toIndex: Int) =
+        canDrop(fromIndex, toIndex, Position.INTO)
+
+    /**
+     * Invokes [moveRow] after converting [fromIndex] and [toIndex] using [viewIndexToModelIndex].
+     *
+     * @see moveRow
+     * @see RefinedDropSupport
+     */
+    override fun drop(fromIndex: Int, toIndex: Int, position: Position) =
+        wrapDrop { moveRow(viewIndexToModelIndex(fromIndex), viewIndexToModelIndex(toIndex), position) }
 
 
     /**
-     * Returns `true` if and only if [node] does not have children.
+     * Returns `true` if and only if [node] is contained in this model and [node] does not have children.
      *
-     * Throws an exception if [node] is not a [StateNode] or if [node] is not contained in this model.
+     * Throws an exception if [node] is not a [StateNode].
+     *
+     * Unlike methods such as [getChild], this method does not throw an exception if [node] is not contained in this
+     * model, because [com.intellij.ui.tree.ui.DefaultTreeUI] calls this method on non-nodes during drag-and-drop.
      */
     override fun isLeaf(node: Any): Boolean {
         require(node is StateNode) {
             Bundle("template_list.error.unknown_node_type", "node", node.javaClass.canonicalName)
         }
-        require(node in nodes) { Bundle("template_list.error.node_not_in_tree") }
 
-        return !node.canHaveChildren || node.children.isEmpty()
+        return node in nodes && (!node.canHaveChildren || node.children.isEmpty())
     }
 
     /**

src/main/resources/randomness.properties

@@ -122,8 +122,8 @@ template.name.unknown=<unknown>
 template.ui.name_option=&Name:
 template.title=Template
 template_list.error.cannot_insert_root='TemplateList' cannot have parent so cannot be inserted.
+template_list.error.cannot_move_row=Cannot move row %1$d %2$s row %3$d.
 template_list.error.cannot_remove_root=Cannot remove root from model.
-template_list.error.cannot_swap_rows=Cannot swap row at index %1$d with row at index %2$d.
 template_list.error.duplicate_name=Template names should be unique. Rename template '%1$s'.
 template_list.error.failed_to_save_settings=Failed To Save Settings
 template_list.error.infertile_parent=Cannot access child of parent that cannot have children.

src/test/kotlin/com/fwdekker/randomness/InsertActionTest.kt

@@ -14,7 +14,7 @@ import io.kotest.matchers.shouldBe
 /**
  * Integration tests for [InsertAction].
  */
-@Suppress("detekt:FunctionMaxLength", "detekt:FunctionName", "detekt:TooManyFunctions")
+@Suppress("detekt:FunctionMaxLength", "detekt:FunctionName", "detekt:TooManyFunctions") // Acceptable for tests
 class InsertActionTest : BasePlatformTestCase() {
     private lateinit var document: Document
     private lateinit var caretModel: CaretModel