boot-clj / boot

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

Use clojure.spec for task options #667

Closed burn2delete closed 6 years ago

burn2delete commented 6 years ago

It would be awesome if we could use clojure.spec instead of the current DSL system.

arichiardi commented 6 years ago

Oh that would be awesome yes!

seancorfield commented 6 years ago

@flyboarder Could you explain what you envisage? clojure.spec is not a string parsing system...

martinklepsch commented 6 years ago

Spec is nice but I actually don't have many gripes with the current option parsing system. 🙂

I think the following questions would be important to answer or at least think about before pushing ahead with something like this:

burn2delete commented 6 years ago

My thought is for cases which are usually solved by the current code option, ie I would like an option to handle multiple data types as valid params. As well it can offload the option DSL logic to the developer to implement via spec. We can provide a small set of specs which support the current system.

martinklepsch commented 6 years ago

@flyboarder Not sure I fully understand what you have in mind. Maybe you could provide some "dreamcode" examples of how usage would look with your envisioned feature added? In my experience this helps understanding a lot :)

seancorfield commented 6 years ago

Yeah, I'm with Martin here @flyboarder -- after reading your recent comment, I still don't understand what you are suggesting or how spec would help. Can you give a specific, concrete example of what the task options and logic would look like today, and what you think it might look like with spec?

As it stands, I can't even imagine what problem you are trying to describe, I'm afraid.

burn2delete commented 6 years ago

Wishful task: (mytask :someopt {:my "data"}) or (mytask :someopt ["still" "valid"])

Current DSL:

[s someopt  VAL [str] "Option description."]
;; cannot use anything other than a vector of strings, unless option is set to code

Wishful DSL:

(spec/def ::myspec (spec/or map? vector?))

[s someopt VAL ::myspec "Option description."]
;; implement custom spec to validate option

Is this example more clear?

seancorfield commented 6 years ago

Nope, that doesn't help at all, sorry.

The [str] DSL says each option will be (converted to) a string and conj'd together to produce a vector (of strings).

How would ::myspec indicate that repeated -s arguments are processed? How could those -s options even became a hash map?

martinklepsch commented 6 years ago

I wouldn't say it doesn't help at all @seancorfield 🙂

It does answer the question about usage from Clojure but Sean is right, it's not obvious how this would inform CLI parsing.

burn2delete commented 6 years ago

I dont envision this changing the string parsing from the current method, it would however provide a means to validate the data after it is parsed from the cli. Boot already handles the conversion to hashmap.

seancorfield commented 6 years ago

But the [str] DSL is what tells Boot how to parse the arguments into a data structure (and how to handle multiple instances of the arguments etc). If you replace [str] with a spec, Boot will no longer know how to parse the strings or produce a data structure.

Instead of [str], consider something like [int] or #{sym} or regex -- I just don't see how you can replace those with a spec and expect Boot to "do the right thing".

burn2delete commented 6 years ago

What the DSL provides is a fixed way to parse and validate. Currently you can implement this behaviour by using code or edn and then using spec within a task. My thought is that Boot could detect the keyword within the DSL and use edn or code as the parser however switch to using the spec as the validator function.

Although using spec within a task might be the more universal alternative, as it doesn't affect boot.

seancorfield commented 6 years ago

Okay, I can see how a spec could be an alternative of a single form of the DSL, and Boot could assume that DSL form and then validate with the spec. However, a spec could only stand in for a single, specific DSL form (since Boot's behavior must be unambiguous) -- so it would seem like it could only be either an alternative for code or edn...?

The question then is which better as the behavior for dealing with a spec: just read-string the input strings (i.e., edn) or read-string and then eval the input strings (i.e., code). I could see an argument either way (but might lean toward code as the most general?).

Then, I guess you'd need to support [::my-spec], #{::my-spec}, etc to allow for Boot to collect multiple instances of arguments into a collection (and read & eval & validate each argument).

And another question would be whether the spec is used just for s/valid? or whether Boot should s/conform the data -- and presumably s/explain if the input argument(s) are not valid / do not conform?

This feels like a very narrow use case that would push a lot of restrictions on Boot (see https://github.com/boot-clj/boot/issues/667#issuecomment-351936491 -- Martin's earlier comment) for relatively little gain over just having a task explicitly use clojure.spec directly.

(and, just as a caveat, I'm a big fan of spec and we've use it heavily in production at work since it became available, but haven't felt any loss of it directly in Boot -- although we do have a macro in our build.boot that has an s/fdef spec)

burn2delete commented 6 years ago

I agree with @seancorfield there might be little gain for this. Currently it would probably be easier to implement a macro within the task that uses spec to validate the opts map, or call spec/valid? for individual options.