Merge pull request #553 from boriel/bugfix/config_file_options

Bugfix/config file options

Author: boriel

poetry.lock

@@ -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/api/config.py

@@ -16,9 +16,9 @@
 from typing import Optional
 
 from src.api import global_
+from src.api import config
 
 from src.api.constants import CLASS
-from src.api.config import OPTIONS
 
 
 # Exports only these functions. Others
@@ -39,14 +39,14 @@ def msg_output(msg: str) -> None:
     if msg in global_.error_msg_cache:
         return
 
-    OPTIONS.stderr.write("%s\n" % msg)
+    config.OPTIONS.stderr.write("%s\n" % msg)
     global_.error_msg_cache.add(msg)
 
 
 def info(msg: str) -> None:
-    if OPTIONS.debug_level < 1:
+    if config.OPTIONS.debug_level < 1:
         return
-    OPTIONS.stderr.write("info: %s\n" % msg)
+    config.OPTIONS.stderr.write("info: %s\n" % msg)
 
 
 def error(lineno: int, msg: str, fname: Optional[str] = None) -> None:
@@ -55,13 +55,13 @@ def error(lineno: int, msg: str, fname: Optional[str] = None) -> None:
     if fname is None:
         fname = global_.FILENAME
 
-    if global_.has_errors > OPTIONS.max_syntax_errors:
+    if global_.has_errors > config.OPTIONS.max_syntax_errors:
         msg = 'Too many errors. Giving up!'
 
     msg = "%s:%i: error:%s %s" % (fname, lineno, ERROR_PREFIX, msg)
     msg_output(msg)
 
-    if global_.has_errors > OPTIONS.max_syntax_errors:
+    if global_.has_errors > config.OPTIONS.max_syntax_errors:
         sys.exit(1)
 
     global_.has_errors += 1
@@ -71,7 +71,7 @@ def warning(lineno: int, msg: str, fname: Optional[str] = None) -> None:
     """ Generic warning error routine
     """
     global_.has_warnings += 1
-    if global_.has_warnings <= OPTIONS.expected_warnings:
+    if global_.has_warnings <= config.OPTIONS.expected_warnings:
         return
 
     if fname is None:
@@ -107,7 +107,7 @@ def decorator(func: Callable) -> Callable:
         def wrapper(*args, **kwargs):
             global WARNING_PREFIX
             if global_.ENABLED_WARNINGS.get(code, True):
-                if not OPTIONS.hide_warning_codes:
+                if not config.OPTIONS.hide_warning_codes:
                     WARNING_PREFIX = f'warning: [W{code}]'
                 func(*args, **kwargs)
                 WARNING_PREFIX = ''
@@ -122,7 +122,7 @@ def wrapper(*args, **kwargs):
 def warning_implicit_type(lineno: int, id_: str, type_: str = None):
     """ Warning: Using default implicit type 'x'
     """
-    if OPTIONS.strict:
+    if config.OPTIONS.strict:
         syntax_error_undeclared_type(lineno, id_)
         return
 
@@ -164,7 +164,7 @@ def warning_empty_if(lineno: int):
 def warning_not_used(lineno: int, id_: str, kind: str = 'Variable', fname: Optional[str] = None):
     """ Emits an optimization warning
     """
-    if OPTIONS.optimization_level > 0:
+    if config.OPTIONS.optimization_level > 0:
         warning(lineno, "%s '%s' is never used" % (kind, id_), fname=fname)
 
 

src/api/errmsg.py

@@ -188,6 +188,11 @@
 
 
 # ----------------------------------------------------------------------
-# Warning codes and whether they're enabled or not
+# Warning options
 # ----------------------------------------------------------------------
+
+# Warning codes and whether they're enabled or not
 ENABLED_WARNINGS: Dict[str, bool] = {}
+
+# Number of expected warnings (won't be issued)
+EXPECTED_WARNINGS: int = 0

src/api/global_.py

@@ -10,12 +10,13 @@
 # ----------------------------------------------------------------------
 
 import json
+import enum
 
 from typing import Dict
 from typing import List
 from typing import Any
 
-from .errors import Error
+from src.api.errors import Error
 
 __all__ = ['Option', 'Options', 'ANYTYPE', 'Action']
 
@@ -161,13 +162,16 @@ def pop(self) -> Any:
 # ----------------------------------------------------------------------
 # Options commands
 # ----------------------------------------------------------------------
-class Action:
+@enum.unique
+class Action(str, enum.Enum):
     ADD = 'add'
     ADD_IF_NOT_DEFINED = 'add_if_not_defined'
     CLEAR = 'clear'
     LIST = 'list'
 
-    allowed = {ADD, CLEAR, LIST, ADD_IF_NOT_DEFINED}
+    @classmethod
+    def valid(cls, action: str) -> bool:
+        return action in list(cls)
 
 
 # ----------------------------------------------------------------------
@@ -258,9 +262,9 @@ def check_allowed_args(action: str, kwargs_, allowed_args, required_args=None):
             if not args or args == (Action.LIST,):
                 return {x: y for x, y in self._options.items()}
 
-        assert args, f"Missing one action of {', '.join(Action.allowed)}"
-        assert len(args) == 1 and args[0] in Action.allowed, \
-            f"Only one action of {', '.join(Action.allowed)} can be specified"
+        assert args, f"Missing one action of {', '.join(Action)}"
+        assert len(args) == 1 and Action.valid(args[0]), \
+            f"Only one action of {', '.join(Action)} can be specified"
 
         # clear
         if args[0] == Action.CLEAR:
@@ -282,6 +286,7 @@ def check_allowed_args(action: str, kwargs_, allowed_args, required_args=None):
             kwargs['type_'] = kwargs['type']
             del kwargs['type']
             self.__add_option(**kwargs)
+            return
 
         if args[0] == Action.ADD_IF_NOT_DEFINED:
             kwargs['type'] = kwargs.get('type')

src/api/options.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,

src/zxbc/args_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_arg_parser.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)