Respond to comments

Author: multiplemonomials

tools/python/mbed_tools/build/_internal/config/schemas.py

@@ -31,7 +31,7 @@ class ConfigEntryDetails(BaseModel):
     macro_name: str | None = None
     """
     Name of the macro that this setting shall generate. If unset, defaults to
-    'MBED_CONF_<namespace uppercase>_<config entry name uppercase>', where
+    ``'MBED_CONF_<namespace uppercase>_<config entry name uppercase>'``, where
     <namespace> is the current namespace, i.e. the value of the "name" property for mbed_lib.json5 files,
     "target" for targets.json5 entries, and "app" in mbed_app.json5.
     """
@@ -127,7 +127,7 @@ class BaseJSONConfig(BaseModel):
     """
     List of overrides to unconditionally apply when this lib is included.
 
-    Overrides take the form "[<namespace>.]<setting>": <value> and cause the value of the given config
+    Overrides take the form ``"[<namespace>.]<setting>": <value>`` and cause the value of the given config
     setting to be changed to the given value. If the namespace is omitted, it will be set to the current
     namespace (i.e. the value of the "name" property for mbed_lib.json5 files, "target" for targets.json5 entries, and
     "app" in mbed_app.json5).
@@ -188,13 +188,13 @@ class MbedLibJSON(BaseJSONConfig):
     List of overrides applied based on target labels. This is similar to the "overrides" section,
     but allows applying the override only if the target has a specific label. Labels generally
     come from the names of the target and its parents, but targets can also add extra ones.
-    For example:
+    For example::
 
-    "target_overrides": {
-        "MIMXRT105X": {
-            "some-setting": some-value
+        "target_overrides": {
+            "MIMXRT105X": {
+                "some-setting": some-value
+            }
         }
-    }
 
     would apply the override only for targets in the MIMXRT105X target family.
 
@@ -279,14 +279,14 @@ class TargetJSON(BaseJSONConfig):
     from the "closest" ancestor is used, and all other values are discarded.
 
     To determine the attribute's value, the inheritance tree is flattened into a list using a depth-first traversal
-    that visits the first parent of each target first. For example, the inheritance tree diagram for target "A" below:
+    that visits the first parent of each target first. For example, the inheritance tree diagram for target "A" below::
 
-    D     E
-    |     |
-    B     C
-    |_____|
-       |
-       A
+        D     E
+        |     |
+        B     C
+        |_____|
+           |
+           A
 
     Would give us an inheritance order of [A, B, D, C, E]. Then, the overriding attribute's value would be taken
     from the first target in this list to contain the attribute.
@@ -301,14 +301,14 @@ class TargetJSON(BaseJSONConfig):
 
     UNLIKE overriding and merging attributes, accumulating attributes use a breadth-first search to flatten
     the inheritance hierarchy. (why? no idea! Probably for legacy compatibility...).  For example, an inheritance
-    tree diagram for the target "A" below
+    tree diagram for the target "A" below::
 
-    D     E
-    |     |
-    B     C
-    |_____|
-       |
-       A
+        D     E
+        |     |
+        B     C
+        |_____|
+           |
+           A
 
     Would give us an inheritance order of [A, B, C, D, E]. To process this, the first occurrence of the bare
     attribute name in the above list is found, and then we work backwards towards target A processing "_add" and
@@ -366,14 +366,14 @@ class TargetJSON(BaseJSONConfig):
 
     Note that this specific attribute has some special behavior, in that the value after the "=" in the macro
     definition can be ignored when finding matches to remove. So if the parent target adds a macro "FOO=7", then
-    doing "macros_remove": ["FOO"] in a child target is enough to remove it.
+    doing ``"macros_remove": ["FOO"]`` in a child target is enough to remove it.
     """
 
     extra_labels: list[str] = Field(default_factory=list)
     """
     Additional labels to add for this target and targets that inherit from it.
-    Labels are added to the `MBED_TARGET_LABELS` list in CMake, and become compile definitions
-    in the format `TARGET_<label>`.
+    Labels are added to the ``MBED_TARGET_LABELS`` list in CMake, and become compile definitions
+    in the format ``TARGET_<label>``.
     They are also used to control which directories in the source code are scanned for mbed_lib.json5 files.
 
     This is an accumulating attribute in its base form.
@@ -464,7 +464,7 @@ class TargetJSON(BaseJSONConfig):
 
     See mbed-os/targets/drivers.json5 for a full list of recognized peripheral names.
 
-    The peripheral list becomes compile definitions in the format `DEVICE_<name>`.
+    The peripheral list becomes compile definitions in the format ``DEVICE_<name>``.
 
     This is an accumulating attribute in its base form.
     """
@@ -598,13 +598,13 @@ class MbedAppJSON(BaseJSONConfig):
     List of overrides applied based on target labels. This is similar to the "overrides" section,
     but allows applying the override only if the target has a specific label. Labels generally
     come from the names of the target and its parents, but targets can also add extra ones.
-    For example:
+    For example::
 
-    "target_overrides": {
-        "MIMXRT105X": {
-            "some-setting": some-value
+        "target_overrides": {
+            "MIMXRT105X": {
+                "some-setting": some-value
+            }
         }
-    }
 
     would apply the override only for targets in the MIMXRT105X target family.
 

tools/python/mbed_tools/build/_internal/config/source.py

@@ -90,7 +90,7 @@ def check_and_transform_config_name(context: str, config_name: str) -> str:
     """
     if config_name.lower() != config_name:
         logger.warning(
-            f"Config setting '{config_name}' in {context} contains uppercase letters. This style is not recommended."
+            f"Config setting '{config_name}' in {context} contains uppercase letters. This style is not recommended and these will be replaced by lowercase letters."
         )
     if "." in config_name:
         logger.warning(
@@ -101,7 +101,7 @@ def check_and_transform_config_name(context: str, config_name: str) -> str:
             f"Config setting '{config_name}' in {context} contains an underscore. This style is not recommended as it may cause confusion (config names should be in skewer-case). Underscores are replaced by hyphens when Mbed processes JSON settings."
         )
 
-    return config_name.replace("_", "-")
+    return config_name.replace("_", "-").lower()
 
 
 def from_mbed_lib_json_file(

tools/python/mbed_tools/targets/_internal/targets_json_parsers/accumulating_attribute_parser.py

@@ -97,29 +97,15 @@ def _add_attribute_element(accumulator: Dict[str, Any], attribute_name: str, ele
         accumulator[attribute_name].append(element)
 
 
-def _macros_element_matches(element_to_remove: str, element_to_check: str) -> bool:
-    """
-    Checks if an element meets the criteria to be removed from list.
-
-    Some attribute elements (eg. macros) can be defined with a number value
-    eg. MACRO_SOMETHING=5. If we are then instructed to remove
-    MACRO_SOMETHING then this element needs to be recognised and removed
-    in addition to exact matches.
-
-    Args:
-        element_to_remove: the element as taken from list to be removed from an attribute
-        element_to_check: an element that currently makes up part of an attribute definition
-
-    Returns:
-        A boolean reflecting whether the element is a match and should be removed
-    """
-    return element_to_check == element_to_remove or element_to_check.startswith(f"{element_to_remove}=")
-
-
 def _remove_attribute_element(accumulator: Dict[str, Any], attribute_name: str, elements_to_remove: List[Any]) -> None:
     """
     Removes an attribute element from an attribute.
 
+    Note that macros can be defined with a number value
+    eg. "MACRO_SOMETHING=5". If we are then instructed to remove
+    MACRO_SOMETHING then this element needs to be recognised and removed
+    in addition to exact matches. This logic is specific to the "macros" attribute_name
+
     Args:
         accumulator: a store of attributes to be updated
         attribute_name: name of the attribute to update
@@ -129,9 +115,9 @@ def _remove_attribute_element(accumulator: Dict[str, Any], attribute_name: str,
     combinations_to_check = itertools.product(existing_elements, elements_to_remove)
     checked_elements_to_remove = [
         existing_element
-        for existing_element, element in combinations_to_check
-        if (attribute_name == "macros" and _macros_element_matches(element, existing_element))
-        or (element == existing_element)
+        for existing_element, element_to_remove in combinations_to_check
+        if (attribute_name == "macros" and existing_element.startswith(f"{element_to_remove}="))
+        or (element_to_remove == existing_element)
     ]
 
     for element in checked_elements_to_remove:

tools/python_tests/mbed_tools/targets/_internal/targets_json_parsers/test_accumulating_attribute_parser.py

@@ -14,7 +14,6 @@
     _targets_accumulate_hierarchy,
     _determine_accumulated_attributes,
     _remove_attribute_element,
-    _macros_element_matches,
 )
 
 
@@ -162,32 +161,6 @@ def test_determine_accumulated_attributes_basic_add(self):
         self.assertEqual(orig_accumulation_order, accumulation_order)
 
 
-class TestElementMatches(TestCase):
-    def test_element_matches_exactly(self):
-        element_to_remove = "SOMETHING"
-        element_to_check = "SOMETHING"
-
-        self.assertTrue(_macros_element_matches(element_to_remove, element_to_check))
-
-    def test_element_no_match(self):
-        element_to_remove = "SOMETHING"
-        element_to_check = "SOMETHING_ELSE"
-
-        self.assertFalse(_macros_element_matches(element_to_remove, element_to_check))
-
-    def test_element_matches_with_number_arg(self):
-        element_to_remove = "SOMETHING"
-        element_to_check = "SOMETHING=5"
-
-        self.assertTrue(_macros_element_matches(element_to_remove, element_to_check))
-
-    def test_element_no_match_with_number_arg(self):
-        element_to_remove = "SOMETHING"
-        element_to_check = "SOMETHING_DIFFERENT=5"
-
-        self.assertFalse(_macros_element_matches(element_to_remove, element_to_check))
-
-
 class TestRemoveAttributeElement(TestCase):
     def test_remove_element_without_numbers(self):
         current_attribute_state = {"attribute_1": ["ONE", "TWO=2", "THREE"]}