diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c0e678486..582388d634 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,8 @@ #### :nail_care: Polish +- Rewatch cli: do not show build command options in the root help. https://github.com/rescript-lang/rescript/pull/7715 + #### :house: Internal # 12.0.0-beta.13 diff --git a/rewatch/src/cli.rs b/rewatch/src/cli.rs index 0e5dc564a2..2e7ca401af 100644 --- a/rewatch/src/cli.rs +++ b/rewatch/src/cli.rs @@ -1,6 +1,19 @@ -use std::{ffi::OsString, ops::Deref}; - -use clap::{Args, Parser, Subcommand}; +// We currently use https://docs.rs/clap/latest/clap/ v4 for command line parsing. +// However, it does not fully fit our use case as it does not support default commands, +// but we want to default to the "build" command if no other command is specified. +// +// Various workarounds exist, but each with its own drawbacks. +// The workaround implemented here (injecting "build" into the args at the right place +// and then parsing again if no other command matches at the first parse attempt) +// avoids flattening all build command options into the root help, but requires careful +// handling of edge cases regarding global flags. +// Correctness is ensured by a comprehensive test suite. +// +// However, we may want to revisit the decision to use clap after the v12 release. + +use std::{env, ffi::OsString, ops::Deref}; + +use clap::{Args, CommandFactory, Parser, Subcommand, error::ErrorKind}; use clap_verbosity_flag::InfoLevel; use regex::Regex; @@ -20,8 +33,14 @@ pub enum FileExtension { /// ReScript - Fast, Simple, Fully Typed JavaScript from the Future #[derive(Parser, Debug)] +// The shipped binary is `rescript.exe` everywhere, but users invoke it as `rescript` (e.g. +// via `npm run rescript`). Without forcing `bin_name`, clap would print `rescript.exe` in help, +// which leaks the packaging detail into the CLI UX. +#[command(name = "rescript", bin_name = "rescript")] #[command(version)] -#[command(args_conflicts_with_subcommands = true)] +#[command( + after_help = "Note: If no command is provided, the build command is run by default. See `rescript help build` for more information." +)] pub struct Cli { /// Verbosity: /// -v -> Debug @@ -35,10 +54,121 @@ pub struct Cli { /// The command to run. If not provided it will default to build. #[command(subcommand)] - pub command: Option, + pub command: Command, +} - #[command(flatten)] - pub build_args: BuildArgs, +/// Parse argv from the current process while treating `build` as the implicit default subcommand +/// when clap indicates the user omitted one. This keeps the top-level help compact while still +/// supporting bare `rescript …` invocations that expect to run the build. +pub fn parse_with_default() -> Result { + // Use `args_os` so non-UTF bytes still reach clap for proper error reporting on platforms that + // allow arbitrary argv content. + let raw_args: Vec = env::args_os().collect(); + parse_with_default_from(&raw_args) +} + +/// Parse the provided argv while applying the implicit `build` defaulting rules. +pub fn parse_with_default_from(raw_args: &[OsString]) -> Result { + match Cli::try_parse_from(raw_args) { + Ok(cli) => Ok(cli), + Err(err) => { + if should_default_to_build(&err, raw_args) { + let fallback_args = build_default_args(raw_args); + Cli::try_parse_from(&fallback_args) + } else { + Err(err) + } + } + } +} + +fn should_default_to_build(err: &clap::Error, args: &[OsString]) -> bool { + match err.kind() { + ErrorKind::MissingSubcommand + | ErrorKind::DisplayHelpOnMissingArgumentOrSubcommand + | ErrorKind::UnknownArgument + | ErrorKind::InvalidSubcommand => { + let first_non_global = first_non_global_arg(args); + match first_non_global { + Some(arg) => !is_known_subcommand(arg), + None => true, + } + } + _ => false, + } +} + +fn is_global_flag(arg: &OsString) -> bool { + matches!( + arg.to_str(), + Some( + "-v" | "-vv" + | "-vvv" + | "-vvvv" + | "-q" + | "-qq" + | "-qqq" + | "-qqqq" + | "--verbose" + | "--quiet" + | "-h" + | "--help" + | "-V" + | "--version" + ) + ) +} + +fn first_non_global_arg(args: &[OsString]) -> Option<&OsString> { + args.iter().skip(1).find(|arg| !is_global_flag(arg)) +} + +fn is_known_subcommand(arg: &OsString) -> bool { + let Some(arg_str) = arg.to_str() else { + return false; + }; + + Cli::command().get_subcommands().any(|subcommand| { + subcommand.get_name() == arg_str || subcommand.get_all_aliases().any(|alias| alias == arg_str) + }) +} + +fn build_default_args(raw_args: &[OsString]) -> Vec { + // Preserve clap's global flag handling semantics by keeping `-v/-q/-h/-V` in front of the + // inserted `build` token while leaving the rest of the argv untouched. This mirrors clap's own + // precedence rules so the second parse sees an argument layout it would have produced if the + // user had typed `rescript build …` directly. + let mut result = Vec::with_capacity(raw_args.len() + 1); + if raw_args.is_empty() { + return vec![OsString::from("build")]; + } + + let mut globals = Vec::new(); + let mut others = Vec::new(); + let mut saw_double_dash = false; + + for arg in raw_args.iter().skip(1) { + if !saw_double_dash { + if arg == "--" { + saw_double_dash = true; + others.push(arg.clone()); + continue; + } + + if is_global_flag(arg) { + globals.push(arg.clone()); + continue; + } + } + + others.push(arg.clone()); + } + + result.push(raw_args[0].clone()); + result.extend(globals); + result.push(OsString::from("build")); + result.extend(others); + result } #[derive(Args, Debug, Clone)] @@ -136,6 +266,144 @@ pub struct BuildArgs { pub warn_error: Option, } +#[cfg(test)] +mod tests { + use super::*; + use clap::error::ErrorKind; + use log::LevelFilter; + + fn parse(args: &[&str]) -> Result { + let raw_args: Vec = args.iter().map(OsString::from).collect(); + parse_with_default_from(&raw_args) + } + + // Default command behaviour. + #[test] + fn no_subcommand_defaults_to_build() { + let cli = parse(&["rescript"]).expect("expected default build command"); + assert!(matches!(cli.command, Command::Build(_))); + } + + #[test] + fn defaults_to_build_with_folder_shortcut() { + let cli = parse(&["rescript", "someFolder"]).expect("expected build command"); + + match cli.command { + Command::Build(build_args) => assert_eq!(build_args.folder.folder, "someFolder"), + other => panic!("expected build command, got {other:?}"), + } + } + + #[test] + fn trailing_global_flag_is_treated_as_global() { + let cli = parse(&["rescript", "my-project", "-v"]).expect("expected build command"); + + assert_eq!(cli.verbose.log_level_filter(), LevelFilter::Debug); + match cli.command { + Command::Build(build_args) => assert_eq!(build_args.folder.folder, "my-project"), + other => panic!("expected build command, got {other:?}"), + } + } + + #[test] + fn double_dash_keeps_following_args_positional() { + let cli = parse(&["rescript", "--", "-v"]).expect("expected build command"); + + assert_eq!(cli.verbose.log_level_filter(), LevelFilter::Info); + match cli.command { + Command::Build(build_args) => assert_eq!(build_args.folder.folder, "-v"), + other => panic!("expected build command, got {other:?}"), + } + } + + #[test] + fn unknown_subcommand_help_uses_global_help() { + let err = parse(&["rescript", "xxx", "--help"]).expect_err("expected global help"); + assert_eq!(err.kind(), ErrorKind::DisplayHelp); + } + + // Build command specifics. + #[test] + fn build_help_shows_subcommand_help() { + let err = parse(&["rescript", "build", "--help"]).expect_err("expected subcommand help"); + assert_eq!(err.kind(), ErrorKind::DisplayHelp); + let rendered = err.to_string(); + assert!( + rendered.contains("Usage: rescript build"), + "unexpected help: {rendered:?}" + ); + assert!(!rendered.contains("Usage: rescript [OPTIONS] ")); + } + + #[test] + fn build_allows_global_verbose_flag() { + let cli = parse(&["rescript", "build", "-v"]).expect("expected build command"); + assert_eq!(cli.verbose.log_level_filter(), LevelFilter::Debug); + assert!(matches!(cli.command, Command::Build(_))); + } + + #[test] + fn build_option_is_parsed_normally() { + let cli = parse(&["rescript", "build", "--no-timing"]).expect("expected build command"); + + match cli.command { + Command::Build(build_args) => assert!(build_args.no_timing), + other => panic!("expected build command, got {other:?}"), + } + } + + // Subcommand flag handling. + #[test] + fn respects_global_flag_before_subcommand() { + let cli = parse(&["rescript", "-v", "watch"]).expect("expected watch command"); + + assert!(matches!(cli.command, Command::Watch(_))); + } + + #[test] + fn invalid_option_for_subcommand_does_not_fallback() { + let err = parse(&["rescript", "watch", "--no-timing"]).expect_err("expected watch parse failure"); + assert_eq!(err.kind(), ErrorKind::UnknownArgument); + } + + // Version/help flag handling. + #[test] + fn version_flag_before_subcommand_displays_version() { + let err = parse(&["rescript", "-V", "build"]).expect_err("expected version display"); + assert_eq!(err.kind(), ErrorKind::DisplayVersion); + } + + #[test] + fn version_flag_after_subcommand_is_rejected() { + let err = parse(&["rescript", "build", "-V"]).expect_err("expected unexpected argument"); + assert_eq!(err.kind(), ErrorKind::UnknownArgument); + } + + #[test] + fn global_help_flag_shows_help() { + let err = parse(&["rescript", "--help"]).expect_err("expected clap help error"); + assert_eq!(err.kind(), ErrorKind::DisplayHelp); + let rendered = err.to_string(); + assert!(rendered.contains("Usage: rescript [OPTIONS] ")); + } + + #[test] + fn global_version_flag_shows_version() { + let err = parse(&["rescript", "--version"]).expect_err("expected clap version error"); + assert_eq!(err.kind(), ErrorKind::DisplayVersion); + } + + #[cfg(unix)] + #[test] + fn non_utf_argument_returns_error() { + use std::os::unix::ffi::OsStringExt; + + let args = vec![OsString::from("rescript"), OsString::from_vec(vec![0xff])]; + let err = parse_with_default_from(&args).expect_err("expected clap to report invalid utf8"); + assert_eq!(err.kind(), ErrorKind::InvalidUtf8); + } +} + #[derive(Args, Clone, Debug)] pub struct WatchArgs { #[command(flatten)] @@ -181,7 +449,7 @@ impl From for WatchArgs { #[derive(Subcommand, Clone, Debug)] pub enum Command { - /// Build the project + /// Build the project (default command) Build(BuildArgs), /// Build, then start a watcher Watch(WatchArgs), diff --git a/rewatch/src/main.rs b/rewatch/src/main.rs index f7f09eca1e..382aa19c81 100644 --- a/rewatch/src/main.rs +++ b/rewatch/src/main.rs @@ -1,14 +1,13 @@ use anyhow::Result; -use clap::Parser; use log::LevelFilter; use std::{io::Write, path::Path}; use rescript::{build, cli, cmd, format, lock, watcher}; fn main() -> Result<()> { - let args = cli::Cli::parse(); + let cli = cli::parse_with_default().unwrap_or_else(|err| err.exit()); - let log_level_filter = args.verbose.log_level_filter(); + let log_level_filter = cli.verbose.log_level_filter(); env_logger::Builder::new() .format(|buf, record| writeln!(buf, "{}:\n{}", record.level(), record.args())) @@ -16,7 +15,7 @@ fn main() -> Result<()> { .target(env_logger::fmt::Target::Stdout) .init(); - let mut command = args.command.unwrap_or(cli::Command::Build(args.build_args)); + let mut command = cli.command; if let cli::Command::Build(build_args) = &command { if build_args.watch {