Repository navigation
fix(parser): recognize options after rest positional items - #12
Merged
Merged
Conversation
A :rest-p positional took every remaining token, so options written
after its first item were collected as positionals: `search foo src
--output count` produced paths ("src" "--output" "count"). This
contradicted the documented interspersed-arguments behavior. The scanner
now takes rest items one token at a time and keeps checking later tokens
as options until a literal `--`; the rest spec is applied once at the end,
so min/max counts, parsing and defaults behave as before.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
A
:rest-ppositional took every remaining token, so options written after its first item were collected as positionals.search foo src --output countproduced paths("src" "--output" "count").docs/src/guide/cli-behavior.md("Interspersed arguments") already promised that command and global options may follow positionals. A consumer (aitools) had to replace its rest positional with 16 optional positionals to work around it.What to review
src/parser-cps.lisp: the mixed-argument scanner takes rest items one token at a time and keeps checking later tokens as options until a literal--. The rest spec is applied once when the tokens run out, so:min-count/:max-count, value parsing and defaults behave as before. Only internal%functions changed arity.nshell build.ns --json) now getscli-unknown-optionand needs--or a:stop-parsing-poption.docs/src/guide/migration-guide.mdnotes this for nshell. Downstream CLIs were not checked.Verification outside CI
t/parser-cps-rest-positional-test.lisp(15 tests) covers long options in both spellings, flags, short clusters, global options after the subcommand,--(including a second--), negative numbers allowed and rejected, zero rest items, min/max counts across split items, unknown options,--help, bare-, and a root-level rest positional. Against the unfixed parser, 13 of them failed.A consumer executable (aitools,
asdf:program-op) builds against this commit, loadingcl-clistarts no threads, andaitools replace x y a.txt --expect-count 4now reads the option after the path.Release
mainalready declares 1.4.0 (asd, demo, README pin) from unreleased feature commits (define-option,define-positional, thecl-cli/concurrentsystem), and v1.4.0 was never tagged. This fix ships in that release.