<< Back to previous view

[TCLI-10] potential incorrect parsing Created: 23/Jul/14  Updated: 10/Aug/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?



 Comments   
Comment by john walker [ 10/Aug/14 3:01 PM ]

I am not able to reproduce this behavior. Would you create a gist or use code tags?





[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





Generated at Fri Aug 29 17:28:38 CDT 2014 using JIRA 4.4#649-r158309.