Refactor: Minor refactor and fixups for typos.

- main.py: Minor fixup for usage string.
- firmware_fileio.py: Minor clarification of debugging statement.
- logger.py: One line typo in comment.
- argparse_typed.py: Minor refactor to improve clarity and maintainability.

Author: glenn20

src/mp_image_tool_esp32/argparse_typed.py

@@ -46,19 +46,19 @@ def __init__(self, parser: ArgumentParser, typed_namespace: Namespace | None):
         # `typed_namespace` may also contain a type conversion function for some types
         self.type_conversions = getattr(typed_namespace, "_type_conversions", {})
 
-    def _get_argument_type(self, name: str) -> Any:
-        # Get the field name by replacing "-" with "_" in the last word of options
-        name = name.lstrip("-").replace("-", "_")
-        argtype = self.argument_types.get(name)  # Get the argument type hint
-        if not argtype:
-            raise ValueError(f"Argument `{name}` not found in {self.typed_namespace}")
+    def _get_argument_type(self, argument_name: str) -> Any:
+        # Get the argument type hint from the typed_namespace
+        argument_type = self.argument_types.get(argument_name)
+        if not argument_type:
+            raise ValueError(f"Argument `{argument_name}` not found in {self.typed_namespace}")
         # The type_conversions map may contain a function to convert the argument type
-        return self.type_conversions.get(argtype, argtype)
+        return self.type_conversions.get(argument_type, argument_type)
 
     def add_argument(
-        self, options: Sequence[str], metavars: Sequence[str], action: str, help: str
+        self, arguments: Sequence[str], metavars: Sequence[str], action: str, help: str
     ) -> None:
         kwargs: dict[str, Any] = {}
+        # Sort out the metavar options first
         if len(metavars) > 1:
             if metavars[-1] == "...":
                 kwargs["metavar"] = metavars[0]
@@ -70,15 +70,11 @@ def add_argument(
             kwargs["metavar"] = metavars[0]
 
         # Get the type hint for the argument from the typed_namespace
-        argtype = self._get_argument_type(options[-1])
-        if (
-            argtype
-            and argtype is not bool
-            and argtype is not str
-            and len(metavars) == 1
-        ):
-            kwargs["type"] = argtype
-        elif argtype is bool and not action:
+        argument_name = arguments[-1].lstrip("-").replace("-", "_")
+        argument_type = self._get_argument_type(argument_name)
+        if argument_type and argument_type not in (bool, str) and len(metavars) == 1:
+            kwargs["type"] = argument_type
+        elif argument_type is bool and not action:
             # default for bool, unless action or fun have been set
             action = "store_true"
 
@@ -87,7 +83,7 @@ def add_argument(
         if help:
             kwargs["help"] = help
 
-        self.parser.add_argument(*options, **kwargs)
+        self.parser.add_argument(*arguments, **kwargs)
 
 
 # usage is a multiline string of the form:
@@ -131,21 +127,29 @@ def parser(usage: str, typed_namespace: Namespace | None = None) -> ArgumentPars
     prog, description, arguments, epilog = (
         paragraph.strip() for paragraph in f"{usage}\n\n".split("\n\n", 3)
     )
-    typed_parser = TypedArgumentParser(
-        ArgumentParser(prog=prog, description=description, epilog=epilog),
-        typed_namespace,
-    )
+
+    # Construct a regular argparse ArgumentParser
+    argparser = ArgumentParser(prog=prog, description=description, epilog=epilog)
+    # Construct a TypedArgumentParser using the regular parser and the typed_namespace
+    typed_parser = TypedArgumentParser(argparser, typed_namespace)
 
     # Process each of the arguments in the `arguments` string (one per line)
     for line in arguments.splitlines():
+        # Split the line into parts: ARGUMENTS | HELP STRING | ACTION
+        # eg.  "-i --ignore          | Ignore file    | T"
         argstr, help, action, *_ = (s.strip() for s in f"{line}|||".split("|", 3))
 
         # options contains leading words starting with "-" if any, else all words.
-        # eg. "-i --input FILE" -> opts = ["-i", "--input"], metavars = ["FILE"]
-        # eg. "filename" -> opts = ["filename"], metavars = []
-        words = argstr.split()
-        options = list(takewhile(lambda s: s.startswith("-"), words)) or words
-        metavars = words[len(options) :]  # trailing words not in options.
-        typed_parser.add_argument(options, metavars, action, help)
+        # eg. "-i --input FILE" -> options = ["-i", "--input"], metavars = ["FILE"]
+        # eg. "filename" -> options = ["filename"], metavars = []
+        argument_names = argstr.split()
+        option_names = list(takewhile(lambda s: s.startswith("-"), argument_names))
+        if not option_names:
+            # There are no options, so all words are argument names.
+            typed_parser.add_argument(argument_names, [], action, help)
+        else:
+            # If an option is provided, the metavars are the rest of the words.
+            metavars = argument_names[len(option_names) :]
+            typed_parser.add_argument(option_names, metavars, action, help)
 
     return typed_parser.parser

src/mp_image_tool_esp32/firmware_fileio.py

@@ -140,7 +140,7 @@ def __enter__(self) -> FirmwareDeviceIO:
 
     def read(self, size: int | None = None) -> bytes:
         size = size if size is not None else self._end - self._pos
-        log.debug(f"Reading {size:#x} bytes from {self._pos:#x}...")
+        log.debug(f"Reading {size:#x} bytes from offset {self._pos:#x}...")
         data = self.esptool.read_flash(self._pos, size)
         if len(data) != size:
             raise ValueError(f"Read {len(data)} bytes from device, expected {size}.")

src/mp_image_tool_esp32/logger.py

@@ -47,7 +47,7 @@ def render_message(
     console=console,
     highlighter=NullHighlighter(),
     markup=True,
-    show_time=False,  # Don't show the timem level or path in log messages
+    show_time=False,  # Don't show the time, level or path in log messages
     show_level=False,
     show_path=False,
     rich_tracebacks=True,

src/mp_image_tool_esp32/main.py

@@ -124,15 +124,15 @@ class TypedNamespace(argparse.Namespace):
                             "--table default" (default (non-OTA) partition table),\
                             "--table nvs=7B,factory=2M,vfs=0". \
                             SUBTYPE is optional in most cases (inferred from name).
-    --delete NAME1[,NAME2] | delete the named partitions
+    --delete NAME1[,NAME2,...] | delete the named partitions
     --add NAME1:SUBTYPE:OFFSET:SIZE[,NAME2,...] \
                         | add new partitions to table
-    --resize NAME1=SIZE1[,NAME2=SIZE2] \
+    --resize NAME1=SIZE1[,NAME2=SIZE2,...] \
                         | resize partitions \
                             eg. --resize factory=2M,nvs=5B,vfs=0. \
                             If SIZE is 0, expand partition to available space
-    --erase NAME1[,NAME2] | erase the named partitions
-    --erase-fs NAME1[,NAME2] \
+    --erase NAME1[,NAME2,...] | erase the named partitions
+    --erase-fs NAME1[,NAME2,...] \
                         | erase first 4 blocks of a partition on flash storage.\
                             Micropython will initialise filesystem on next boot.
     --read NAME1=FILE1[,NAME2=FILE2,bootloader=FILE,...] \