Merge pull request #68 from jmbarbone/67-flag-na

67 flag na

.lintr

@@ -1,3 +1,5 @@
-linters: linters_with_defaults() # see vignette("lintr")
+linters: linters_with_defaults(
+  indentation_linter = NULL
+  )
 encoding: "UTF-8"
 exclusions: list("tests/spelling.R")

NEWS.md

@@ -1,20 +1,26 @@
 # scribe (development version)
 
+- `flag` action now accepts `NA` as a default [#67](https://github.com/jmbarbone/scribe/issues/67)
+
 # scribe 0.2.0
 
 ## Fixes
 
--   `--help` no longer fails when `scribeArg` has `length(info) > 1` [#59](https://github.com/jmbarbone/scribe/issues/59)
+- `--help` no longer fails when `scribeArg` has `length(info) > 1` [#59](https://github.com/jmbarbone/scribe/issues/59)
 
 ## New features
 
--   `execute` is a new field for `scribeArg` where a function can be called [#63](https://github.com/jmbarbone/scribe/issues/63)
--   `stop` is a new field for `scribeArg` which controls how further arguments are parsed and allows for early stops [#60](https://github.com/jmbarbone/scribe/issues/60)
--   `options()` for `{scribe}` are now listed in `?scribe` documentation and set in `.onAttach()` [#57](https://github.com/jmbarbone/scribe/issues/57)
--   `scribeArgs` can now be given a separate `scribeArg` as a default [#54](https://github.com/jmbarbone/scribe/issues/54)
--   positional arguments now can have default values [#52](https://github.com/jmbarbone/scribe/issues/52)
--   `scribeArgs` with `action = 'flag'` now accept `default = TRUE` [#55](https://github.com/jmbarbone/scribe/issues/55)
+- `execute` is a new field for `scribeArg` where a function can be called [#63](https://github.com/jmbarbone/scribe/issues/63)
+- `stop` is a new field for `scribeArg` which controls how further arguments are parsed and allows for early stops [#60](https://github.com/jmbarbone/scribe/issues/60)
+- `options()` for `{scribe}` are now listed in `?scribe` documentation and set in `.onAttach()` [#57](https://github.com/jmbarbone/scribe/issues/57)
+- `scribeArgs` can now be given a separate `scribeArg` as a default [#54](https://github.com/jmbarbone/scribe/issues/54)
+- positional arguments now can have default values [#52](https://github.com/jmbarbone/scribe/issues/52)
+- `scribeArgs` with `action = 'flag'` now accept `default = TRUE` [#55](https://github.com/jmbarbone/scribe/issues/55) and (when option `no = TRUE`) can also accept `NA` [#67](https://github.com/jmbarbone/scribe/issues/67) 
+
+## Breaking
+
+- `scribeArgs` with `action = "flag"` will now throw an `error` instead of a `warning` when `default` is not `logical(1)` [#68](https://github.com/jmbarbone/scribe/issues/68) 
 
 # scribe 0.1.0
 
--   Added a `NEWS.md` file to track changes to the package.
+- Added a `NEWS.md` file to track changes to the package.

R/arg.R

@@ -132,12 +132,11 @@ arg_initialize <- function( # nolint: cyclocomp_linter.
           default <- FALSE
         }
 
-        if (!(is.logical(default) && length(default) == 1 && !is.na(default))) {
-          warning(
-            "flag must be NULL, TRUE, or FALSE when action=\"flag\"",
+        if (!(is.logical(default) && length(default) == 1)) {
+          stop(
+            "flag must be NULL, TRUE, FALSE, or NA when action=\"flag\"",
             call. = FALSE
           )
-          default <- FALSE
         }
       }
 
@@ -148,6 +147,16 @@ arg_initialize <- function( # nolint: cyclocomp_linter.
       if (n != 0L) {
         stop("n must be 0L when action=\"flag\"", call. = FALSE)
       }
+
+      if (isFALSE(options$no) && is.na(default)) {
+        warning(
+          "default=NA is not a valid default when option no=FALSE",
+          "\nusing to FALSE instead",
+          call. = FALSE
+        )
+
+        default <- FALSE
+      }
     },
     list = {
       # don' assume default choices
@@ -213,7 +222,6 @@ arg_initialize <- function( # nolint: cyclocomp_linter.
     }
   }
 
-
   if (is.logical(stop) && length(stop) == 1L) {
     stop <- if (is.na(stop)) {
       "soft"

tests/testthat/test-class-args.R

@@ -126,6 +126,48 @@ test_that("length(info) > 1 [#57]", {
   expect_output(ca$parse())
 })
 
+test_that("flag can default to NA [#67]", {
+  ca <- command_args()
+  ca$add_argument("--foo", action = "flag", default = NA)
+
+  obj <- ca$parse()
+  exp <- list(foo = NA)
+  expect_identical(obj, exp)
+
+  ca$set_input("--foo")
+  obj <- ca$parse()
+  exp <- list(foo = TRUE)
+  expect_identical(obj, exp)
+
+  ca$set_input("--no-foo")
+  obj <- ca$parse()
+  exp <- list(foo = FALSE)
+  expect_identical(obj, exp)
+
+  # now with 'no' turned off.  this will cause the default to become FALSE
+  ca <- command_args()
+  expect_warning(ca$add_argument(
+    "--foo",
+    action = "flag",
+    default = NA,
+    options = list(no = FALSE)
+  ))
+
+  obj <- ca$parse()
+  exp <- list(foo = FALSE)
+  expect_identical(obj, exp)
+
+  ca$set_input("--foo")
+  obj <- ca$parse()
+  exp <- list(foo = TRUE)
+  expect_identical(obj, exp)
+
+  ca$set_input("--no-foo")
+  expect_warning(obj <- ca$parse())
+  exp <- list(foo = FALSE)
+  expect_identical(obj, exp)
+})
+
 test_that("snapshots", {
   arg <- new_arg("...", info = "help text")
   expect_output(arg$show())

tests/testthat/test-class-command-args.R

@@ -98,18 +98,16 @@ test_that("$add_argument(action = 'flag') [#17]", {
   exp <- list(foo = FALSE)
   expect_identical(obj, exp)
 
-  obj <- command_args("-f")$add_argument("-f", "--foo", action = "flag")$parse()
+  obj <- ca$set_input("-f")$parse()
   exp <- list(foo = TRUE)
   expect_identical(obj, exp)
 
-  obj <- command_args("--foo")$add_argument("-f", "--foo", action = "flag")$parse() # nolint: line_length_linter.
+  obj <- ca$set_input("--foo")$parse()
   exp <- list(foo = TRUE)
   expect_identical(obj, exp)
 
-  # nolint start: line_length_linter.
-  expect_warning(command_args()$add_argument("f", action = "flag", default = "1"))
-  expect_warning(command_args()$add_argument("f", action = "flag", default = "1"))
-  # nolint end: line_length_linter.
+  expect_error(command_args()$add_argument("f", action = "flag", default = "1"))
+  expect_error(command_args()$add_argument("f", action = "flag", default = "1"))
 })
 
 test_that("$add_argument()", {