[gjs] console: Process arguments better

[Philip Chimento <pchimento src gnome org>]

commit 7ea4903876659f9e65e85adda18b62970a7d2880
Author: Philip Chimento <philip endlessm com>
Date:   Wed Oct 19 17:13:41 2016 -0700

    console: Process arguments better
    
    We now process our command line arguments as follows: all arguments
    before a program name or a -c or --command argument are passed to the GJS
    interpreter itself. All arguments after the program are passed into the
    program's ARGV array.
    
    This solves some weirdness where "gjs -c '...' --help" was showing the
    GJS help instead of passing the --help option onto the program given with
    -c, as expected.
    
    This corresponds to the way Python processes its command line arguments,
    and also opens up the possibility of --version or other arguments that
    might want to be accepted both by GJS and a script.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=772033

 Makefile-test.am        |    4 ++
 NEWS                    |   18 ++++++++++
 gjs/console.cpp         |   84 ++++++++++++++++++++++++++++++----------------
 test/testCommandLine.sh |   64 +++++++++++++++++++++++++++++++++++
 4 files changed, 141 insertions(+), 29 deletions(-)
---
diff --git a/Makefile-test.am b/Makefile-test.am
index 06e6400..16cdc13 100644
--- a/Makefile-test.am
+++ b/Makefile-test.am
@@ -128,6 +128,10 @@ TESTS_ENVIRONMENT =                                                        \
        G_FILENAME_ENCODING=latin1      # ensure filenames are not utf8 \
        DBUS_UUIDGEN="$(DBUS_UUIDGEN)"
 
+simple_tests = test/testCommandLine.sh
+EXTRA_DIST += $(simple_tests)
+TESTS = $(simple_tests)
+
 if CODE_COVERAGE_ENABLED
 TESTS_ENVIRONMENT += \
        GJS_UNIT_COVERAGE_OUTPUT=lcov \
diff --git a/NEWS b/NEWS
index f24773e..79cebca 100644
--- a/NEWS
+++ b/NEWS
@@ -32,6 +32,24 @@ NEXT
   gjs> a[1]
   0
 
+- Backwards-incompatible change: we have changed the way gjs-console interprets
+  command-line arguments. Previously there was a heuristic to try to decide
+  whether "--help" given on the command line was meant for GJS itself or for a
+  script being launched. This did not work in some cases, for example:
+
+  $ gjs -c 'if (ARGV.indexOf("--help") == -1) throw "ugh"' --help
+
+  would print the GJS help.
+
+  Now, all arguments meant for GJS itself must come _before_ the name of the
+  script to execute or any script given with a "-c" argument. Any arguments
+  _after_ the filename or script are passed on to the script. This is the way
+  that Python handles its command line arguments as well.
+
+  If you previously relied on any -I arguments after your script being added to
+  the search path, then you should either reorder those arguments, or handle -I
+  inside your script, adding paths to imports.searchPath manually.
+
 Version 1.46.0
 --------------
 
diff --git a/gjs/console.cpp b/gjs/console.cpp
index f32954f..917f9fb 100644
--- a/gjs/console.cpp
+++ b/gjs/console.cpp
@@ -45,18 +45,17 @@ static GOptionEntry entries[] = {
     { NULL }
 };
 
-G_GNUC_NORETURN
-static void
-print_help (GOptionContext *context,
-            bool            main_help)
+static char **
+strndupv(int n, char **strv)
 {
-  gchar *help;
-
-  help = g_option_context_get_help (context, main_help, NULL);
-  g_print ("%s", help);
-  g_free (help);
-
-  exit (0);
+    int ix;
+    if (n == 0)
+        return NULL;
+    char **retval = g_new(char *, n + 1);
+    for (ix = 0; ix < n; ix++)
+        retval[ix] = g_strdup(strv[ix]);
+    retval[n] = NULL;
+    return retval;
 }
 
 int
@@ -70,26 +69,52 @@ main(int argc, char **argv)
     const char *filename;
     const char *program_name;
     gsize len;
-    int code;
+    int code, argc_copy = argc, gjs_argc = argc, script_argc, ix;
+    char **argv_copy = g_strdupv(argv), **argv_copy_addr = argv_copy;
+    char **gjs_argv, **gjs_argv_addr;
+    char * const *script_argv;
 
     setlocale(LC_ALL, "");
 
     context = g_option_context_new(NULL);
 
-    /* pass unknown through to the JS script */
     g_option_context_set_ignore_unknown_options(context, true);
     g_option_context_set_help_enabled(context, false);
 
     g_option_context_add_main_entries(context, entries, NULL);
-    if (!g_option_context_parse(context, &argc, &argv, &error))
+    if (!g_option_context_parse(context, &argc_copy, &argv_copy, &error))
         g_error("option parsing failed: %s", error->message);
 
-    if (argc >= 2) {
-        if (strcmp(argv[1], "-h") == 0 || strcmp(argv[1], "--help") == 0)
-            print_help(context, true);
-        else if (strcmp(argv[1], "--help-all") == 0)
-            print_help(context, false);
+    /* Split options so we pass unknown ones through to the JS script */
+    for (ix = 1; ix < argc; ix++) {
+        /* Check if a file was given and split after it */
+        if (argc_copy >= 2 && strcmp(argv[ix], argv_copy[1]) == 0) {
+            /* Filename given; split after this argument */
+            gjs_argc = ix + 1;
+            break;
+        }
+
+        /* Check if -c or --command was given and split after following arg */
+        if (command != NULL &&
+            (strcmp(argv[ix], "-c") == 0 || strcmp(argv[ix], "--command") == 0)) {
+            gjs_argc = ix + 2;
+            break;
+        }
     }
+    gjs_argv_addr = gjs_argv = strndupv(gjs_argc, argv);
+    script_argc = argc - gjs_argc;
+    script_argv = argv + gjs_argc;
+    g_strfreev(argv_copy_addr);
+
+    /* Parse again, only the GJS options this time */
+    include_path = NULL;
+    coverage_prefixes = NULL;
+    coverage_output_path = NULL;
+    command = NULL;
+    g_option_context_set_ignore_unknown_options(context, false);
+    g_option_context_set_help_enabled(context, true);
+    if (!g_option_context_parse(context, &gjs_argc, &gjs_argv, &error))
+        g_error("option parsing failed: %s", error->message);
 
     g_option_context_free (context);
 
@@ -97,22 +122,22 @@ main(int argc, char **argv)
         script = command;
         len = strlen(script);
         filename = "<command line>";
-        program_name = argv[0];
-    } else if (argc <= 1) {
+        program_name = gjs_argv[0];
+    } else if (gjs_argc == 1) {
         script = g_strdup("const Console = imports.console; Console.interact();");
         len = strlen(script);
         filename = "<stdin>";
-        program_name = argv[0];
-    } else /*if (argc >= 2)*/ {
+        program_name = gjs_argv[0];
+    } else {
+        /* All unprocessed options should be in script_argv */
+        g_assert(gjs_argc == 2);
         error = NULL;
-        if (!g_file_get_contents(argv[1], &script, &len, &error)) {
+        if (!g_file_get_contents(gjs_argv[1], &script, &len, &error)) {
             g_printerr("%s\n", error->message);
             exit(1);
         }
-        filename = argv[1];
-        program_name = argv[1];
-        argc--;
-        argv++;
+        filename = gjs_argv[1];
+        program_name = gjs_argv[1];
     }
 
     js_context = (GjsContext*) g_object_new(GJS_TYPE_CONTEXT,
@@ -135,7 +160,7 @@ main(int argc, char **argv)
 
     /* prepare command line arguments */
     if (!gjs_context_define_string_array(js_context, "ARGV",
-                                         argc - 1, (const char**)argv + 1,
+                                         script_argc, (const char **) script_argv,
                                          &error)) {
         code = 1;
         g_printerr("Failed to defined ARGV: %s", error->message);
@@ -153,6 +178,7 @@ main(int argc, char **argv)
     }
 
  out:
+    g_strfreev(gjs_argv_addr);
 
     /* Probably doesn't make sense to write statistics on failure */
     if (coverage && code == 0)
diff --git a/test/testCommandLine.sh b/test/testCommandLine.sh
new file mode 100755
index 0000000..ddb31ec
--- /dev/null
+++ b/test/testCommandLine.sh
@@ -0,0 +1,64 @@
+#!/bin/sh -e
+
+gjs="$TOP_BUILDDIR"/gjs-console
+
+# this JS script fails if either 1) --help is not passed to it, or 2) the string
+# "sentinel" is not in its search path
+cat <<EOF >help.js
+const System = imports.system;
+if (imports.searchPath.indexOf('sentinel') == -1)
+    System.exit(1);
+if (ARGV.indexOf('--help') == -1)
+    System.exit(1);
+System.exit(0);
+EOF
+
+fail () {
+    >&2 echo "FAIL: $1"
+    exit 1
+}
+
+# gjs --help prints GJS help
+"$gjs" --help >/dev/null || \
+    fail "--help should succeed"
+test -n "`"$gjs" --help`" || \
+    fail "--help should print something"
+
+# print GJS help even if it's not the first argument
+"$gjs" -I . --help >/dev/null || \
+    fail "should succeed when --help is not first arg"
+test -n "`"$gjs" -I . --help`" || \
+    fail "should print something when --help is not first arg"
+
+# --help before a script file name prints GJS help
+"$gjs" --help help.js >/dev/null || \
+    fail "--help should succeed before a script file"
+test -n "`"$gjs" --help help.js`" || \
+    fail "--help should print something before a script file"
+
+# --help before a -c argument prints GJS help
+script='imports.system.exit(1)'
+"$gjs" --help -c "$script" >/dev/null || \
+    fail "--help should succeed before -c"
+test -n "`"$gjs" --help -c "$script"`" || \
+    fail "--help should print something before -c"
+
+# --help after a script file name is passed to the script
+"$gjs" -I sentinel help.js --help || \
+    fail "--help after script file should be passed to script"
+test -z "`"$gjs" -I sentinel help.js --help`" || \
+    fail "--help after script file should not print anything"
+
+# --help after a -c argument is passed to the script
+script='if(ARGV[0] !== "--help") imports.system.exit(1)'
+"$gjs" -c "$script" --help || \
+    fail "--help after -c should be passed to script"
+test -z "`"$gjs" -c "$script" --help`" || \
+    fail "--help after -c should not print anything"
+
+# -I after a program is not consumed by GJS
+if "$gjs" help.js --help -I sentinel; then
+    fail "-I after script file should not be added to search path"
+fi
+
+rm -f help.js