docopt.rs icon indicating copy to clipboard operation
docopt.rs copied to clipboard

Expand Error::WithProgramUsage with just the unknown flag

Open MatejLach opened this issue 10 years ago • 2 comments

I was trying to implement a fix for cargo (the issue), however the only way of improving the error message involves cutting out the bad flag/option out of the error string returned by docopt.

I am not very familiar with docopt, so may be there is a better way, but it would be very useful for the Error::WithProgramUsage variant to encapsulate just the raw, user-entered flag/option that was invalid, instead of just having the whole error string including the passed-in flag returned.

Is this something that would be possible to implement? Thanks and great work on the crate!

MatejLach avatar Nov 18 '15 11:11 MatejLach

@MatejLach Yes, this is definitely something that would be desirable. I wasn't particularly principled about error handling when I wrote the parser, so all of the errors returned are plain strings. This effectively makes your task to improve Cargo impossible (that's bad).

The right way to fix this is to:

  1. Create a new enum type called ParseError.
  2. Go through src/parse.rs and replace all string-based error messages with the appropriate ParseError variant.
  3. Modify Error in src/dopt.rs. Specifically, change Argv(String) to Parse(ParseError). Also, make ParseError public.
  4. Implement Display on ParseError (probably using the same or similar strings that were replaced in step 2).

Then I think that will do it. Notably, this will be a breaking change. Fortunately, I suspect Cargo is one of the few projects doing this kind of detailed handling of errors (but we can search for others and send them PRs, most fixes should be trivial). For example, the ParseError enum should probably contain an UnknownFlag { got: String, possibles: Vec<String> } variant, where got is the name of the flag and possibles is the list of possible flags that could be matched. Then the Display impl for ParseError would do the fuzzy search done here: https://github.com/docopt/docopt.rs/blob/master/src/parse.rs#L966-L978

It might be possible to do all of this without completely understanding the parser (which is quite hairy).

I'll tackle this issue myself at some point, but I'm not sure when. Until then, I'd be happy to mentor anyone through it.

BurntSushi avatar Nov 18 '15 12:11 BurntSushi

@BurntSushi Oh, great, I am definitely interested in looking into it, but with several Uni deadlines looming, it may take a little longer for me to get this done, so if anyone else wants to look into this until then, don't mind me :-)

EDIT: I started seriously looking into parsing and am currently reading Parsing Techniques, as it seems that the parser would benefit from greater refactoring work, since, as you described, it "is quite hairy" and such would benefit from a major refactoring to enable easier extensibility. EDIT2: Specifically, I'm wondering about the possibility of a rewrite in something like nom down the line.

MatejLach avatar Nov 18 '15 14:11 MatejLach