<< Back to previous view

[TCLI-10] potential incorrect parsing Created: 23/Jul/14  Updated: 23/Jul/14

Status: Open
Project: tools.cli
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Travis Camechis Assignee: Gareth Jones
Resolution: Unresolved Votes: 0
Labels: None


 Description   

If i have these options defined

(def cli-options [ ["-i" "--interactive" "interactive cli mode"]
["-H" "--help" "show help"]
["h" "-host host" "hostname"
["-u" "--user user" "username to login to rulegate"]])

if I enter a command as follows

myprog -h myhost -u foo

my option output looks like { :host myhost :user foo }

however if I leave the host value off
{:host -u}

It appears to be treating the -u as the value for -h. I would expect a missing argument error. Is this the correct behavior?






[TCLI-9] Support multiple validations Created: 08/Feb/14  Updated: 22/Apr/14

Status: Open
Project: tools.cli
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Jean Niklas L'orange Assignee: Gareth Jones
Resolution: Unresolved Votes: 0
Labels: None


 Description   

In many cases, I have input arguments where I want to perform multiple validations, and return different error messages based upon that. Would it be reasonable to have multiple validation functions, or being able to have a validation function returning nil if no errors exists, and an error string if not?

If this seems like an interesting idea, I believe that the easiest and simplest solution would be to support a single validation function returning the actual error message. The reationale for this is that multiple validations may require a complex rule system (ordering? should it short-circuit, or should it return multiple error messages?), and as such, it's better to have those rules evident in the validation function.



 Comments   
Comment by Sung Pae [ 13/Feb/14 10:35 PM ]

My apologies for this delayed response. I thought that I would be emailed on creation of new issues, but this does not appear to be the case. This is the second time this has happened, so I will clearly have to rig something up to keep on top of this board.

> In many cases, I have input arguments where I want to perform multiple validations, and return different error messages based upon that. Would it be reasonable to have multiple validation functions, or being able to have a validation function returning nil if no errors exists, and an error string if not?

> If this seems like an interesting idea, I believe that the easiest and simplest solution would be to support a single validation function returning the actual error message. The reationale for this is that multiple validations may require a complex rule system (ordering? should it short-circuit, or should it return multiple error messages?), and as such, it's better to have those rules evident in the validation function.

The single largest reason I opted against implementing validations like this was that I dislike the corresponding linguistic and syntactic awkwardness of inverting the logic:

:validate [pred "Must satisfy pred"] ;; Error message is optional

vs

:validate #(when-not (pred %) "Must satisfy pred")

The first approach looks like a standard assertion, while the second requires an explicit branch and must return a truthy value to register a failure.¹

Now, the second example is more powerful for the reasons you outline, but since the vast majority of command line option arguments are semantically simple values such as numbers, filenames, and hostnames, I chose to optimize for the common case in order to have the terser syntax.

That said, I am happy to be convinced otherwise. Could you please provide an example of an option specification where returning different error messages for different failures is significantly better than something like the following?

["o" "-option EXAMPLE"
:validate [#(and (foo? %) (bar? %) (baz? %))
"Must be a foo that is also a bar and a baz"]]

I am currently of the mind that a per-option error summary is sufficient for an advanced user interface like the command line, and becomes complete with a link to a documentation URL (or man page).

Thank you.

¹ I usually name functions of the second type `validation-errors` instead of `validate` because I find this confusing:

(when-not (validate input) "This seems awkward.")

Comment by Jean Niklas L'orange [ 14/Feb/14 9:42 AM ]

Hello Sung,

No worries about the response time.

It's not necessarily the fact that it is more verbose, just that I would like to give good, descriptive error messages back. While it's easy for each of the different errors, I think it's better to say exactly what the problem is. For instance, if I want to take in an input directory path, I want to ensure that is is a legal path, and if the file already exists, it must be a directory:

["-i" "--input-directory DIR"
 :validate 
 #(if-not (ok-path? %)
    (format "Input directory path '%s' is malformed" %)
    (let [f (File. %)]
      (if (and (.exists f) (not (.isDirectory f)))
        (format "Input directory '%s' is a file, not a directory" %))))]

However, I can understand that such functionality can be a bit over the top for a CLI library. As long as you've compared/considered the different possibilities, I am content with whatever choice you take.

Comment by Sung Pae [ 17/Feb/14 11:17 PM ]

Saying yes to this kind of enhancement should be quite easy, but I only have a single reservation: adding a second type of validation function that is a logical inverse of the existing type seems confusing.

What do you think about using exceptions to carry error messages?

https://gist.github.com/guns/9065033
(gaah I don't know how to use JIRA at all)

The existing implementation already catches Exceptions, throws away the error message, and returns nil. This is more verbose than fn -> optarg -> nil | String, but it is aligned to the current logical convention.

Comment by Sung Pae [ 22/Apr/14 12:21 AM ]

Hello Jean,

I've implemented your first suggestion (support multiple validations) and pushed it to master. I think it's a nice compromise.

From the patch header:

This is an implemenation (sic) of the first suggestion, multiple validation
functions.

The :validation entry can now have multiple entries, which are
partitioned into pairs, then split into the vector entries
:validation-fn and :validation-msg. It would be nice to have
a pluralized key name here, but we decline this for backwards
compatibility.

Non-collection values of :validate-fn and :validate-msg continue to be
supported by conditional wrapping.

Example:

    [["-f" "--file PATH"
      :parse-fn io/file
      :validate [#(.exists %) "File does not exist"
                 #(.isFile %) "Path is not a regular file"
                 #(.canRead %) "File is unreadable"]]]

Addresses TCLI-9.

https://github.com/clojure/tools.cli/commit/56ecd5f305d444bbf6dcd28f4f4adce58ebc0de4





[TCLI-8] Correct test that trivially always passes Created: 31/Dec/13  Updated: 02/Jan/14  Resolved: 02/Jan/14

Status: Resolved
Project: tools.cli
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Trivial
Reporter: Andy Fingerhut Assignee: Sung Pae
Resolution: Completed Votes: 0
Labels: None

Attachments: File tcli-8-v1.diff    

 Description   

The following tools.cli test always trivially passes, because (= expr) is always true:

(is (= {:verbose false}))

Found using pre-release version of Eastwood Clojure lint tool.



 Comments   
Comment by Andy Fingerhut [ 31/Dec/13 1:57 AM ]

Patch tcli-8-v1.diff corrects the test.

Comment by Sung Pae [ 02/Jan/14 1:33 AM ]

Thank you, patch applied!





[TCLI-7] summarize throws when called with an empty sequence of options Created: 17/Dec/13  Updated: 02/Jan/14  Resolved: 02/Jan/14

Status: Resolved
Project: tools.cli
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Hugo Duncan Assignee: Sung Pae
Resolution: Completed Votes: 0
Labels: None


 Description   

When calling opt-parse with an empty sequence of options, summarize throws an exception.

actual: clojure.lang.ArityException: Wrong number of args (1) passed to: core$map
 at clojure.lang.AFn.throwArity (AFn.java:437)
    clojure.lang.RestFn.invoke (RestFn.java:412)
    clojure.lang.AFn.applyToHelper (AFn.java:161)
    clojure.lang.RestFn.applyTo (RestFn.java:132)
    clojure.core$apply.invoke (core.clj:619)
    clojure.tools.cli$summarize.invoke (cli.clj:375)

https://github.com/hugoduncan/tools.cli/compare/feature;fix-summary-no-options?expand=1



 Comments   
Comment by Sung Pae [ 02/Jan/14 1:32 AM ]

Thank you, patch applied!

I am sorry for the delay; I mistakenly thought I would receive email notifications on new issues so have not been polling JIRA.

I hope to ameliorate this soon.





[TCLI-6] Merge optparse-clj and increase modularity Created: 20/Nov/13  Updated: 10/Dec/13  Resolved: 10/Dec/13

Status: Resolved
Project: tools.cli
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Sung Pae Assignee: Sung Pae
Resolution: Completed Votes: 0
Labels: enhancement


 Description   

On Sun 25 Aug 2013 at 09:05:15PM -0500, gaz jones wrote:

> Hey, i am the current maintainer of tools.cli - i have very little
> time to make any changes to it at the moment (kids ). I'm not sure
> what the process is for adding you as a developer or transferring
> ownership etc but if I'm happy to do so as I have no further plans for
> working on it.

Hello Gareth,

Sorry for delay in action. I submitted my Clojure CA that week and have
been casually awaiting confirmation ever since.

Only today have I noticed that my name has appeared on
http://clojure.org/contributing, so I suppose that is confirmation
enough.

This is my proposal for tools.cli:

  • Merge the CLI arguments lexer from github.com/guns/optparse-clj to
    support GNU option parsing conventions.

Examples:
https://github.com/guns/optparse-clj#features

guns.cli.optparse/tokenize-arguments:
https://github.com/guns/optparse-clj/blob/master/src-cljx/guns/cli/optparse.cljx#L25-74

GNU options parsing spec:
https://www.gnu.org/software/libc/manual/html_node/Argument-Syntax.html

  • Adapt tools.cli/cli to use the arguments lexer, then freeze it to
    maintain backwards compatibility.
  • Create a new function tools.cli/parse-opts, based largely on the
    design of guns.cli.optparse/parse, that supports the following
    features:
  • Granular options specification map.

Given the following setup:

(ns my.ns
...)

(def cli-options
[["s" "-server HOSTNAME" "Remote server"
:default (java.net.InetAddress/getByName \"example.com\")
:default-desc "example.com"
:parse-fn #(java.net.InetAddress/getByName %)
:assert-fn (partial instance? Inet4Address)
:assert-msg "%s is not an IPv4 host"]
[...]
])

A call to (clojure.tools.cli/compile-option-specs cli-options)
will result in the following PersistentArrayMap:

{option-id ; :server
{:short-opt String ; "-s"
:long-opt String ; "--server"
:required String ; "HOSTNAME"
:desc String ; "Remote server"
:default Object ; #<Inet4Address example.com/93.184.216.119>
:default-desc String ; "example.com"
:parse-fn IFn ; #(InetAddress/getByName %)
:assoc-fn IFn ; assoc
:assert-fn IFn ; (partial instance? Inet4Address)
:assert-msg String ; "%s is not an IPv4 host"
}}

The optspec compiler will verify uniqueness of option-id,
:short-opt, and :long-opt values and throw an AssertionError on
failure.

The optspec map is a PAM to preserve options ordering for summary
generation.

  • Customizable options summary.

tools.cli/parse-opts will return an options summary string to
the caller. Printing the summary with a banner will be the
responsibility of the caller.

The default options summary will look like:

-p, --port NUMBER 8080 Remote port
-s, --server HOSTNAME example.com Remote server
--detach Detach and run in the background
-h, --help

The above format can be changed by supplying an optional :summary-fn
flag that will receive the optspec map values from above and return
a summary string. The default summary-fn will be a public var.

This addresses TCLI-3.

  • Optional in-order options processing, with trailing options parsed
    by default.

This is necessary for managing different option sets for
subcommands. Indirectly addresses TCLI-5.

  • No runtime exceptions.

While parse-opts will throw an AssertionError for duplicate
option-id, :short-opt, and :long-opt values during compilation,
option parsing errors will no longer throw exceptions.

Instead, a map of {option-id error-string} will be provided, or nil
if there are no errors. Correspondingly, parse-opts will have the
following function prototype:

parse-opts:
[argument-seq [& option-vectors] & compiler-flags]
->
{:options PersistentArrayMap ; optspec above
:arguments PersistentVector ; non-optarg arguments
:summary String ; options summary produced by summary-fn
:errors {Keyword String} ; error messages by option
}

The expected usage of this function will look like:

(def usage-banner
"Usage: my-program [options] arg1 arg2\n\nOptions:\n")

(defn exit [status msg]
(println msg)
(System/exit status))

(defn -main [& argv]
(let [{:keys [options arguments summary errors]}
(cli/parse-opts argv cli-options :in-order true)]
(when (:help options)
(exit 0 (str usage-banner summary)))
(when (not= (count arguments) 2)
(exit 1 (str usage-banner summary)))
(when errors
(exit 1 (string/join "\n" (cons "The following errors have occured:\n"
(vals errors)))))
(apply my-program! options arguments)))

  • ClojureScript support.

github.com/guns/optparse-clj currently supports CLJS/node.js via
the cljx preprocessor and an extern file for the Closure Compiler
`process` namespace.

If this is desirable for tools.cli, a similar setup will be applied,
except that I will attempt to avoid cljx for easier hacking.

Comments are appreciated, and I am eager to amend this proposal to gain
community acceptance.

Cheers,
Sung Pae



 Comments   
Comment by Sung Pae [ 20/Nov/13 6:27 PM ]

A more nicely formatted version of the above is available here:

https://gist.github.com/guns/7573819/raw

EDIT:

There is an error in the function prototype above. The actual return value should be:

```
{:options {Keyword Object} ; {:server "my-server.com"}
:arguments PersistentVector ; non-optarg arguments
:summary String ; options summary produced by summary-fn
:errors {Keyword String} ; error messages by option
}
```

Specifically, :options will of course return a PHM of option keywords to their parsed values.

Comment by Gareth Jones [ 20/Nov/13 9:22 PM ]

Sounds good. One thing to add - there is an open issue on github to allow a single dash as an argument (https://github.com/clojure/tools.cli/issues/17) e.g. to indicate you wish to use stdin to read from. This seems to be used as a convention in several *nix cli utilities (such as tar for example). I was intending on adding a patch to support that by simply allowing '-' to be a valid argument name. If you can work that (or something equivalent) into your changes too that would be great!

Thanks,

Gareth

Comment by Sung Pae [ 22/Nov/13 3:57 PM ]

Sorry for the delayed response; I am finding JIRA a bit confusing.

In my experience with options parsers from other languages, '-' as
stdin is typically left to the discretion of the caller as there are
situations where you might want '-' to be a normal argument (e.g. for
describing arithmetic expressions or ranges).

Therefore I believe the proper answer to issue 17 is for the user to
call

`(replace {"-" *in*} rest-args)`

on the remaining arguments vector, and not to bake this behavior into
tools.cli.

I will be happy to make this argument to kyptin on Github if you find
this reasoning persuasive.

Meanwhile, I will wait patiently for someone on the core team to respond
to your post on clojure-dev.

Thank you!

Sung Pae

Comment by Sung Pae [ 10/Dec/13 1:20 PM ]

clojure.tools.cli/parse-opts completes the conceptual merge of optparse-clj.





[TCLI-5] Optionally allow unkown options Created: 25/Oct/13  Updated: 02/Jan/14  Resolved: 02/Jan/14

Status: Resolved
Project: tools.cli
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Hugo Duncan Assignee: Sung Pae
Resolution: Declined Votes: 0
Labels: None

Attachments: File allow_unkown_options    

 Description   

When building a cli with subcommands, it can be useful to allow passing through unkown options as arguments, so they can be processed in a subcommand.

The current API doesn't allow for passing any configuration to the `cli` command, so this adds an `allow-unknown-opts?` var, which when bound to true, will pass through unknown options.



 Comments   
Comment by Hugo Duncan [ 31/Oct/13 8:03 AM ]

A patched version of tools.cli is available on clojars as `[com.palletops/tools.cli "0.2.4"]`

Comment by Sung Pae [ 10/Dec/13 1:50 PM ]

Hello,

In the next version of tools.cli, parse-opts accepts an :in-order option to process arguments sequentially until the first unknown token.

The README has an explanation:

https://github.com/clojure/tools.cli#in-order-processing-for-subcommands

This is not the same as passing through unknown options, but it is the convention I am familiar with in large programs with subcommands.

Is this solution satisfactory for you?

The only times I have ever implemented a pass-through option for an options parser was for programs that wrap other command line programs. In these cases, the wrapper had a possibly intersecting superset of options, and I wanted to provide access to the underlying command's options without specifying them in my wrapper.

This worked okay, but the resulting options interface was complex, and since the relationship of the wrapper and underlying command was implicit, a bit fragile.

A major problem with the superset parser was that all unknown options were assumed to require arguments. This is necessary because it is impossible to tell whether `-ab` is meant to be interpreted as `-a -b` or `-a "b"` without knowing whether `-a` requires an argument. We can work around this by requiring that pass-through options always specify option arguments with an equal sign, but now we have introduced subtle differences for different tiers of options.

In contrast to all that, the subcommand options processing style of the git program and others like it allow many intersecting sets of options to be present in a single command invocation simply by following the easily understood convention:

cmd [cmd-opts] subcmd1 [subcmd1-opts]

I hope you find this argument persuasive, but I am happy to discuss it further!

Comment by Hugo Duncan [ 17/Dec/13 9:45 AM ]

The new `:in-order` option provides a better solution.

Comment by Sung Pae [ 02/Jan/14 1:36 AM ]

Thank you for your thoughts on this.





[TCLI-4] Paramters without value. Created: 11/Sep/13  Updated: 15/Dec/13  Resolved: 15/Dec/13

Status: Resolved
Project: tools.cli
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Ivan Boldyrev Assignee: Sung Pae
Resolution: Completed Votes: 0
Labels: None


 Description   

Currently there is only one type of parameters without value: boolean parameters introduced with :flag true.

But what if parameters define actions that have to be performed, and some actions need no argument? Then presence "-no-action1" with "-action" and help message would be strange at least, if not misleading.



 Comments   
Comment by Paul Stadig [ 02/Oct/13 8:01 AM ]

This is basically the case with a help option. If you specify

["-h" "--help" "Display usage" :default false :flag true]

as an option, then you get banner like this

Switches               Default     Desc                    
 --------               -------     ----                    
 -h, --no-help, --help  false       Display usage
Comment by Sung Pae [ 10/Dec/13 1:19 PM ]

In the upcoming version of tools.cli, parse-opts does not print a default value for boolean options or options without a specified :default.

In addition, option summaries may be customized with the :summary-fn option:

cf. https://github.com/clojure/tools.cli/issues/13#issuecomment-30258137

Comment by Sung Pae [ 15/Dec/13 3:14 PM ]

I am going to close this issue as it has been addressed by 0.3.0.

Thank you!





[TCLI-3] Change contract to provide access to banner on parse error Created: 09/Feb/13  Updated: 15/Dec/13  Resolved: 15/Dec/13

Status: Resolved
Project: tools.cli
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Philip Aston Assignee: Sung Pae
Resolution: Completed Votes: 1
Labels: None


 Description   

If a user provides an invalid option, most applications would want to display the banner.

tools.cli throws an Exception with minimal information, and (banner-for) is marked private.

The exception could be replaced with one that provides access to the banner, but I think it would be better (easier to use, more idiomatic?) to change the contract so that the banner is always returned. Given the exception string is also useful, perhaps change cli to return a map with keys [:options :extra-args :banner :parse-failure]?



 Comments   
Comment by Gunnar Völkel [ 08/Mar/13 4:19 AM ]

I agree with the reporter. Throwing an exception for invalid arguments ("[...] is not a valid argument") is not appropriate since you usually want to display that error message along with the banner.

Comment by Sung Pae [ 10/Dec/13 1:26 PM ]

In the upcoming 0.3.0 release, parse-opts does not throw exceptions on parse errors, but collects them into an errors vector which the caller checks to see if parsing was successful.

parse-opts does have a :post assertion on the uniqueness of :id, :short-opt, and :long-opt entries in the compiled option maps, but these errors are meant to help development, and may even be switched off by disabling assert at compile time.

Comment by Sung Pae [ 15/Dec/13 3:15 PM ]

I am going to close this issue as it has been addressed by 0.3.0.

Thank you!





[TCLI-2] Allow caller-supplied parse-fn Created: 11/Nov/12  Updated: 06/Aug/13  Resolved: 06/Aug/13

Status: Resolved
Project: tools.cli
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Pierre-Yves Ritschard Assignee: Gareth Jones
Resolution: Completed Votes: 0
Labels: enhancement

Attachments: File TCLI-2.diff    
Patch: Code and Test

 Description   

As for :parse-fn, a function can be supplied, this
is useful for standard use-cases such as: -vv, or when
you want to build a list from values.

PR: https://github.com/clojure/tools.cli/pull/11



 Comments   
Comment by Andy Fingerhut [ 11/Nov/12 12:38 PM ]

Pierre-Yves, there are instructions for creating patches under the headings "Development" and "Adding patches" on this page: http://dev.clojure.org/display/design/JIRA+workflow

Submissions to this module do require the author to sign a CA. Instructions here: http://clojure.org/contributing

Comment by Pierre-Yves Ritschard [ 11/Apr/13 9:48 AM ]

Suggested Patch

Comment by Pierre-Yves Ritschard [ 11/Apr/13 9:48 AM ]

Hello, now that I'm a registered contributor, I attached a file as suggested in the workflow wiki

Comment by Gareth Jones [ 06/Aug/13 1:13 PM ]

Applied this patch, and will release as 0.2.4 shortly. Thanks.

Comment by Gareth Jones [ 06/Aug/13 1:13 PM ]

Applied





[TCLI-1] Do not include keys when no value provided and no :default Created: 18/May/12  Updated: 24/Aug/12  Resolved: 24/Aug/12

Status: Closed
Project: tools.cli
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Minor
Reporter: Stuart Sierra Assignee: Stuart Sierra
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File TCLI-1-0001.patch     Text File TCLI-1-0002.patch    
Patch: Code and Test

 Description   

I was trying to use tools.cli in conjunction with a configuration map
loaded from a file, and use clojure.core/merge to combine the results.

This didn't work because tools.cli always uses a default value of
nil, even when no default value is specified. The nil always
overrides defaults from another source.

Example before the patch:

(def my-defaults
  {:foo 1})

(merge my-defaults
       (first (clojure.tools.cli/cli
               [] ; no arguments given
               ["--foo"]))) ; no default specified
;;=> {:foo nil}

This enhancement modifies tools.cli to completely omit arguments which
have no default specified and no value given on the command line.

After the patch, the above example returns:

;;=> {:foo 1}


 Comments   
Comment by Stuart Sierra [ 20/Jul/12 7:41 AM ]

New patch file 0002 fixes syntax error in the previous patch; updated description to show correct results.

Comment by Stuart Sierra [ 24/Aug/12 8:31 AM ]

Patch applied in commit https://github.com/clojure/tools.cli/commit/f9d92395cb788dd08cb144035a9d5fd8706d10b5





Generated at Thu Jul 24 01:31:59 CDT 2014 using JIRA 4.4#649-r158309.