[2/4] dts: customise argparse error message

Message ID 20240122182611.1904974-3-luca.vizzarro@arm.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series dts: error and usage improvements |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Luca Vizzarro Jan. 22, 2024, 6:26 p.m. UTC
  This commit customises the arguments parsing class' error message,
making it so the confusing usage is not displayed in these occurrences,
but the user is redirected to use the help argument instead.

Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>
Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
---
 dts/framework/settings.py | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)
  

Comments

Juraj Linkeš Jan. 29, 2024, 1:04 p.m. UTC | #1
Unaddressed
On Mon, Jan 22, 2024 at 7:26 PM Luca Vizzarro <luca.vizzarro@arm.com> wrote:
>
> This commit customises the arguments parsing class' error message,
> making it so the confusing usage is not displayed in these occurrences,

I'm curious, what exactly is confusing about the message?

> but the user is redirected to use the help argument instead.
>
> Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>
> Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
> ---
>  dts/framework/settings.py | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/dts/framework/settings.py b/dts/framework/settings.py
> index 2d0365e763..acfe5cad44 100644
> --- a/dts/framework/settings.py
> +++ b/dts/framework/settings.py
> @@ -170,6 +170,15 @@ def _parse_revision_id(rev_id: str) -> str:
>          )
>
>
> +class ArgumentParser(argparse.ArgumentParser):
> +    """ArgumentParser with a custom error message."""
> +    def error(self, message):
> +        print(f"{self.prog}: error: {message}\n", file=sys.stderr)
> +        self.exit(2,
> +                  "For help and usage, "
> +                  "run the command with the --help flag.\n")
> +
> +
>  @dataclass(slots=True)
>  class Settings:
>      """Default framework-wide user settings.
> @@ -200,8 +209,8 @@ class Settings:
>  SETTINGS: Settings = Settings()
>
>
> -def _get_parser() -> argparse.ArgumentParser:
> -    parser = argparse.ArgumentParser(
> +def _get_parser() -> ArgumentParser:
> +    parser = ArgumentParser(
>          description="Run DPDK test suites. All options may be specified with the environment "
>          "variables provided in brackets. Command line arguments have higher priority.",
>          formatter_class=argparse.ArgumentDefaultsHelpFormatter,
> --
> 2.34.1
>
  
Luca Vizzarro Feb. 23, 2024, 7:12 p.m. UTC | #2
On 29/01/2024 13:04, Juraj Linkeš wrote:
> I'm curious, what exactly is confusing about the message?

Unfortunately a bit too much time has passed... but if I remember 
correctly I think that given the great amount of arguments, whenever the 
message is printed a bit too much information is given to the user. So 
bottomline, too crowded
  
Juraj Linkeš Feb. 26, 2024, 9:09 a.m. UTC | #3
On Fri, Feb 23, 2024 at 8:12 PM Luca Vizzarro <Luca.Vizzarro@arm.com> wrote:
>
> On 29/01/2024 13:04, Juraj Linkeš wrote:
> > I'm curious, what exactly is confusing about the message?
>
> Unfortunately a bit too much time has passed... but if I remember
> correctly I think that given the great amount of arguments, whenever the
> message is printed a bit too much information is given to the user. So
> bottomline, too crowded

The original message is:
./main.py -wdghf
usage: main.py [-h] [--config-file CONFIG_FILE] [--output-dir
OUTPUT_DIR] [-t TIMEOUT] [-v] [-s] [--tarball TARBALL]
[--compile-timeout COMPILE_TIMEOUT] [--test-suite TEST_SUITE
[TEST_CASES ...]] [--re-run RE_RUN]
main.py: error: unrecognized arguments: -wdghf

So this is what's confusing. I guess it doesn't mention that the user
should use the help argument and that's where the confusion was? From
my point of view that's just standard (to run a command with -h in
case of an error such as the one above), but maybe it is better to
state it explicitly.
  

Patch

diff --git a/dts/framework/settings.py b/dts/framework/settings.py
index 2d0365e763..acfe5cad44 100644
--- a/dts/framework/settings.py
+++ b/dts/framework/settings.py
@@ -170,6 +170,15 @@  def _parse_revision_id(rev_id: str) -> str:
         )
 
 
+class ArgumentParser(argparse.ArgumentParser):
+    """ArgumentParser with a custom error message."""
+    def error(self, message):
+        print(f"{self.prog}: error: {message}\n", file=sys.stderr)
+        self.exit(2,
+                  "For help and usage, "
+                  "run the command with the --help flag.\n")
+
+
 @dataclass(slots=True)
 class Settings:
     """Default framework-wide user settings.
@@ -200,8 +209,8 @@  class Settings:
 SETTINGS: Settings = Settings()
 
 
-def _get_parser() -> argparse.ArgumentParser:
-    parser = argparse.ArgumentParser(
+def _get_parser() -> ArgumentParser:
+    parser = ArgumentParser(
         description="Run DPDK test suites. All options may be specified with the environment "
         "variables provided in brackets. Command line arguments have higher priority.",
         formatter_class=argparse.ArgumentDefaultsHelpFormatter,