Consider de-deprecating `:collect`

[lread]

Scenario

I am experimenting with migrating antq from using clojure.tools.cli to babashka.cli.

To not break compatibility for antq users I need to support repeated values separated by colons.

For example:

--foo a:b --foo c:d:e --foo f

Should result in a parsed {:foo ["a" "b" "c" "d" "e" "f"]}

One of the args with repeated values is validated, so it would be convenient to do the work within bb cli and take advantage of bb cli's :validate.

Exploration

I had a peek at babashka.cli and noticed its deprecated :collect might support this scenario.

My initial attempt seemed to work:

(require '[babashka.cli :as cli]
         '[clojure.string :as str])

(cli/parse-opts ["--foo" "a:b" "--foo" "c:d:e" "--foo" "f"]
                {:coerce {:foo #(str/split % #":")}
                 :collect {:foo concat}})
;; => {:foo ("a" "b" "c" "d" "e" "f")}

On Slack, borkdude mentioned this might be considered hacky because :coerce was intended to turn a single value into a single value. He offered up an alternative:

(cli/parse-opts ["--foo" "a:b" "--foo" "c:d:e" "--foo" "f"]
                {:collect {:foo (fn [coll foo]
                                  (into (or coll [])
                                        (str/split foo #":")))}})
;; => {:foo ["a" "b" "c" "d" "e" "f"]}

Problem with :spec (solved)

I noticed that, probably because it is deprecated, :collect does not work with :spec.

Borkdude made a fix for this 6a90c55, and I have verified that it works for both the above techniques

(cli/parse-opts ["--foo" "a:b" "--foo" "c:d:e" "--foo" "f"]
                {:spec {:foo  {:coerce #(str/split % #":")
                               :collect concat}}})
;; => {:foo ("a" "b" "c" "d" "e" "f")}


(cli/parse-opts ["--foo" "a:b:c" "--foo" "d:e:f"]
                {:spec {:foo
                        {:collect (fn [coll foo] 
                                    (into (or coll []) 
                                          (str/split foo #":")))}}})
;; => {:foo ["a" "b" "c" "d" "e" "f"]}

Options?

We might have jumped the gun with a solution, but that's probably ok.

Options might have been:

Option 1 - Do nothing, won't fix

Moot. 6a90c55 allows me to proceed via option 3 or 4.

Option 2 - Explicitly support scenario of splitting arg values

Babashka.cli already supports this scenario: --foo a b c --foo c d e --foo f:

  (cli/parse-opts ["--foo" "a" "b" "--foo" "c" "d" "e" "--foo" "f"]
                  {:coerce {:foo [:string]}})
  ;; => {:foo ["a" "b" "c" "d" "e" "f"]}

I realize the shell has split the command line args here, but
maybe also support splitting arg values through some new bb cli option?
:split #(str/split % #":")
This isn't terribly different than the trials above.

Pros:

• Clearly untangles from coercion (or collection).
• Avoids de-deprecating :collect.

Cons:

• Formalizing this might be too niche?
• Another invention to support.
• We'd probably want these kinds of args are generally cross-os shell friendly, I think they are, but Windows often surprises.

Option 3 - Use :coerce with :collect

See my initial example above.

Pros:

• works!

Cons:

• A bit of a hack
• Need to de-deprecate :collect

Opton 4 - Use :collect

See borkdude's example above.

Pros:

• works!
• Not hacky

Cons:

• Need to de-deprecate :collect

Decision

On Slack, we chose option 4.
Option 2 might be worth evaluating, but I don't feel at all strongly about it.
I'm happy with option 4.
Do you agree @borkdude?

Next Up

After agreement, I will create a PR that will close out this issue:

1. docs: de-deprecate :collect with an example. Will describe as niche.
2. tests: add test(s), probably under spec-test
3. docs: changelog

[borkdude]

I don't think we need a new option, but docs would be great