From 4138c7192003cc20e7740bd70a9b2e082c9a725c Mon Sep 17 00:00:00 2001 From: dw-0 Date: Thu, 26 Sep 2024 20:52:19 +0200 Subject: [PATCH] fix: fix section adding and exception handling (#548) Signed-off-by: Dominik Willner --- .../simple_config_parser.py | 25 +++-- .../tests/assets/test_config_2.cfg | 33 +++++++ .../tests/assets/test_config_3.cfg | 94 +++++++++++++++++++ .../tests/line_parsing/test_line_parsing.py | 2 +- .../tests/public_api/conftest.py | 26 +++++ .../tests/public_api/test_options_api.py | 19 +--- .../tests/public_api/test_read_file.py | 10 +- .../tests/public_api/test_sections_api.py | 15 --- .../tests/value_conversion/test_get_conv.py | 35 +++++-- kiauh/utils/config_utils.py | 6 ++ 10 files changed, 210 insertions(+), 55 deletions(-) create mode 100644 kiauh/core/submodules/simple_config_parser/tests/assets/test_config_2.cfg create mode 100644 kiauh/core/submodules/simple_config_parser/tests/assets/test_config_3.cfg create mode 100644 kiauh/core/submodules/simple_config_parser/tests/public_api/conftest.py diff --git a/kiauh/core/submodules/simple_config_parser/src/simple_config_parser/simple_config_parser.py b/kiauh/core/submodules/simple_config_parser/src/simple_config_parser/simple_config_parser.py index 88f2ae0..d3d7e87 100644 --- a/kiauh/core/submodules/simple_config_parser/src/simple_config_parser/simple_config_parser.py +++ b/kiauh/core/submodules/simple_config_parser/src/simple_config_parser/simple_config_parser.py @@ -192,11 +192,24 @@ class SimpleConfigParser: self.config[section] = {"_raw": f"[{section}]\n"} def _check_set_section_spacing(self): - prev_section = self.get_sections()[-1] - prev_section_content = self.config[prev_section] - last_item = list(prev_section_content.keys())[-1] - if last_item.startswith("#_") and last_item.keys()[-1] != "\n": - prev_section_content[last_item].append("\n") + prev_section_name: str = self.get_sections()[-1] + prev_section_content: Dict = self.config[prev_section_name] + last_option_name: str = list(prev_section_content.keys())[-1] + + if last_option_name.startswith("#_"): + last_elem_value: str = prev_section_content[last_option_name][-1] + + # if the last section is a collector, we first check if the last element + # in the collector ends with a newline. if it does not, we append a newline. + # this can happen if the config file does not end with a newline. + if not last_elem_value.endswith("\n"): + prev_section_content[last_option_name][-1] = f"{last_elem_value}\n" + + # if the last item in a collector is not a newline, we append a newline, so + # that the new section is seperated from the options of the previous section + # by a newline + if last_elem_value != "\n": + prev_section_content[last_option_name].append("\n") else: prev_section_content[self._generate_rand_id()] = ["\n"] @@ -298,7 +311,7 @@ class SimpleConfigParser: """Return the value of the given option in the given section as a converted value""" try: return conv(self.getval(section, option, fallback)) - except ValueError as e: + except (ValueError, TypeError, AttributeError) as e: if fallback is not _UNSET: return fallback raise ValueError( diff --git a/kiauh/core/submodules/simple_config_parser/tests/assets/test_config_2.cfg b/kiauh/core/submodules/simple_config_parser/tests/assets/test_config_2.cfg new file mode 100644 index 0000000..f963713 --- /dev/null +++ b/kiauh/core/submodules/simple_config_parser/tests/assets/test_config_2.cfg @@ -0,0 +1,33 @@ +# a comment at the very top +# should be treated as the file header + +# up to the first section, including all blank lines + +[section_1] +option_1: value_1 +option_1_1: True # this is a boolean +option_1_2: 5 ; this is an integer +option_1_3: 1.123 #;this is a float + +[section_2] ; comment +option_2: value_2 + +; comment + +[section_3] +option_3: value_3 # comment + +[section_4] +# comment +option_4: value_4 + +[section number 5] +#option_5: value_5 +option_5 = this.is.value-5 +multi_option: + # these are multi-line values + value_5_1 + value_5_2 ; here is a comment + value_5_3 +option_5_1: value_5_1 +# config ending with a comment diff --git a/kiauh/core/submodules/simple_config_parser/tests/assets/test_config_3.cfg b/kiauh/core/submodules/simple_config_parser/tests/assets/test_config_3.cfg new file mode 100644 index 0000000..1563926 --- /dev/null +++ b/kiauh/core/submodules/simple_config_parser/tests/assets/test_config_3.cfg @@ -0,0 +1,94 @@ +# a comment at the very top +# should be treated as the file header + +# up to the first section, including all blank lines + +[section_1] +option_1: value_1 +option_1_1: True # this is a boolean +option_1_2: 5 ; this is an integer +option_1_3: 1.123 #;this is a float + +[section_2] ; comment +option_2: value_2 + +; comment + +[section_3] +option_3: value_3 # comment + +[section_4] +# comment +option_4: value_4 + +[section number 5] +#option_5: value_5 +option_5 = this.is.value-5 +multi_option: + # these are multi-line values + value_5_1 + value_5_2 ; here is a comment + value_5_3 +option_5_1: value_5_1 + +[gcode_macro M117] +rename_existing: M117.1 +gcode: + {% if rawparams %} + {% set escaped_msg = rawparams.split(';', 1)[0].split('\x23', 1)[0]|replace('"', '\\"') %} + SET_DISPLAY_TEXT MSG="{escaped_msg}" + RESPOND TYPE=command MSG="{escaped_msg}" + {% else %} + SET_DISPLAY_TEXT + {% endif %} + +# SDCard 'looping' (aka Marlin M808 commands) support +# +# Support SDCard looping +[sdcard_loop] +[gcode_macro M486] +gcode: + # Parameters known to M486 are as follows: + # [C] Cancel the current object + # [P] Cancel the object with the given index + # [S] Set the index of the current object. + # If the object with the given index has been canceled, this will cause + # the firmware to skip to the next object. The value -1 is used to + # indicate something that isn’t an object and shouldn’t be skipped. + # [T] Reset the state and set the number of objects + # [U] Un-cancel the object with the given index. This command will be + # ignored if the object has already been skipped + + {% if 'exclude_object' not in printer %} + {action_raise_error("[exclude_object] is not enabled")} + {% endif %} + + {% if 'T' in params %} + EXCLUDE_OBJECT RESET=1 + + {% for i in range(params.T | int) %} + EXCLUDE_OBJECT_DEFINE NAME={i} + {% endfor %} + {% endif %} + + {% if 'C' in params %} + EXCLUDE_OBJECT CURRENT=1 + {% endif %} + + {% if 'P' in params %} + EXCLUDE_OBJECT NAME={params.P} + {% endif %} + + {% if 'S' in params %} + {% if params.S == '-1' %} + {% if printer.exclude_object.current_object %} + EXCLUDE_OBJECT_END NAME={printer.exclude_object.current_object} + {% endif %} + {% else %} + EXCLUDE_OBJECT_START NAME={params.S} + {% endif %} + {% endif %} + + {% if 'U' in params %} + EXCLUDE_OBJECT RESET=1 NAME={params.U} + {% endif %} diff --git a/kiauh/core/submodules/simple_config_parser/tests/line_parsing/test_line_parsing.py b/kiauh/core/submodules/simple_config_parser/tests/line_parsing/test_line_parsing.py index e1b95d5..b306d14 100644 --- a/kiauh/core/submodules/simple_config_parser/tests/line_parsing/test_line_parsing.py +++ b/kiauh/core/submodules/simple_config_parser/tests/line_parsing/test_line_parsing.py @@ -32,7 +32,7 @@ def test_section_parsing(parser): parser.config.keys() ), f"Expected keys: {expected_keys}, got: {parser.config.keys()}" assert parser.in_option_block is False - assert parser.current_section == "section number 5" + assert parser.current_section == parser.get_sections()[-1] assert parser.config["section_2"]["_raw"] == "[section_2] ; comment" diff --git a/kiauh/core/submodules/simple_config_parser/tests/public_api/conftest.py b/kiauh/core/submodules/simple_config_parser/tests/public_api/conftest.py new file mode 100644 index 0000000..7c932c6 --- /dev/null +++ b/kiauh/core/submodules/simple_config_parser/tests/public_api/conftest.py @@ -0,0 +1,26 @@ +# ======================================================================= # +# Copyright (C) 2024 Dominik Willner # +# # +# https://github.com/dw-0/simple-config-parser # +# # +# This file may be distributed under the terms of the GNU GPLv3 license # +# ======================================================================= # +from pathlib import Path + +import pytest + +from src.simple_config_parser.simple_config_parser import SimpleConfigParser +from tests.utils import load_testdata_from_file + +BASE_DIR = Path(__file__).parent.parent.joinpath("assets") +CONFIG_FILES = ["test_config_1.cfg", "test_config_2.cfg", "test_config_3.cfg"] + + +@pytest.fixture(params=CONFIG_FILES) +def parser(request): + parser = SimpleConfigParser() + file_path = BASE_DIR.joinpath(request.param) + for line in load_testdata_from_file(file_path): + parser._parse_line(line) # noqa + + return parser diff --git a/kiauh/core/submodules/simple_config_parser/tests/public_api/test_options_api.py b/kiauh/core/submodules/simple_config_parser/tests/public_api/test_options_api.py index 352e556..c094a32 100644 --- a/kiauh/core/submodules/simple_config_parser/tests/public_api/test_options_api.py +++ b/kiauh/core/submodules/simple_config_parser/tests/public_api/test_options_api.py @@ -5,28 +5,13 @@ # # # This file may be distributed under the terms of the GNU GPLv3 license # # ======================================================================= # -from pathlib import Path import pytest from src.simple_config_parser.simple_config_parser import ( NoOptionError, NoSectionError, - SimpleConfigParser, ) -from tests.utils import load_testdata_from_file - -BASE_DIR = Path(__file__).parent.parent.joinpath("assets") -TEST_DATA_PATH = BASE_DIR.joinpath("test_config_1.cfg") - - -@pytest.fixture -def parser(): - parser = SimpleConfigParser() - for line in load_testdata_from_file(TEST_DATA_PATH): - parser._parse_line(line) # noqa - - return parser def test_get_options(parser): @@ -72,6 +57,7 @@ def test_getval(parser): def test_getval_fallback(parser): assert parser.getval("section_1", "option_128", "fallback") == "fallback" + assert parser.getval("section_1", "option_128", None) is None def test_getval_exceptions(parser): @@ -104,6 +90,7 @@ def test_getint_from_boolean(parser): def test_getint_fallback(parser): assert parser.getint("section_1", "option_128", 128) == 128 + assert parser.getint("section_1", "option_128", None) is None def test_getboolean(parser): @@ -130,6 +117,7 @@ def test_getboolean_from_float(parser): def test_getboolean_fallback(parser): assert parser.getboolean("section_1", "option_128", True) is True assert parser.getboolean("section_1", "option_128", False) is False + assert parser.getboolean("section_1", "option_128", None) is None def test_getfloat(parser): @@ -154,6 +142,7 @@ def test_getfloat_from_boolean(parser): def test_getfloat_fallback(parser): assert parser.getfloat("section_1", "option_128", 1.234) == 1.234 + assert parser.getfloat("section_1", "option_128", None) is None def test_set_existing_option(parser): diff --git a/kiauh/core/submodules/simple_config_parser/tests/public_api/test_read_file.py b/kiauh/core/submodules/simple_config_parser/tests/public_api/test_read_file.py index b8b523b..f9272df 100644 --- a/kiauh/core/submodules/simple_config_parser/tests/public_api/test_read_file.py +++ b/kiauh/core/submodules/simple_config_parser/tests/public_api/test_read_file.py @@ -7,8 +7,6 @@ # ======================================================================= # from pathlib import Path -import pytest - from src.simple_config_parser.simple_config_parser import ( SimpleConfigParser, ) @@ -17,12 +15,8 @@ BASE_DIR = Path(__file__).parent.parent.joinpath("assets") TEST_DATA_PATH = BASE_DIR.joinpath("test_config_1.cfg") -@pytest.fixture -def parser(): - return SimpleConfigParser() - - -def test_read_file(parser): +def test_read_file(): + parser = SimpleConfigParser() parser.read_file(TEST_DATA_PATH) assert parser.config is not None assert parser.config.keys() is not None diff --git a/kiauh/core/submodules/simple_config_parser/tests/public_api/test_sections_api.py b/kiauh/core/submodules/simple_config_parser/tests/public_api/test_sections_api.py index 0e55ecf..35ffc21 100644 --- a/kiauh/core/submodules/simple_config_parser/tests/public_api/test_sections_api.py +++ b/kiauh/core/submodules/simple_config_parser/tests/public_api/test_sections_api.py @@ -5,27 +5,12 @@ # # # This file may be distributed under the terms of the GNU GPLv3 license # # ======================================================================= # -from pathlib import Path import pytest from src.simple_config_parser.simple_config_parser import ( DuplicateSectionError, - SimpleConfigParser, ) -from tests.utils import load_testdata_from_file - -BASE_DIR = Path(__file__).parent.parent.joinpath("assets") -TEST_DATA_PATH = BASE_DIR.joinpath("test_config_1.cfg") - - -@pytest.fixture -def parser(): - parser = SimpleConfigParser() - for line in load_testdata_from_file(TEST_DATA_PATH): - parser._parse_line(line) # noqa - - return parser def test_get_sections(parser): diff --git a/kiauh/core/submodules/simple_config_parser/tests/value_conversion/test_get_conv.py b/kiauh/core/submodules/simple_config_parser/tests/value_conversion/test_get_conv.py index 47828c2..893ecc1 100644 --- a/kiauh/core/submodules/simple_config_parser/tests/value_conversion/test_get_conv.py +++ b/kiauh/core/submodules/simple_config_parser/tests/value_conversion/test_get_conv.py @@ -25,50 +25,65 @@ def parser(): return parser -def test_get_conv(parser): - # Test conversion to int +def test_get_int_conv(parser): should_be_int = parser._get_conv("section_1", "option_1_2", int) assert isinstance(should_be_int, int) - # Test conversion to float + +def test_get_float_conv(parser): should_be_float = parser._get_conv("section_1", "option_1_3", float) assert isinstance(should_be_float, float) - # Test conversion to boolean + +def test_get_bool_conv(parser): should_be_bool = parser._get_conv( "section_1", "option_1_1", parser._convert_to_boolean ) assert isinstance(should_be_bool, bool) - # Test fallback for int + +def test_get_int_conv_fallback(parser): should_be_fallback_int = parser._get_conv( "section_1", "option_128", int, fallback=128 ) assert isinstance(should_be_fallback_int, int) assert should_be_fallback_int == 128 + assert parser._get_conv("section_1", "option_128", int, None) is None - # Test fallback for float + +def test_get_float_conv_fallback(parser): should_be_fallback_float = parser._get_conv( "section_1", "option_128", float, fallback=1.234 ) assert isinstance(should_be_fallback_float, float) assert should_be_fallback_float == 1.234 - # Test fallback for boolean + assert parser._get_conv("section_1", "option_128", float, None) is None + + +def test_get_bool_conv_fallback(parser): should_be_fallback_bool = parser._get_conv( "section_1", "option_128", parser._convert_to_boolean, fallback=True ) assert isinstance(should_be_fallback_bool, bool) assert should_be_fallback_bool is True - # Test ValueError exception for invalid int conversion + assert ( + parser._get_conv("section_1", "option_128", parser._convert_to_boolean, None) + is None + ) + + +def test_get_int_conv_exception(parser): with pytest.raises(ValueError): parser._get_conv("section_1", "option_1", int) - # Test ValueError exception for invalid float conversion + +def test_get_float_conv_exception(parser): with pytest.raises(ValueError): parser._get_conv("section_1", "option_1", float) - # Test ValueError exception for invalid boolean conversion + +def test_get_bool_conv_exception(parser): with pytest.raises(ValueError): parser._get_conv("section_1", "option_1", parser._convert_to_boolean) diff --git a/kiauh/utils/config_utils.py b/kiauh/utils/config_utils.py index 1f5179c..fd13ce0 100644 --- a/kiauh/utils/config_utils.py +++ b/kiauh/utils/config_utils.py @@ -48,6 +48,8 @@ def add_config_section( scp.write_file(cfg_file) + Logger.print_ok("OK!") + def add_config_section_at_top(section: str, instances: List[InstanceType]) -> None: # TODO: this could be implemented natively in SimpleConfigParser @@ -69,6 +71,8 @@ def add_config_section_at_top(section: str, instances: List[InstanceType]) -> No cfg_file.unlink() tmp_cfg_path.rename(cfg_file) + Logger.print_ok("OK!") + def remove_config_section(section: str, instances: List[InstanceType]) -> None: for instance in instances: @@ -87,3 +91,5 @@ def remove_config_section(section: str, instances: List[InstanceType]) -> None: scp.remove_section(section) scp.write_file(cfg_file) + + Logger.print_ok("OK!")