From 33b95b60e367534223a7eea3a82ff21a3fa3ff1a Mon Sep 17 00:00:00 2001 From: Veronica Berglyd Olsen <1619840+vkbo@users.noreply.github.com> Date: Tue, 18 Oct 2022 20:43:33 +0200 Subject: [PATCH] Fix missing data path issue (#1180) --- novelwriter/common.py | 21 ++++- novelwriter/config.py | 125 +++++++++------------------- novelwriter/gui/theme.py | 6 +- tests/test_base/test_base_common.py | 29 ++++++- tests/test_base/test_base_config.py | 32 ------- 5 files changed, 90 insertions(+), 123 deletions(-) diff --git a/novelwriter/common.py b/novelwriter/common.py index 69f6bc050..ec4bd5bdb 100644 --- a/novelwriter/common.py +++ b/novelwriter/common.py @@ -35,7 +35,7 @@ from PyQt5.QtWidgets import qApp from novelwriter.enum import nwItemClass, nwItemType, nwItemLayout -from novelwriter.error import logException +from novelwriter.error import formatException, logException from novelwriter.constants import nwConst, nwUnicode logger = logging.getLogger(__name__) @@ -475,6 +475,25 @@ def makeFileNameSafe(value): return cleanName +def ensureFolder(dirPath, parentPath=None, errLog=None): + """Make sure a folder exists, and if it doesn't, create it. + """ + try: + if parentPath: + dirPath = os.path.join(parentPath, dirPath) + if not os.path.isdir(dirPath): + os.mkdir(dirPath) + except Exception as exc: + logger.error("Could not create folder: %s", dirPath) + logException() + if isinstance(errLog, list): + errLog.append(f"Could not create folder: {dirPath}") + errLog.append(formatException(exc)) + return False + + return True + + def sha256sum(filePath): """Make a shasum of a file using a buffer. Based on: https://stackoverflow.com/a/44873382/5825851 diff --git a/novelwriter/config.py b/novelwriter/config.py index e86c850d4..90c30c426 100644 --- a/novelwriter/config.py +++ b/novelwriter/config.py @@ -37,7 +37,7 @@ ) from novelwriter.error import logException, formatException -from novelwriter.common import splitVersionNumber, formatTimeStamp, NWConfigParser +from novelwriter.common import ensureFolder, splitVersionNumber, formatTimeStamp, NWConfigParser from novelwriter.constants import nwFiles, nwUnicode logger = logging.getLogger(__name__) @@ -54,23 +54,21 @@ def __init__(self): self.appName = "novelWriter" self.appHandle = self.appName.lower() - # Config Error Handling - self.hasError = False # True if the config class encountered an error - self.errData = [] # List of error messages - # Set Paths - self.cmdOpen = None # Path from command line for project to be opened on launch - self.confPath = None # Folder where the config is saved - self.confFile = None # The config file name - self.dataPath = None # Folder where app data is stored - self.lastPath = None # The last user-selected folder (browse dialogs) - self.appPath = None # The full path to the novelwriter package folder - self.appRoot = None # The full path to the novelwriter root folder - self.appIcon = None # The full path to the novelwriter icon file - self.assetPath = None # The full path to the novelwriter/assets folder - self.pdfDocs = None # The location of the PDF manual, if it exists + self.cmdOpen = None # Path from command line for project to be opened on launch + self.confPath = None # Folder where the config is saved + self.confFile = None # The config file name + self.dataPath = None # Folder where app data is stored + self.lastPath = None # The last user-selected folder (browse dialogs) + self.appPath = None # The full path to the novelwriter package folder + self.appRoot = None # The full path to the novelwriter root folder + self.appIcon = None # The full path to the novelwriter icon file + self.assetPath = None # The full path to the novelwriter/assets folder + self.pdfDocs = None # The location of the PDF manual, if it exists # Runtime Settings and Variables + self.hasError = False # True if the config class encountered an error + self.errData = [] # List of error messages self.confChanged = False # True whenever the config has chenged, false after save # General @@ -94,14 +92,14 @@ def __init__(self): self.qtTrans = {} # Sizes - self.winGeometry = [1200, 650] - self.prefGeometry = [700, 615] - self.projColWidth = [200, 60, 140] - self.mainPanePos = [300, 800] - self.docPanePos = [400, 400] - self.viewPanePos = [500, 150] - self.outlnPanePos = [500, 150] - self.isFullScreen = False + self.winGeometry = [1200, 650] + self.prefGeometry = [700, 615] + self.projColWidth = [200, 60, 140] + self.mainPanePos = [300, 800] + self.docPanePos = [400, 400] + self.viewPanePos = [500, 150] + self.outlnPanePos = [500, 150] + self.isFullScreen = False # Features self.hideVScroll = False # Hide vertical scroll bars on main widgets @@ -270,20 +268,8 @@ def initConfig(self, confPath=None, dataPath=None): logger.info("Setting data path from alternative path: %s", dataPath) self.dataPath = dataPath - logger.verbose("Config path: %s", self.confPath) - logger.verbose("Data path: %s", self.dataPath) - - # Check Data Path Subdirs - dataDirs = ["syntax", "themes"] - for dataDir in dataDirs: - dirPath = os.path.join(self.dataPath, dataDir) - if not os.path.isdir(dirPath): - try: - os.mkdir(dirPath) - logger.info("Created folder: %s", dirPath) - except Exception: - logger.error("Could not create folder: %s", dirPath) - logException() + logger.debug("Config path: %s", self.confPath) + logger.debug("Data path: %s", self.dataPath) self.confFile = self.appHandle+".conf" self.lastPath = os.path.expanduser("~") @@ -308,18 +294,20 @@ def initConfig(self, confPath=None, dataPath=None): logger.verbose("App path: %s", self.appPath) logger.verbose("Last path: %s", self.lastPath) - # If the config folder does not exist, create it. - # This assumes that the os config folder itself exists. - if not os.path.isdir(self.confPath): - try: - os.mkdir(self.confPath) - except Exception as exc: - logger.error("Could not create folder: %s", self.confPath) - logException() - self.hasError = True - self.errData.append("Could not create folder: %s" % self.confPath) - self.errData.append(formatException(exc)) - self.confPath = None + # If the config and data folders don't not exist, create them + # This assumes that the os config and data folders exist + if not ensureFolder(self.confPath, errLog=self.errData): + self.hasError = True + self.confPath = None + + if not ensureFolder(self.dataPath, errLog=self.errData): + self.hasError = True + self.dataPath = None + + # We don't error on these failing since they are not essential + if self.dataPath is not None: + ensureFolder("syntax", parentPath=self.dataPath) + ensureFolder("themes", parentPath=self.dataPath) # Check if config file exists if self.confPath is not None: @@ -330,20 +318,6 @@ def initConfig(self, confPath=None, dataPath=None): # If it does not exist, save a copy of the default values self.saveConfig() - # If the data folder does not exist, create it. - # This assumes that the os data folder itself exists. - if self.dataPath is not None: - if not os.path.isdir(self.dataPath): - try: - os.mkdir(self.dataPath) - except Exception as exc: - logger.error("Could not create folder: %s", self.dataPath) - logException() - self.hasError = True - self.errData.append("Could not create folder: %s" % self.dataPath) - self.errData.append(formatException(exc)) - self.dataPath = None - # Load recent projects cache self.loadRecentCache() @@ -389,7 +363,7 @@ def initLocalisation(self, nwApp): def listLanguages(self, lngSet): """List localisation files in the i18n folder. The default GUI - language 'en_GB' is British English. + language is British English (en_GB). """ if lngSet == self.LANG_NW: fPre = "nw_" @@ -741,29 +715,6 @@ def removeFromRecentCache(self, thePath): # Setters ## - def setConfPath(self, newPath): - """Set the path and filename to the config file. - """ - if newPath is None: - return True - if not os.path.isfile(newPath): - logger.error("File not found, using default config path instead") - return False - self.confPath = os.path.dirname(newPath) - self.confFile = os.path.basename(newPath) - return True - - def setDataPath(self, newPath): - """Set the data path. - """ - if newPath is None: - return True - if not os.path.isdir(newPath): - logger.error("Path not found, using default data path instead") - return False - self.dataPath = os.path.abspath(newPath) - return True - def setLastPath(self, lastPath): """Set the last used path (by the user). """ diff --git a/novelwriter/gui/theme.py b/novelwriter/gui/theme.py index 131628c05..e737faab1 100644 --- a/novelwriter/gui/theme.py +++ b/novelwriter/gui/theme.py @@ -120,11 +120,13 @@ def __init__(self): self._availThemes = {} self._availSyntax = {} - self._listConf(self._availSyntax, os.path.join(self.mainConf.dataPath, "syntax")) self._listConf(self._availSyntax, os.path.join(self.mainConf.assetPath, "syntax")) - self._listConf(self._availThemes, os.path.join(self.mainConf.dataPath, "themes")) self._listConf(self._availThemes, os.path.join(self.mainConf.assetPath, "themes")) + if self.mainConf.dataPath: # Not guaranteed to be set + self._listConf(self._availSyntax, os.path.join(self.mainConf.dataPath, "syntax")) + self._listConf(self._availThemes, os.path.join(self.mainConf.dataPath, "themes")) + self.updateFont() self.updateTheme() self.iconCache.updateTheme() diff --git a/tests/test_base/test_base_common.py b/tests/test_base/test_base_common.py index 0ce153b46..e93026b28 100644 --- a/tests/test_base/test_base_common.py +++ b/tests/test_base/test_base_common.py @@ -33,7 +33,8 @@ isTitleTag, isItemClass, isItemType, isItemLayout, hexToInt, checkIntRange, minmax, checkIntTuple, formatInt, formatTimeStamp, formatTime, simplified, splitVersionNumber, transferCase, fuzzyTime, numberToRoman, jsonEncode, - readTextFile, makeFileNameSafe, sha256sum, getGuiItem, NWConfigParser + readTextFile, makeFileNameSafe, ensureFolder, sha256sum, getGuiItem, + NWConfigParser ) @@ -541,6 +542,32 @@ def testBaseCommon_MakeFileNameSafe(): # END Test testBaseCommon_MakeFileNameSafe +@pytest.mark.base +def testBaseCommon_EnsureFolder(monkeypatch, fncDir): + """Test the ensureFolder function. + """ + newDir1 = os.path.join(fncDir, "newDir1") + newDir2 = os.path.join(fncDir, "newDir2") + newDir3 = os.path.join(fncDir, "newDir3") + + assert ensureFolder(None) is False + + assert ensureFolder(newDir1) is True + assert os.path.isdir(newDir1) + + assert ensureFolder("newDir2", parentPath=fncDir) is True + assert os.path.isdir(newDir2) + + with monkeypatch.context() as mp: + mp.setattr("os.mkdir", causeOSError) + errLog = [] + assert ensureFolder("newDir3", parentPath=fncDir, errLog=errLog) is False + assert errLog[0] == f"Could not create folder: {newDir3}" + assert not os.path.isdir(newDir3) + +# END Test testBaseCommon_EnsureFolder + + @pytest.mark.base def testBaseCommon_Sha256Sum(monkeypatch, fncDir, ipsumText): """Test the sha256sum function. diff --git a/tests/test_base/test_base_config.py b/tests/test_base/test_base_config.py index 49387ab05..f67214936 100644 --- a/tests/test_base/test_base_config.py +++ b/tests/test_base/test_base_config.py @@ -316,38 +316,6 @@ def testBaseConfig_RecentCache(monkeypatch, tmpConf, tmpDir, fncDir): # END Test testBaseConfig_RecentCache -@pytest.mark.base -def testBaseConfig_SetPath(tmpConf, tmpDir): - """Test path setters. - """ - # Conf Path - assert tmpConf.setConfPath(None) - assert not tmpConf.setConfPath(os.path.join("somewhere", "over", "the", "rainbow")) - assert tmpConf.setConfPath(os.path.join(tmpDir, "novelwriter.conf")) - assert tmpConf.confPath == tmpDir - assert tmpConf.confFile == "novelwriter.conf" - assert not tmpConf.confChanged - - # Data Path - assert tmpConf.setDataPath(None) - assert not tmpConf.setDataPath(os.path.join("somewhere", "over", "the", "rainbow")) - assert tmpConf.setDataPath(tmpDir) - assert tmpConf.dataPath == tmpDir - assert not tmpConf.confChanged - - # Last Path - assert tmpConf.setLastPath(None) - assert tmpConf.lastPath == "" - - assert tmpConf.setLastPath(os.path.join(tmpDir, "file.tmp")) - assert tmpConf.lastPath == tmpDir - - assert tmpConf.setLastPath("") - assert tmpConf.lastPath == "" - -# END Test testBaseConfig_SetPath - - @pytest.mark.base def testBaseConfig_SettersGetters(tmpConf, tmpDir, outDir, refDir): """Set various sizes and positions