fix: fix section adding and exception handling (#548)

Signed-off-by: Dominik Willner <th33xitus@gmail.com>

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(

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

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<flag>] Cancel the current object
+  #   [P<index>] Cancel the object with the given index
+  #   [S<index>] 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<count>] Reset the state and set the number of objects
+  #   [U<index>] 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 %}

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"
 
 

kiauh/core/submodules/simple_config_parser/tests/public_api/conftest.py

@@ -0,0 +1,26 @@
+# ======================================================================= #
+#  Copyright (C) 2024 Dominik Willner <th33xitus@gmail.com>               #
+#                                                                         #
+#  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

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):

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

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):

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)

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!")