From 0810f1e82bd9ae101100bd19b7bc7ec4ce405a06 Mon Sep 17 00:00:00 2001 From: Carl Gay Date: Sat, 4 Jan 2025 19:56:39 -0500 Subject: [PATCH] Better handling of "--" This is the simplest fix I could see for the second part ("Also, and more importantly...") of #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. --- command-line-parser.dylan | 31 ++++++++++--------------------- library.dylan | 1 + tests/options-test.dylan | 17 +++++++++++++++++ 3 files changed, 28 insertions(+), 21 deletions(-) diff --git a/command-line-parser.dylan b/command-line-parser.dylan index df78bc4..e432804 100644 --- a/command-line-parser.dylan +++ b/command-line-parser.dylan @@ -114,6 +114,8 @@ define abstract class () slot command-subcommands :: = #[], init-keyword: subcommands:; slot selected-subcommand :: false-or() = #f; + // All arguments following the first "--" positional argument, if any. + slot unconsumed-arguments :: = #(); end class; // The --help option is added by default but we provide a way to turn it off here. @@ -598,8 +600,10 @@ end; //====================================================================== // Break up our arguments around '--' in the traditional fashion. -define function split-args(argv) +define function split-args (argv) => (clean-args :: , extra-args :: ) + // TODO(cgay): A "--foo --" should be valid. + // https://github.com/dylan-lang/command-line-parser/issues/47 let splitter = find-key(argv, curry(\=, "--")); if (splitter) let clean-args = copy-sequence(argv, end: splitter); @@ -710,9 +714,9 @@ define open generic parse-command-line (parser :: , argv :: ) => (); -// Parse the command line, side-effecting the parser, its options, and its -// subcommands with the parsed values. `args` is a sequence of command line -// arguments (strings). It must not include the command name. +// Parse the command line, side-effecting the parser, its options, and its subcommands +// with the parsed values. `args` is a sequence of command line arguments (strings) that +// does not include the command name. define method parse-command-line (parser :: , args :: ) => () @@ -721,24 +725,9 @@ define method parse-command-line let chopped-args = chop-args(clean-args); tokenize-args(parser, chopped-args); process-tokens(program-name(), parser, #f); - if (~empty?(extra-args)) - // Append any more positional options from after the '--'. If there's a - // subcommand the extra args go with that. - // (This feels hackish. Can we handle this directly in process-tokens?) let command = parser.selected-subcommand | parser; - let option = last(command.command-options); - if (~(instance?(option, ) - & option.option-repeated?)) - let opts = command.positional-options; - usage-error("Only %d positional argument%s allowed.", - opts.size, - if (opts.size = 1) "" else "s" end); - end; - for (arg in extra-args) - option.option-value := add!(option.option-value, - parse-option-value(arg, option.option-type)); - end for; + command.unconsumed-arguments := extra-args; end; end method; @@ -838,7 +827,7 @@ end function; Parameterless options: -b, --bar, --no-bar - Present or absent. May have opposites; latter values override + Present or absent. May have opposites; later values override previous values. Parameter options: diff --git a/library.dylan b/library.dylan index 398cd20..6286b2e 100644 --- a/library.dylan +++ b/library.dylan @@ -70,6 +70,7 @@ define module command-line-parser parse-command-line, get-option-value, print-help, + unconsumed-arguments, parse-option-value; diff --git a/tests/options-test.dylan b/tests/options-test.dylan index c2532da..efb7209 100644 --- a/tests/options-test.dylan +++ b/tests/options-test.dylan @@ -217,3 +217,20 @@ define test test-positional-option-parsing () assert-equal("d", p4.option-value); assert-equal(#["e", "f"], p5.option-value); end test; + +define test test-unconsumed-arguments () + let p1 = make(, name: "p1", help: "?", required?: #f); + let cmd1 = make(, help: "?", options: list(p1)); + assert-no-errors(parse-command-line(cmd1, #["x", "--", "y", "z"])); + assert-equal(#["y", "z"], cmd1.unconsumed-arguments); + assert-no-errors(parse-command-line(cmd1, #["--", "y", "z"])); + assert-equal(#["y", "z"], cmd1.unconsumed-arguments); + + // Same assertions but with the final positional option allowing more than one arg. + let p2 = make(, name: "p1", help: "?", required?: #f, repeated?: #t); + let cmd2 = make(, help: "?", options: list(p2)); + assert-no-errors(parse-command-line(cmd2, #["x", "--", "y", "z"])); + assert-equal(#["y", "z"], cmd2.unconsumed-arguments); + assert-no-errors(parse-command-line(cmd2, #["--", "y", "z"])); + assert-equal(#["y", "z"], cmd2.unconsumed-arguments); +end test;