From 9bc7250532aa038dca8877ff774ab1ada5c91504 Mon Sep 17 00:00:00 2001 From: Veronica Berglyd Olsen <1619840+vkbo@users.noreply.github.com> Date: Sat, 23 Apr 2022 21:33:11 +0200 Subject: [PATCH 1/2] Allow deleting non-empty folders --- novelwriter/gui/projtree.py | 154 ++++++++++++---------------- tests/test_gui/test_gui_projtree.py | 77 ++++++++------ 2 files changed, 115 insertions(+), 116 deletions(-) diff --git a/novelwriter/gui/projtree.py b/novelwriter/gui/projtree.py index cb59c9ee..f7a11db4 100644 --- a/novelwriter/gui/projtree.py +++ b/novelwriter/gui/projtree.py @@ -444,90 +444,8 @@ class GuiProjectTree(QTreeWidget): return False wCount = self._getItemWordCount(tHandle) - if nwItemS.itemType == nwItemType.FILE: - logger.debug("User requested file '%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) + if nwItemS.itemType == nwItemType.ROOT: + logger.debug("User requested a 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 diff --git a/tests/test_gui/test_gui_projtree.py b/tests/test_gui/test_gui_projtree.py index 84563086..59d02c2d 100644 --- a/tests/test_gui/test_gui_projtree.py +++ b/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 - 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 + # Delete the second file, 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 - assert nwTree.deleteItem("000000000000d") is False # Folder - assert nwTree.deleteItem("0000000000008") is False # Root + trashHandle = nwGUI.theProject.projTree.trashRoot() - # Add a folder we can delete - nwTree.setSelectedHandle("000000000000a") # Character Root + # Add a folder with two files + 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 - with monkeypatch.context() as mp: - mp.setattr("PyQt5.QtWidgets.QTreeWidgetItem.parent", lambda *a: None) - 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 + # Delete the folder, which moves everything to Trash + assert nwTree.getTreeFromHandle("0000000000014") == [ + "0000000000014", "0000000000015", "0000000000016" + ] 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 - assert nwTree.deleteItem("000000000000a") is True - assert "000000000000a" not in nwGUI.theProject.projTree + # Delete again, which should delete folder and all files + assert nwTree.deleteItem("0000000000014") is True + 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() From bb53b2530cb9f1e68bf4aaba79b8cf65b56d1aaf Mon Sep 17 00:00:00 2001 From: Veronica Berglyd Olsen <1619840+vkbo@users.noreply.github.com> Date: Sat, 23 Apr 2022 21:41:14 +0200 Subject: [PATCH 2/2] Fix error messages when emptying trash --- novelwriter/gui/projtree.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/novelwriter/gui/projtree.py b/novelwriter/gui/projtree.py index f7a11db4..4b4d5e2d 100644 --- a/novelwriter/gui/projtree.py +++ b/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, - and the deletion is always permanent. + delete the files on disk. Root folders are deleted if they're empty + only, and the deletion is always permanent. """ if not self.theParent.hasProject: logger.error("No project open")