From 90241cd1d8bc648d5a22e18c9e2ab6fcdc102a10 Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Fri, 25 Jul 2025 08:30:49 +0200 Subject: [PATCH 01/18] Rewatch cli help: do not show build command options in the root help --- rewatch/src/cli.rs | 8 +++----- rewatch/src/main.rs | 25 +++++++++++++++++++++---- 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/rewatch/src/cli.rs b/rewatch/src/cli.rs index 0e5dc564a2..ac0b71fdda 100644 --- a/rewatch/src/cli.rs +++ b/rewatch/src/cli.rs @@ -22,6 +22,7 @@ pub enum FileExtension { #[derive(Parser, Debug)] #[command(version)] #[command(args_conflicts_with_subcommands = true)] +#[command(after_help = "NOTE: If no subcommand is provided, `build` is run by default.")] pub struct Cli { /// Verbosity: /// -v -> Debug @@ -35,10 +36,7 @@ pub struct Cli { /// The command to run. If not provided it will default to build. #[command(subcommand)] - pub command: Option, - - #[command(flatten)] - pub build_args: BuildArgs, + pub command: Command, } #[derive(Args, Debug, Clone)] @@ -181,7 +179,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..1753aa0d03 100644 --- a/rewatch/src/main.rs +++ b/rewatch/src/main.rs @@ -1,14 +1,17 @@ use anyhow::Result; use clap::Parser; use log::LevelFilter; -use std::{io::Write, path::Path}; +use std::{env, io::Write, path::Path}; use rescript::{build, cli, cmd, format, lock, watcher}; fn main() -> Result<()> { - let args = cli::Cli::parse(); + let mut args: Vec = env::args().collect(); + handle_default_arg(&mut args); - let log_level_filter = args.verbose.log_level_filter(); + let cli = cli::Cli::parse_from(args); + + 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 +19,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 { @@ -111,3 +114,17 @@ fn get_lock(folder: &str) -> lock::Lock { acquired_lock => acquired_lock, } } + +fn handle_default_arg(args: &mut Vec) { + let first_arg = args.get(1); + let global_flags = ["-h", "--help", "-V", "--version"]; + + let needs_default_arg = match first_arg { + Some(arg) if !arg.starts_with("-") || global_flags.contains(&arg.as_str()) => false, + _ => true, + }; + + if needs_default_arg { + args.insert(1, "build".to_string()); + } +} From ccb5ead03f007e5869c6d66d61bc220a41b995ec Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Fri, 25 Jul 2025 11:16:58 +0200 Subject: [PATCH 02/18] Satisfy clippy --- rewatch/src/main.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rewatch/src/main.rs b/rewatch/src/main.rs index 1753aa0d03..335f7ee037 100644 --- a/rewatch/src/main.rs +++ b/rewatch/src/main.rs @@ -120,8 +120,8 @@ fn handle_default_arg(args: &mut Vec) { let global_flags = ["-h", "--help", "-V", "--version"]; let needs_default_arg = match first_arg { - Some(arg) if !arg.starts_with("-") || global_flags.contains(&arg.as_str()) => false, - _ => true, + Some(arg) => arg.starts_with("-") && !global_flags.contains(&arg.as_str()), + None => true, }; if needs_default_arg { From c1a358d3447b4b31459e92d2102fd18eb7902d45 Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Fri, 25 Jul 2025 11:18:08 +0200 Subject: [PATCH 03/18] CHANGELOG # Conflicts: # CHANGELOG.md # Conflicts: # CHANGELOG.md --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) 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 From 3761d007ecf00a5d697b0f439977fa0540eb0fb9 Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Fri, 25 Jul 2025 13:36:53 +0200 Subject: [PATCH 04/18] Text + Formatting --- rewatch/src/cli.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/rewatch/src/cli.rs b/rewatch/src/cli.rs index ac0b71fdda..d3343b2600 100644 --- a/rewatch/src/cli.rs +++ b/rewatch/src/cli.rs @@ -22,7 +22,9 @@ pub enum FileExtension { #[derive(Parser, Debug)] #[command(version)] #[command(args_conflicts_with_subcommands = true)] -#[command(after_help = "NOTE: If no subcommand is provided, `build` is run by default.")] +#[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 From abd0db01f8fa1b2ef9abea0a1d5ad9d376f81191 Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Fri, 26 Sep 2025 18:09:43 +0200 Subject: [PATCH 05/18] Fix implementation --- rewatch/src/main.rs | 73 ++++++++++++++++++++++++++++++++++++--------- 1 file changed, 59 insertions(+), 14 deletions(-) diff --git a/rewatch/src/main.rs b/rewatch/src/main.rs index 335f7ee037..9633ec2ae0 100644 --- a/rewatch/src/main.rs +++ b/rewatch/src/main.rs @@ -1,15 +1,13 @@ use anyhow::Result; -use clap::Parser; +use clap::{Parser, error::ErrorKind}; use log::LevelFilter; use std::{env, io::Write, path::Path}; use rescript::{build, cli, cmd, format, lock, watcher}; fn main() -> Result<()> { - let mut args: Vec = env::args().collect(); - handle_default_arg(&mut args); - - let cli = cli::Cli::parse_from(args); + let raw_args: Vec = env::args().collect(); + let cli = parse_cli(raw_args); let log_level_filter = cli.verbose.log_level_filter(); @@ -115,16 +113,63 @@ fn get_lock(folder: &str) -> lock::Lock { } } -fn handle_default_arg(args: &mut Vec) { - let first_arg = args.get(1); - let global_flags = ["-h", "--help", "-V", "--version"]; +fn parse_cli(raw_args: Vec) -> cli::Cli { + match cli::Cli::try_parse_from(&raw_args) { + Ok(cli) => cli, + Err(err) => { + if should_default_to_build(&err, &raw_args) { + let mut fallback_args = raw_args.clone(); + let insert_at = index_after_global_flags(&fallback_args); + fallback_args.insert(insert_at, "build".into()); + + match cli::Cli::try_parse_from(&fallback_args) { + Ok(cli) => cli, + Err(fallback_err) => fallback_err.exit(), + } + } else { + err.exit() + } + } + } +} - let needs_default_arg = match first_arg { - Some(arg) => arg.starts_with("-") && !global_flags.contains(&arg.as_str()), - None => true, - }; +fn should_default_to_build(err: &clap::Error, args: &[String]) -> bool { + match err.kind() { + ErrorKind::MissingSubcommand | ErrorKind::DisplayHelpOnMissingArgumentOrSubcommand => true, + ErrorKind::UnknownArgument | ErrorKind::InvalidSubcommand => { + args.iter().skip(1).any(|arg| !is_global_flag(arg)) + } + _ => false, + } +} - if needs_default_arg { - args.insert(1, "build".to_string()); +fn index_after_global_flags(args: &[String]) -> usize { + let mut idx = 1; + while let Some(arg) = args.get(idx) { + if is_global_flag(arg) { + idx += 1; + } else { + break; + } } + idx.min(args.len()) +} + +fn is_global_flag(arg: &str) -> bool { + matches!( + arg, + "-v" | "-vv" + | "-vvv" + | "-vvvv" + | "-q" + | "-qq" + | "-qqq" + | "-qqqq" + | "--verbose" + | "--quiet" + | "-h" + | "--help" + | "-V" + | "--version" + ) } From f7889aa1a6baf8be9a7175320e1ceebcda8da781 Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Fri, 26 Sep 2025 18:21:47 +0200 Subject: [PATCH 06/18] Added tests --- rewatch/src/cli.rs | 1 - rewatch/src/main.rs | 60 ++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 54 insertions(+), 7 deletions(-) diff --git a/rewatch/src/cli.rs b/rewatch/src/cli.rs index d3343b2600..72bc0125af 100644 --- a/rewatch/src/cli.rs +++ b/rewatch/src/cli.rs @@ -21,7 +21,6 @@ pub enum FileExtension { /// ReScript - Fast, Simple, Fully Typed JavaScript from the Future #[derive(Parser, Debug)] #[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." )] diff --git a/rewatch/src/main.rs b/rewatch/src/main.rs index 9633ec2ae0..8383e9b142 100644 --- a/rewatch/src/main.rs +++ b/rewatch/src/main.rs @@ -7,7 +7,7 @@ use rescript::{build, cli, cmd, format, lock, watcher}; fn main() -> Result<()> { let raw_args: Vec = env::args().collect(); - let cli = parse_cli(raw_args); + let cli = parse_cli(raw_args).unwrap_or_else(|err| err.exit()); let log_level_filter = cli.verbose.log_level_filter(); @@ -113,9 +113,9 @@ fn get_lock(folder: &str) -> lock::Lock { } } -fn parse_cli(raw_args: Vec) -> cli::Cli { +fn parse_cli(raw_args: Vec) -> Result { match cli::Cli::try_parse_from(&raw_args) { - Ok(cli) => cli, + Ok(cli) => Ok(cli), Err(err) => { if should_default_to_build(&err, &raw_args) { let mut fallback_args = raw_args.clone(); @@ -123,11 +123,11 @@ fn parse_cli(raw_args: Vec) -> cli::Cli { fallback_args.insert(insert_at, "build".into()); match cli::Cli::try_parse_from(&fallback_args) { - Ok(cli) => cli, - Err(fallback_err) => fallback_err.exit(), + Ok(cli) => Ok(cli), + Err(fallback_err) => Err(fallback_err), } } else { - err.exit() + Err(err) } } } @@ -173,3 +173,51 @@ fn is_global_flag(arg: &str) -> bool { | "--version" ) } + +#[cfg(test)] +mod tests { + use super::*; + + fn parse(args: &[&str]) -> Result { + parse_cli(args.iter().map(|arg| arg.to_string()).collect()) + } + + #[test] + fn defaults_to_build_without_args() { + let cli = parse(&["rescript"]).expect("expected default build command"); + + match cli.command { + cli::Command::Build(build_args) => assert_eq!(build_args.folder.folder, "."), + other => panic!("expected build command, got {other:?}"), + } + } + + #[test] + fn defaults_to_build_with_folder_shortcut() { + let cli = parse(&["rescript", "someFolder"]).expect("expected build command"); + + match cli.command { + cli::Command::Build(build_args) => assert_eq!(build_args.folder.folder, "someFolder"), + other => panic!("expected build command, got {other:?}"), + } + } + + #[test] + fn respects_global_flag_before_subcommand() { + let cli = parse(&["rescript", "-v", "watch"]).expect("expected watch command"); + + assert!(matches!(cli.command, cli::Command::Watch(_))); + } + + #[test] + fn help_flag_does_not_default_to_build() { + let err = parse(&["rescript", "--help"]).expect_err("expected clap help error"); + assert_eq!(err.kind(), ErrorKind::DisplayHelp); + } + + #[test] + fn version_flag_does_not_default_to_build() { + let err = parse(&["rescript", "--version"]).expect_err("expected clap version error"); + assert_eq!(err.kind(), ErrorKind::DisplayVersion); + } +} From 0c397103e003f5aeb5c6642d3347bb071a6750e4 Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Fri, 26 Sep 2025 18:52:55 +0200 Subject: [PATCH 07/18] Bug fix --- rewatch/src/main.rs | 35 +++++++++++++++++++++++++++++++---- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/rewatch/src/main.rs b/rewatch/src/main.rs index 8383e9b142..4ce5b92e52 100644 --- a/rewatch/src/main.rs +++ b/rewatch/src/main.rs @@ -1,5 +1,5 @@ use anyhow::Result; -use clap::{Parser, error::ErrorKind}; +use clap::{CommandFactory, Parser, error::ErrorKind}; use log::LevelFilter; use std::{env, io::Write, path::Path}; @@ -134,11 +134,19 @@ fn parse_cli(raw_args: Vec) -> Result { } fn should_default_to_build(err: &clap::Error, args: &[String]) -> bool { + let first_non_global = first_non_global_arg(args); + match err.kind() { - ErrorKind::MissingSubcommand | ErrorKind::DisplayHelpOnMissingArgumentOrSubcommand => true, - ErrorKind::UnknownArgument | ErrorKind::InvalidSubcommand => { - args.iter().skip(1).any(|arg| !is_global_flag(arg)) + ErrorKind::MissingSubcommand | ErrorKind::DisplayHelpOnMissingArgumentOrSubcommand => { + match first_non_global { + Some(arg) => !is_known_subcommand(arg), + None => true, + } } + ErrorKind::UnknownArgument | ErrorKind::InvalidSubcommand => match first_non_global { + Some(arg) if is_known_subcommand(arg) => false, + _ => true, + }, _ => false, } } @@ -174,6 +182,19 @@ fn is_global_flag(arg: &str) -> bool { ) } +fn first_non_global_arg(args: &[String]) -> Option<&str> { + args.iter() + .skip(1) + .find(|arg| !is_global_flag(arg)) + .map(|s| s.as_str()) +} + +fn is_known_subcommand(arg: &str) -> bool { + cli::Cli::command().get_subcommands().any(|subcommand| { + subcommand.get_name() == arg || subcommand.get_all_aliases().any(|alias| alias == arg) + }) +} + #[cfg(test)] mod tests { use super::*; @@ -209,6 +230,12 @@ mod tests { assert!(matches!(cli.command, cli::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); + } + #[test] fn help_flag_does_not_default_to_build() { let err = parse(&["rescript", "--help"]).expect_err("expected clap help error"); From cd61d37d05a0d40c10149a106eb42537d9923529 Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Fri, 26 Sep 2025 19:12:11 +0200 Subject: [PATCH 08/18] Simplify and satisfy clippy --- rewatch/src/main.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/rewatch/src/main.rs b/rewatch/src/main.rs index 4ce5b92e52..4c95994b3e 100644 --- a/rewatch/src/main.rs +++ b/rewatch/src/main.rs @@ -134,19 +134,17 @@ fn parse_cli(raw_args: Vec) -> Result { } fn should_default_to_build(err: &clap::Error, args: &[String]) -> bool { - let first_non_global = first_non_global_arg(args); - match err.kind() { - ErrorKind::MissingSubcommand | ErrorKind::DisplayHelpOnMissingArgumentOrSubcommand => { + 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, } } - ErrorKind::UnknownArgument | ErrorKind::InvalidSubcommand => match first_non_global { - Some(arg) if is_known_subcommand(arg) => false, - _ => true, - }, _ => false, } } From 15a8ab202956da7c605290b488548470fc9de31c Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Fri, 26 Sep 2025 19:18:26 +0200 Subject: [PATCH 09/18] Fix non-utf8 error --- rewatch/src/main.rs | 78 ++++++++++++++++++++++++++++----------------- 1 file changed, 49 insertions(+), 29 deletions(-) diff --git a/rewatch/src/main.rs b/rewatch/src/main.rs index 4c95994b3e..7a3da6bd9d 100644 --- a/rewatch/src/main.rs +++ b/rewatch/src/main.rs @@ -1,12 +1,14 @@ use anyhow::Result; use clap::{CommandFactory, Parser, error::ErrorKind}; use log::LevelFilter; -use std::{env, io::Write, path::Path}; +use std::{env, ffi::OsString, io::Write, path::Path}; use rescript::{build, cli, cmd, format, lock, watcher}; fn main() -> Result<()> { - let raw_args: Vec = env::args().collect(); + // 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(); let cli = parse_cli(raw_args).unwrap_or_else(|err| err.exit()); let log_level_filter = cli.verbose.log_level_filter(); @@ -113,14 +115,14 @@ fn get_lock(folder: &str) -> lock::Lock { } } -fn parse_cli(raw_args: Vec) -> Result { +fn parse_cli(raw_args: Vec) -> Result { match cli::Cli::try_parse_from(&raw_args) { Ok(cli) => Ok(cli), Err(err) => { if should_default_to_build(&err, &raw_args) { let mut fallback_args = raw_args.clone(); let insert_at = index_after_global_flags(&fallback_args); - fallback_args.insert(insert_at, "build".into()); + fallback_args.insert(insert_at, OsString::from("build")); match cli::Cli::try_parse_from(&fallback_args) { Ok(cli) => Ok(cli), @@ -133,7 +135,7 @@ fn parse_cli(raw_args: Vec) -> Result { } } -fn should_default_to_build(err: &clap::Error, args: &[String]) -> bool { +fn should_default_to_build(err: &clap::Error, args: &[OsString]) -> bool { match err.kind() { ErrorKind::MissingSubcommand | ErrorKind::DisplayHelpOnMissingArgumentOrSubcommand @@ -149,7 +151,7 @@ fn should_default_to_build(err: &clap::Error, args: &[String]) -> bool { } } -fn index_after_global_flags(args: &[String]) -> usize { +fn index_after_global_flags(args: &[OsString]) -> usize { let mut idx = 1; while let Some(arg) = args.get(idx) { if is_global_flag(arg) { @@ -161,44 +163,52 @@ fn index_after_global_flags(args: &[String]) -> usize { idx.min(args.len()) } -fn is_global_flag(arg: &str) -> bool { +fn is_global_flag(arg: &OsString) -> bool { matches!( - arg, - "-v" | "-vv" - | "-vvv" - | "-vvvv" - | "-q" - | "-qq" - | "-qqq" - | "-qqqq" - | "--verbose" - | "--quiet" - | "-h" - | "--help" - | "-V" - | "--version" + arg.to_str(), + Some( + "-v" | "-vv" + | "-vvv" + | "-vvvv" + | "-q" + | "-qq" + | "-qqq" + | "-qqqq" + | "--verbose" + | "--quiet" + | "-h" + | "--help" + | "-V" + | "--version" + ) ) } -fn first_non_global_arg(args: &[String]) -> Option<&str> { - args.iter() - .skip(1) - .find(|arg| !is_global_flag(arg)) - .map(|s| s.as_str()) +fn first_non_global_arg(args: &[OsString]) -> Option<&OsString> { + args.iter().skip(1).find(|arg| !is_global_flag(arg)) } -fn is_known_subcommand(arg: &str) -> bool { +fn is_known_subcommand(arg: &OsString) -> bool { + let Some(arg_str) = arg.to_str() else { + return false; + }; + cli::Cli::command().get_subcommands().any(|subcommand| { - subcommand.get_name() == arg || subcommand.get_all_aliases().any(|alias| alias == arg) + subcommand.get_name() == arg_str || subcommand.get_all_aliases().any(|alias| alias == arg_str) }) } #[cfg(test)] mod tests { use super::*; + use std::ffi::OsString; fn parse(args: &[&str]) -> Result { - parse_cli(args.iter().map(|arg| arg.to_string()).collect()) + parse_cli(args.iter().map(OsString::from).collect()) + } + + fn parse_os(args: Vec) -> Result { + parse_cli(args) } #[test] @@ -245,4 +255,14 @@ mod tests { 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_os(args).expect_err("expected clap to report invalid utf8"); + assert_eq!(err.kind(), ErrorKind::InvalidUtf8); + } } From 75b10d392ea0bd153bb0efc8d54735ca9c1d4f3b Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Fri, 26 Sep 2025 19:24:47 +0200 Subject: [PATCH 10/18] Satisfy clippy --- rewatch/src/main.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/rewatch/src/main.rs b/rewatch/src/main.rs index 7a3da6bd9d..1d9b581b8d 100644 --- a/rewatch/src/main.rs +++ b/rewatch/src/main.rs @@ -207,10 +207,6 @@ mod tests { parse_cli(args.iter().map(OsString::from).collect()) } - fn parse_os(args: Vec) -> Result { - parse_cli(args) - } - #[test] fn defaults_to_build_without_args() { let cli = parse(&["rescript"]).expect("expected default build command"); @@ -262,7 +258,7 @@ mod tests { use std::os::unix::ffi::OsStringExt; let args = vec![OsString::from("rescript"), OsString::from_vec(vec![0xff])]; - let err = parse_os(args).expect_err("expected clap to report invalid utf8"); + let err = parse_cli(args).expect_err("expected clap to report invalid utf8"); assert_eq!(err.kind(), ErrorKind::InvalidUtf8); } } From 81864509b26ce603a455c73c2924a01f3a71a534 Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Sat, 27 Sep 2025 08:24:55 +0200 Subject: [PATCH 11/18] rewatch: harden default command parsing --- rewatch/src/cli.rs | 1 + rewatch/src/main.rs | 62 ++++++++++++++++++++++++++++++++++----------- 2 files changed, 48 insertions(+), 15 deletions(-) diff --git a/rewatch/src/cli.rs b/rewatch/src/cli.rs index 72bc0125af..99aa269711 100644 --- a/rewatch/src/cli.rs +++ b/rewatch/src/cli.rs @@ -20,6 +20,7 @@ pub enum FileExtension { /// ReScript - Fast, Simple, Fully Typed JavaScript from the Future #[derive(Parser, Debug)] +#[command(name = "rescript", bin_name = "rescript")] #[command(version)] #[command( after_help = "Note: If no command is provided, the build command is run by default. See `rescript help build` for more information." diff --git a/rewatch/src/main.rs b/rewatch/src/main.rs index 1d9b581b8d..5eb8b53f30 100644 --- a/rewatch/src/main.rs +++ b/rewatch/src/main.rs @@ -120,9 +120,7 @@ fn parse_cli(raw_args: Vec) -> Result { Ok(cli) => Ok(cli), Err(err) => { if should_default_to_build(&err, &raw_args) { - let mut fallback_args = raw_args.clone(); - let insert_at = index_after_global_flags(&fallback_args); - fallback_args.insert(insert_at, OsString::from("build")); + let fallback_args = build_default_args(&raw_args); match cli::Cli::try_parse_from(&fallback_args) { Ok(cli) => Ok(cli), @@ -151,18 +149,6 @@ fn should_default_to_build(err: &clap::Error, args: &[OsString]) -> bool { } } -fn index_after_global_flags(args: &[OsString]) -> usize { - let mut idx = 1; - while let Some(arg) = args.get(idx) { - if is_global_flag(arg) { - idx += 1; - } else { - break; - } - } - idx.min(args.len()) -} - fn is_global_flag(arg: &OsString) -> bool { matches!( arg.to_str(), @@ -198,9 +184,44 @@ fn is_known_subcommand(arg: &OsString) -> bool { }) } +fn build_default_args(raw_args: &[OsString]) -> Vec { + 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 +} + #[cfg(test)] mod tests { use super::*; + use log::LevelFilter; use std::ffi::OsString; fn parse(args: &[&str]) -> Result { @@ -234,6 +255,17 @@ mod tests { assert!(matches!(cli.command, cli::Command::Watch(_))); } + #[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 { + cli::Command::Build(build_args) => assert_eq!(build_args.folder.folder, "my-project"), + other => panic!("expected build command, got {other:?}"), + } + } + #[test] fn invalid_option_for_subcommand_does_not_fallback() { let err = parse(&["rescript", "watch", "--no-timing"]).expect_err("expected watch parse failure"); From 64181e9c1eed8478771b072da6eb5fe41804f0fb Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Sat, 27 Sep 2025 08:45:00 +0200 Subject: [PATCH 12/18] rewatch: add baseline clap behavior tests --- rewatch/src/main.rs | 77 ++++++++++++++++++++++++++++++++++++--------- 1 file changed, 63 insertions(+), 14 deletions(-) diff --git a/rewatch/src/main.rs b/rewatch/src/main.rs index 5eb8b53f30..412f6301d6 100644 --- a/rewatch/src/main.rs +++ b/rewatch/src/main.rs @@ -228,14 +228,11 @@ mod tests { parse_cli(args.iter().map(OsString::from).collect()) } + // Default command behaviour. #[test] - fn defaults_to_build_without_args() { + fn no_subcommand_defaults_to_build() { let cli = parse(&["rescript"]).expect("expected default build command"); - - match cli.command { - cli::Command::Build(build_args) => assert_eq!(build_args.folder.folder, "."), - other => panic!("expected build command, got {other:?}"), - } + assert!(matches!(cli.command, cli::Command::Build(_))); } #[test] @@ -249,37 +246,89 @@ mod tests { } #[test] - fn respects_global_flag_before_subcommand() { - let cli = parse(&["rescript", "-v", "watch"]).expect("expected watch command"); + fn trailing_global_flag_is_treated_as_global() { + let cli = parse(&["rescript", "my-project", "-v"]).expect("expected build command"); - assert!(matches!(cli.command, cli::Command::Watch(_))); + assert_eq!(cli.verbose.log_level_filter(), LevelFilter::Debug); + match cli.command { + cli::Command::Build(build_args) => assert_eq!(build_args.folder.folder, "my-project"), + 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"); + 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, cli::Command::Build(_))); + } + + #[test] + fn build_option_is_parsed_normally() { + let cli = parse(&["rescript", "build", "--no-timing"]).expect("expected build command"); + match cli.command { - cli::Command::Build(build_args) => assert_eq!(build_args.folder.folder, "my-project"), + cli::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, cli::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 help_flag_does_not_default_to_build() { + 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 version_flag_does_not_default_to_build() { + fn global_version_flag_shows_version() { let err = parse(&["rescript", "--version"]).expect_err("expected clap version error"); assert_eq!(err.kind(), ErrorKind::DisplayVersion); } From f6389441a88f008298e88aa648218c0e9e4fd659 Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Sat, 27 Sep 2025 09:00:28 +0200 Subject: [PATCH 13/18] add test for "--" --- rewatch/src/main.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/rewatch/src/main.rs b/rewatch/src/main.rs index 412f6301d6..fe6934c1ab 100644 --- a/rewatch/src/main.rs +++ b/rewatch/src/main.rs @@ -256,6 +256,17 @@ mod tests { } } + #[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 { + cli::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"); From 5ee35e9330c1d6552e5802c747bb5c2a0046489c Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Mon, 29 Sep 2025 09:49:04 +0200 Subject: [PATCH 14/18] add comments --- rewatch/src/cli.rs | 3 +++ rewatch/src/main.rs | 9 +++++++++ 2 files changed, 12 insertions(+) diff --git a/rewatch/src/cli.rs b/rewatch/src/cli.rs index 99aa269711..7306e0a9cd 100644 --- a/rewatch/src/cli.rs +++ b/rewatch/src/cli.rs @@ -20,6 +20,9 @@ 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( diff --git a/rewatch/src/main.rs b/rewatch/src/main.rs index fe6934c1ab..117e437963 100644 --- a/rewatch/src/main.rs +++ b/rewatch/src/main.rs @@ -116,6 +116,11 @@ fn get_lock(folder: &str) -> lock::Lock { } fn parse_cli(raw_args: Vec) -> Result { + // Clap's builder API cannot express "build" as an implicit default subcommand without also + // polluting the top-level help output. To keep help focused while still honoring "rescript" + // invocations that omit a command, we first attempt the canonical parse and only synthesize a + // `build` subcommand when clap tells us the invocation failed because no subcommand (or a + // positional that maps to one) was provided. match cli::Cli::try_parse_from(&raw_args) { Ok(cli) => Ok(cli), Err(err) => { @@ -185,6 +190,10 @@ fn is_known_subcommand(arg: &OsString) -> bool { } 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")]; From c554209c55a93c745aef42f6c56a284686559d6a Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Mon, 29 Sep 2025 10:02:16 +0200 Subject: [PATCH 15/18] Move default arg handling to cli module --- rewatch/src/cli.rs | 246 +++++++++++++++++++++++++++++++++++++++++- rewatch/src/main.rs | 252 +------------------------------------------- 2 files changed, 246 insertions(+), 252 deletions(-) diff --git a/rewatch/src/cli.rs b/rewatch/src/cli.rs index 7306e0a9cd..b7096da869 100644 --- a/rewatch/src/cli.rs +++ b/rewatch/src/cli.rs @@ -1,6 +1,6 @@ use std::{ffi::OsString, ops::Deref}; -use clap::{Args, Parser, Subcommand}; +use clap::{error::ErrorKind, Args, CommandFactory, Parser, Subcommand}; use clap_verbosity_flag::InfoLevel; use regex::Regex; @@ -44,6 +44,112 @@ pub struct Cli { pub command: Command, } +/// Parse argv 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(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)] pub struct FolderArg { /// The relative path to where the main rescript.json resides. IE - the root of your project. @@ -139,6 +245,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(&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(&args).expect_err("expected clap to report invalid utf8"); + assert_eq!(err.kind(), ErrorKind::InvalidUtf8); + } +} + #[derive(Args, Clone, Debug)] pub struct WatchArgs { #[command(flatten)] diff --git a/rewatch/src/main.rs b/rewatch/src/main.rs index 117e437963..6b42579e64 100644 --- a/rewatch/src/main.rs +++ b/rewatch/src/main.rs @@ -1,5 +1,4 @@ use anyhow::Result; -use clap::{CommandFactory, Parser, error::ErrorKind}; use log::LevelFilter; use std::{env, ffi::OsString, io::Write, path::Path}; @@ -9,7 +8,7 @@ fn main() -> 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(); - let cli = parse_cli(raw_args).unwrap_or_else(|err| err.exit()); + let cli = cli::parse_with_default(&raw_args).unwrap_or_else(|err| err.exit()); let log_level_filter = cli.verbose.log_level_filter(); @@ -114,252 +113,3 @@ fn get_lock(folder: &str) -> lock::Lock { acquired_lock => acquired_lock, } } - -fn parse_cli(raw_args: Vec) -> Result { - // Clap's builder API cannot express "build" as an implicit default subcommand without also - // polluting the top-level help output. To keep help focused while still honoring "rescript" - // invocations that omit a command, we first attempt the canonical parse and only synthesize a - // `build` subcommand when clap tells us the invocation failed because no subcommand (or a - // positional that maps to one) was provided. - match cli::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); - - match cli::Cli::try_parse_from(&fallback_args) { - Ok(cli) => Ok(cli), - Err(fallback_err) => Err(fallback_err), - } - } 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::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 -} - -#[cfg(test)] -mod tests { - use super::*; - use log::LevelFilter; - use std::ffi::OsString; - - fn parse(args: &[&str]) -> Result { - parse_cli(args.iter().map(OsString::from).collect()) - } - - // Default command behaviour. - #[test] - fn no_subcommand_defaults_to_build() { - let cli = parse(&["rescript"]).expect("expected default build command"); - assert!(matches!(cli.command, cli::Command::Build(_))); - } - - #[test] - fn defaults_to_build_with_folder_shortcut() { - let cli = parse(&["rescript", "someFolder"]).expect("expected build command"); - - match cli.command { - cli::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 { - cli::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 { - cli::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, cli::Command::Build(_))); - } - - #[test] - fn build_option_is_parsed_normally() { - let cli = parse(&["rescript", "build", "--no-timing"]).expect("expected build command"); - - match cli.command { - cli::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, cli::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_cli(args).expect_err("expected clap to report invalid utf8"); - assert_eq!(err.kind(), ErrorKind::InvalidUtf8); - } -} From 7dea5e52a8f53739b444fc4b503ea222fe6bdd4d Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Mon, 29 Sep 2025 10:29:21 +0200 Subject: [PATCH 16/18] format --- rewatch/src/cli.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rewatch/src/cli.rs b/rewatch/src/cli.rs index b7096da869..cefc50dc17 100644 --- a/rewatch/src/cli.rs +++ b/rewatch/src/cli.rs @@ -1,6 +1,6 @@ use std::{ffi::OsString, ops::Deref}; -use clap::{error::ErrorKind, Args, CommandFactory, Parser, Subcommand}; +use clap::{Args, CommandFactory, Parser, Subcommand, error::ErrorKind}; use clap_verbosity_flag::InfoLevel; use regex::Regex; From b9d24276a87346255580c8e6500d1aa6765b4f55 Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Mon, 29 Sep 2025 11:15:19 +0200 Subject: [PATCH 17/18] Move env::args_os().collect into cli.rs --- rewatch/src/cli.rs | 22 +++++++++++++++------- rewatch/src/main.rs | 7 ++----- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/rewatch/src/cli.rs b/rewatch/src/cli.rs index cefc50dc17..0d76c64438 100644 --- a/rewatch/src/cli.rs +++ b/rewatch/src/cli.rs @@ -1,4 +1,4 @@ -use std::{ffi::OsString, ops::Deref}; +use std::{env, ffi::OsString, ops::Deref}; use clap::{Args, CommandFactory, Parser, Subcommand, error::ErrorKind}; use clap_verbosity_flag::InfoLevel; @@ -44,10 +44,18 @@ pub struct Cli { pub command: Command, } -/// Parse argv 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(raw_args: &[OsString]) -> Result { +/// 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) => { @@ -253,7 +261,7 @@ mod tests { fn parse(args: &[&str]) -> Result { let raw_args: Vec = args.iter().map(OsString::from).collect(); - parse_with_default(&raw_args) + parse_with_default_from(&raw_args) } // Default command behaviour. @@ -378,7 +386,7 @@ mod tests { use std::os::unix::ffi::OsStringExt; let args = vec![OsString::from("rescript"), OsString::from_vec(vec![0xff])]; - let err = parse_with_default(&args).expect_err("expected clap to report invalid utf8"); + let err = parse_with_default_from(&args).expect_err("expected clap to report invalid utf8"); assert_eq!(err.kind(), ErrorKind::InvalidUtf8); } } diff --git a/rewatch/src/main.rs b/rewatch/src/main.rs index 6b42579e64..382aa19c81 100644 --- a/rewatch/src/main.rs +++ b/rewatch/src/main.rs @@ -1,14 +1,11 @@ use anyhow::Result; use log::LevelFilter; -use std::{env, ffi::OsString, io::Write, path::Path}; +use std::{io::Write, path::Path}; use rescript::{build, cli, cmd, format, lock, watcher}; fn main() -> 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(); - let cli = cli::parse_with_default(&raw_args).unwrap_or_else(|err| err.exit()); + let cli = cli::parse_with_default().unwrap_or_else(|err| err.exit()); let log_level_filter = cli.verbose.log_level_filter(); From 603172f9b995da578288b7fb482532fb5d4152b8 Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Mon, 29 Sep 2025 14:14:31 +0200 Subject: [PATCH 18/18] Add comment regarding workaround --- rewatch/src/cli.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/rewatch/src/cli.rs b/rewatch/src/cli.rs index 0d76c64438..2e7ca401af 100644 --- a/rewatch/src/cli.rs +++ b/rewatch/src/cli.rs @@ -1,3 +1,16 @@ +// 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};