Allow deleting non-empty folders (#1048)

novelwriter/gui/projtree.py

@@ -404,7 +404,7 @@ class GuiProjectTree(QTreeWidget):
             return False
 
         logger.verbose("Deleting %d file(s) from Trash", nTrash)
-        for tHandle in self.getTreeFromHandle(trashHandle):
+        for tHandle in reversed(self.getTreeFromHandle(trashHandle)):
             if tHandle == trashHandle:
                 continue
             self.deleteItem(tHandle, alreadyAsked=True, bulkAction=True)
@@ -418,8 +418,8 @@ class GuiProjectTree(QTreeWidget):
         """Delete an item from the project tree. As a first step, files are
         moved to the Trash folder. Permanent deletion is a second step. This
         second step also deletes the item from the project object as well as
-        delete the files on disk. Folders are deleted if they're empty only,
+        delete the files on disk. Root folders are deleted if they're empty
-        and the deletion is always permanent.
+        only, and the deletion is always permanent.
         """
         if not self.theParent.hasProject:
             logger.error("No project open")
@@ -444,90 +444,8 @@ class GuiProjectTree(QTreeWidget):
             return False
 
         wCount = self._getItemWordCount(tHandle)
-        if nwItemS.itemType == nwItemType.FILE:
+        if nwItemS.itemType == nwItemType.ROOT:
-            logger.debug("User requested file '%s' deleted", tHandle)
+            logger.debug("User requested a root folder '%s' deleted", tHandle)
-            trItemP = trItemS.parent()
-            trItemT = self._addTrashRoot()
-            if trItemP is None or trItemT is None:
-                logger.error("Could not delete item")
-                return False
-
-            if self.theProject.projTree.isTrash(tHandle):
-                # If the file is in the trash folder already, as the
-                # user if they want to permanently delete the file.
-                doPermanent = False
-                if not alreadyAsked:
-                    msgYes = self.theParent.askQuestion(
-                        self.tr("Delete File"),
-                        self.tr("Permanently delete file '{0}'?").format(nwItemS.itemName)
-                    )
-                    if msgYes:
-                        doPermanent = True
-                else:
-                    doPermanent = True
-
-                if doPermanent:
-                    logger.debug("Permanently deleting file with handle '%s'", tHandle)
-
-                    delDoc = NWDoc(self.theProject, tHandle)
-                    if not delDoc.deleteDocument():
-                        self.theParent.makeAlert([
-                            self.tr("Could not delete document file."), delDoc.getError()
-                        ], nwAlert.ERROR)
-                        return False
-
-                    self.propagateCount(tHandle, 0)
-                    tIndex = trItemP.indexOfChild(trItemS)
-                    trItemC = trItemP.takeChild(tIndex)
-
-                    if self.theParent.docEditor.docHandle() == tHandle:
-                        self.theParent.closeDocument()
-
-                    self.theIndex.deleteHandle(tHandle)
-                    self._deleteTreeItem(tHandle)
-                    self._setTreeChanged(True)
-                    self.wordCountsChanged.emit()
-
-            else:
-                # The file is not already in the trash folder, so we
-                # move it there.
-                msgYes = self.theParent.askQuestion(
-                    self.tr("Delete File"),
-                    self.tr("Move file '{0}' to Trash?").format(nwItemS.itemName),
-                )
-                if msgYes:
-                    logger.debug("Moving file '%s' to trash", tHandle)
-
-                    self.propagateCount(tHandle, 0)
-                    tIndex = trItemP.indexOfChild(trItemS)
-                    trItemC = trItemP.takeChild(tIndex)
-                    trItemT.addChild(trItemC)
-                    self._postItemMove(tHandle, wCount)
-                    self._recordLastMove(trItemS, trItemP, tIndex)
-                    self._setTreeChanged(True)
-
-        elif nwItemS.itemType == nwItemType.FOLDER:
-            logger.debug("User requested folder '%s' deleted", tHandle)
-            trItemP = trItemS.parent()
-            if trItemP is None:
-                logger.error("Could not delete folder")
-                return False
-
-            tIndex = trItemP.indexOfChild(trItemS)
-            if trItemS.childCount() == 0:
-                trItemP.takeChild(tIndex)
-                self._deleteTreeItem(tHandle)
-                self._setTreeChanged(True)
-            else:
-                self.theParent.makeAlert(self.tr(
-                    "Cannot delete folder. It is not empty. "
-                    "Recursive deletion is not supported. "
-                    "Please delete the content first."
-                ), nwAlert.ERROR)
-                return False
-
-        elif nwItemS.itemType == nwItemType.ROOT:
-            logger.debug("User requested root folder '%s' deleted", tHandle)
             tIndex = self.indexOfTopLevelItem(trItemS)
             if trItemS.childCount() == 0:
                 self.takeTopLevelItem(tIndex)
@@ -541,6 +459,60 @@ class GuiProjectTree(QTreeWidget):
                 ), nwAlert.ERROR)
                 return False
 
+        else:
+            logger.debug("User requested a file or folder '%s' deleted", tHandle)
+            trItemP = trItemS.parent()
+            trItemT = self._addTrashRoot()
+            if trItemP is None or trItemT is None:
+                logger.error("Could not delete item")
+                return False
+
+            if self.theProject.projTree.isTrash(tHandle):
+                # If the file is in the trash folder already, as the
+                # user if they want to permanently delete the file.
+                doPermanent = False
+                if not alreadyAsked:
+                    msgYes = self.theParent.askQuestion(
+                        self.tr("Delete"),
+                        self.tr("Permanently delete '{0}'?").format(nwItemS.itemName)
+                    )
+                    if msgYes:
+                        doPermanent = True
+                else:
+                    doPermanent = True
+
+                if doPermanent:
+                    logger.debug("Permanently deleting item with handle '%s'", tHandle)
+
+                    self.propagateCount(tHandle, 0)
+                    tIndex = trItemP.indexOfChild(trItemS)
+                    trItemC = trItemP.takeChild(tIndex)
+                    for dHandle in reversed(self.getTreeFromHandle(tHandle)):
+                        if self.theParent.docEditor.docHandle() == dHandle:
+                            self.theParent.closeDocument()
+                        self._deleteTreeItem(dHandle)
+
+                    self._setTreeChanged(True)
+                    self.wordCountsChanged.emit()
+
+            else:
+                # The item is not already in the trash folder, so we
+                # move it there.
+                msgYes = self.theParent.askQuestion(
+                    self.tr("Delete"),
+                    self.tr("Move '{0}' to Trash?").format(nwItemS.itemName),
+                )
+                if msgYes:
+                    logger.debug("Moving item '%s' to trash", tHandle)
+
+                    self.propagateCount(tHandle, 0)
+                    tIndex = trItemP.indexOfChild(trItemS)
+                    trItemC = trItemP.takeChild(tIndex)
+                    trItemT.addChild(trItemC)
+                    self._postItemMove(tHandle, wCount)
+                    self._recordLastMove(trItemS, trItemP, tIndex)
+                    self._setTreeChanged(True)
+
         return True
 
     def setTreeItemValues(self, tHandle):
@@ -863,11 +835,21 @@ class GuiProjectTree(QTreeWidget):
         return self._treeMap.get(tHandle, None)
 
     def _deleteTreeItem(self, tHandle):
-        """Delete a tree item from the project and the map.
+        """Permanently delete a tree item from the project and the map.
         """
+        if self.theProject.projTree.checkType(tHandle, nwItemType.FILE):
+            delDoc = NWDoc(self.theProject, tHandle)
+            if not delDoc.deleteDocument():
+                self.theParent.makeAlert([
+                    self.tr("Could not delete document file."), delDoc.getError()
+                ], nwAlert.ERROR)
+                return False
+
+        self.theIndex.deleteHandle(tHandle)
         del self.theProject.projTree[tHandle]
         self._treeMap.pop(tHandle, None)
-        return
+
+        return True
 
     def _scanChildren(self, theList, tItem, tIndex):
         """This is a recursive function returning all items in a tree

tests/test_gui/test_gui_projtree.py

@@ -298,9 +298,6 @@ def testGuiProjTree_DeleteItems(qtbot, caplog, monkeypatch, nwGUI, fncDir, mockR
         "0000000000010", "0000000000011", "0000000000012",
     ]
 
-    # Delete File
-    # ===========
-
     # Delete item without focus -> blocked
     monkeypatch.setattr(GuiProjectTree, "hasFocus", lambda *a: False)
     nwTree.setSelectedHandle("0000000000012")
@@ -319,6 +316,16 @@ def testGuiProjTree_DeleteItems(qtbot, caplog, monkeypatch, nwGUI, fncDir, mockR
     assert nwTree.deleteItem("0000000000000") is False
     assert "Could not find tree item" in caplog.text
 
+    # Delete Folder/Root
+    # ==================
+
+    # Deleting non-empty folders is blocked
+    assert nwTree.deleteItem("0000000000008") is False  # Novel Root
+    assert nwTree.deleteItem("000000000000a") is True   # Character Root
+
+    # Delete File
+    # ===========
+
     # Block adding trash folder
     funcPointer = nwTree._addTrashRoot
     nwTree._addTrashRoot = lambda *a: None
@@ -352,12 +359,7 @@ def testGuiProjTree_DeleteItems(qtbot, caplog, monkeypatch, nwGUI, fncDir, mockR
         trashHandle, "0000000000011"
     ]
 
-    # Try to delete the second document, but block the deletion
+    # Delete the second file, and skip asking for permission
-    with monkeypatch.context() as mp:
-        mp.setattr("novelwriter.core.document.NWDoc.deleteDocument", lambda *a: False)
-        assert nwTree.deleteItem("0000000000011") is False
-
-    # Delete proper, and skip asking for permission
     assert os.path.isfile(os.path.join(prjDir, "content", "0000000000011.nwd"))
     assert "0000000000011" in nwGUI.theProject.projTree
     assert nwTree.deleteItem("0000000000011", alreadyAsked=True) is True
@@ -365,33 +367,36 @@ def testGuiProjTree_DeleteItems(qtbot, caplog, monkeypatch, nwGUI, fncDir, mockR
     assert "0000000000011" not in nwGUI.theProject.projTree
     assert nwTree.getTreeFromHandle(trashHandle) == [trashHandle]
 
-    # Delete Folder/Root
+    # Delete Folder
-    # ==================
+    # =============
 
-    # Deleting non-empty folders is blocked
+    trashHandle = nwGUI.theProject.projTree.trashRoot()
-    assert nwTree.deleteItem("000000000000d") is False  # Folder
-    assert nwTree.deleteItem("0000000000008") is False  # Root
 
-    # Add a folder we can delete
+    # Add a folder with two files
-    nwTree.setSelectedHandle("000000000000a")  # Character Root
+    nwTree.setSelectedHandle("0000000000009")
     assert nwTree.newTreeItem(nwItemType.FOLDER) is True
-    assert "0000000000014" in nwGUI.theProject.projTree
+    nwTree.setSelectedHandle("0000000000014")
+    assert nwTree.newTreeItem(nwItemType.FILE) is True
+    assert nwTree.newTreeItem(nwItemType.FILE) is True
+    assert os.path.isfile(os.path.join(fncDir, "project", "content", "0000000000015.nwd"))
+    assert os.path.isfile(os.path.join(fncDir, "project", "content", "0000000000016.nwd"))
 
-    # Try to delete, but block parent item lookup
+    # Delete the folder, which moves everything to Trash
-    with monkeypatch.context() as mp:
+    assert nwTree.getTreeFromHandle("0000000000014") == [
-        mp.setattr("PyQt5.QtWidgets.QTreeWidgetItem.parent", lambda *a: None)
+        "0000000000014", "0000000000015", "0000000000016"
-        caplog.clear()
+    ]
-        assert nwTree.deleteItem("0000000000014") is False
-        assert "Could not delete folder" in caplog.text
-        assert "0000000000014" in nwGUI.theProject.projTree
-
-    # Delete folder properly
     assert nwTree.deleteItem("0000000000014") is True
-    assert "0000000000014" not in nwGUI.theProject.projTree
+    assert nwTree.getTreeFromHandle(trashHandle) == [
+        trashHandle, "0000000000014", "0000000000015", "0000000000016"
+    ]
+    assert os.path.isfile(os.path.join(fncDir, "project", "content", "0000000000015.nwd"))
+    assert os.path.isfile(os.path.join(fncDir, "project", "content", "0000000000016.nwd"))
 
-    # Delete the Character root
+    # Delete again, which should delete folder and all files
-    assert nwTree.deleteItem("000000000000a") is True
+    assert nwTree.deleteItem("0000000000014") is True
-    assert "000000000000a" not in nwGUI.theProject.projTree
+    assert nwTree.getTreeFromHandle(trashHandle) == [trashHandle]
+    assert not os.path.isfile(os.path.join(fncDir, "project", "content", "0000000000015.nwd"))
+    assert not os.path.isfile(os.path.join(fncDir, "project", "content", "0000000000016.nwd"))
 
     # Empty Trash
     # ===========
@@ -421,6 +426,18 @@ def testGuiProjTree_DeleteItems(qtbot, caplog, monkeypatch, nwGUI, fncDir, mockR
     assert nwTree.getTreeFromHandle(trashHandle) == [trashHandle]
     assert nwTree._treeChanged is True
 
+    # Try to delete a file, but block the underlying deletion of the file on disk
+    assert os.path.isfile(os.path.join(fncDir, "project", "content", "000000000000e.nwd"))
+    with monkeypatch.context() as mp:
+        mp.setattr("novelwriter.core.document.NWDoc.deleteDocument", lambda *a: False)
+        assert nwTree.deleteItem("000000000000e") is True
+        assert nwTree.deleteItem("000000000000e") is True
+        assert os.path.isfile(os.path.join(fncDir, "project", "content", "000000000000e.nwd"))
+
+    # Delete proper
+    assert nwTree._deleteTreeItem("000000000000e") is True
+    assert not os.path.isfile(os.path.join(fncDir, "project", "content", "000000000000e.nwd"))
+
     # Clean up
     # qtbot.stopForInteraction()
     nwGUI.closeProject()