diff --git a/NEWS.md b/NEWS.md index 38fcb9d..0178b7c 100644 --- a/NEWS.md +++ b/NEWS.md @@ -10,6 +10,9 @@ types instead of coercing them. Integer options no longer accept float or logical values such as `10.2` or `true`; float options still accept integer values (#18). +- Command switches are now required by default. When a command is + omitted, Rapp prints scoped help; add `#| required: false` above the + `switch()` to allow running without a command (#21). ## New features diff --git a/R/app.R b/R/app.R index e5a0af9..90d0c9c 100644 --- a/R/app.R +++ b/R/app.R @@ -81,6 +81,10 @@ is_command_switch <- function(e) { typeof(switch_expr) == "character" || is_simple_assignment_call(switch_expr) } +command_switch_help_on_missing <- function(anno) { + !isFALSE((anno %||% list())[["required"]]) +} + .simple_call_syms <- c("+", "-", "c", "character", "integer", "double", "numeric") @@ -117,6 +121,8 @@ get_app_inputs <- function(app, exprs = app$exprs, pos = integer()) { if (length(commands)) { stop("Only one app command switch() block allowed per expression level") } + switch_expr <- e[[2L]] + switch_anno <- parse_expr_anno(getSrcLineNo(exprs[i]), lines, is_hashpipe) branches <- as.list(e)[-(1:2)] if (".val_pos_in_exprs" %in% names(branches)) { stop('command name ".val_pos_in_exprs" not permitted.') @@ -136,9 +142,10 @@ get_app_inputs <- function(app, exprs = app$exprs, pos = integer()) { } ) names(commands) <- gsub("_", "-", names(commands), fixed = TRUE) - switch_expr <- e[[2L]] commands$.val_pos_in_exprs <- c(pos, i, 2L, if (is.call(switch_expr)) 3L) + attr(commands, "help_on_missing_command") <- + command_switch_help_on_missing(switch_anno) next } diff --git a/R/args.R b/R/args.R index 4e6ea20..9e0b4be 100644 --- a/R/args.R +++ b/R/args.R @@ -168,6 +168,11 @@ process_args <- function(args, app) { app$exprs[[spec$.val_pos_in_exprs]] <- val } + command_names <- setdiff(names(app_commands), ".val_pos_in_exprs") + missing_required_command <- + length(command_names) && + isTRUE(attr(app_commands, "help_on_missing_command")) + if (length(positional_args) || length(app_args)) { # we've parsed all the command line args, # we can now match positional args @@ -204,12 +209,20 @@ process_args <- function(args, app) { } if (length(specs) < length(positional_args)) { + unrecognized_args <- positional_args[ + seq.int(length(specs) + 1L, length(positional_args)) + ] stop( "Arguments not recognized: ", - paste0(positional_args[-seq_along(specs)], collapse = " ") + paste0(unrecognized_args, collapse = " ") ) } + if (missing_required_command) { + print_app_help(app, command_path = command_path, yaml = FALSE) + return(FALSE) + } + if (length(specs) != length(positional_args)) { for (i in rev(seq_along(specs))) { if (isFALSE(specs[[i]][["required"]])) { @@ -239,6 +252,11 @@ process_args <- function(args, app) { } } + if (missing_required_command) { + print_app_help(app, command_path = command_path, yaml = FALSE) + return(FALSE) + } + TRUE } diff --git a/R/help.R b/R/help.R index 01f88bf..050f7f5 100644 --- a/R/help.R +++ b/R/help.R @@ -453,8 +453,16 @@ print_app_help <- function(app, yaml = TRUE, command_path = character()) { if (any_opts) { usage_components <- c(usage_components, "[OPTIONS]") } - if (length(setdiff(names(current_commands), ".val_pos_in_exprs"))) { - usage_components <- c(usage_components, "") + command_names <- setdiff(names(current_commands), ".val_pos_in_exprs") + if (length(command_names)) { + command_usage <- if ( + isFALSE(attr(current_commands, "help_on_missing_command")) + ) { + "[]" + } else { + "" + } + usage_components <- c(usage_components, command_usage) } usage_components <- c(usage_components, build_usage_args(current_args)) usage_line <- paste("Usage:", paste(usage_components, collapse = " ")) diff --git a/README.md b/README.md index de55430..0390989 100644 --- a/README.md +++ b/README.md @@ -77,12 +77,15 @@ arguments, and commands. The sections below cover the supported patterns. ### Help -All Rapps comes with built-in flags for help. +All Rapps come with built-in flags for help. - `--help` shows usage, description, and options for the app (and for subcommands when used after a command, e.g., `todo list --help`). - `--help-yaml` prints machine-readable metadata for the app as YAML. +When a command is missing, Rapp automatically prints the same help as +`--help`. + ### Options Simple assignments of scalar (length-1) literals at the top level of the @@ -212,11 +215,13 @@ This changes the usage to `Usage: greet []` (with brackets). ### Commands Use a `switch()` statement whose first argument is either a character -scalar or an assignment (for example `switch("")` or -`switch(command <- "", ...)`) to declare commands. The corresponding -branch runs when the matching command is supplied on the command line. -Declare command specific options and positional arguments with the same -rules inside the branch. +scalar or an assignment to declare commands. Command switches are +required by default; if no command is supplied, Rapp prints help for the +current command level. To allow running without a command, add +`#| required: false` above the `switch()`. The corresponding branch runs +when the matching command is supplied on the command line. Declare +command specific options and positional arguments with the same rules +inside the branch. ``` r #!/usr/bin/env Rapp @@ -358,7 +363,7 @@ command line arguments. | Assignment of `TRUE` or `FALSE`
`foo <- TRUE` | Boolean switch
`APP --foo` or `APP --no-foo` | | Assignment of `c()` or `list()`
`foo <- c()` | Repeatable option
`APP --foo val1 --foo val2` | | Assignment of `NULL` to name with `...`
`args... <- NULL` | Positional Arg Collector
`APP foo bar baz` | -| Switch with string literal
`switch("", cmd1 = {}, cmd2 = {})` | Commands
`APP cmd1 --help`
`APP cmd2 --help` | +| Switch with string literal
`switch("", cmd1 = {}, cmd2 = {})` | Required commands
`APP --help`
`APP cmd1 --help`
`APP cmd2 --help` | ### Running interactively diff --git a/tests/testthat/_snaps/subcommands.md b/tests/testthat/_snaps/subcommands.md new file mode 100644 index 0000000..f9df528 --- /dev/null +++ b/tests/testthat/_snaps/subcommands.md @@ -0,0 +1,202 @@ +# missing literal command switch prints help + + Code + yaml12::write_yaml(snapshot) + Output + --- + app: |- + #!/usr/bin/env Rapp + #| name: required-command-test + #| description: Exercise missing command help. + + switch('', + #| title: List entries + list = { cat('list called\n') } + ) + invocation: + - usage: $ required-command-test + output: |- + Usage: required-command-test + + Exercise missing command help. + + Commands: + list List entries + + For help with a specific command, run: `required-command-test --help`. + - usage: $ required-command-test list + output: list called + ... + +# missing command assignment prints help by default + + Code + yaml12::write_yaml(snapshot) + Output + --- + app: |- + #!/usr/bin/env Rapp + #| name: assigned-command-test + #| description: Exercise missing command help. + + switch(command <- '', + #| title: List entries + list = { cat(command, '\n', sep = '') } + ) + invocation: + - usage: $ assigned-command-test + output: |- + Usage: assigned-command-test + + Exercise missing command help. + + Commands: + list List entries + + For help with a specific command, run: `assigned-command-test --help`. + - usage: $ assigned-command-test list + output: list + ... + +# required false command switch allows missing command + + Code + yaml12::write_yaml(snapshot) + Output + --- + app: |- + #!/usr/bin/env Rapp + #| name: optional-command-test + + #| required: false + switch(command <- '', + #| title: List entries + list = { cat(command, '\n', sep = '') } + ) + cat('no command\n') + invocation: + - usage: $ optional-command-test + output: no command + - usage: $ optional-command-test list + output: |- + list + no command + - usage: $ optional-command-test --help + output: |- + Usage: optional-command-test [] + + optional-command-test + + Commands: + list List entries + + For help with a specific command, run: `optional-command-test --help`. + ... + +# missing command prints help before matching positionals + + Code + yaml12::write_yaml(snapshot) + Output + --- + app: |- + #!/usr/bin/env Rapp + #| name: command-with-positional-test + + #| description: Input path. + input <- NULL + + switch('', + #| title: Run command + run = { cat('run ', input, '\n', sep = '') } + ) + cat('no command\n') + invocation: + - usage: $ command-with-positional-test data.csv + output: |- + Usage: command-with-positional-test + + command-with-positional-test + + Commands: + run Run command + + Arguments: + Input path. + + For help with a specific command, run: `command-with-positional-test --help`. + - usage: $ command-with-positional-test run data.csv + output: |- + run data.csv + no command + ... + +# missing nested command prints scoped help + + Code + yaml12::write_yaml(snapshot) + Output + --- + app: |- + #!/usr/bin/env Rapp + #| name: nested-required-command-test + + switch('', + #| title: Parent command + parent = { + switch('', + #| title: Child command + child = { cat('child called\n') } + ) + } + ) + invocation: + - usage: $ nested-required-command-test parent + output: |- + Parent command + + Usage: nested-required-command-test parent + + Commands: + child Child command + + For help with a specific command, run: `nested-required-command-test parent --help`. + - usage: $ nested-required-command-test parent child + output: child called + ... + +# optional parent command preserves required child help + + Code + yaml12::write_yaml(snapshot) + Output + --- + app: |- + #!/usr/bin/env Rapp + #| name: optional-parent-required-child-test + + #| required: false + switch(parent_cmd <- '', + #| title: Parent command + parent = { + switch(child_cmd <- NULL, + #| title: Child command + child = { cat('child called\n') } + ) + } + ) + invocation: + - usage: $ optional-parent-required-child-test parent + output: |- + Parent command + + Usage: optional-parent-required-child-test parent + + Commands: + child Child command + + For help with a specific command, run: `optional-parent-required-child-test parent --help`. + - usage: $ optional-parent-required-child-test parent child + output: child called + ... + diff --git a/tests/testthat/apps/kitchen-sink.R b/tests/testthat/apps/kitchen-sink.R index 3111404..1b9fe22 100644 --- a/tests/testthat/apps/kitchen-sink.R +++ b/tests/testthat/apps/kitchen-sink.R @@ -35,6 +35,7 @@ optional_positional <- NULL #| required: false optional_positional_default <- "foo" +#| required: false switch( mode <- "", #| title: Summary Mode diff --git a/tests/testthat/apps/nested-commands.R b/tests/testthat/apps/nested-commands.R index 686456e..59e8989 100644 --- a/tests/testthat/apps/nested-commands.R +++ b/tests/testthat/apps/nested-commands.R @@ -8,6 +8,7 @@ switch( parent_opt <- "parent-default" parent_switch <- TRUE + #| required: false switch( child_cmd <- "", child1 = { diff --git a/tests/testthat/apps/simple-commands.R b/tests/testthat/apps/simple-commands.R index 2002715..0d4a8f8 100644 --- a/tests/testthat/apps/simple-commands.R +++ b/tests/testthat/apps/simple-commands.R @@ -2,6 +2,7 @@ global_opt <- "global_opt_default" +#| required: false switch( cmd <- "", diff --git a/tests/testthat/test-subcommands.R b/tests/testthat/test-subcommands.R index 7abba89..3f1f0cd 100644 --- a/tests/testthat/test-subcommands.R +++ b/tests/testthat/test-subcommands.R @@ -10,12 +10,279 @@ capture_nested_env <- function(args = character()) { capture_app_env(nested_app, args) } +command_invocation <- function(usage, args = character()) { + stopifnot(is.character(usage), length(usage) == 1) + stopifnot(is.character(args)) + + list(usage = usage, args = args) +} + +snapshot_command_runs <- function(app_path, ...) { + invocations <- list(...) + stopifnot(length(invocations) > 0) + + runs <- lapply(invocations, function(invocation) { + output <- capture.output(result <- Rapp::run(app_path, invocation$args)) + + list( + usage = paste0("$ ", invocation$usage), + output = output, + result = result + ) + }) + + snapshot <- list( + app = paste(readLines(app_path), collapse = "\n"), + invocation = lapply(runs, function(run) { + list( + usage = run$usage, + output = paste(run$output, collapse = "\n") + ) + }) + ) + expect_snapshot(yaml12::write_yaml(snapshot)) + + runs +} + test_that("simple app uses defaults without args", { env <- capture_simple_env() expect_identical(env$cmd, "") expect_identical(env$global_opt, "global_opt_default") }) +test_that("missing literal command switch prints help", { + app_path <- local_rapp_app( + c( + "#!/usr/bin/env Rapp", + "#| name: required-command-test", + "#| description: Exercise missing command help.", + "", + "switch('',", + " #| title: List entries", + " list = { cat('list called\\n') }", + ")" + ), + prefix = "rapp-required-command-" + ) + + runs <- snapshot_command_runs( + app_path, + command_invocation("required-command-test"), + command_invocation("required-command-test list", "list") + ) + lines <- runs[[1]]$output + + expect_null(runs[[1]]$result) + expect_identical(runs[[2]]$output, "list called") + + expect_true(any(grepl( + "Usage: required-command-test ", + lines, + fixed = TRUE + ))) + expect_true(any(grepl("Commands:", lines, fixed = TRUE))) + expect_true(any(grepl("list", lines, fixed = TRUE))) +}) + +test_that("missing command assignment prints help by default", { + app_path <- local_rapp_app( + c( + "#!/usr/bin/env Rapp", + "#| name: assigned-command-test", + "#| description: Exercise missing command help.", + "", + "switch(command <- '',", + " #| title: List entries", + " list = { cat(command, '\\n', sep = '') }", + ")" + ), + prefix = "rapp-assigned-command-" + ) + + runs <- snapshot_command_runs( + app_path, + command_invocation("assigned-command-test"), + command_invocation("assigned-command-test list", "list") + ) + lines <- runs[[1]]$output + + expect_null(runs[[1]]$result) + expect_identical(runs[[2]]$output, "list") + expect_identical(runs[[2]]$result$command, "list") + + expect_true(any(grepl( + "Usage: assigned-command-test ", + lines, + fixed = TRUE + ))) + expect_true(any(grepl("Commands:", lines, fixed = TRUE))) + expect_true(any(grepl("list", lines, fixed = TRUE))) + +}) + +test_that("required false command switch allows missing command", { + app_path <- local_rapp_app( + c( + "#!/usr/bin/env Rapp", + "#| name: optional-command-test", + "", + "#| required: false", + "switch(command <- '',", + " #| title: List entries", + " list = { cat(command, '\\n', sep = '') }", + ")", + "cat('no command\\n')" + ), + prefix = "rapp-optional-command-" + ) + + runs <- snapshot_command_runs( + app_path, + command_invocation("optional-command-test"), + command_invocation("optional-command-test list", "list"), + command_invocation("optional-command-test --help", "--help") + ) + + expect_identical(runs[[1]]$output, "no command") + expect_identical(runs[[1]]$result$command, "") + expect_identical(runs[[2]]$output, c("list", "no command")) + expect_identical(runs[[2]]$result$command, "list") + + expect_null(runs[[3]]$result) + expect_true(any(grepl( + "Usage: optional-command-test []", + runs[[3]]$output, + fixed = TRUE + ))) +}) + +test_that("missing command prints help before matching positionals", { + app_path <- local_rapp_app( + c( + "#!/usr/bin/env Rapp", + "#| name: command-with-positional-test", + "", + "#| description: Input path.", + "input <- NULL", + "", + "switch('',", + " #| title: Run command", + " run = { cat('run ', input, '\\n', sep = '') }", + ")", + "cat('no command\\n')" + ), + prefix = "rapp-command-with-positional-" + ) + + runs <- snapshot_command_runs( + app_path, + command_invocation("command-with-positional-test data.csv", "data.csv"), + command_invocation( + "command-with-positional-test run data.csv", + c("run", "data.csv") + ) + ) + lines <- runs[[1]]$output + + expect_null(runs[[1]]$result) + expect_identical(runs[[2]]$output, c("run data.csv", "no command")) + + expect_true(any(grepl( + "Usage: command-with-positional-test ", + lines, + fixed = TRUE + ))) + expect_true(any(grepl("Commands:", lines, fixed = TRUE))) + expect_true(any(grepl("run", lines, fixed = TRUE))) +}) + +test_that("missing nested command prints scoped help", { + app_path <- local_rapp_app( + c( + "#!/usr/bin/env Rapp", + "#| name: nested-required-command-test", + "", + "switch('',", + " #| title: Parent command", + " parent = {", + " switch('',", + " #| title: Child command", + " child = { cat('child called\\n') }", + " )", + " }", + ")" + ), + prefix = "rapp-nested-required-command-" + ) + + runs <- snapshot_command_runs( + app_path, + command_invocation("nested-required-command-test parent", "parent"), + command_invocation( + "nested-required-command-test parent child", + c("parent", "child") + ) + ) + lines <- runs[[1]]$output + + expect_null(runs[[1]]$result) + expect_identical(runs[[2]]$output, "child called") + + expect_true(any(grepl( + "Usage: nested-required-command-test parent ", + lines, + fixed = TRUE + ))) + expect_true(any(grepl("Commands:", lines, fixed = TRUE))) + expect_true(any(grepl("child", lines, fixed = TRUE))) +}) + +test_that("optional parent command preserves required child help", { + app_path <- local_rapp_app( + c( + "#!/usr/bin/env Rapp", + "#| name: optional-parent-required-child-test", + "", + "#| required: false", + "switch(parent_cmd <- '',", + " #| title: Parent command", + " parent = {", + " switch(child_cmd <- NULL,", + " #| title: Child command", + " child = { cat('child called\\n') }", + " )", + " }", + ")" + ), + prefix = "rapp-optional-parent-required-child-" + ) + + runs <- snapshot_command_runs( + app_path, + command_invocation( + "optional-parent-required-child-test parent", + "parent" + ), + command_invocation( + "optional-parent-required-child-test parent child", + c("parent", "child") + ) + ) + lines <- runs[[1]]$output + + expect_null(runs[[1]]$result) + expect_identical(runs[[2]]$output, "child called") + + expect_true(any(grepl( + "Usage: optional-parent-required-child-test parent ", + lines, + fixed = TRUE + ))) + expect_true(any(grepl("Commands:", lines, fixed = TRUE))) + expect_true(any(grepl("child", lines, fixed = TRUE))) +}) + test_that("global option is recognised before and after a command", { env_pre <- capture_simple_env(c("--global-opt", "override", "cmd1")) env_post <- capture_simple_env(c("cmd1", "--global-opt", "late")) diff --git a/tests/testthat/test-todo.R b/tests/testthat/test-todo.R index 396d1f7..63590e0 100644 --- a/tests/testthat/test-todo.R +++ b/tests/testthat/test-todo.R @@ -33,6 +33,21 @@ test_that("todo help output", { ) }) +test_that("todo without a command prints help", { + lines <- run_todo_app() + + expect_true(any(grepl("Todo manager", lines, fixed = TRUE))) + expect_true(any(grepl("Usage: todo [OPTIONS] ", lines, fixed = TRUE))) + expect_true(any(grepl("Commands:", lines, fixed = TRUE))) +}) + +test_that("todo rejects unknown command tokens", { + expect_error( + run_todo_app("lisst", capture = FALSE), + "Arguments not recognized: lisst" + ) +}) + test_that("todo commands update the store", { store <- tempfile(fileext = ".yml") on.exit(unlink(store), add = TRUE)