[PATCH 09/12] rteval: Improve argparse implementation and remove manual sys.argv parsing

[John Kacur <jkacur@redhat.com>]

This commit modernizes the argument parsing by using argparse properly
and eliminating fragile manual sys.argv manipulation.

Key improvements:

1. Use nargs='+' for -Z/--summarize and -H/--raw-histogram options
   - These now properly accept one or more XML files as arguments
   - Removed complex manual argument splitting logic (lines 158-174)
   - Help text now correctly shows: -Z XMLFILE [XMLFILE ...]

2. Use nargs='?' with const='' for -S/--source-download option
   - Simplified handling of optional kernel version argument
   - Removed manual sys.argv.count() checks (line 179)
   - Default value provided when flag is present without argument

3. Use parse_known_args() for early argument detection
   - Config file (-f) detection now uses proper argparse
   - Logging flags (-v/-D/-q) detection now uses proper argparse
   - Removed all manual sys.argv indexing and counting

4. Return cmd_opts instead of cmd_args from parse_options()
   - File arguments now directly accessible via cmd_opts attributes
   - Clearer separation between options and file arguments

5. Improved error handling
   - argparse provides better error messages for malformed arguments
   - Supports both -Z file1 file2 and --summarize=file1 formats

This eliminates 40+ lines of fragile manual parsing code and makes
the argument handling more robust and maintainable.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: John Kacur <jkacur@redhat.com>
---
 rteval-cmd | 93 +++++++++++++++++++++++-------------------------------
 1 file changed, 39 insertions(+), 54 deletions(-)

diff --git a/rteval-cmd b/rteval-cmd
index 8fd37b98b069..00873ce1196b 100755
--- a/rteval-cmd
+++ b/rteval-cmd
@@ -124,11 +124,11 @@ def parse_options(cfg, parser, cmdargs):
                       action='store_true', default=rtevcfg.debugging,
                       help=f'turn on debug prints (default: {rtevcfg.debugging})')
     parser.add_argument("-Z", '--summarize', dest='rteval___summarize',
-                      action='store_true', default=False,
-                      help='summarize an already existing XML report')
+                      nargs='+', default=None, metavar='XMLFILE',
+                      help='summarize one or more already existing XML reports')
     parser.add_argument("-H", '--raw-histogram', dest='rteval___rawhistogram',
-                      action='store_true', default=False,
-                      help='Generate raw histogram data for an already existing XML report')
+                      nargs='+', default=None, metavar='XMLFILE',
+                      help='Generate raw histogram data for one or more already existing XML reports')
     parser.add_argument("-f", "--inifile", dest="rteval___inifile",
                       type=str, default=None, metavar="FILE",
                       help="initialization file for configuring loads and behavior")
@@ -144,43 +144,19 @@ def parse_options(cfg, parser, cmdargs):
     parser.add_argument("-V", "--version", dest="rteval___version",
                       action='store_true', default=False,
                       help='print rteval version and exit')
-    parser.add_argument("-S", "--source-download", nargs="*", dest="rteval___srcdownload",
-                        type=str, default=None, metavar="KERNEL_VERSION",
+    parser.add_argument("-S", "--source-download", nargs="?", dest="rteval___srcdownload",
+                        type=str, default=None, const='', metavar="KERNEL_VERSION",
                         help='download a source kernel from kernel.org and exit')
     parser.add_argument("--noload", dest="rteval___noload",
                         action="store_true", default=False,
                         help="only run the measurements (don't run loads)")
 
-
-    if not cmdargs:
-        cmdargs = ["--help"]
-
-    # if -Z/--summarize is specified, add the files to be summarized to cmd_args, and add -Z to cmd_opts
-    cmd_args = []
-    if (sys.argv.count('-Z')+sys.argv.count('--summarize')) > 0:
-        try:
-            ind = cmdargs.index('-Z')
-        except ValueError:
-            ind = cmdargs.index('--summarize')
-        cmd_args = cmdargs[ind+1:]
-        cmdargs = cmdargs[:ind+1]
-    # if -H/--raw-histogram is specified, add the files to be summarized to cmd_args, and add -H to cmd_opts
-    elif (sys.argv.count('-H')+sys.argv.count('--raw-histogram')) > 0:
-        try:
-            ind = cmdargs.index('-H')
-        except ValueError:
-            ind = cmdargs.index('--raw-histogram')
-        cmd_args = cmdargs[ind+1:]
-        cmdargs = cmdargs[:ind+1]
-
     cmd_opts = parser.parse_args(args=cmdargs)
 
     # if no kernel version was provided for --source-download, set version to default
-    if (sys.argv.count('-S')+sys.argv.count('--source-download')) > 0:
-        if cmd_opts.rteval___srcdownload == []:
+    if cmd_opts.rteval___srcdownload is not None:
+        if cmd_opts.rteval___srcdownload == '':
             cmd_opts.rteval___srcdownload = ModuleParameters()["source"]["default"].replace(".tar.xz", "")
-        else:
-            cmd_opts.rteval___srcdownload = cmd_opts.rteval___srcdownload[0]
 
     if cmd_opts.rteval___version:
         print(f"rteval version {RTEVAL_VERSION}")
@@ -205,7 +181,7 @@ def parse_options(cfg, parser, cmdargs):
     # Update the config object with the parsed arguments
     cfg.UpdateFromOptionParser(cmd_opts)
 
-    return cmd_args
+    return cmd_opts
 
 if __name__ == '__main__':
     from rteval.sysinfo import dmi
@@ -225,13 +201,14 @@ if __name__ == '__main__':
 
         # Before really parsing options, see if we have been given a config file in the args
         # and load it - just so that default values are according to the config file
-        try:
-            cfgfile = sys.argv[sys.argv.index('-f')+1]
-            config.Load(cfgfile)
-        except IndexError:
-            # Missing file argument
-            raise RuntimeError('The -f option requires a file name to the configuration file')
-        except ValueError:
+        # Use a minimal parser to extract just the -f option
+        config_parser = argparse.ArgumentParser(add_help=False)
+        config_parser.add_argument('-f', '--inifile', dest='inifile')
+        early_args, _ = config_parser.parse_known_args()
+
+        if early_args.inifile:
+            config.Load(early_args.inifile)
+        else:
             # No configuration file given, load defaults
             config.Load()
 
@@ -247,20 +224,29 @@ if __name__ == '__main__':
                 'sysstat' : 'module'})
 
         # Prepare log levels before loading modules, not to have unwanted log messages
+        # Use a minimal parser to extract logging-related flags early
         rtevcfg = config.GetSection('rteval')
-        if (sys.argv.count('-v')+sys.argv.count('--verbose')) > 0:
+        log_parser = argparse.ArgumentParser(add_help=False)
+        log_parser.add_argument('-v', '--verbose', action='store_true')
+        log_parser.add_argument('-D', '--debug', action='store_true')
+        log_parser.add_argument('-q', '--quiet', action='store_true')
+        log_parser.add_argument('--idle-set', action='store_true')
+        early_args, _ = log_parser.parse_known_args()
+
+        if early_args.verbose:
             rtevcfg.verbose = True
-        if (sys.argv.count('-D')+sys.argv.count('--debug')) > 0:
+        if early_args.debug:
             rtevcfg.debugging = True
-        if (sys.argv.count('-q')+sys.argv.count('--quiet')) > 0:
+        if early_args.quiet:
             rtevcfg.quiet = True
+
         loglev = (not rtevcfg.quiet and (Log.ERR | Log.WARN)) \
                 | (rtevcfg.verbose and Log.INFO) \
                 | (rtevcfg.debugging and Log.DEBUG)
         logger.SetLogVerbosity(loglev)
 
         # check if cpupower is being used
-        if sys.argv.count('--idle-set') > 0:
+        if early_args.idle_set:
             rtevcfg.update({'usingCpupower': True})
 
         # Load modules
@@ -271,7 +257,7 @@ if __name__ == '__main__':
         parser = argparse.ArgumentParser()
         loadmods.SetupModuleOptions(parser)
         measuremods.SetupModuleOptions(parser)
-        cmd_args = parse_options(config, parser, sys.argv[1:])
+        cmd_opts = parse_options(config, parser, sys.argv[1:])
 
         if rtevcfg.noload:
             if rtevcfg.onlyload:
@@ -371,16 +357,15 @@ if __name__ == '__main__':
         logger.log(Log.DEBUG, f"workdir: {rtevcfg.workdir}")
 
         # if --summarize was specified then just parse the XML, print it and exit
-        if rtevcfg.summarize or rtevcfg.rawhistogram:
-            if len(cmd_args) < 1:
-                raise RuntimeError("Must specify at least one XML file with --summarize!")
-
-            for x in cmd_args:
-                if rtevcfg.summarize:
-                    summarize(x, rtevcfg.xslt_report)
-                elif rtevcfg.rawhistogram:
-                    summarize(x, rtevcfg.xslt_histogram)
+        if cmd_opts.rteval___summarize:
+            for xmlfile in cmd_opts.rteval___summarize:
+                summarize(xmlfile, rtevcfg.xslt_report)
+            sys.exit(0)
 
+        # if --raw-histogram was specified then just parse the XML, print it and exit
+        if cmd_opts.rteval___rawhistogram:
+            for xmlfile in cmd_opts.rteval___rawhistogram:
+                summarize(xmlfile, rtevcfg.xslt_histogram)
             sys.exit(0)
 
         if os.getuid() != 0:
-- 
2.51.1