Skip to content
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 6 additions & 6 deletions rewatch/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,11 @@ pub enum FileExtension {

/// ReScript - Fast, Simple, Fully Typed JavaScript from the Future
#[derive(Parser, Debug)]
#[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
Expand All @@ -35,10 +38,7 @@ pub struct Cli {

/// The command to run. If not provided it will default to build.
#[command(subcommand)]
pub command: Option<Command>,

#[command(flatten)]
pub build_args: BuildArgs,
pub command: Command,
}

#[derive(Args, Debug, Clone)]
Expand Down Expand Up @@ -181,7 +181,7 @@ impl From<BuildArgs> 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),
Expand Down
253 changes: 248 additions & 5 deletions rewatch/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,22 +1,25 @@
use anyhow::Result;
use clap::Parser;
use clap::{CommandFactory, Parser, error::ErrorKind};
use log::LevelFilter;
use std::{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 args = cli::Cli::parse();
// 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<OsString> = env::args_os().collect();
let cli = parse_cli(raw_args).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()))
.filter_level(log_level_filter)
.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 {
Expand Down Expand Up @@ -111,3 +114,243 @@ fn get_lock(folder: &str) -> lock::Lock {
acquired_lock => acquired_lock,
}
}

fn parse_cli(raw_args: Vec<OsString>) -> Result<cli::Cli, clap::Error> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes me wonder if we can check if the first argument is any of the verbs, if not prepend the vector with build.
That way we avoid the double parse in the error case.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what I had in the beginning, and it had other issues.
See the previous codex reviews, it took quite a few attempts until everything worked correctly.

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<OsString> {
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<cli::Cli, clap::Error> {
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] <COMMAND>"));
}

#[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] <COMMAND>"));
}

#[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);
}
}
Loading