Finish cli help consistency improvements

Author: DZakh

jscomp/bsb_exe/rescript_main.ml

@@ -234,6 +234,6 @@ let () =
         start.pos_lnum Ext_json_parse.report_error e;
       exit 2
   | Bsb_arg.Bad s | Sys_error s ->
-      Format.fprintf Format.err_formatter "@{<error>Error:@} %s@." s;
+      Format.fprintf Format.err_formatter "@{<error>Error:@} %s" s;
       exit 2
   | e -> Ext_pervasives.reraise e

jscomp/build_tests/cli_help/input.js

@@ -78,15 +78,23 @@ let out = child_process.spawnSync(`../../../rescript`, ["build", "--help"], {
   cwd: __dirname,
 });
 assert.equal(out.stdout, buildHelp);
+assert.equal(out.stderr, "");
 assert.equal(out.status, 0);
 
-// Shows build help with --help arg
+// FIXME: Help works incorrectly in watch mode
 out = child_process.spawnSync(`../../../rescript`, ["build", "-w", "--help"], {
   encoding: "utf8",
   cwd: __dirname,
 });
 // FIXME: Shouldn't have "Start compiling" for help
 assert.equal(out.stdout, ">>>> Start compiling\n" + buildHelp);
+// FIXME: Don't run the watcher when showing help
+assert.match(
+  out.stderr,
+  new RegExp(
+    "Uncaught Exception Error: ENOENT: no such file or directory, watch 'bsconfig.json'\n"
+  )
+);
 // FIXME: Should be 0
 assert.equal(out.status, 1);
 
@@ -96,14 +104,16 @@ out = child_process.spawnSync(`../../../rescript`, ["build", "-h"], {
   cwd: __dirname,
 });
 assert.equal(out.stdout, buildHelp);
+assert.equal(out.stderr, "");
 assert.equal(out.status, 0);
 
 // Exits with build help with unknown arg
 out = child_process.spawnSync(`../../../rescript`, ["build", "-wtf"], {
   encoding: "utf8",
   cwd: __dirname,
 });
-assert.equal(out.stderr, 'Error: Unknown option "-wtf".\n' + buildHelp + "\n");
+assert.equal(out.stdout, "");
+assert.equal(out.stderr, 'Error: Unknown option "-wtf".\n' + buildHelp);
 assert.equal(out.status, 2);
 
 // Shows cli help with --help arg
@@ -112,6 +122,7 @@ out = child_process.spawnSync(`../../../rescript`, ["--help"], {
   cwd: __dirname,
 });
 assert.equal(out.stdout, cliHelp);
+assert.equal(out.stderr, "");
 assert.equal(out.status, 0);
 
 // Shows cli help with -h arg
@@ -120,6 +131,7 @@ out = child_process.spawnSync(`../../../rescript`, ["-h"], {
   cwd: __dirname,
 });
 assert.equal(out.stdout, cliHelp);
+assert.equal(out.stderr, "");
 assert.equal(out.status, 0);
 
 // Shows cli help with help command
@@ -128,13 +140,15 @@ out = child_process.spawnSync(`../../../rescript`, ["help"], {
   cwd: __dirname,
 });
 assert.equal(out.stdout, cliHelp);
+assert.equal(out.stderr, "");
 assert.equal(out.status, 0);
 
 // Shows cli help with unknown command
 out = child_process.spawnSync(`../../../rescript`, ["built"], {
   encoding: "utf8",
   cwd: __dirname,
 });
+assert.equal(out.stdout, "");
 assert.equal(out.stderr, `Error: Unknown command or flag "built".\n` + cliHelp);
 assert.equal(out.status, 2);
 
@@ -143,6 +157,7 @@ out = child_process.spawnSync(`../../../rescript`, ["-w"], {
   encoding: "utf8",
   cwd: __dirname,
 });
+assert.equal(out.stdout, "");
 assert.equal(out.stderr, `Error: Unknown command or flag "-w".\n` + cliHelp);
 assert.equal(out.status, 2);
 
@@ -152,6 +167,7 @@ out = child_process.spawnSync(`../../../rescript`, ["clean", "--help"], {
   cwd: __dirname,
 });
 assert.equal(out.stdout, cleanHelp);
+assert.equal(out.stderr, "");
 assert.equal(out.status, 0);
 
 // Shows clean help with -h arg
@@ -160,66 +176,68 @@ out = child_process.spawnSync(`../../../rescript`, ["clean", "-h"], {
   cwd: __dirname,
 });
 assert.equal(out.stdout, cleanHelp);
+assert.equal(out.stderr, "");
 assert.equal(out.status, 0);
 
 // Exits with clean help with unknown arg
 out = child_process.spawnSync(`../../../rescript`, ["clean", "-wtf"], {
   encoding: "utf8",
   cwd: __dirname,
 });
-assert.equal(out.stderr, 'Error: Unknown option "-wtf".\n' + cleanHelp + "\n");
+assert.equal(out.stdout, "");
+assert.equal(out.stderr, 'Error: Unknown option "-wtf".\n' + cleanHelp);
 assert.equal(out.status, 2);
 
 // Shows format help with --help arg
 out = child_process.spawnSync(`../../../rescript`, ["format", "--help"], {
   encoding: "utf8",
   cwd: __dirname,
 });
-// Note: it writes to stderr
-assert.equal(out.stderr, formatHelp);
+assert.equal(out.stdout, formatHelp);
+assert.equal(out.stderr, "");
 assert.equal(out.status, 0);
 
 // Shows format help with -h arg
 out = child_process.spawnSync(`../../../rescript`, ["format", "-h"], {
   encoding: "utf8",
   cwd: __dirname,
 });
-// Note: it writes to stderr
-assert.equal(out.stderr, formatHelp);
+assert.equal(out.stdout, formatHelp);
+assert.equal(out.stderr, "");
 assert.equal(out.status, 0);
 
 // Shows convert help with --help arg
 out = child_process.spawnSync(`../../../rescript`, ["convert", "--help"], {
   encoding: "utf8",
   cwd: __dirname,
 });
-// Note: it writes to stderr
-assert.equal(out.stderr, convertHelp);
+assert.equal(out.stdout, convertHelp);
+assert.equal(out.stderr, "");
 assert.equal(out.status, 0);
 
 // Shows convert help with -h arg
 out = child_process.spawnSync(`../../../rescript`, ["convert", "-h"], {
   encoding: "utf8",
   cwd: __dirname,
 });
-// Note: it writes to stderr
-assert.equal(out.stderr, convertHelp);
+assert.equal(out.stdout, convertHelp);
+assert.equal(out.stderr, "");
 assert.equal(out.status, 0);
 
 // Shows dump help with --help arg
 out = child_process.spawnSync(`../../../rescript`, ["dump", "--help"], {
   encoding: "utf8",
   cwd: __dirname,
 });
-// Note: it writes to stderr
-assert.equal(out.stderr, dumpHelp);
+assert.equal(out.stdout, dumpHelp);
+assert.equal(out.stderr, "");
 assert.equal(out.status, 0);
 
 // Shows dump help with -h arg
 out = child_process.spawnSync(`../../../rescript`, ["dump", "-h"], {
   encoding: "utf8",
   cwd: __dirname,
 });
-// Note: it writes to stderr
-assert.equal(out.stderr, dumpHelp);
+assert.equal(out.stdout, dumpHelp);
+assert.equal(out.stderr, "");
 assert.equal(out.status, 0);

jscomp/ext/bsc_args.ml

@@ -86,16 +86,16 @@ let stop_raise ~usage ~(error : error) (speclist : t) =
       Ext_buffer.output_buffer stdout b;
       exit 0
   | Unknown s ->
-      b +> "unknown option: '";
+      b +> "Unknown option \"";
       b +> s;
-      b +> "'.\n"
+      b +> "\".\n"
   | Missing s ->
-      b +> "option '";
+      b +> "Option \"";
       b +> s;
-      b +> "' needs an argument.\n");
+      b +> "\" needs an argument.\n");
   usage_b b ~usage speclist;
   bad_arg (Ext_buffer.contents b)
-
+    
 let parse_exn ~usage ~argv ?(start = 1) ?(finish = Array.length argv)
     (speclist : t) (anonfun : rev_args:string list -> unit) =
   let current = ref start in

scripts/rescript_arg.js

@@ -84,13 +84,13 @@ function stop_raise(usage, error, specs) {
     case "Unknown":
       if (["-help", "--help", "-h"].includes(error.data)) {
         usage_b(b, usage, specs);
-        process.stderr.write(b.val);
+        process.stdout.write(b.val);
         process.exit(0);
       } else {
-        b.add("unknown option: '").add(error.data).add("'.\n");
+        b.add(`Unknown option "${error.data}".\n'`);
       }
     case "Missing":
-      b.add("option '").add(error.data).add("' needs an argument.\n");
+      b.add(`Option "${error.data}" needs an argument.\n'`);
   }
   usage_b(b, usage, specs);
   bad_arg(b.val);