fix: ensure options are retrieved from .ini file

Some boolean options, when not specified, default to "False", which
overwrites "True" if set in the .INI file.

Now they defaults to None (null), and won't overwrite previous setting.

Author: boriel

src/api/config.py

@@ -12,26 +12,32 @@
 import os
 import sys
 import configparser
+import enum
 
-from src import api
+from enum import Enum
+from typing import Dict, Callable
+from src.api import errmsg
 
 # The options container
-from . import options
-from . import global_
 
-from .options import ANYTYPE, Action
+from src.api import options
+from src.api import global_
+
+from src.api.options import ANYTYPE, Action
 
 
 # ------------------------------------------------------
 # Common setup and configuration for all tools
 # ------------------------------------------------------
-class ConfigSections:
+@enum.unique
+class ConfigSections(str, Enum):
     ZXBC = 'zxbc'
     ZXBASM = 'zxbasm'
     ZXBPP = 'zxbpp'
 
 
-class OPTION:
+@enum.unique
+class OPTION(str, Enum):
     OUTPUT_FILENAME = 'output_filename'
     INPUT_FILENAME = 'input_filename'
     STDERR_FILENAME = 'stderr_filename'
@@ -96,23 +102,23 @@ def load_config_from_file(filename: str, section: str, options_: options.Options
         cfg = configparser.ConfigParser()
         cfg.read(filename, encoding='utf-8')
     except (configparser.DuplicateSectionError, configparser.DuplicateOptionError):
-        api.errmsg.msg_output(f"Invalid config file '{filename}': it has duplicated fields")
+        errmsg.msg_output(f"Invalid config file '{filename}': it has duplicated fields")
         if stop_on_error:
             sys.exit(1)
         return False
     except FileNotFoundError:
-        api.errmsg.msg_output(f"Config file '{filename}' not found")
+        errmsg.msg_output(f"Config file '{filename}' not found")
         if stop_on_error:
             sys.exit(1)
         return False
 
     if section not in cfg.sections():
-        api.errmsg.msg_output(f"Section '{section}' not found in config file '{filename}'")
+        errmsg.msg_output(f"Section '{section}' not found in config file '{filename}'")
         if stop_on_error:
             sys.exit(1)
         return False
 
-    parsing = {
+    parsing: Dict[type, Callable] = {
         int: cfg.getint,
         float: cfg.getfloat,
         bool: cfg.getboolean
@@ -137,7 +143,7 @@ def save_config_into_file(filename: str, section: str, options_: options.Options
         try:
             cfg.read(filename, encoding='utf-8')
         except (configparser.DuplicateSectionError, configparser.DuplicateOptionError):
-            api.errmsg.msg_output(f"Invalid config file '{filename}': it has duplicated fields")
+            errmsg.msg_output(f"Invalid config file '{filename}': it has duplicated fields")
             if stop_on_error:
                 sys.exit(1)
             return False
@@ -157,7 +163,7 @@ def save_config_into_file(filename: str, section: str, options_: options.Options
         with open(filename, 'wt', encoding='utf-8') as f:
             cfg.write(f)
     except IOError:
-        api.errmsg.msg_output(f"Can't write config file '{filename}'")
+        errmsg.msg_output(f"Can't write config file '{filename}'")
         if stop_on_error:
             sys.exit(1)
         return False
@@ -189,10 +195,10 @@ def init():
     OPTIONS(Action.ADD, name=OPTION.MEMORY_MAP, type=str, default=None, ignore_none=True)
     OPTIONS(Action.ADD, name=OPTION.FORCE_ASM_BRACKET, type=bool, default=False, ignore_none=True)
 
-    OPTIONS(Action.ADD, name=OPTION.USE_BASIC_LOADER, type=bool, default=False)  # Whether to use a loader
+    OPTIONS(Action.ADD, name=OPTION.USE_BASIC_LOADER, type=bool, default=False, ignore_none=True)
 
     # Whether to add autostart code (needs basic loader = true)
-    OPTIONS(Action.ADD, name=OPTION.AUTORUN, type=bool, default=False)
+    OPTIONS(Action.ADD, name=OPTION.AUTORUN, type=bool, default=False, ignore_none=True)
     OPTIONS(Action.ADD, name=OPTION.OUTPUT_FILE_TYPE, type=str, default='bin')  # bin, tap, tzx etc...
     OPTIONS(Action.ADD, name=OPTION.INCLUDE_PATH, type=str, default='')  # Include path, like '/var/lib:/var/include'
 

src/zxbc/args_parser.py

@@ -42,9 +42,9 @@ def parser() -> argparse.ArgumentParser:
     output_file_type_group.add_argument('--parse-only', action='store_true',
                                         help='Only parses to check for syntax and semantic errors')
 
-    parser_.add_argument('-B', '--BASIC', action='store_true', dest='basic',
+    parser_.add_argument('-B', '--BASIC', action='store_true', dest='basic', default=None,
                          help="Creates a BASIC loader which loads the rest of the CODE. Requires -T ot -t")
-    parser_.add_argument('-a', '--autorun', action='store_true',
+    parser_.add_argument('-a', '--autorun', action='store_true', default=None,
                          help="Sets the program to be run once loaded")
 
     parser_.add_argument('-S', '--org', type=str,

tests/api/test_arg_parser.py

@@ -0,0 +1,26 @@
+import unittest
+
+from src.zxbc.args_parser import parser
+
+
+class TestArgParser(unittest.TestCase):
+    """ Test argument options from the cmdline
+    """
+    def setUp(self):
+        self.parser = parser()
+
+    def test_autorun_defaults_to_none(self):
+        """ Some boolean options, when not specified from the command line
+        must return None (null) instead of False to preserve .INI saved
+        value.
+        """
+        options = self.parser.parse_args(["test.bas"])
+        self.assertIsNone(options.autorun)
+
+    def test_loader_defaults_to_none(self):
+        """ Some boolean options, when not specified from the command line
+        must return None (null) instead of False to preserve .INI saved
+        value.
+        """
+        options = self.parser.parse_args(["test.bas"])
+        self.assertIsNone(options.basic)

tests/api/test_config.py

@@ -12,9 +12,9 @@ class TestConfig(unittest.TestCase):
     """
     def setUp(self):
         config.OPTIONS(config.Action.CLEAR)
+        config.init()
 
     def test_init(self):
-        config.init()
         self.assertEqual(config.OPTIONS.debug_level, 0)
         self.assertEqual(config.OPTIONS.stdin, sys.stdin)
         self.assertEqual(config.OPTIONS.stdout, sys.stdout)
@@ -46,7 +46,6 @@ def test_init(self):
         self.assertEqual(config.OPTIONS.strict, False)
 
     def test_initted_values(self):
-        config.init()
         self.assertEqual(sorted(config.OPTIONS._options.keys()), [
             '__DEFINES',
             config.OPTION.ARCH,
@@ -82,3 +81,19 @@ def test_initted_values(self):
             config.OPTION.USE_BASIC_LOADER,
             config.OPTION.ASM_ZXNEXT
         ])
+
+    def test_loader_ignore_none(self):
+        """ Some settings must ignore "None" assignments, since
+        this means the user didn't specify anything from the command line
+        """
+        config.OPTIONS.use_basic_loader = True
+        config.OPTIONS.use_basic_loader = None
+        self.assertEqual(config.OPTIONS.use_basic_loader, True)
+
+    def test_autorun_ignore_none(self):
+        """ Some settings must ignore "None" assignments, since
+        this means the user didn't specify anything from the command line
+        """
+        config.OPTIONS.autorun = True
+        config.OPTIONS.autorun = None
+        self.assertEqual(config.OPTIONS.autorun, True)