Config class improvements (#826)

* Subclass ConfigParser
* Remove redundant variables and update tests
* Update file description
* Move NWConfigParser class to common.py
* Improve coverage of config class
* Use NWConfigParser also in themes.py
* Update config class file header
* Fix comment
* Update the usage of ConfigParser to recommended practice and extend tests
* Improve logging for info level a bit

tests/conftest.py

@@ -91,10 +91,7 @@ def fncDir(tmpDir):
         shutil.rmtree(fncDir)
     if not os.path.isdir(fncDir):
         os.mkdir(fncDir)
-    yield fncDir
-    if os.path.isdir(fncDir):
-        shutil.rmtree(fncDir)
-    return
+    return fncDir
 
 
 @pytest.fixture(scope="function")

tests/test_base/test_base_common.py

@@ -19,14 +19,19 @@ You should have received a copy of the GNU General Public License
 along with this program. If not, see <https://www.gnu.org/licenses/>.
 """
 
+import os
 import time
 import pytest
 
+from datetime import datetime
+
+from tools import writeFile
+
 from nw.common import (
-    checkString, checkBool, checkInt, formatInt, transferCase,
-    fuzzyTime, checkHandle, formatTimeStamp, formatTime, hexToInt,
+    checkString, checkBool, checkInt, formatInt, transferCase, fuzzyTime,
+    checkHandle, formatTimeStamp, parseTimeStamp, formatTime, hexToInt,
     makeFileNameSafe, isHandle, isTitleTag, isItemClass, isItemType,
-    isItemLayout, numberToRoman
+    isItemLayout, numberToRoman, NWConfigParser
 )
 
 
@@ -65,11 +70,11 @@ def testBaseCommon_CheckBool():
     """
     assert checkBool(None, 3, True) is None
     assert checkBool("None", 3, True) is None
-    assert checkBool("True", False, False)
-    assert not checkBool("False", True, False)
+    assert checkBool("True", False, False) is True
+    assert checkBool("False", True, False) is False
     assert checkBool("Boo", None, False) is None
-    assert not checkBool(0, None, False)
-    assert checkBool(1, None, False)
+    assert checkBool(0, None, False) is False
+    assert checkBool(1, None, False) is True
     assert checkBool(2, None, False) is None
     assert checkBool(0.0, None, False) is None
     assert checkBool(1.0, None, False) is None
@@ -96,13 +101,12 @@ def testBaseCommon_CheckHandle():
 def testBaseCommon_IsHandle():
     """Test the isHandle function.
     """
-    assert isHandle("47666c91c7ccf")
-
-    assert not isHandle("47666C91C7CCF")
-    assert not isHandle("h7666c91c7ccf")
-    assert not isHandle("None")
-    assert not isHandle(None)
-    assert not isHandle("STUFF")
+    assert isHandle("47666c91c7ccf") is True
+    assert isHandle("47666C91C7CCF") is False
+    assert isHandle("h7666c91c7ccf") is False
+    assert isHandle("None") is False
+    assert isHandle(None) is False
+    assert isHandle("STUFF") is False
 
 # END Test testBaseCommon_IsHandle
 
@@ -111,16 +115,16 @@ def testBaseCommon_IsHandle():
 def testBaseCommon_IsTitleTag():
     """Test the isItemClass function.
     """
-    assert isTitleTag("T123456")
+    assert isTitleTag("T123456") is True
 
-    assert not isTitleTag("t123456")
-    assert not isTitleTag("S123456")
-    assert not isTitleTag("T12345A")
-    assert not isTitleTag("T1234567")
+    assert isTitleTag("t123456") is False
+    assert isTitleTag("S123456") is False
+    assert isTitleTag("T12345A") is False
+    assert isTitleTag("T1234567") is False
 
-    assert not isTitleTag("None")
-    assert not isTitleTag(None)
-    assert not isTitleTag("STUFF")
+    assert isTitleTag("None") is False
+    assert isTitleTag(None) is False
+    assert isTitleTag("STUFF") is False
 
 # END Test testBaseCommon_IsTitleTag
 
@@ -129,21 +133,21 @@ def testBaseCommon_IsTitleTag():
 def testBaseCommon_IsItemClass():
     """Test the isItemClass function.
     """
-    assert isItemClass("NO_CLASS")
-    assert isItemClass("NOVEL")
-    assert isItemClass("PLOT")
-    assert isItemClass("CHARACTER")
-    assert isItemClass("WORLD")
-    assert isItemClass("TIMELINE")
-    assert isItemClass("OBJECT")
-    assert isItemClass("ENTITY")
-    assert isItemClass("CUSTOM")
-    assert isItemClass("ARCHIVE")
-    assert isItemClass("TRASH")
+    assert isItemClass("NO_CLASS") is True
+    assert isItemClass("NOVEL") is True
+    assert isItemClass("PLOT") is True
+    assert isItemClass("CHARACTER") is True
+    assert isItemClass("WORLD") is True
+    assert isItemClass("TIMELINE") is True
+    assert isItemClass("OBJECT") is True
+    assert isItemClass("ENTITY") is True
+    assert isItemClass("CUSTOM") is True
+    assert isItemClass("ARCHIVE") is True
+    assert isItemClass("TRASH") is True
 
-    assert not isItemClass("None")
-    assert not isItemClass(None)
-    assert not isItemClass("STUFF")
+    assert isItemClass("None") is False
+    assert isItemClass(None) is False
+    assert isItemClass("STUFF") is False
 
 # END Test testBaseCommon_IsItemClass
 
@@ -152,15 +156,15 @@ def testBaseCommon_IsItemClass():
 def testBaseCommon_IsItemType():
     """Test the isItemType function.
     """
-    assert isItemType("NO_TYPE")
-    assert isItemType("ROOT")
-    assert isItemType("FOLDER")
-    assert isItemType("FILE")
-    assert isItemType("TRASH")
+    assert isItemType("NO_TYPE") is True
+    assert isItemType("ROOT") is True
+    assert isItemType("FOLDER") is True
+    assert isItemType("FILE") is True
+    assert isItemType("TRASH") is True
 
-    assert not isItemType("None")
-    assert not isItemType(None)
-    assert not isItemType("STUFF")
+    assert isItemType("None") is False
+    assert isItemType(None) is False
+    assert isItemType("STUFF") is False
 
 # END Test testBaseCommon_IsItemType
 
@@ -169,19 +173,19 @@ def testBaseCommon_IsItemType():
 def testBaseCommon_IsItemLayout():
     """Test the isItemLayout function.
     """
-    assert isItemLayout("NO_LAYOUT")
-    assert isItemLayout("TITLE")
-    assert isItemLayout("BOOK")
-    assert isItemLayout("PAGE")
-    assert isItemLayout("PARTITION")
-    assert isItemLayout("UNNUMBERED")
-    assert isItemLayout("CHAPTER")
-    assert isItemLayout("SCENE")
-    assert isItemLayout("NOTE")
+    assert isItemLayout("NO_LAYOUT") is True
+    assert isItemLayout("TITLE") is True
+    assert isItemLayout("BOOK") is True
+    assert isItemLayout("PAGE") is True
+    assert isItemLayout("PARTITION") is True
+    assert isItemLayout("UNNUMBERED") is True
+    assert isItemLayout("CHAPTER") is True
+    assert isItemLayout("SCENE") is True
+    assert isItemLayout("NOTE") is True
 
-    assert not isItemLayout("None")
-    assert not isItemLayout(None)
-    assert not isItemLayout("STUFF")
+    assert isItemLayout("None") is False
+    assert isItemLayout(None) is False
+    assert isItemLayout("STUFF") is False
 
 # END Test testBaseCommon_IsItemLayout
 
@@ -234,11 +238,29 @@ def testBaseCommon_FormatTime():
 # END Test testBaseCommon_FormatTime
 
 
+@pytest.mark.base
+def testBaseCommon_ParseTimeStamp():
+    """Test the parseTimeStamp function.
+    """
+    localEpoch = datetime(2000, 1, 1).timestamp()
+    assert parseTimeStamp(None, 0.0, allowNone=True) is None
+    assert parseTimeStamp("None", 0.0, allowNone=True) is None
+    assert parseTimeStamp("None", 0.0) == 0.0
+    assert parseTimeStamp("2000-01-01 00:00:00", 123.0) == localEpoch
+    assert parseTimeStamp("2000-13-01 00:00:00", 123.0) == 123.0
+    assert parseTimeStamp("2000-01-32 00:00:00", 123.0) == 123.0
+
+# END Test testBaseCommon_ParseTimeStamp
+
+
 @pytest.mark.base
 def testBaseCommon_FormatInt():
     """Test the formatInt function.
     """
-    assert formatInt(1000) == "1000"
+    # Normal Cases
+    assert formatInt(1) == "1"
+    assert formatInt(12) == "12"
+    assert formatInt(123) == "123"
     assert formatInt(1234) == "1.23\u2009k"
     assert formatInt(12345) == "12.3\u2009k"
     assert formatInt(123456) == "123\u2009k"
@@ -247,6 +269,11 @@ def testBaseCommon_FormatInt():
     assert formatInt(123456789) == "123\u2009M"
     assert formatInt(1234567890) == "1.23\u2009G"
 
+    # Exceptions
+    assert formatInt(12.3) == "ERR"
+    assert formatInt(None) == "ERR"
+    assert formatInt("42") == "ERR"
+
 # END Test testBaseCommon_FormatInt
 
 
@@ -339,3 +366,87 @@ def testBaseCommon_RomanNumbers():
     assert numberToRoman(999, True) == "cmxcix"
 
 # END Test testBaseCommon_RomanNumbers
+
+
+@pytest.mark.base
+def testBaseCommon_NWConfigParser(fncDir):
+    """Test the NWConfigParser subclass.
+    """
+    tstConf = os.path.join(fncDir, "test.cfg")
+    writeFile(tstConf, (
+        "[main]\n"
+        "stropt = value\n"
+        "intopt1 = 42\n"
+        "intopt2 = 42.43\n"
+        "boolopt1 = true\n"
+        "boolopt2 = TRUE\n"
+        "boolopt3 = 1\n"
+        "boolopt4 = 0\n"
+        "list1 = a, b, c\n"
+        "list2 = 17, 18, 19\n"
+    ))
+
+    cfgParser = NWConfigParser()
+    cfgParser.read(tstConf)
+
+    # Readers
+    # =======
+
+    # Read String
+    assert cfgParser.rdStr("main", "stropt",   "stuff") == "value"
+    assert cfgParser.rdStr("main", "boolopt1", "stuff") == "true"
+    assert cfgParser.rdStr("main", "intopt1",  "stuff") == "42"
+
+    assert cfgParser.rdStr("nope", "stropt",   "stuff") == "stuff"
+    assert cfgParser.rdStr("main", "blabla",   "stuff") == "stuff"
+
+    # Read Boolean
+    assert cfgParser.rdBool("main", "boolopt1", None) is True
+    assert cfgParser.rdBool("main", "boolopt2", None) is True
+    assert cfgParser.rdBool("main", "boolopt3", None) is True
+    assert cfgParser.rdBool("main", "boolopt4", None) is False
+    assert cfgParser.rdBool("main", "intopt1",  None) is None
+
+    assert cfgParser.rdBool("nope", "boolopt1", None) is None
+    assert cfgParser.rdBool("main", "blabla",   None) is None
+
+    # Read Integer
+    assert cfgParser.rdInt("main", "intopt1", 13) == 42
+    assert cfgParser.rdInt("main", "intopt2", 13) == 13
+    assert cfgParser.rdInt("main", "stropt",  13) == 13
+
+    assert cfgParser.rdInt("nope", "intopt1", 13) == 13
+    assert cfgParser.rdInt("main", "blabla",  13) == 13
+
+    # Read String List
+    assert cfgParser.rdStrList("main", "list1", []) == []
+    assert cfgParser.rdStrList("main", "list1", ["x"]) == ["a"]
+    assert cfgParser.rdStrList("main", "list1", ["x", "y"]) == ["a", "b"]
+    assert cfgParser.rdStrList("main", "list1", ["x", "y", "z"]) == ["a", "b", "c"]
+    assert cfgParser.rdStrList("main", "list1", ["x", "y", "z", "w"]) == ["a", "b", "c", "w"]
+
+    assert cfgParser.rdStrList("main", "stropt", ["x"]) == ["value"]
+    assert cfgParser.rdStrList("main", "intopt1", ["x"]) == ["42"]
+
+    assert cfgParser.rdStrList("nope", "list1", ["x"]) == ["x"]
+    assert cfgParser.rdStrList("main", "blabla", ["x"]) == ["x"]
+
+    # Read Integer List
+    assert cfgParser.rdIntList("main", "list2", []) == []
+    assert cfgParser.rdIntList("main", "list2", [1]) == [17]
+    assert cfgParser.rdIntList("main", "list2", [1, 2]) == [17, 18]
+    assert cfgParser.rdIntList("main", "list2", [1, 2, 3]) == [17, 18, 19]
+    assert cfgParser.rdIntList("main", "list2", [1, 2, 3, 4]) == [17, 18, 19, 4]
+
+    assert cfgParser.rdIntList("main", "stropt", [1]) == [1]
+    assert cfgParser.rdIntList("main", "boolopt1", [1]) == [1]
+
+    assert cfgParser.rdIntList("nope", "list2", [1]) == [1]
+    assert cfgParser.rdIntList("main", "blabla", [1]) == [1]
+
+    # Internal
+    # ========
+
+    assert cfgParser._parseLine("main", "stropt", None, 999) is None
+
+# END Test testBaseCommon_NWConfigParser

tests/test_base/test_base_config.py

@@ -19,10 +19,9 @@ You should have received a copy of the GNU General Public License
 along with this program. If not, see <https://www.gnu.org/licenses/>.
 """
 
-import pytest
-import sys
 import os
-import configparser
+import sys
+import pytest
 
 from shutil import copyfile
 
@@ -161,6 +160,20 @@ def testBaseConfig_Init(monkeypatch, tmpDir, fncDir, outDir, refDir, filesDir):
         assert tstConf.hasError is False
         assert tstConf.errData == []
 
+    # Check handling of novelWriter as a package
+    with monkeypatch.context() as mp:
+        tstConf.initConfig(confPath=tmpDir, dataPath=tmpDir)
+        assert tstConf.confPath == tmpDir
+        assert tstConf.dataPath == tmpDir
+        appRoot = tstConf.appRoot
+
+        mp.setattr("os.path.isfile", lambda *a: True)
+        tstConf.initConfig(confPath=tmpDir, dataPath=tmpDir)
+        assert tstConf.confPath == tmpDir
+        assert tstConf.dataPath == tmpDir
+        assert tstConf.appRoot == os.path.dirname(appRoot)
+        assert tstConf.appPath == os.path.dirname(appRoot)
+
     assert tstConf.loadConfig()
     assert tstConf.saveConfig()
 
@@ -187,19 +200,34 @@ def testBaseConfig_Init(monkeypatch, tmpDir, fncDir, outDir, refDir, filesDir):
     assert tstConf.saveConfig()
 
     # Localisation
+    # ============
+
     i18nDir = os.path.join(fncDir, "i18n")
     os.mkdir(i18nDir)
     os.mkdir(os.path.join(i18nDir, "stuff"))
     tstConf.nwLangPath = i18nDir
 
-    copyfile(os.path.join(filesDir, "nw_en_GB.qm"), os.path.join(fncDir, "nw_en_GB.qm"))
+    copyfile(os.path.join(filesDir, "nw_en_GB.qm"), os.path.join(i18nDir, "nw_en_GB.qm"))
     writeFile(os.path.join(i18nDir, "nw_en_GB.ts"), "")
     writeFile(os.path.join(i18nDir, "nw_abcd.qm"), "")
 
     tstApp = MockApp()
     tstConf.initLocalisation(tstApp)
+
+    # Check Lists
     theList = tstConf.listLanguages(tstConf.LANG_NW)
     assert theList == [("en_GB", "British English")]
+    theList = tstConf.listLanguages(tstConf.LANG_PROJ)
+    assert theList == [("en", "English")]
+    theList = tstConf.listLanguages(None)
+    assert theList == []
+
+    # Add Language
+    copyfile(os.path.join(filesDir, "nw_en_GB.qm"), os.path.join(i18nDir, "nw_fr.qm"))
+    writeFile(os.path.join(i18nDir, "nw_fr.ts"), "")
+
+    theList = tstConf.listLanguages(tstConf.LANG_NW)
+    assert theList == [("en_GB", "British English"), ("fr", "Français")]
 
     copyfile(confFile, testFile)
     assert cmpFiles(testFile, compFile, [2, 9, 10])
@@ -311,6 +339,7 @@ def testBaseConfig_SettersGetters(tmpConf, tmpDir, outDir, refDir):
 
     # GUI Scaling
     # ===========
+
     tmpConf.guiScale = 1.0
     assert tmpConf.pxInt(10) == 10
     assert tmpConf.pxInt(13) == 13
@@ -343,6 +372,19 @@ def testBaseConfig_SettersGetters(tmpConf, tmpDir, outDir, refDir):
 
     assert tmpConf.setWinSize(1200, 650)
 
+    # Preferences Size
+    tmpConf.guiScale = 2.0
+    assert tmpConf.setPreferencesSize(70, 70)
+    assert tmpConf.getPreferencesSize() == [70, 70]
+    assert tmpConf.prefGeometry == [35, 35]
+
+    tmpConf.guiScale = 1.0
+    assert tmpConf.setPreferencesSize(70, 70)
+    assert tmpConf.getPreferencesSize() == [70, 70]
+    assert tmpConf.prefGeometry == [70, 70]
+
+    assert tmpConf.setPreferencesSize(700, 615)
+
     # Project Tree Columns
     tmpConf.guiScale = 2.0
     assert tmpConf.setTreeColWidths([10, 20, 25])
@@ -436,6 +478,7 @@ def testBaseConfig_SettersGetters(tmpConf, tmpDir, outDir, refDir):
 
     # Getters Only
     # ============
+
     tmpConf.guiScale = 1.0
     assert tmpConf.getTextWidth() == 600
     assert tmpConf.getTextMargin() == 40
@@ -450,6 +493,7 @@ def testBaseConfig_SettersGetters(tmpConf, tmpDir, outDir, refDir):
 
     # Flag Setters
     # ============
+
     assert not tmpConf.setShowRefPanel(False)
     assert not tmpConf.showRefPanel
     assert tmpConf.setShowRefPanel(True)
@@ -482,74 +526,16 @@ def testBaseConfig_Internal(monkeypatch, tmpConf):
     # Function _packList
     assert tmpConf._packList(["A", 1, 2.0, None, False]) == "A, 1, 2.0, None, False"
 
-    # Function _unpackList
-    assert tmpConf._unpackList("1, 2, 3", [0, 0, 0], tmpConf.CNF_I_LST) == [1, 2, 3]
-    assert tmpConf._unpackList("1, 2   ", [0, 0, 0], tmpConf.CNF_I_LST) == [1, 2, 0]
-    assert tmpConf._unpackList("A, B, C", [0, 0, 0], tmpConf.CNF_I_LST) == [0, 0, 0]
-    assert tmpConf._unpackList("1, 2, 3", ["X", "Y", "Z"], tmpConf.CNF_S_LST) == ["1", "2", "3"]
-    assert tmpConf._unpackList("A, B   ", ["X", "Y", "Z"], tmpConf.CNF_S_LST) == ["A", "B", "Z"]
-    assert tmpConf._unpackList("A, B, C", ["X", "Y", "Z"], tmpConf.CNF_S_LST) == ["A", "B", "C"]
-    assert tmpConf._unpackList("A, B, C", ["X", "Y", "Z"], tmpConf.CNF_STR) == ["X", "Y", "Z"]
-
-    # Function _parseLine
-    cnfParse = configparser.ConfigParser()
-    cnfParse.read_string(
-        "[Main]\n"
-        "val_string = stuff\n"
-        "val_int = 123\n"
-        "val_bool = True\n"
-        "val_list_string = A, B, C\n"
-        "val_list_int = 1, 2, 3\n"
-    )
-
-    assert tmpConf._parseLine(
-        cnfParse, "Main", "val_string", tmpConf.CNF_STR, "default"
-    ) == "stuff"
-    assert tmpConf._parseLine(
-        cnfParse, "Main", "nope", tmpConf.CNF_STR, "default"
-    ) == "default"
-
-    assert tmpConf._parseLine(
-        cnfParse, "Main", "val_int", tmpConf.CNF_INT, "0"
-    ) == 123
-    assert tmpConf._parseLine(
-        cnfParse, "Main", "nope", tmpConf.CNF_INT, 0
-    ) == 0
-    assert tmpConf._parseLine(
-        cnfParse, "Main", "val_string", tmpConf.CNF_INT, 0
-    ) == 0
-
-    assert tmpConf._parseLine(
-        cnfParse, "Main", "val_bool", tmpConf.CNF_BOOL, False
-    ) is True
-    assert tmpConf._parseLine(
-        cnfParse, "Main", "nope", tmpConf.CNF_BOOL, False
-    ) is False
-    assert tmpConf._parseLine(
-        cnfParse, "Main", "val_string", tmpConf.CNF_BOOL, False
-    ) is False
-
-    assert tmpConf._parseLine(
-        cnfParse, "Main", "val_list_string", tmpConf.CNF_S_LST, ["W", "X", "Y", "Z"]
-    ) == ["A", "B", "C", "Z"]
-    assert tmpConf._parseLine(
-        cnfParse, "Main", "nope", tmpConf.CNF_S_LST, ["W", "X", "Y", "Z"]
-    ) == ["W", "X", "Y", "Z"]
-
-    assert tmpConf._parseLine(
-        cnfParse, "Main", "val_list_int", tmpConf.CNF_I_LST, [6, 7, 8, 9]
-    ) == [1, 2, 3, 9]
-    assert tmpConf._parseLine(
-        cnfParse, "Main", "nope", tmpConf.CNF_S_LST, [6, 7, 8, 9]
-    ) == [6, 7, 8, 9]
-
     # Function _checkNone
     assert tmpConf._checkNone(None) is None
     assert tmpConf._checkNone("None") is None
-    assert tmpConf._checkNone("stuff") == "stuff"
+    assert tmpConf._checkNone("none") is None
+    assert tmpConf._checkNone("NONE") is None
+    assert tmpConf._checkNone("NoNe") is None
+    assert tmpConf._checkNone(123456) == 123456
 
     # Function _checkOptionalPackages
-    # (Assumes enchant package exists ans is importable)
+    # (Assumes enchant package exists and is importable)
     tmpConf._checkOptionalPackages()
     assert tmpConf.hasEnchant is True
 
@@ -559,12 +545,12 @@ def testBaseConfig_Internal(monkeypatch, tmpConf):
         assert tmpConf.hasEnchant is False
 
     with monkeypatch.context() as mp:
-        mp.setattr("shutil.which", lambda *args: "stuff")
+        mp.setattr("shutil.which", lambda *a: "stuff")
         tmpConf._checkOptionalPackages()
         assert tmpConf.hasAssistant is True
 
     with monkeypatch.context() as mp:
-        mp.setattr("shutil.which", lambda *args: None)
+        mp.setattr("shutil.which", lambda *a: None)
         tmpConf._checkOptionalPackages()
         assert tmpConf.hasAssistant is False