Merge pull request #21 from cgay/keyed-option

Fix <keyed-option> parsing and add test

Author: cgay

command-line-parser.dylan

@@ -320,6 +320,8 @@ define function tokens-remaining?
   ~parser.parser-tokens.empty?
 end;
 
+// TODO(cgay): This says it returns false-or, but it doesn't. I think it would be an
+// improvement to make it do that and get rid of tokens-remaining?.
 define function peek-token
     (parser :: <command>) => (token :: false-or(<token>))
   unless (tokens-remaining?(parser))
@@ -611,8 +613,8 @@ define function tokenize-args
       end
     end,
     // Add a token to our deque
-    method token (class :: <class>, value :: <string>,
-                  #rest keys, #key, #all-keys) => ()
+    method add-token (class :: <class>, value :: <string>,
+                      #rest keys, #key, #all-keys) => ()
       apply(add-argument-token, parser, class, value, keys);
     end,
     method parse-short-option (arg)
@@ -624,14 +626,13 @@ define function tokenize-args
                 & opt.option-might-have-parameters?
                 & i + 1 < arg.size)
             // Take rest of argument, and use it as a parameter.
-            token(<short-option-token>, name, tightly-bound?: #t);
-            token(<argument-token>,
-                  copy-sequence(arg, start: i + 1));
+            add-token(<short-option-token>, name, tightly-bound?: #t);
+            add-token(<argument-token>, copy-sequence(arg, start: i + 1));
             done();
           else
-            // A solitary option with no parameter.
-            // TODO(cgay): why do we not exit the loop here??
-            token(<short-option-token>, name);
+            // A solitary option with no parameter. Do not exit the loop because there
+            // may be multiple short options strung together as in 'ls -ltR'.
+            add-token(<short-option-token>, name);
           end if;
         end for;
       end block;
@@ -640,22 +641,22 @@ define function tokenize-args
     let arg = pop(args);
     case
       (arg = "=") =>
-        token(<equals-token>, "=");
-        token(<argument-token>, next-arg());
+        add-token(<equals-token>, "=");
+        add-token(<argument-token>, next-arg());
 
       starts-with?(arg, "--") =>
-        token(<long-option-token>, copy-sequence(arg, start: 2));
+        add-token(<long-option-token>, copy-sequence(arg, start: 2));
 
       starts-with?(arg, "-") =>
         if (arg.size = 1)
           // Probably a fake filename representing stdin ('cat -')
-          token(<argument-token>, "-");
+          add-token(<argument-token>, "-");
         else
           parse-short-option(arg);
         end if;
 
       otherwise =>
-        token(<argument-token>, arg);
+        add-token(<argument-token>, arg);
     end case;
   end until;
 end function tokenize-args;

parsers.dylan

@@ -195,14 +195,23 @@ define method parse-option
  => ()
   pop-token(parser);
   let key = pop-token(parser).token-value;
-  let value =
-    if (instance?(peek-token(parser), <equals-token>))
-      pop-token(parser);
-      parse-option-value(pop-token(parser).token-value,
-                         option.option-type)
-    else
-      #t
-    end;
+  let equal-pos = find-key(key, curry(\=, '='));
+  let value = if (equal-pos)
+                // We have `-D key=value` as tokens #("-D", "key=value"). The "key=value"
+                // can't be further broken down into tokens by the tokenizer because it
+                // doesn't know that it is parsing a <keyed-option> at that point, so do
+                // it here.
+                let v = copy-sequence(key, start: equal-pos + 1);
+                key := copy-sequence(key, end: equal-pos);
+                parse-option-value(v, option.option-type);
+              elseif (tokens-remaining?(parser)
+                      & instance?(peek-token(parser), <equals-token>))
+                pop-token(parser);
+                parse-option-value(pop-token(parser).token-value,
+                                   option.option-type)
+              else
+                #t
+              end;
   option.option-value[key] := value;
 end method parse-option;
 

tests/options-test.dylan

@@ -1,5 +1,15 @@
 Module: command-line-parser-test-suite
 
+// Note: when debugging tests it can be useful to wrap them in
+//   block ()
+//     ...
+//   exception (ex :: <usage-error>)
+//     signal(make(<simple-error>, ...))
+//   end;
+// so that Testworks' --debug=crashes works. This is only an issue when debugging
+// command-line-parser tests. We might want to just wrap them all in a macro that
+// does that.
+
 define method parse-one (option, argv)
   let p = make(<command-line-parser>,
                help: "x",
@@ -64,7 +74,32 @@ define test test-parameter-option ()
 end test;
 
 define test test-keyed-option ()
-end;
+  local method opt ()
+          // Make a new one each time to ensure option-value slot cleared.
+          make(<keyed-option>, names: #("define", "D"), help: "help")
+        end;
+  let cmd = parse-one(opt(), #[]);
+  assert-true(empty?(get-option-value(cmd, "define")));
+  assert-true(empty?(get-option-value(cmd, "D")));
+
+  // When no "=value" given value defaults to #t?
+  let cmd = parse-one(opt(), #["-Dkey"]);
+  assert-equal(1, size(get-option-value(cmd, "D")));
+  assert-equal(#t, get-option-value(cmd, "define")["key"]);
+
+  // The rest should all set "key" => "value".
+  for (args in #(#("-Dkey=value"),
+                 #("-D", "key=value"),
+                 #("-D", "key", "=", "value"),
+                 #("--define", "key=value"),
+                 #("--define", "key", "=", "value")))
+    let cmd = parse-one(opt(), args);
+    let table = get-option-value(cmd, "define");
+    assert-true(table);
+    assert-equal(1, size(table));
+    assert-equal("value", table["key"]);
+  end;
+end test;
 
 define test test-repeated-parameter-option ()
 end;