malli-cli icon indicating copy to clipboard operation
malli-cli copied to clipboard

`:optional` schema property doesn't work properly

Open neuromantik33 opened this issue 3 years ago • 3 comments

In your example running the validate and explain (with humanize), it seems that the optional keyword doesn't work.

(->> (m/decode Config [] cli/cli-transformer)
     (m/explain Config)
     me/humanize)
=> {:show-config? ["missing required key"], :help ["missing required key"], :proxy ["missing required key"]}

Your tests with a similar use-case pass because the optional options have a :default false entry.

Also the test suite is failing

$ lein test

lein test piotr-yuxuan.domain.gnu-test

lein test piotr-yuxuan.domain.posix-test

lein test piotr-yuxuan.malli-cli-test

lein test :only piotr-yuxuan.malli-cli-test/args-transformer-test

FAIL in (args-transformer-test) (malli_cli_test.clj:241)
expected: (= (m/decode MyCliSchema cli-args (mt/transformer malli-cli/cli-transformer)) {:log-level :all, :help false, :upload-api "http://localhost:8080", :database "http://localhost:8888", :async-parallelism 64, :create-market-dataset false})
  actual: (not (= {:log-level :all, :upload-api "http://localhost:8080", :database "http://localhost:8888", :async-parallelism 64} {:log-level :all, :help false, :upload-api "http://localhost:8080", :database "http://localhost:8888", :async-parallelism 64, :create-market-dataset false}))

Not sure if it is malli issue or here but I'm really hoping I can use this lib to no longer have to use tools.cli or omniconf.

Thanks in advance

neuromantik33 avatar Dec 02 '21 22:12 neuromantik33

This updated config seems to work, but is it a typo in the README?

(def Config
  (m/schema
    [:map {:closed true, :decode/args-transformer cli/args-transformer}
     [:show-config? {:optional true} [boolean? {:description "Print actual configuration value and exit."
                                                :arg-number  0}]]
     [:help {:optional true} [boolean? {:description  "Display usage summary and exit."
                                        :short-option "-h"
                                        :arg-number   0}]]
     [:upload-api [string? {:description  "Address of target upload-api instance. If not set from the command line, lookup env var $CORP_UPLOAD_API."
                            :short-option "-a"
                            ;; Cli option will be looked up, then env var, then default.
                            :env-var      "CORP_UPLOAD_API"
                            :default      "http://localhost:8080"}]]
     [:log-level [:enum {:description             "Non-idempotent -v increases log level, --log-level sets it."
                         ;; Express how to decode a string into an enum value.
                         :decode/string           keyword
                         :short-option            "-v"
                         :short-option/arg-number 0
                         :short-option/update-fn  cli/non-idempotent-option
                         :default                 :error
                         ;; Used in summary to pretty-print the default value to a string.
                         :default->str            name}
                  :off :fatal :error :warn :info :debug :trace :all]]
     [:proxy [:map
                [:host string?]
                ;; malli will parse and return an integer.
                [:port pos-int?]]]]))

neuromantik33 avatar Dec 02 '21 23:12 neuromantik33

I'm sorry it took me so long to become aware that you had open an issue! Looking into it now.

piotr-yuxuan avatar Jan 02 '22 12:01 piotr-yuxuan

Ho, you're completely right @neuromantik33, indeed it is a typo! I can fix it, or perhaps you would prefer to open a PR and be credited for this? (if so, please keep the change as minimal as possible, and I'm open to change my mind in separate issues about map value indentation or alias change from malli-cli to cli)

piotr-yuxuan avatar Jan 02 '22 12:01 piotr-yuxuan

Hello @neuromantik33 thank you for opening an issue. I believe the example has been fixed.

piotr-yuxuan avatar Jun 07 '23 19:06 piotr-yuxuan