boot-clj / boot

Build tooling for Clojure.
https://boot-clj.github.io/
Eclipse Public License 1.0
1.75k stars 182 forks source link

Options DSL doesn't handle type spec using a set as expected #510

Open danielschranz opened 8 years ago

danielschranz commented 8 years ago

For a task I'm currently writing I tried to specify the parameter type as #{kw regex} expecting it to be a set that can contain keywords and/or regexps.

I did a quick experiment by replacing assert-type (https://github.com/boot-clj/boot/blob/master/boot/core/src/boot/cli.clj#L65) with the following:

(declare assert-type)

(defn- assert-any-type [types arg]
  (some #(assert-type % arg) types))

(defn- assert-type [type args]
  (cond
    (symbol? type) ((assert-atom type) args)
    (set? type) (map (partial assert-any-type type) args)
    :else (every? identity (mapv assert-type type args))))

Which seems to work but now I'm actually unsure if this is the correct interpretation. The type specification of #{kw regex} could also be interpreted as a single parameter that can be a keyword or a regexp.

The wiki only states Options can also be declared as collections of these primitive types. Sets, maps, and vectors are supported. but doesn't specify the semantics.

In any case - and if I'm reading the code correctly - the current implementation (see https://github.com/boot-clj/boot/blob/master/boot/core/src/boot/cli.clj#L68) doesn't make sense for a type specification like #{kw regex} as it checks each parameter in the order it pulls the types out of the set. (similar for maps?)

Is this a case where the task should specify the parameter more generic as edn or code and handle type checking itself?

zilti commented 8 years ago

I, too, would be interested in a solution for this problem.

micha commented 7 years ago

@danielschranz The purpose of the type declaration is to facilitate an easy to understand relationship between command line arguments (which can only be strings) and kw args in the REPL. It's designed primarily for simplicity and clarity. It is definitely not designed to be a replacement for something like clojure.spec.

There is no way for the command line argument parser to know if the string ":foo" is a keyword or a regular expression, for example.

There are a few options for your use case:

  1. Use the edn or code types as you mentioned. This means that at the command line the user would need to do some awkward quoting, like boot yourtask -o '#":foo"'. Personally I always do everything I can do to avoid that, because multiple layers of quoting gets really hairy. This solution would require assertions to enforce type correctness.
  2. Declare the option as untyped (using the ^:! meta on the type, like ^:! #{str}), and perform the coercion using your own logic. The metadata disables runtime type checking of the kw args when invoked from Clojure (ie. you can pass any type as the option arg from Clojure). This avoids the awkward quoting issues but does require that you are able to distinguish between kw strings and regex strings by some heuristic (since all command line option arguments will be parsed as strings), and it also will require assertions to enforce type correctness.
  3. Split the option into two options, one for kws and one for regexps. This might be the best solution for the user because it seems like the simplest one to understand, and no extra quoting is required on the command line. This solution does not require extra assertions to enforce type correctness.

I myself like the 3rd option the best, I think.

danielschranz commented 7 years ago

Really sorry I didn't see the response earlier - didn't get my usual email notification. I'll have a think which of your solutions would work best in my case. Thanks for your input!