From c6de8a7b51cef0e7e2f9d6e3cdbc10aa3a9a73a0 Mon Sep 17 00:00:00 2001 From: Dmitry Zakharov Date: Fri, 13 Oct 2023 14:26:33 +0300 Subject: [PATCH 1/7] Fx compile status with help command --- .../build_tests/cli_compile_status/input.js | 43 ++++ .../cli_compile_status/rescript.json | 5 + jscomp/build_tests/cli_help/input.js | 202 ++++++++++++++++++ rescript | 109 +++++----- 4 files changed, 306 insertions(+), 53 deletions(-) create mode 100755 jscomp/build_tests/cli_compile_status/input.js create mode 100644 jscomp/build_tests/cli_compile_status/rescript.json create mode 100755 jscomp/build_tests/cli_help/input.js 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..7311ed95d6 --- /dev/null +++ b/jscomp/build_tests/cli_help/input.js @@ -0,0 +1,202 @@ +// @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" + + "The default `rescript` is equivalent to `rescript build` subcommand\n" + + "\n"; + +const buildHelp = + "Usage: rescript build -- \n" + + "\n" + + "`rescript build` builds the project with dependencies\n" + + "\n" + + "`rescript -- -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); + +// Shows build help with -h arg +out = child_process.spawnSync(`../../../rescript`, ["build", "-h"], { + encoding: "utf8", + cwd: __dirname, +}); +assert.equal(out.stdout, buildHelp); + +// 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"); + +// Shows cli help with --help arg +out = child_process.spawnSync(`../../../rescript`, ["--help"], { + encoding: "utf8", + cwd: __dirname, +}); +assert.equal(out.stdout, cliHelp); + +// Shows cli help with -h arg +out = child_process.spawnSync(`../../../rescript`, ["-h"], { + encoding: "utf8", + cwd: __dirname, +}); +assert.equal(out.stdout, cliHelp); + +// Shows cli help with help command +out = child_process.spawnSync(`../../../rescript`, ["help"], { + encoding: "utf8", + cwd: __dirname, +}); +assert.equal(out.stdout, cliHelp); + +// Shows cli help with unknown command +out = child_process.spawnSync(`../../../rescript`, ["built"], { + encoding: "utf8", + cwd: __dirname, +}); +// Should write to stderr instead ??? +assert.equal(out.stdout, cliHelp); + +// Shows cli help with unknown args +out = child_process.spawnSync(`../../../rescript`, ["-w"], { + encoding: "utf8", + cwd: __dirname, +}); +// Should write to stderr instead ??? +assert.equal(out.stdout, cliHelp); + +// Shows clean help with --help arg +out = child_process.spawnSync(`../../../rescript`, ["clean", "--help"], { + encoding: "utf8", + cwd: __dirname, +}); +assert.equal(out.stdout, cleanHelp); + +// Shows clean help with -h arg +out = child_process.spawnSync(`../../../rescript`, ["clean", "-h"], { + encoding: "utf8", + cwd: __dirname, +}); +assert.equal(out.stdout, cleanHelp); + +// 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"); + +// 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); + +// 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); + +// 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); + +// 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); + +// 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); + +// 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); diff --git a/rescript b/rescript index 23fe253b46..41fb14a8ba 100755 --- a/rescript +++ b/rescript @@ -512,59 +512,62 @@ 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; + 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 "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": + 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); } - 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); } } From 83962472435c33375f26d3751162947a2c60b3ed Mon Sep 17 00:00:00 2001 From: Dmitry Zakharov Date: Fri, 13 Oct 2023 14:35:25 +0300 Subject: [PATCH 2/7] Update changelog --- CHANGELOG.md | 2 ++ rescript | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 806e17c452..1ef1cca303 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 diff --git a/rescript b/rescript index 41fb14a8ba..97f60c2583 100755 --- a/rescript +++ b/rescript @@ -512,6 +512,10 @@ Please pick a different one using the \`-ws [host:]port\` flag from bsb.`); buildFinishedCallback(0); }); } else { + // 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); From bbaf8ca9992c8b00c19c429863492628f2062fa1 Mon Sep 17 00:00:00 2001 From: Dmitry Zakharov Date: Thu, 26 Oct 2023 15:54:36 +0400 Subject: [PATCH 3/7] Update tests --- jscomp/build_tests/cli_help/input.js | 30 ++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/jscomp/build_tests/cli_help/input.js b/jscomp/build_tests/cli_help/input.js index 7311ed95d6..3a6c9c4bb4 100755 --- a/jscomp/build_tests/cli_help/input.js +++ b/jscomp/build_tests/cli_help/input.js @@ -80,6 +80,17 @@ let out = child_process.spawnSync(`../../../rescript`, ["build", "--help"], { cwd: __dirname, }); assert.equal(out.stdout, buildHelp); +assert.equal(out.status, 0); + +// Shows build help with --help arg +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: Should be 0 +assert.equal(out.status, 1); // Shows build help with -h arg out = child_process.spawnSync(`../../../rescript`, ["build", "-h"], { @@ -87,6 +98,7 @@ out = child_process.spawnSync(`../../../rescript`, ["build", "-h"], { cwd: __dirname, }); assert.equal(out.stdout, buildHelp); +assert.equal(out.status, 0); // Exits with build help with unknown arg out = child_process.spawnSync(`../../../rescript`, ["build", "-wtf"], { @@ -94,6 +106,7 @@ out = child_process.spawnSync(`../../../rescript`, ["build", "-wtf"], { cwd: __dirname, }); assert.equal(out.stderr, "Error: unknown option: '-wtf'.\n" + buildHelp + "\n"); +assert.equal(out.status, 2); // Shows cli help with --help arg out = child_process.spawnSync(`../../../rescript`, ["--help"], { @@ -101,6 +114,8 @@ out = child_process.spawnSync(`../../../rescript`, ["--help"], { cwd: __dirname, }); assert.equal(out.stdout, cliHelp); +// FIXME: Should be 0 +assert.equal(out.status, 2); // Shows cli help with -h arg out = child_process.spawnSync(`../../../rescript`, ["-h"], { @@ -108,6 +123,7 @@ out = child_process.spawnSync(`../../../rescript`, ["-h"], { cwd: __dirname, }); assert.equal(out.stdout, cliHelp); +assert.equal(out.status, 0); // Shows cli help with help command out = child_process.spawnSync(`../../../rescript`, ["help"], { @@ -115,6 +131,7 @@ out = child_process.spawnSync(`../../../rescript`, ["help"], { cwd: __dirname, }); assert.equal(out.stdout, cliHelp); +assert.equal(out.status, 0); // Shows cli help with unknown command out = child_process.spawnSync(`../../../rescript`, ["built"], { @@ -123,6 +140,8 @@ out = child_process.spawnSync(`../../../rescript`, ["built"], { }); // Should write to stderr instead ??? assert.equal(out.stdout, cliHelp); +// FIXME: Should show a warning that it's an unknown command +assert.equal(out.status, 2); // Shows cli help with unknown args out = child_process.spawnSync(`../../../rescript`, ["-w"], { @@ -131,6 +150,8 @@ out = child_process.spawnSync(`../../../rescript`, ["-w"], { }); // Should write to stderr instead ??? assert.equal(out.stdout, cliHelp); +// FIXME: Should show a warning that it's an unknown arg +assert.equal(out.status, 2); // Shows clean help with --help arg out = child_process.spawnSync(`../../../rescript`, ["clean", "--help"], { @@ -138,6 +159,7 @@ out = child_process.spawnSync(`../../../rescript`, ["clean", "--help"], { cwd: __dirname, }); assert.equal(out.stdout, cleanHelp); +assert.equal(out.status, 0); // Shows clean help with -h arg out = child_process.spawnSync(`../../../rescript`, ["clean", "-h"], { @@ -145,6 +167,7 @@ out = child_process.spawnSync(`../../../rescript`, ["clean", "-h"], { cwd: __dirname, }); assert.equal(out.stdout, cleanHelp); +assert.equal(out.status, 0); // Exits with clean help with unknown arg out = child_process.spawnSync(`../../../rescript`, ["clean", "-wtf"], { @@ -152,6 +175,7 @@ out = child_process.spawnSync(`../../../rescript`, ["clean", "-wtf"], { cwd: __dirname, }); assert.equal(out.stderr, "Error: unknown option: '-wtf'.\n" + cleanHelp + "\n"); +assert.equal(out.status, 2); // Shows format help with --help arg out = child_process.spawnSync(`../../../rescript`, ["format", "--help"], { @@ -160,6 +184,7 @@ out = child_process.spawnSync(`../../../rescript`, ["format", "--help"], { }); // Note: it writes to stderr assert.equal(out.stderr, formatHelp); +assert.equal(out.status, 0); // Shows format help with -h arg out = child_process.spawnSync(`../../../rescript`, ["format", "-h"], { @@ -168,6 +193,7 @@ out = child_process.spawnSync(`../../../rescript`, ["format", "-h"], { }); // Note: it writes to stderr assert.equal(out.stderr, formatHelp); +assert.equal(out.status, 0); // Shows convert help with --help arg out = child_process.spawnSync(`../../../rescript`, ["convert", "--help"], { @@ -176,6 +202,7 @@ out = child_process.spawnSync(`../../../rescript`, ["convert", "--help"], { }); // Note: it writes to stderr assert.equal(out.stderr, convertHelp); +assert.equal(out.status, 0); // Shows convert help with -h arg out = child_process.spawnSync(`../../../rescript`, ["convert", "-h"], { @@ -184,6 +211,7 @@ out = child_process.spawnSync(`../../../rescript`, ["convert", "-h"], { }); // Note: it writes to stderr assert.equal(out.stderr, convertHelp); +assert.equal(out.status, 0); // Shows dump help with --help arg out = child_process.spawnSync(`../../../rescript`, ["dump", "--help"], { @@ -192,6 +220,7 @@ out = child_process.spawnSync(`../../../rescript`, ["dump", "--help"], { }); // Note: it writes to stderr assert.equal(out.stderr, dumpHelp); +assert.equal(out.status, 0); // Shows dump help with -h arg out = child_process.spawnSync(`../../../rescript`, ["dump", "-h"], { @@ -200,3 +229,4 @@ out = child_process.spawnSync(`../../../rescript`, ["dump", "-h"], { }); // Note: it writes to stderr assert.equal(out.stderr, dumpHelp); +assert.equal(out.status, 0); From 71ec60a2af3a341fcead6df618bf392e4f0094ce Mon Sep 17 00:00:00 2001 From: Dmitry Zakharov Date: Thu, 26 Oct 2023 18:09:04 +0400 Subject: [PATCH 4/7] Improve cli consistency --- jscomp/bsb/bsb_arg.ml | 8 ++++---- jscomp/build_tests/cli_help/input.js | 19 ++++++------------- rescript | 26 ++++++++++---------------- 3 files changed, 20 insertions(+), 33 deletions(-) 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/build_tests/cli_help/input.js b/jscomp/build_tests/cli_help/input.js index 3a6c9c4bb4..08f769f7b0 100755 --- a/jscomp/build_tests/cli_help/input.js +++ b/jscomp/build_tests/cli_help/input.js @@ -22,9 +22,7 @@ const cliHelp = "\n" + "Run `rescript -h` for subcommand help. Examples:\n" + " rescript build -h\n" + - " rescript format -h\n" + - "The default `rescript` is equivalent to `rescript build` subcommand\n" + - "\n"; + " rescript format -h\n"; const buildHelp = "Usage: rescript build -- \n" + @@ -105,7 +103,7 @@ 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.stderr, 'Error: Unknown option "-wtf".\n' + buildHelp + "\n"); assert.equal(out.status, 2); // Shows cli help with --help arg @@ -114,8 +112,7 @@ out = child_process.spawnSync(`../../../rescript`, ["--help"], { cwd: __dirname, }); assert.equal(out.stdout, cliHelp); -// FIXME: Should be 0 -assert.equal(out.status, 2); +assert.equal(out.status, 0); // Shows cli help with -h arg out = child_process.spawnSync(`../../../rescript`, ["-h"], { @@ -138,9 +135,7 @@ out = child_process.spawnSync(`../../../rescript`, ["built"], { encoding: "utf8", cwd: __dirname, }); -// Should write to stderr instead ??? -assert.equal(out.stdout, cliHelp); -// FIXME: Should show a warning that it's an unknown command +assert.equal(out.stderr, `Error: Unknown command or flag "built".\n` + cliHelp); assert.equal(out.status, 2); // Shows cli help with unknown args @@ -148,9 +143,7 @@ out = child_process.spawnSync(`../../../rescript`, ["-w"], { encoding: "utf8", cwd: __dirname, }); -// Should write to stderr instead ??? -assert.equal(out.stdout, cliHelp); -// FIXME: Should show a warning that it's an unknown arg +assert.equal(out.stderr, `Error: Unknown command or flag "-w".\n` + cliHelp); assert.equal(out.status, 2); // Shows clean help with --help arg @@ -174,7 +167,7 @@ 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.stderr, 'Error: Unknown option "-wtf".\n' + cleanHelp + "\n"); assert.equal(out.status, 2); // Shows format help with --help arg diff --git a/rescript b/rescript index 97f60c2583..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() { @@ -545,13 +542,6 @@ Please pick a different one using the \`-ws [host:]port\` flag from bsb.`); 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), @@ -561,16 +551,20 @@ Please pick a different one using the \`-ws [host:]port\` flag from bsb.`); break; case "-h": case "-help": + case "--help": case "help": - help(); + console.log(helpMessage); break; case "-v": case "-version": + case "--version": + case "version": console.log(require("./package.json").version); break; default: - console.error(`Unknown subcommand or flags: ${maybeSubcommand}`); - help(); + console.error( + `Error: Unknown command or flag "${maybeSubcommand}".\n${helpMessage}` + ); process.exit(2); } } From df001a798ffa745625b468f1c1371509b7a3c3a1 Mon Sep 17 00:00:00 2001 From: Dmitry Zakharov Date: Thu, 26 Oct 2023 18:46:48 +0400 Subject: [PATCH 5/7] Finish cli help consistency improvements --- jscomp/bsb_exe/rescript_main.ml | 2 +- jscomp/build_tests/cli_help/input.js | 48 +++++++++++++++++++--------- jscomp/ext/bsc_args.ml | 8 ++--- scripts/rescript_arg.js | 6 ++-- 4 files changed, 41 insertions(+), 23 deletions(-) diff --git a/jscomp/bsb_exe/rescript_main.ml b/jscomp/bsb_exe/rescript_main.ml index b9050b5700..83e808d893 100644 --- a/jscomp/bsb_exe/rescript_main.ml +++ b/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:@} %s@." s; + Format.fprintf Format.err_formatter "@{Error:@} %s" s; exit 2 | e -> Ext_pervasives.reraise e diff --git a/jscomp/build_tests/cli_help/input.js b/jscomp/build_tests/cli_help/input.js index 08f769f7b0..83207ccff6 100755 --- a/jscomp/build_tests/cli_help/input.js +++ b/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,6 +104,7 @@ 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 @@ -103,7 +112,8 @@ 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,6 +140,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 unknown command @@ -135,6 +148,7 @@ 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,6 +176,7 @@ 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 @@ -167,7 +184,8 @@ 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 @@ -175,8 +193,8 @@ 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 @@ -184,8 +202,8 @@ 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 @@ -193,8 +211,8 @@ 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 @@ -202,8 +220,8 @@ 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 @@ -211,8 +229,8 @@ 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 @@ -220,6 +238,6 @@ 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); 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/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); From b04113bd1ae92fe758df80778dbae259ef18b6c3 Mon Sep 17 00:00:00 2001 From: Dmitry Zakharov Date: Thu, 26 Oct 2023 18:51:21 +0400 Subject: [PATCH 6/7] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ef1cca303..034f1b6daf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,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 From f1d405990953845b595347863c523b570398ed85 Mon Sep 17 00:00:00 2001 From: Dmitry Zakharov Date: Fri, 27 Oct 2023 16:18:45 +0400 Subject: [PATCH 7/7] Fix build help --- jscomp/bsb_exe/rescript_main.ml | 2 +- jscomp/build_tests/cli_help/input.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/jscomp/bsb_exe/rescript_main.ml b/jscomp/bsb_exe/rescript_main.ml index 83e808d893..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 diff --git a/jscomp/build_tests/cli_help/input.js b/jscomp/build_tests/cli_help/input.js index 83207ccff6..2c38947612 100755 --- a/jscomp/build_tests/cli_help/input.js +++ b/jscomp/build_tests/cli_help/input.js @@ -29,7 +29,7 @@ const buildHelp = "\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" + "\n" + "Options:\n" + " -w Watch mode\n" +