diff --git a/CHANGELOG.md b/CHANGELOG.md index 806e17c452..034f1b6daf 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/jscomp/bsb/bsb_arg.ml b/jscomp/bsb/bsb_arg.ml index 13c383b485..2e5e4cf272 100644 --- a/jscomp/bsb/bsb_arg.ml +++ b/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) diff --git a/jscomp/bsb_exe/rescript_main.ml b/jscomp/bsb_exe/rescript_main.ml index b9050b5700..c8474ee48f 100644 --- a/jscomp/bsb_exe/rescript_main.ml +++ b/jscomp/bsb_exe/rescript_main.ml @@ -90,7 +90,7 @@ let clean_usage = let build_usage = "Usage: rescript build -- \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:@} %s@." s; + Format.fprintf Format.err_formatter "@{Error:@} %s" s; exit 2 | e -> Ext_pervasives.reraise e diff --git a/jscomp/build_tests/cli_compile_status/input.js b/jscomp/build_tests/cli_compile_status/input.js new file mode 100755 index 0000000000..be04df447f --- /dev/null +++ b/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` + ) +); diff --git a/jscomp/build_tests/cli_compile_status/rescript.json b/jscomp/build_tests/cli_compile_status/rescript.json new file mode 100644 index 0000000000..c7c9717c60 --- /dev/null +++ b/jscomp/build_tests/cli_compile_status/rescript.json @@ -0,0 +1,5 @@ +{ + "name": "test", + "version": "0.1.0", + "sources": [] +} diff --git a/jscomp/build_tests/cli_help/input.js b/jscomp/build_tests/cli_help/input.js new file mode 100755 index 0000000000..2c38947612 --- /dev/null +++ b/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 \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 -h` for subcommand help. Examples:\n" + + " rescript build -h\n" + + " rescript format -h\n"; + +const buildHelp = + "Usage: rescript build -- \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 \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 [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 [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 [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); diff --git a/jscomp/ext/bsc_args.ml b/jscomp/ext/bsc_args.ml index 721c191c6b..5f1fd2e19c 100644 --- a/jscomp/ext/bsc_args.ml +++ b/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) diff --git a/rescript b/rescript index 23fe253b46..baee8efdf1 100755 --- a/rescript +++ b/rescript @@ -68,8 +68,8 @@ if (process.env.NINJA_ANSI_FORCED === undefined) { } else { dlog(`NINJA_ANSI_FORCED: "${process.env.NINJA_ANSI_FORCED}"`); } -function help() { - console.log(`Usage: rescript + +const helpMessage = `Usage: rescript \`rescript\` is equivalent to \`rescript build\` @@ -87,10 +87,7 @@ Subcommands: Run \`rescript -h\` for subcommand help. Examples: rescript build -h - rescript format -h -The default \`rescript\` is equivalent to \`rescript build\` subcommand -`); -} + rescript format -h`; var isBuilding = false; function releaseBuild() { @@ -512,59 +509,63 @@ Please pick a different one using the \`-ws [host:]port\` flag from bsb.`); buildFinishedCallback(0); }); } else { - switch (maybeSubcommand) { - case "info": - case "clean": { - delegateCommand(process_argv.slice(2)); - break; - } - case undefined: - case "build": { - logStartCompiling(); - delegateCommand(process_argv.slice(2), logFinishCompiling); - break; + // We want to show the compile time for build + // But bsb might show a help message in some cases + // We don't want to show the compile time in the case + // But we can only be sure about that when building without any additional args + const isDefinitelyBuild = + maybeSubcommand === undefined || + (maybeSubcommand === "build" && process_argv.length === 3); + + if (isDefinitelyBuild) { + logStartCompiling(); + delegateCommand(process_argv.slice(2), logFinishCompiling); + } else { + switch (maybeSubcommand) { + case "info": + case "clean": + case "build": { + delegateCommand(process_argv.slice(2)); + break; + } + case "format": + require("./scripts/rescript_format.js").main( + process.argv.slice(3), + rescript_exe, + bsc_exe + ); + break; + case "dump": + require("./scripts/rescript_dump.js").main( + process.argv.slice(3), + rescript_exe, + bsc_exe + ); + break; + case "convert": + require("./scripts/rescript_convert.js").main( + process.argv.slice(3), + rescript_exe, + bsc_exe + ); + break; + case "-h": + case "-help": + case "--help": + case "help": + console.log(helpMessage); + break; + case "-v": + case "-version": + case "--version": + case "version": + console.log(require("./package.json").version); + break; + default: + console.error( + `Error: Unknown command or flag "${maybeSubcommand}".\n${helpMessage}` + ); + process.exit(2); } - case "format": - require("./scripts/rescript_format.js").main( - process.argv.slice(3), - rescript_exe, - bsc_exe - ); - break; - case "dump": - require("./scripts/rescript_dump.js").main( - process.argv.slice(3), - rescript_exe, - bsc_exe - ); - break; - case "dump": - require("./scripts/rescript_dump.js").main( - process.argv.slice(3), - rescript_exe, - bsc_exe - ); - break; - case "convert": - // Todo - require("./scripts/rescript_convert.js").main( - process.argv.slice(3), - rescript_exe, - bsc_exe - ); - break; - case "-h": - case "-help": - case "help": - help(); - break; - case "-v": - case "-version": - console.log(require("./package.json").version); - break; - default: - console.error(`Unknown subcommand or flags: ${maybeSubcommand}`); - help(); - process.exit(2); } } diff --git a/scripts/rescript_arg.js b/scripts/rescript_arg.js index 86454ae612..3cbd5373d7 100644 --- a/scripts/rescript_arg.js +++ b/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);