fix(parser): recognize options after rest positional items

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")

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...]`

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);

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

src/parser-cps.lisp

@@ -39,58 +39,84 @@ 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
                                      option-values action remaining)
        (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

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)))))

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)))