Args parsing and command structure #3
Conversation
|
|
||
| ``` clojure | ||
| (sharetribe.flex-cli.core/main-dev-str "process list -m bike-soil") | ||
| ``` |
There was a problem hiding this comment.
Question: Can you use the command line and Emacs REPL interchangeably? For example, if you login in command line, is the Emacs REPL using that same logged in state? Or does this depend on your Emacs config and how it reads env vars etc.?
There was a problem hiding this comment.
The REPL is connected to the app you start from the commandline, so I guess the answer is yes.
| ``` clojure | ||
| (sharetribe.flex-cli.core/main-dev-str "process list -m bike-soil") | ||
| ``` | ||
|
|
There was a problem hiding this comment.
Just a side note: this is clearly a personal preference thing and I think you have a different preference compared to me, but I really dislike long README files. For me they are hard to navigate and contain really important information and very rarely needed info mixed with each other. This makes it hard to find the important things from all the noise.
I like the README to only contain the bare essentials of getting started, then a link to other docs for more info. For example, see the Flex Docs README: https://github.com/sharetribe/flex-docs
This requires a bit more thinking when the docs are added, but I think the benefit of making it more approachable for new devs outweights the drawbacks. No need to do anything now, but we can discuss this at some point.
There was a problem hiding this comment.
I don't have strong. Everything goes.
| :short-opt "-h"} | ||
| {:id :version | ||
| :long-opt "--version" | ||
| :short-opt "-V"}] |
There was a problem hiding this comment.
Is this upper-case on purpose? Is -v reserved for "verbose" etc.?
I tend to type command -v by default to check the version of any CLI tool.
There was a problem hiding this comment.
Yeah lower-case is for "verbose". The lower-case verbose can be used also so that each "v" increments the verbosity level, e.g. -vvv means verbosity level 3.
| (s/def ::opts (s/coll-of ::opt)) | ||
| (s/def ::sub-cmds (s/coll-of ::sub-cmd)) | ||
| (s/def ::handler any?) | ||
| (s/def ::sub-cmd (s/keys :req-un [::name] |
There was a problem hiding this comment.
Hmm, there is no spec for ::name here. Is that a problem? I'm not sure how these things work.
|
|
||
| (defn parse-error [parse-result] | ||
| (doseq [e (-> parse-result :data :errors)] | ||
| (println e)) |
There was a problem hiding this comment.
Should this go to stderr?
|
|
||
| (defn command-not-found [parse-result] | ||
| (println "Command not found:" (-> parse-result :data :arguments first)) | ||
| {:exit-status 1}) |
There was a problem hiding this comment.
Maybe stderr here as well?
I've always used exit status 2 for incorrect script usage that prints the usage help. I was about to suggest it here, but after googling I'm not quite sure: http://tldp.org/LDP/abs/html/exitcodes.html
| ;; how | ||
| ;; the "help" | ||
| ;; command is | ||
| ;; invoked |
There was a problem hiding this comment.
That is some super ugly formatting in the comment. :D Maybe move it to the line above?
| ;; the "help" | ||
| ;; command is | ||
| ;; invoked | ||
| (:version options) (version/version (dissoc options :version)) ;; Same |
There was a problem hiding this comment.
Does this behave correctly with both flex --version (version/version is run) and flex process pull [...] --version (version/version is not run)? I.e. it only checks the version before the subcommand.
| (cond | ||
| (:error parse-result) (error parse-result) | ||
| (= ::main (:handler parse-result)) (main parse-result) | ||
| (:handler parse-result) ((:handler parse-result) (:options parse-result)))] |
There was a problem hiding this comment.
Maybe there could be destructuring above for {:keys [error handler main options]} parse-result to make this a bit cleaner?
|
|
||
| (defn ^:dev/after-load after-load | ||
| "Callback function that is run after code hot loading. Prompts | ||
| command-line args and will call the main function with given args." |
There was a problem hiding this comment.
I don't see how this prompts for anything? Also not sure what kind of workflow that would be in practice?
| @@ -0,0 +1,52 @@ | |||
| (ns sharetribe.flex-cli.parse | |||
There was a problem hiding this comment.
The namespace name is a little ambiguous. args-parse ? args ? Not sure what's better but perhaps getting into the habit of adding at least a short namespace doc str to explain the purpose of the namespace would be a good idea here to use from the beginning.
In this PR: