Fix compile status with help command (rescript-lang#6439)

* Fx compile status with help command

* Update changelog

* Update tests

* Improve cli consistency

* Finish cli help consistency improvements

* Update changelog

* Fix build help

Author: DZakh

CHANGELOG.md

@@ -28,6 +28,8 @@
 - Fix printing of exotic JSX names https://github.com/rescript-lang/rescript-compiler/pull/6451
 - Fix locations when code with `await` fails to compile (all locations would point to the internal function `unsafe_await`) https://github.com/rescript-lang/rescript-compiler/pull/6452
 - Fix renaming fields (with @as) in inline records doesn't work when destructuring https://github.com/rescript-lang/rescript-compiler/pull/6456
+- Fix `rc.4` regressions:
+  - Don't show compilation time when calling `rescript build -help` command. https://github.com/rescript-lang/rescript-compiler/pull/6439
 
 #### :house: Internal
 
@@ -37,6 +39,7 @@
 
 - Add [`Deno`](https://deno.land/api?s=Deno) to reserved names, so that modules named `Deno` don't clash with the globally exposed `Deno` object. https://github.com/rescript-lang/rescript-compiler/pull/6428
 - Disable ESLint/TSLint on gentype outputs properly. https://github.com/rescript-lang/rescript-compiler/pull/6442
+- Improve `rescript` CLI to use `stdout`/`stderr` appropriately for help command's message. https://github.com/rescript-lang/rescript-compiler/pull/6439
 
 # 11.0.0-rc.4
 

jscomp/bsb/bsb_arg.ml

@@ -79,13 +79,13 @@ 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)
 

jscomp/bsb_exe/rescript_main.ml

@@ -90,7 +90,7 @@ let clean_usage =
 let build_usage =
   "Usage: rescript build <options> -- <ninja_options>\n\n\
    `rescript build` builds the project with dependencies\n\n\
-   `rescript -- -h` for Ninja options (internal usage only; unstable)\n"
+   `rescript build -- -h` for Ninja options (internal usage only; unstable)\n"
 
 let install_target () =
   let ( // ) = Filename.concat in
@@ -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_compile_status/input.js

@@ -0,0 +1,43 @@
+// @ts-check
+
+const assert = require("assert");
+const child_process = require("child_process");
+
+// Shows compile time for `rescript build` command
+let out = child_process.spawnSync(`../../../rescript`, ["build"], {
+  encoding: "utf8",
+  cwd: __dirname,
+});
+assert.match(
+  out.stdout,
+  new RegExp(`>>>> Start compiling
+Dependency Finished
+>>>> Finish compiling \\d+ mseconds`)
+);
+
+// Shows compile time for `rescript` command
+out = child_process.spawnSync(`../../../rescript`, {
+  encoding: "utf8",
+  cwd: __dirname,
+});
+assert.match(
+  out.stdout,
+  new RegExp(`>>>> Start compiling
+Dependency Finished
+>>>> Finish compiling \\d+ mseconds`)
+);
+
+// Doesn't show compile time for `rescript build -verbose` command
+// Because we can't be sure that -verbose is a valid argument
+// And bsb won't fail with a usage message.
+// It works this way not only for -verbose, but any other arg, including -h/--help/-help
+out = child_process.spawnSync(`../../../rescript`, ["build", "-verbose"], {
+  encoding: "utf8",
+  cwd: __dirname,
+});
+assert.match(
+  out.stdout,
+  new RegExp(
+    `Package stack: test  \nDependency Finished\nninja.exe -C lib/bs \n`
+  )
+);

jscomp/build_tests/cli_compile_status/rescript.json

@@ -0,0 +1,5 @@
+{
+  "name": "test",
+  "version": "0.1.0",
+  "sources": []
+}

jscomp/build_tests/cli_help/input.js

@@ -0,0 +1,243 @@
+// @ts-check
+
+const assert = require("assert");
+const child_process = require("child_process");
+
+const cliHelp =
+  "Usage: rescript <options> <subcommand>\n" +
+  "\n" +
+  "`rescript` is equivalent to `rescript build`\n" +
+  "\n" +
+  "Options:\n" +
+  "  -v, -version  display version number\n" +
+  "  -h, -help     display help\n" +
+  "\n" +
+  "Subcommands:\n" +
+  "  build\n" +
+  "  clean\n" +
+  "  format\n" +
+  "  convert\n" +
+  "  dump\n" +
+  "  help\n" +
+  "\n" +
+  "Run `rescript <subcommand> -h` for subcommand help. Examples:\n" +
+  "  rescript build -h\n" +
+  "  rescript format -h\n";
+
+const buildHelp =
+  "Usage: rescript build <options> -- <ninja_options>\n" +
+  "\n" +
+  "`rescript build` builds the project with dependencies\n" +
+  "\n" +
+  "`rescript build -- -h` for Ninja options (internal usage only; unstable)\n" +
+  "\n" +
+  "Options:\n" +
+  "  -w          Watch mode\n" +
+  "  -ws         [host]:port set up host & port for WebSocket build notifications\n" +
+  "  -verbose    Set the output to be verbose\n" +
+  "  -with-deps  *deprecated* This is the default behavior now. This option will be removed in a future release\n";
+
+const cleanHelp =
+  "Usage: rescript clean <options>\n" +
+  "\n" +
+  "`rescript clean` cleans build artifacts\n" +
+  "\n" +
+  "Options:\n" +
+  "  -verbose    Set the output to be verbose\n" +
+  "  -with-deps  *deprecated* This is the default behavior now. This option will be removed in a future release\n";
+
+const formatHelp =
+  "Usage: rescript format <options> [files]\n" +
+  "\n" +
+  "`rescript format` formats the current directory\n" +
+  "\n" +
+  "Options:\n" +
+  "  -stdin  [.res|.resi|.ml|.mli] Read the code from stdin and print\n" +
+  "          the formatted code to stdout in ReScript syntax\n" +
+  "  -all    Format the whole project \n" +
+  "  -check  Check formatting for file or the whole project. Use `-all` to check the whole project\n";
+
+const convertHelp =
+  "Usage: rescript convert <options> [files]\n" +
+  "\n" +
+  "`rescript convert` converts the current directory\n" +
+  "\n" +
+  "**This command removes old OCaml files and creates new ReScript \n" +
+  "files. Make sure your work is saved first!**\n" +
+  "\n" +
+  "Options:\n" +
+  "  -all  Convert the whole project\n";
+
+const dumpHelp =
+  "Usage: rescript dump <options> [target]\n" +
+  "`rescript dump` dumps the information for the target\n";
+
+// Shows build help with --help arg
+let out = child_process.spawnSync(`../../../rescript`, ["build", "--help"], {
+  encoding: "utf8",
+  cwd: __dirname,
+});
+assert.equal(out.stdout, buildHelp);
+assert.equal(out.stderr, "");
+assert.equal(out.status, 0);
+
+// 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);
+
+// Shows build help with -h arg
+out = child_process.spawnSync(`../../../rescript`, ["build", "-h"], {
+  encoding: "utf8",
+  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.stdout, "");
+assert.equal(out.stderr, 'Error: Unknown option "-wtf".\n' + buildHelp);
+assert.equal(out.status, 2);
+
+// Shows cli help with --help arg
+out = child_process.spawnSync(`../../../rescript`, ["--help"], {
+  encoding: "utf8",
+  cwd: __dirname,
+});
+assert.equal(out.stdout, cliHelp);
+assert.equal(out.stderr, "");
+assert.equal(out.status, 0);
+
+// Shows cli help with -h arg
+out = child_process.spawnSync(`../../../rescript`, ["-h"], {
+  encoding: "utf8",
+  cwd: __dirname,
+});
+assert.equal(out.stdout, cliHelp);
+assert.equal(out.stderr, "");
+assert.equal(out.status, 0);
+
+// Shows cli help with help command
+out = child_process.spawnSync(`../../../rescript`, ["help"], {
+  encoding: "utf8",
+  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);
+
+// Shows cli help with unknown args
+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);
+
+// Shows clean help with --help arg
+out = child_process.spawnSync(`../../../rescript`, ["clean", "--help"], {
+  encoding: "utf8",
+  cwd: __dirname,
+});
+assert.equal(out.stdout, cleanHelp);
+assert.equal(out.stderr, "");
+assert.equal(out.status, 0);
+
+// Shows clean help with -h arg
+out = child_process.spawnSync(`../../../rescript`, ["clean", "-h"], {
+  encoding: "utf8",
+  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.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,
+});
+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,
+});
+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,
+});
+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,
+});
+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,
+});
+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,
+});
+assert.equal(out.stdout, dumpHelp);
+assert.equal(out.stderr, "");
+assert.equal(out.status, 0);

jscomp/ext/bsc_args.ml

@@ -86,13 +86,13 @@ 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)