From 1923a55443cc5e861718912c036777e8fafd4099 Mon Sep 17 00:00:00 2001
From: "Veronica K. B. Olsen" <1619840+vkbo@users.noreply.github.com>
Date: Thu, 15 Oct 2020 19:08:35 +0200
Subject: [PATCH] Code cleanup in core classes and rename of parHandle to
 itemParent in NWItem class

---
 nw/core/document.py   | 11 +++++++----
 nw/core/index.py      |  6 +++---
 nw/core/item.py       | 19 +++++++++++--------
 nw/core/project.py    |  6 +++---
 nw/core/tree.py       | 10 +++++-----
 nw/gui/build.py       |  4 ++--
 nw/gui/docmerge.py    |  2 +-
 nw/gui/docsplit.py    |  4 ++--
 nw/gui/projtree.py    | 12 ++++++------
 tests/test_item.py    | 10 +++++-----
 tests/test_project.py |  4 ++--
 11 files changed, 47 insertions(+), 41 deletions(-)

diff --git a/nw/core/document.py b/nw/core/document.py
index 00fd7146..1452e9dd 100644
--- a/nw/core/document.py
+++ b/nw/core/document.py
@@ -67,7 +67,9 @@ class NWDoc():
 
     def openDocument(self, tHandle, showStatus=True, isOrphan=False):
         """Open a document from handle, capturing potential file system
-        errors and parse meta data.
+        errors and parse meta data. If the document doesn't exist on
+        disk, return an empty string. If something went wrong, return
+        None.
         """
         if not isHandle(tHandle):
             return None
@@ -125,8 +127,8 @@ class NWDoc():
         return theText
 
     def saveDocument(self, docText):
-        """Save the document via temp file in case of save failure, and
-        in any case keep a backup of the file.
+        """Save the document. The file is saved via a temp file in case
+        of save failure. Returns True if successful, False if not.
         """
         if self._docHandle is None:
             return False
@@ -139,6 +141,7 @@ class NWDoc():
         docPath = os.path.join(self.theProject.projContent, docFile)
         docTemp = os.path.join(self.theProject.projContent, docFile+"~")
 
+        # DocMeta line
         if self._theItem is None:
             docMeta = ""
         else:
@@ -171,7 +174,7 @@ class NWDoc():
         return True
 
     def deleteDocument(self, tHandle):
-        """Permanently delete a document source file and its backups
+        """Permanently delete a document source file and related files
         from the project data folder.
         """
         if not isHandle(tHandle):
diff --git a/nw/core/index.py b/nw/core/index.py
index f715d835..c08562e1 100644
--- a/nw/core/index.py
+++ b/nw/core/index.py
@@ -279,7 +279,7 @@ class NWIndex():
         if theItem.itemLayout == nwItemLayout.NO_LAYOUT:
             logger.info("Not indexing no-layout item %s" % tHandle)
             return False
-        if theItem.parHandle is None:
+        if theItem.itemParent is None:
             logger.info("Not indexing orphaned item %s" % tHandle)
             return False
 
@@ -288,7 +288,7 @@ class NWIndex():
         self.textCounts[tHandle] = [cC, wC, pC]
 
         # If the file is archived or trashed, we don't index the file itself
-        if self.theProject.projTree.isTrashRoot(theItem.parHandle):
+        if self.theProject.projTree.isTrashRoot(theItem.itemParent):
             logger.info("Not indexing trash item %s" % tHandle)
             return False
         if theRoot.itemClass == nwItemClass.ARCHIVE:
@@ -583,7 +583,7 @@ class NWIndex():
 
     def getCounts(self, tHandle, sTitle=None):
         """Returns the counts for a file, or a section of a file
-        starting at title nTitle.
+        starting at title sTitle if it is provided.
         """
         cC = 0
         wC = 0
diff --git a/nw/core/item.py b/nw/core/item.py
index 702287dd..944b5f0c 100644
--- a/nw/core/item.py
+++ b/nw/core/item.py
@@ -42,7 +42,7 @@ class NWItem():
 
         self.itemName   = ""
         self.itemHandle = None
-        self.parHandle  = None
+        self.itemParent = None
         self.itemOrder  = None
         self.itemType   = nwItemType.NO_TYPE
         self.itemClass  = nwItemClass.NO_CLASS
@@ -70,7 +70,7 @@ class NWItem():
         xPack = etree.SubElement(xParent, "item", attrib={
             "handle" : str(self.itemHandle),
             "order"  : str(self.itemOrder),
-            "parent" : str(self.parHandle),
+            "parent" : str(self.itemParent),
         })
         self._subPack(xPack, "name",   text=str(self.itemName))
         self._subPack(xPack, "type",   text=str(self.itemType.name))
@@ -85,6 +85,7 @@ class NWItem():
             self._subPack(xPack, "cursorPos", text=str(self.cursorPos), none=False)
         else:
             self._subPack(xPack, "expanded", text=str(self.isExpanded))
+
         return
 
     def unpackXML(self, xItem):
@@ -101,7 +102,7 @@ class NWItem():
             return False
 
         if "parent" in xItem.attrib:
-            self.parHandle = xItem.attrib["parent"]
+            self.itemParent = xItem.attrib["parent"]
 
         setMap = {
             "name"      : self.setName,
@@ -131,9 +132,11 @@ class NWItem():
         """
         if not none and (text is None or text == "None"):
             return None
-        xSub = etree.SubElement(xParent, name, attrib=attrib)
+        xAttr = {} if attrib is None else attrib
+        xSub = etree.SubElement(xParent, name, attrib=xAttr)
         if text is not None:
             xSub.text = text
+
         return
 
     ##
@@ -162,14 +165,14 @@ class NWItem():
         """Set the parent handle, and ensure that it is valid.
         """
         if theParent is None:
-            self.parHandle = None
+            self.itemParent = None
         elif isinstance(theParent, str):
             if len(theParent) == 13:
-                self.parHandle = theParent
+                self.itemParent = theParent
             else:
-                self.parHandle = None
+                self.itemParent = None
         else:
-            self.parHandle = None
+            self.itemParent = None
         return
 
     def setOrder(self, theOrder):
diff --git a/nw/core/project.py b/nw/core/project.py
index 15fd7d27..622bcc38 100644
--- a/nw/core/project.py
+++ b/nw/core/project.py
@@ -1139,16 +1139,16 @@ class NWProject():
                 # Technically a bug since treeOrder is built from the
                 # same data as projTree
                 continue
-            elif tItem.parHandle is None:
+            elif tItem.itemParent is None:
                 # Item is a root, or already been identified as an
                 # orphaned item
                 sentItems.append(tHandle)
                 yield tItem
-            elif tItem.parHandle in sentItems:
+            elif tItem.itemParent in sentItems:
                 # Item's parent has been sent, so all is fine
                 sentItems.append(tHandle)
                 yield tItem
-            elif tItem.parHandle in iterItems:
+            elif tItem.itemParent in iterItems:
                 # Item's parent exists, but hasn't been sent yet, so add
                 # it again to the end
                 logger.warning("Item %s found before its parent" % tHandle)
diff --git a/nw/core/tree.py b/nw/core/tree.py
index c51ae16c..b43230a9 100644
--- a/nw/core/tree.py
+++ b/nw/core/tree.py
@@ -134,7 +134,7 @@ class NWTree():
         for xItem in xContent:
             nwItem = NWItem(self.theProject)
             if nwItem.unpackXML(xItem):
-                self.append(nwItem.itemHandle, nwItem.parHandle, nwItem)
+                self.append(nwItem.itemHandle, nwItem.itemParent, nwItem)
                 nwItem.saveInitialCount()
 
         return True
@@ -261,10 +261,10 @@ class NWTree():
         tItem = self.__getitem__(tHandle)
         if tItem is not None:
             for i in range(nwConst.maxDepth + 1):
-                if tItem.parHandle is None:
+                if tItem.itemParent is None:
                     return tItem
                 else:
-                    tHandle = tItem.parHandle
+                    tHandle = tItem.itemParent
                     tItem = self.__getitem__(tHandle)
         return None
 
@@ -279,10 +279,10 @@ class NWTree():
         if tItem is not None:
             tTree.append(tHandle)
             for i in range(nwConst.maxDepth + 1):
-                if tItem.parHandle is None:
+                if tItem.itemParent is None:
                     return tTree
                 else:
-                    tHandle = tItem.parHandle
+                    tHandle = tItem.itemParent
                     tItem   = self.__getitem__(tHandle)
                     if tItem is None:
                         return tTree
diff --git a/nw/gui/build.py b/nw/gui/build.py
index 1a69b62e..45a1a073 100644
--- a/nw/gui/build.py
+++ b/nw/gui/build.py
@@ -684,8 +684,8 @@ class GuiBuildNovel(QDialog):
         isNone |= theItem.itemLayout == nwItemLayout.NO_LAYOUT
         isNone |= theItem.itemClass == nwItemClass.NO_CLASS
         isNone |= theItem.itemClass == nwItemClass.TRASH
-        isNone |= theItem.parHandle == self.theProject.projTree.trashRoot()
-        isNone |= theItem.parHandle is None
+        isNone |= theItem.itemParent == self.theProject.projTree.trashRoot()
+        isNone |= theItem.itemParent is None
         isNote  = theItem.itemLayout == nwItemLayout.NOTE
         isNovel = not isNone and not isNote
 
diff --git a/nw/gui/docmerge.py b/nw/gui/docmerge.py
index 2418a161..d7d128fe 100644
--- a/nw/gui/docmerge.py
+++ b/nw/gui/docmerge.py
@@ -127,7 +127,7 @@ class GuiDocMerge(QDialog):
             ), nwAlert.ERROR)
             return
 
-        nHandle = self.theProject.newFile(srcItem.itemName, srcItem.itemClass, srcItem.parHandle)
+        nHandle = self.theProject.newFile(srcItem.itemName, srcItem.itemClass, srcItem.itemParent)
         newItem = self.theProject.projTree[nHandle]
         newItem.setStatus(srcItem.itemStatus)
 
diff --git a/nw/gui/docsplit.py b/nw/gui/docsplit.py
index 3f497c18..1a5e9491 100644
--- a/nw/gui/docsplit.py
+++ b/nw/gui/docsplit.py
@@ -154,7 +154,7 @@ class GuiDocSplit(QDialog):
             return
 
         # Check that another folder can be created
-        parTree = self.theProject.projTree.getItemPath(srcItem.parHandle)
+        parTree = self.theProject.projTree.getItemPath(srcItem.itemParent)
         if len(parTree) >= nwConst.maxDepth - 1:
             self.theParent.makeAlert((
                 "Cannot add new folder for the document split. "
@@ -176,7 +176,7 @@ class GuiDocSplit(QDialog):
 
         # Create the folder
         fHandle = self.theProject.newFolder(
-            srcItem.itemName, srcItem.itemClass, srcItem.parHandle
+            srcItem.itemName, srcItem.itemClass, srcItem.itemParent
         )
         self.theParent.treeView.revealNewTreeItem(fHandle)
         logger.verbose("Creating folder %s" % fHandle)
diff --git a/nw/gui/projtree.py b/nw/gui/projtree.py
index 9f4bb5e6..6b1ee7bd 100644
--- a/nw/gui/projtree.py
+++ b/nw/gui/projtree.py
@@ -219,7 +219,7 @@ class GuiProjectTree(QTreeWidget):
             pItem = self.theProject.projTree[pHandle]
             if pItem.itemType == nwItemType.FILE:
                 nHandle = pHandle
-                pHandle = pItem.parHandle
+                pHandle = pItem.itemParent
 
             # If we again have no home, give up
             if pHandle is None:
@@ -270,7 +270,7 @@ class GuiProjectTree(QTreeWidget):
         """
         nwItem = self.theProject.projTree[tHandle]
         trItem = self._addTreeItem(nwItem, nHandle)
-        pHandle = nwItem.parHandle
+        pHandle = nwItem.itemParent
         if pHandle is not None and pHandle in self.theMap:
             self.theMap[pHandle].setExpanded(True)
         self.clearSelection()
@@ -430,7 +430,7 @@ class GuiProjectTree(QTreeWidget):
                 logger.error("Could not delete item")
                 return False
 
-            pHandle = nwItemS.parHandle
+            pHandle = nwItemS.itemParent
             if self.theProject.projTree.isTrashRoot(pHandle):
                 # If the file is in the trash folder already, as the
                 # user if they want to permanently delete the file.
@@ -815,7 +815,7 @@ class GuiProjectTree(QTreeWidget):
         project tree.
         """
         tHandle = nwItem.itemHandle
-        pHandle = nwItem.parHandle
+        pHandle = nwItem.itemParent
         tClass  = nwItem.itemClass
         newItem = QTreeWidgetItem([""]*4)
 
@@ -1022,11 +1022,11 @@ class GuiProjectTreeMenu(QMenu):
 
         trashHandle = self.theTree.theProject.projTree.trashRoot()
 
-        inTrash = theItem.parHandle == trashHandle and trashHandle is not None
+        inTrash = theItem.itemParent == trashHandle and trashHandle is not None
         isTrash = theItem.itemHandle == trashHandle and trashHandle is not None
         isFile  = theItem.itemType == nwItemType.FILE
         isArch  = theRoot.itemClass == nwItemClass.ARCHIVE
-        isOrph  = isFile and theItem.parHandle is None
+        isOrph  = isFile and theItem.itemParent is None
 
         showOpen      = isFile
         showView      = isFile
diff --git a/tests/test_item.py b/tests/test_item.py
index d1097161..936ba185 100644
--- a/tests/test_item.py
+++ b/tests/test_item.py
@@ -31,13 +31,13 @@ def testItemSettersSimple(nwDummy):
 
     # Parent
     theItem.setParent(None)
-    assert theItem.parHandle is None
+    assert theItem.itemParent is None
     theItem.setParent(123)
-    assert theItem.parHandle is None
+    assert theItem.itemParent is None
     theItem.setParent("0123456789abcdef")
-    assert theItem.parHandle is None
+    assert theItem.itemParent is None
     theItem.setParent("0123456789abc")
-    assert theItem.parHandle == "0123456789abc"
+    assert theItem.itemParent == "0123456789abc"
 
     # Order
     theItem.setOrder(None)
@@ -227,7 +227,7 @@ def testItemXMLPackUnpack(nwDummy):
     # Unpack
     assert theItem.unpackXML(xContent[0])
     assert theItem.itemHandle == "0123456789abc"
-    assert theItem.parHandle == "0123456789abc"
+    assert theItem.itemParent == "0123456789abc"
     assert theItem.itemOrder == 1
     assert theItem.isExpanded
     assert theItem.paraCount == 3
diff --git a/tests/test_project.py b/tests/test_project.py
index 282acbc7..e8f1442b 100644
--- a/tests/test_project.py
+++ b/tests/test_project.py
@@ -521,7 +521,7 @@ def testProjectOrphanedFiles(nwDummy, nwLipsum):
     assert oItem is not None
     assert oItem.itemName == "Mars"
     assert oItem.itemHandle == "636b6aa9b697b"
-    assert oItem.parHandle is None
+    assert oItem.itemParent is None
     assert oItem.itemClass == nwItemClass.WORLD
     assert oItem.itemType == nwItemType.FILE
     assert oItem.itemLayout == nwItemLayout.NOTE
@@ -531,7 +531,7 @@ def testProjectOrphanedFiles(nwDummy, nwLipsum):
     assert oItem is not None
     assert oItem.itemName == "Orphaned File 1"
     assert oItem.itemHandle == "736b6aa9b697b"
-    assert oItem.parHandle is None
+    assert oItem.itemParent is None
     assert oItem.itemClass == nwItemClass.NO_CLASS
     assert oItem.itemType == nwItemType.FILE
     assert oItem.itemLayout == nwItemLayout.NO_LAYOUT