Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle -- correctly #47

Open
cgay opened this issue Jan 3, 2025 · 0 comments
Open

Handle -- correctly #47

cgay opened this issue Jan 3, 2025 · 0 comments

Comments

@cgay
Copy link
Member

cgay commented Jan 3, 2025

(The following is unverified internet content. Please verify before acting on it.)

Currently it looks like -- is handled rather poorly in split-args. It just searches for -- as a token and splits the sequence into "clean args" that precede -- and "extra args" that follow --. This doesn't take into account the possibility of supplying "--" as the value of a named option. That is, --foo=-- and --foo -- should be the same thing. (Unlikely to happen? Probably.)

Also, and more importantly, args following -- appear to be glommed onto the value of the final <option> in the parser (or subcommand). Not sure that that makes any sense. Worse yet, add! is used to add the args and add! makes no guarantee where the value will be added to a sequence; beginning? end? middle? Example:

test-positional-option-parsing failed
  #["e", "--", "f"] = p5.option-value failed [#["e", "--", "f"] and #("f", "e") are not =.
  sizes differ (3 and 2), element 0 is the first non-matching element]

Note that the actual parsed value is out of order: #("f", "e")

$ git diff
diff --git a/tests/options-test.dylan b/tests/options-test.dylan
index c2532da..1d8ee27 100644
--- a/tests/options-test.dylan
+++ b/tests/options-test.dylan
@@ -210,10 +210,10 @@ define test test-positional-option-parsing ()
                  help: "h",
                  options: list(make(<flag-option>, names: "flag", help: "h"),
                                p1, p2, p3, p4, p5));
-  assert-no-errors(parse-command-line(cmd, #["a", "b", "c", "d", "e", "f"]));
+  assert-no-errors(parse-command-line(cmd, #["a", "b", "c", "d", "e", "--", "f"]));
   assert-equal("a", p1.option-value);
   assert-equal("b", p2.option-value);
   assert-equal("c", p3.option-value);
   assert-equal("d", p4.option-value);
-  assert-equal(#["e", "f"], p5.option-value);
+  assert-equal(#["e", "--", "f"], p5.option-value);
 end test;

Pretty sure that <positional-option> values with repeated?: #t are lists and that add! adds to the front of a list.

cgay added a commit to cgay/command-line-parser that referenced this issue Jan 5, 2025
This is the simplest fix I could see for the second part ("Also, and more
importantly...") of dylan-lang#47 ... simply stuff everything after "--" into a new
`unconsumed-arguments` slot in the parser.

While technically this creates a backward incompatibility, the old code resulted in all
the arguments being added to the final option's value list **in reverse order** and it is
highly unlikely that anyone is depending on that broken behavior.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant