emrun: use argparse in more conventional way (#12520)

* emrun: use argparse in more conventional way

This is not quite NFC since passing command line arguments
to the page that start with `-` now requires a separator.

This used to work:

./emrun filename.html --foo --bar

Where as now one needs to add `--` to signal the end of emrun's
argument parsing.

So either:

./emrun filename.html -- --foo --bar

or:

./emrun -- filename.html --foo --bar

* error_check

ChangeLog.md

@@ -31,6 +31,12 @@ Current Trunk
 - Reject promises returned from the factory function created by using the
   MODULARIZE build option if initialization of the module instance fails
   (#12396).
+- emrun: Passing command line flags (arguments that start with `-`) to the
+  program bring run now requires a `--` on the command line to signal the
+  end of `emrun` arguments. e.g:
+    `emrun filename.html -- --arg-for-page`
+  This is standard behaviour for command line parsing and simplifies the
+  emrun logic.
 
 2.0.7: 10/13/2020
 -----------------

emrun.py

@@ -1383,10 +1383,14 @@ def list_processes_by_name(exe_full_path):
 def run():
   global browser_process, browser_exe, processname_killed_atexit, emrun_options, emrun_not_enabled_nag_printed
   usage_str = """\
-emrun [emrun_options] filename.html [html_cmdline_options]
+emrun [emrun_options] filename.html -- [html_cmdline_options]
 
    where emrun_options specifies command line options for emrun itself, whereas
    html_cmdline_options specifies startup arguments to the program.
+
+If you are seeing "unrecognized arguments" when trying to pass
+arguments to your page, remember to add `--` between arguments
+to emrun itself and arguments to your page.
 """
   parser = argparse.ArgumentParser(usage=usage_str)
 
@@ -1401,7 +1405,7 @@ emrun [emrun_options] filename.html [html_cmdline_options]
                            '--browser=/path/to/browser, to avoid emrun being '
                            'detached from the browser process it spawns.')
 
-  parser.add_argument('--no_server',
+  parser.add_argument('--no_server', action='store_true',
                       help='If specified, a HTTP web server is not launched '
                            'to host the page to run.')
 
@@ -1496,28 +1500,11 @@ emrun [emrun_options] filename.html [html_cmdline_options]
   parser.add_argument('--private_browsing', action='store_true',
                       help='If specified, opens browser in private/incognito mode.')
 
-  parser.add_argument('serve', nargs='*')
+  parser.add_argument('serve', nargs='?', default='')
 
-  opts_with_param = ['--browser', '--browser_args', '--timeout_returncode',
-                     '--timeout', '--silence_timeout', '--log_stderr', '--log_stdout',
-                     '--hostname', '--port', '--serve_root']
+  parser.add_argument('cmdlineparams', nargs='*')
 
-  cmdlineparams = []
-  # Split the startup arguments to two parts, delimited by the first (unbound) positional argument.
-  # The first set is args intended for emrun, and the second set is the cmdline args to program.
-  i = 1
-  while i < len(sys.argv):
-    if sys.argv[i] in opts_with_param:
-      i += 1 # Skip next one, it's the value for this opt.
-    elif not sys.argv[i].startswith('-'):
-      cmdlineparams = sys.argv[i + 1:]
-      sys.argv = sys.argv[:i + 1]
-      break
-    i += 1
-
-  options = parser.parse_args(sys.argv[1:])
-  args = options.serve
-  emrun_options = options
+  options = emrun_options = parser.parse_args()
 
   if options.android:
     global ADB
@@ -1543,17 +1530,20 @@ emrun [emrun_options] filename.html [html_cmdline_options]
       list_pc_browsers()
     return
 
-  if len(args) < 1 and (options.system_info or options.browser_info):
+  if not options.serve and (options.system_info or options.browser_info):
     # Don't run if only --system_info or --browser_info was passed.
     options.no_server = options.no_browser = True
 
-  if len(args) < 1 and not (options.no_server and options.no_browser):
+  if not options.serve and not (options.no_server and options.no_browser):
     logi(usage_str)
     logi('')
     logi('Type emrun --help for a detailed list of available options.')
     return
 
-  file_to_serve = args[0] if len(args) else '.'
+  if options.serve:
+    file_to_serve = options.serve
+  else:
+    file_to_serve = '.'
   file_to_serve_is_url = file_to_serve.startswith('file://') or file_to_serve.startswith('http://') or file_to_serve.startswith('https://')
 
   if options.serve_root:
@@ -1570,8 +1560,8 @@ emrun [emrun_options] filename.html [html_cmdline_options]
     url = file_to_serve
   else:
     url = os.path.relpath(os.path.abspath(file_to_serve), serve_dir)
-    if len(cmdlineparams):
-      url += '?' + '&'.join(cmdlineparams)
+    if len(options.cmdlineparams):
+      url += '?' + '&'.join(options.cmdlineparams)
     hostname = socket.gethostbyname(socket.gethostname()) if options.android else options.hostname
     url = 'http://' + hostname + ':' + str(options.port) + '/' + url
 

tests/test_browser.py

@@ -2467,6 +2467,13 @@ void *getBindBuffer() {
                  '--kill_exit', '--port', '6939', '--verbose',
                  '--log_stdout', self.in_dir('stdout.txt'),
                  '--log_stderr', self.in_dir('stderr.txt')]
+
+    # Verify that trying to pass argument to the page without the `--` separator will
+    # generate an actionable error message
+    err = self.expect_fail(args_base + ['--foo'])
+    self.assertContained('error: unrecognized arguments: --foo', err)
+    self.assertContained('remember to add `--` between arguments', err)
+
     if EMTEST_BROWSER is not None:
       # If EMTEST_BROWSER carried command line arguments to pass to the browser,
       # (e.g. "firefox -profile /path/to/foo") those can't be passed via emrun,
@@ -2484,11 +2491,12 @@ void *getBindBuffer() {
           browser_args = parser.parse_known_args(browser_args)[1]
         if browser_args:
           args_base += ['--browser_args', ' ' + ' '.join(browser_args)]
+
     for args in [
         args_base,
         args_base + ['--private_browsing', '--port', '6941']
     ]:
-      args += [self.in_dir('hello_world.html'), '1', '2', '--3']
+      args += [self.in_dir('hello_world.html'), '--', '1', '2', '--3']
       print(shared.shlex_join(args))
       proc = self.run_process(args, check=False)
       self.assertEqual(proc.returncode, 100)