Refine error message output for `mruby` command

- Write message to stderr instead of stdout.
- Avoid duplicate message output (`SyntaxError`, `ScriptError` etc).
- Refine invalid option message.
- Suppress redundant usage output.
- Fix some incorrect exit code.

2 files changed, 60 insertions, 30 deletions

diff --git a/mrbgems/mruby-bin-mruby/bintest/mruby.rb b/mrbgems/mruby-bin-mruby/bintest/mruby.rb
index e8d1510f7..1dced07f9 100644
--- a/mrbgems/mruby-bin-mruby/bintest/mruby.rb
+++ b/mrbgems/mruby-bin-mruby/bintest/mruby.rb
@@ -1,10 +1,16 @@
 require 'tempfile'
+require 'open3'
+
+assert_mruby = ->(exp_out_pattern, exp_err_pattern, exp_success, args) do
+  out, err, stat = Open3.capture3(cmd("mruby"), *args)
+  assert_match(exp_out_pattern, out, "standard output")
+  assert_match(exp_err_pattern, err, "standard error")
+  assert_equal(exp_success, stat.success?, "exit success?")
+end
 
 assert('regression for #1564') do
-  o = `#{cmd('mruby')} -e #{shellquote('<<')} 2>&1`
-  assert_include o, "-e:1:2: syntax error"
-  o = `#{cmd('mruby')} -e #{shellquote('<<-')} 2>&1`
-  assert_include o, "-e:1:3: syntax error"
+  assert_mruby.("", "-e:1:2: syntax error, *", false, %w[-e <<])
+  assert_mruby.("", "-e:1:3: syntax error, *", false, %w[-e <<-])
 end
 
 assert('regression for #1572') do
@@ -66,6 +72,11 @@ RUBY
   assert_equal 0, $?.exitstatus
 end
 
+assert('mruby -c option') do
+  assert_mruby.("Syntax OK\n", "", true, ["-c", "-e", "p 1"])
+  assert_mruby.("", "-e:1:7: syntax error, *", false, ["-c", "-e", "p 1; 1."])
+end
+
 assert('mruby -d option') do
   o = `#{cmd('mruby')} -e #{shellquote('p $DEBUG')}`
   assert_equal "false\n", o
@@ -73,6 +84,14 @@ assert('mruby -d option') do
   assert_equal "true\n", o
 end
 
+assert('mruby -e option (no code specified)') do
+  assert_mruby.("", "* No code specified for -e\n", false, %w[-e])
+end
+
+assert('mruby -h option') do
+  assert_mruby.("Usage: *mruby*", "", true, %w[-h])
+end
+
 assert('mruby -r option') do
   lib = Tempfile.new('lib.rb')
   lib.write <<EOS
@@ -95,3 +114,27 @@ EOS
   assert_equal 'hogeClass', `#{cmd('mruby')} -r #{lib.path} -r #{script.path} -e #{shellquote('print Hoge.class')}`
   assert_equal 0, $?.exitstatus
 end
+
+assert('mruby -r option (no library specified)') do
+  assert_mruby.("", "*: No library specified for -r\n", false, %w[-r])
+end
+
+assert('mruby -r option (file not found)') do
+  assert_mruby.("", "*: Cannot open library file: *", false, %w[-r _no_exists_])
+end
+
+assert('mruby invalid short option') do
+  assert_mruby.("", "*: invalid option -1 *", false, %w[-1])
+end
+
+assert('mruby invalid long option') do
+  assert_mruby.("", "*: invalid option --longopt *", false, %w[--longopt])
+end
+
+assert('unhandled exception') do
+  assert_mruby.("", "* EXCEPTION!*", false, %w[-e raise("EXCEPTION!")])
+end
+
+assert('program file not found') do
+  assert_mruby.("", "*: Cannot open program file*", false, %w[_no_exists_])
+end
diff --git a/mrbgems/mruby-bin-mruby/tools/mruby/mruby.c b/mrbgems/mruby-bin-mruby/tools/mruby/mruby.c
index 498bedef2..c96365c9f 100644
--- a/mrbgems/mruby-bin-mruby/tools/mruby/mruby.c
+++ b/mrbgems/mruby-bin-mruby/tools/mruby/mruby.c
@@ -7,19 +7,6 @@
 #include <mruby/dump.h>
 #include <mruby/variable.h>
 
-#ifdef MRB_DISABLE_STDIO
-static void
-p(mrb_state *mrb, mrb_value obj)
-{
-  mrb_value val = mrb_inspect(mrb, obj);
-
-  fwrite(RSTRING_PTR(val), RSTRING_LEN(val), 1, stdout);
-  putc('\n', stdout);
-}
-#else
-#define p(mrb,obj) mrb_p(mrb,obj)
-#endif
-
 struct _args {
   FILE *rfp;
   char* cmdline;
@@ -119,14 +106,17 @@ append_cmdline:
         }
       }
       else {
-        printf("%s: No code specified for -e\n", *origargv);
-        return EXIT_SUCCESS;
+        fprintf(stderr, "%s: No code specified for -e\n", *origargv);
+        return EXIT_FAILURE;
       }
       break;
+    case 'h':
+      usage(*origargv);
+      exit(EXIT_SUCCESS);
     case 'r':
       if (!item[0]) {
         if (argc <= 1) {
-          printf("%s: No library specified for -r\n", *origargv);
+          fprintf(stderr, "%s: No library specified for -r\n", *origargv);
           return EXIT_FAILURE;
         }
         argc--; argv++;
@@ -158,6 +148,7 @@ append_cmdline:
         exit(EXIT_SUCCESS);
       }
     default:
+      fprintf(stderr, "%s: invalid option %s (-h will show valid options)\n", *origargv, *argv);
       return EXIT_FAILURE;
     }
   }
@@ -167,7 +158,7 @@ append_cmdline:
     else {
       args->rfp = fopen(argv[0], args->mrbfile ? "rb" : "r");
       if (args->rfp == NULL) {
-        printf("%s: Cannot open program file. (%s)\n", *origargv, *argv);
+        fprintf(stderr, "%s: Cannot open program file. (%s)\n", *origargv, *argv);
         return EXIT_FAILURE;
       }
       args->fname = TRUE;
@@ -212,14 +203,13 @@ main(int argc, char **argv)
   mrb_sym zero_sym;
 
   if (mrb == NULL) {
-    fputs("Invalid mrb_state, exiting mruby\n", stderr);
+    fprintf(stderr, "%s: Invalid mrb_state, exiting mruby\n", *argv);
     return EXIT_FAILURE;
   }
 
   n = parse_args(mrb, argc, argv, &args);
   if (n == EXIT_FAILURE || (args.cmdline == NULL && args.rfp == NULL)) {
     cleanup(mrb, &args);
-    usage(argv[0]);
     return n;
   }
   else {
@@ -258,7 +248,7 @@ main(int argc, char **argv)
     for (i = 0; i < args.libc; i++) {
       FILE *lfp = fopen(args.libv[i], args.mrbfile ? "rb" : "r");
       if (lfp == NULL) {
-        printf("Cannot open library file: %s\n", args.libv[i]);
+        fprintf(stderr, "%s: Cannot open library file: %s\n", *argv, args.libv[i]);
         mrbc_context_free(mrb, c);
         cleanup(mrb, &args);
         return EXIT_FAILURE;
@@ -289,13 +279,10 @@ main(int argc, char **argv)
     mrb_gc_arena_restore(mrb, ai);
     mrbc_context_free(mrb, c);
     if (mrb->exc) {
-      if (mrb_undef_p(v)) {
-        mrb_p(mrb, mrb_obj_value(mrb->exc));
-      }
-      else {
+      if (!mrb_undef_p(v)) {
         mrb_print_error(mrb);
       }
-      n = -1;
+      n = EXIT_FAILURE;
     }
     else if (args.check_syntax) {
       printf("Syntax OK\n");
@@ -303,5 +290,5 @@ main(int argc, char **argv)
   }
   cleanup(mrb, &args);
 
-  return n == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
+  return n;
 }