diff --git a/cl-cli.asd b/cl-cli.asd index 611f38e..522a990 100644 --- a/cl-cli.asd +++ b/cl-cli.asd @@ -172,6 +172,7 @@ (:file "t/package-test") (:file "t/parser-dispatch-test") (:file "t/parser-cps-test") + (:file "t/parser-cps-rest-positional-test") (:file "t/parser-dispatch-property-test") (:file "t/model-helpers-mutation-test") (:file "t/parser-dispatch-fuzz-test") diff --git a/docs/src/guide/cli-behavior.md b/docs/src/guide/cli-behavior.md index 0568f02..53e1653 100644 --- a/docs/src/guide/cli-behavior.md +++ b/docs/src/guide/cli-behavior.md @@ -80,6 +80,28 @@ Command and global options may also appear after command positionals when a consumer CLI expects interspersed arguments, such as `demo compile input.lisp --output out.fasl --verbose`. +This holds for a rest positional (`:rest-p t`) too: it collects the positional +tokens around the options rather than every token after its first item. With +positionals `pattern` and `paths` (rest) and an `--output` option, +`grep search foo src --output count lib` yields `paths` `("src" "lib")` and +`output` `"count"`. Long options (`--output count`, `--output=count`), short +options and clusters, flags, and global options are recognized in any +position. + +Only two things end option parsing: + +- a literal `--`: every later token, including another `--` and any + option-looking token, is positional, so `grep search foo -- --output count` + yields `paths` `("--output" "count")`; +- an option declared with `:stop-parsing-p t` (see below). + +Before that point an option-looking token is always parsed as an option, so an +undeclared one signals `cli-unknown-option` instead of joining the rest +positional. A token like `-5` follows the +[negative-number policy](#negative-number-arguments): with +`:allow-negative-numbers t` it is a positional item, otherwise it is a +short-option cluster. A bare `-` is always positional. + ## Stopping parsing for opaque tails Use `:stop-parsing-p t` for options such as shell `-c COMMAND [ARGS...]` diff --git a/docs/src/guide/migration-guide.md b/docs/src/guide/migration-guide.md index cba9586..17449bd 100644 --- a/docs/src/guide/migration-guide.md +++ b/docs/src/guide/migration-guide.md @@ -100,7 +100,10 @@ Use these `cl-cli` features: - built-in help/version flags - `:stop-parsing-p t` on `-c` - root positionals with a trailing `:rest-p t` positional for script argv — - see the [root positional example](../getting-started.md#root-positional-example) + see the [root positional example](../getting-started.md#root-positional-example). + Options are still parsed after the script name, so script arguments that + look like options go after `--` (`nshell build.ns -- --json`); see + [Interspersed arguments](cli-behavior.md#interspersed-arguments) Existing coverage: root positional parsing is shown in the `script-runner` example in [Getting Started](../getting-started.md#root-positional-example); diff --git a/docs/src/guide/option-values.md b/docs/src/guide/option-values.md index 1e8dd4a..af29238 100644 --- a/docs/src/guide/option-values.md +++ b/docs/src/guide/option-values.md @@ -212,7 +212,10 @@ and a custom `:parser` — plus positional-specific shape: (cl-cli:make-positional :key :files :rest-p t :min-count 1 :max-count 8) ``` -A rest positional (`:rest-p t`) collects every remaining token and can bound +A rest positional (`:rest-p t`) collects every remaining positional token +(options after its first item are still parsed as options; see +[Interspersed arguments](cli-behavior.md#interspersed-arguments) for `--` and +`:stop-parsing-p`) and can bound how many values it accepts with `:min-count` / `:max-count` (too few signals `cli-missing-positional`, too many `cli-unexpected-argument`). A required positional may not follow an optional one — declaring one signals diff --git a/src/parser-cps.lisp b/src/parser-cps.lisp index 0b146f3..a748817 100644 --- a/src/parser-cps.lisp +++ b/src/parser-cps.lisp @@ -39,42 +39,66 @@ token streams without additional dynamic stack frames." (%scan-options-prefix validated-specs table tokens initial-values :dispatch nil #'values)))) -(defun %scan-mixed-arguments-consume-positional (pending positional-values remaining) - "Consume one positional token, or signal CLI-USAGE-ERROR when none is pending. +(defun %scan-mixed-arguments-consume-positional + (pending positional-values rest-tokens remaining) + "Consume the first token of REMAINING as the next pending positional, or +signal CLI-USAGE-ERROR when none is pending. -Returns (values new-pending new-positional-values new-remaining)." +A rest positional takes one token at a time and stays pending: the token is +pushed onto REST-TOKENS (most recent first) and the spec is applied only when +the scan ends, so options after the rest positional's first item are still +recognized. + +Returns (values new-pending new-positional-values new-rest-tokens +new-remaining)." (when (null pending) (signal-unexpected-positionals remaining)) (let ((spec (first pending))) - (multiple-value-bind (new-positional-values new-remaining) - (apply-positional-spec spec positional-values remaining) - (values (if (positional-rest-p spec) nil (rest pending)) - new-positional-values - new-remaining)))) + (if (positional-rest-p spec) + (values pending positional-values + (cons (first remaining) rest-tokens) + (rest remaining)) + (multiple-value-bind (new-positional-values new-remaining) + (apply-positional-spec spec positional-values remaining) + (values (rest pending) new-positional-values rest-tokens + new-remaining))))) (defun %scan-mixed-arguments (validated-specs table remaining pending option-values - positional-values action literal-mode-p on-done) + positional-values rest-tokens action literal-mode-p + on-done) "Scan REMAINING as an interleaved option/positional stream, calling ON-DONE with the final (pending option-values positional-values action) once every token is consumed. +Option tokens are recognized anywhere until a literal \"--\" or a +stop-parsing option switches to LITERAL-MODE-P, including after a rest +positional has started collecting: REST-TOKENS holds its items (most recent +first) and is applied to the pending rest spec when REMAINING runs out. + Genuine continuation-passing style, mirroring %SCAN-OPTIONS-PREFIX: every terminal case calls ON-DONE, and every other case is a tail call threading the full scan state through explicit arguments. Implementations that optimize tail calls can process long token streams without additional dynamic stack frames." (cond ((null remaining) - (funcall on-done pending option-values positional-values action)) + (if rest-tokens + (funcall on-done nil option-values + (apply-positional-spec (first pending) positional-values + (reverse rest-tokens)) + action) + (funcall on-done pending option-values positional-values action))) ((and (not literal-mode-p) (string= (first remaining) "--")) (%scan-mixed-arguments validated-specs table (rest remaining) pending - option-values positional-values action t on-done)) + option-values positional-values rest-tokens action t + on-done)) (literal-mode-p - (multiple-value-bind (new-pending new-positional-values new-remaining) + (multiple-value-bind (new-pending new-positional-values new-rest-tokens + new-remaining) (%scan-mixed-arguments-consume-positional pending positional-values - remaining) + rest-tokens remaining) (%scan-mixed-arguments validated-specs table new-remaining new-pending - option-values new-positional-values action t - on-done))) + option-values new-positional-values new-rest-tokens + action t on-done))) (t (multiple-value-bind (status new-values new-remaining new-action) (%consume-option-token-step (first remaining) validated-specs table @@ -82,15 +106,17 @@ calls can process long token streams without additional dynamic stack frames." (case status ((:done :continue) (%scan-mixed-arguments validated-specs table new-remaining pending - new-values positional-values new-action - (eq status :done) on-done)) + new-values positional-values rest-tokens + new-action (eq status :done) on-done)) (t - (multiple-value-bind (new-pending new-positional-values new-remaining) + (multiple-value-bind (new-pending new-positional-values new-rest-tokens + new-remaining) (%scan-mixed-arguments-consume-positional pending positional-values - remaining) + rest-tokens remaining) (%scan-mixed-arguments validated-specs table new-remaining new-pending option-values new-positional-values - action literal-mode-p on-done)))))))) + new-rest-tokens action literal-mode-p + on-done)))))))) (defun %validate-mixed-option-relationships (app command option-values validated-specs) @@ -133,7 +159,7 @@ calls can process long token streams without additional dynamic stack frames." (multiple-value-bind (validated-specs table) (prepare-option-parser-state app option-specs cache) (%scan-mixed-arguments - validated-specs table tokens positional-specs initial-option-values nil + validated-specs table tokens positional-specs initial-option-values nil nil :dispatch nil (lambda (pending option-values positional-values action) (%finalize-mixed-parse app command validated-specs pending diff --git a/t/parser-cps-rest-positional-test.lisp b/t/parser-cps-rest-positional-test.lisp new file mode 100644 index 0000000..c449f9d --- /dev/null +++ b/t/parser-cps-rest-positional-test.lisp @@ -0,0 +1,128 @@ +(in-package :cl-cli/test) + +(defun %rest-search-app (&key (allow-negative-numbers nil) min-count max-count) + (make-app + :name "grep" + :allow-negative-numbers allow-negative-numbers + :global-options (list (flag-option "verbose" :short #\v)) + :commands + (list (make-command + :name "search" + :options (list (value-option "output" :short #\o) + (flag-option "ignore-case" :short #\i)) + :positionals + (list (make-positional :key :pattern :required-p t) + (make-positional :key :paths :rest-p t + :min-count min-count + :max-count max-count)))))) + +(describe-sequential "rest positionals and interspersed options" + (it "recognizes a separated long value option after a rest positional" + (with-parsed-argv (inv (%rest-search-app) + '("grep" "search" "foo" "src" "--output" "count")) + (expect (equal (positional-value inv :pattern) "foo")) + (expect (equal (positional-value inv :paths) '("src"))) + (expect (equal (option-value inv :output) "count")))) + + (it "recognizes an attached long value option after a rest positional" + (with-parsed-argv (inv (%rest-search-app) + '("grep" "search" "foo" "src" "--output=count")) + (expect (equal (positional-value inv :paths) '("src"))) + (expect (equal (option-value inv :output) "count")))) + + (it "recognizes short options and flags after a rest positional" + (with-parsed-argv (inv (%rest-search-app) + '("grep" "search" "foo" "src" "lib" "-i" "-o" "count")) + (expect (equal (positional-value inv :paths) '("src" "lib"))) + (expect (eq (option-value inv :ignore-case) t)) + (expect (equal (option-value inv :output) "count")))) + + (it "recognizes a long flag and a short cluster value between rest items" + (with-parsed-argv (inv (%rest-search-app) + '("grep" "search" "foo" "a" "--ignore-case" "b" "-ocount" "c")) + (expect (equal (positional-value inv :paths) '("a" "b" "c"))) + (expect (eq (option-value inv :ignore-case) t)) + (expect (equal (option-value inv :output) "count")))) + + (it "recognizes a global option after the subcommand's rest positional" + (with-parsed-argv (inv (%rest-search-app) + '("grep" "search" "foo" "src" "--verbose" "lib" "-v")) + (expect (equal (positional-value inv :paths) '("src" "lib"))) + (expect (eq (option-value inv :verbose) t)))) + + (it "keeps option-looking tokens after a literal -- in the rest positional" + (with-parsed-argv (inv (%rest-search-app) + '("grep" "search" "foo" "src" "--" "--output" "count" "-i" "--")) + (expect (equal (positional-value inv :paths) '("src" "--output" "count" "-i" "--"))) + (expect (null (option-value inv :output))) + (expect (null (option-value inv :ignore-case))))) + + (it "still parses options before a literal -- that follows rest items" + (with-parsed-argv (inv (%rest-search-app) + '("grep" "search" "foo" "src" "-i" "--" "-v")) + (expect (equal (positional-value inv :paths) '("src" "-v"))) + (expect (eq (option-value inv :ignore-case) t)) + (expect (null (option-value inv :verbose))))) + + (it "leaves a rest positional empty when only options follow the last required positional" + (with-parsed-argv (inv (%rest-search-app) + '("grep" "search" "foo" "--output" "count")) + (expect (equal (positional-value inv :pattern) "foo")) + (expect (null (positional-value inv :paths))) + (expect (equal (option-value inv :output) "count")))) + + (it "enforces rest :min-count and :max-count over tokens split by options" + (signals cli-missing-positional + (parse-argv (%rest-search-app :min-count 1) + '("grep" "search" "foo" "--output" "count"))) + (signals cli-unexpected-argument + (parse-argv (%rest-search-app :max-count 1) + '("grep" "search" "foo" "a" "--output" "count" "b"))) + (with-parsed-argv (inv (%rest-search-app :min-count 2 :max-count 2) + '("grep" "search" "foo" "a" "-i" "b")) + (expect (equal (positional-value inv :paths) '("a" "b"))))) + + (it "rejects an unknown option after a rest positional instead of collecting it" + (signals cli-unknown-option + (parse-argv (%rest-search-app) + '("grep" "search" "foo" "src" "--no-such-option")))) + + (it "honors --help after rest positional items" + (with-parsed-argv (inv (%rest-search-app) + '("grep" "search" "foo" "src" "--help")) + (expect (eq (invocation-action inv) :help)))) + + (it "applies the negative-number policy to tokens after a rest positional" + (with-parsed-argv (inv (%rest-search-app :allow-negative-numbers t) + '("grep" "search" "foo" "-5" "src" "-1.5" "-i")) + (expect (equal (positional-value inv :paths) '("-5" "src" "-1.5"))) + (expect (eq (option-value inv :ignore-case) t))) + (signals cli-unknown-option + (parse-argv (%rest-search-app) + '("grep" "search" "foo" "src" "-5")))) + + (it "keeps a bare - in the rest positional while parsing later options" + (with-parsed-argv (inv (%rest-search-app) + '("grep" "search" "foo" "-" "--output" "count")) + (expect (equal (positional-value inv :paths) '("-"))) + (expect (equal (option-value inv :output) "count")))) + + (it "passes option-looking script arguments through -- in the nshell example" + (with-parsed-argv (inv (make-example-app "MAKE-NSHELL-APP") + '("nshell" "build.ns" "a" "--" "--json")) + (expect (equal (positional-value inv :script) "build.ns")) + (expect (equal (positional-value inv :script-argv) '("a" "--json")))) + (signals cli-unknown-option + (parse-argv (make-example-app "MAKE-NSHELL-APP") + '("nshell" "build.ns" "--json")))) + + (it "recognizes global options after a root rest positional" + (with-parsed-argv (inv (make-app :name "tool" + :global-options (list (flag-option "verbose" :short #\v) + (value-option "output")) + :positionals (list (make-positional :key :files + :rest-p t))) + '("tool" "a" "--output" "x" "b" "-v")) + (expect (equal (positional-value inv :files) '("a" "b"))) + (expect (equal (option-value inv :output) "x")) + (expect (eq (option-value inv :verbose) t))))) diff --git a/t/parser-cps-test.lisp b/t/parser-cps-test.lisp index 487ca20..6e0b925 100644 --- a/t/parser-cps-test.lisp +++ b/t/parser-cps-test.lisp @@ -35,7 +35,7 @@ (let ((positionals (list (make-positional :key :input :required-p t)))) (cl-weave:with-continuation-values (captured next called-p) (cl-cli::%scan-mixed-arguments - nil nil '("--" "input") positionals nil nil :dispatch nil #'next) + nil nil '("--" "input") positionals nil nil nil :dispatch nil #'next) (expect called-p) (expect (null (first captured))) (expect (null (second captured)))