Skip to content
This repository has been archived by the owner on Jan 1, 2022. It is now read-only.

Supplement clap::Error.info with helper methods #193

Open
2 tasks done
epage opened this issue Dec 6, 2021 · 5 comments
Open
2 tasks done

Supplement clap::Error.info with helper methods #193

epage opened this issue Dec 6, 2021 · 5 comments

Comments

@epage
Copy link
Owner

epage commented Dec 6, 2021

Issue by TheDaemoness
Tuesday Jul 27, 2021 at 00:09 GMT
Originally opened as clap-rs/clap#2628


Please complete the following tasks

  • I have searched the discussions
  • I have searched the existing issues

Clap Version

3.0.0-beta.2

Describe your use case

Preface: thank you very much for your work on clap so far, it has saved me countless hours of tedium.

ErrorKind is a fieldless enum, so additional contextual information required for someone to provide their own custom error messages is provided as a Vec<String> in Error, named info.

Using a Vec to carry this information not self-explanatory in regards to what each element of the Vec represents (e.g. which element represents the invalid value that triggers an InvalidValue or ValueValidation). This would be okay-ish if clap had any of the following:

  • Methods or associated functions on Error or ErrorKind to retrieve specific information
  • Functions, named constants, or an enum that can be used with info to retrieve specific information based on where it should be in the vector.
  • Documentation describing what the values of info will be for a given ErrorKind.

As far as I can tell, none of these are present, which means that in order to write one's own error reporting, one needs to either read the source code of the functions that construct these Errors or do their own testing. I don't think either should be necessary.

Describe the solution you'd like

The solution I'd prefer the most is a significant breaking change (described under Alternatives). I acknowledge that's not desirable, even considering the upcoming 3.0.0 release.

A compromise is to add ways to retrieve this information safely, such as by adding methods to Error that return the relevant information. A few possible examples with placeholder names:

  • pub fn get_arg_or_subcmd(&self) -> Option<&str>
  • pub fn get_missing(&self) -> &[String]
  • pub fn get_value_counts(&self) -> Option<(usize,usize)> //Found, expected.

Implementing each method should be fairly simple, if perhaps a bit repetitive considering the number of variants on ErrorKind that need to be matched. The bodies would mostly be pattern matches followed by accesses to info.

One caveat in the implementation is that Error should contain a private flag to indicate whether the Error was constructed by Error::with_description or not. If so, these functions should return empty values.

Alternatives, if applicable

This problem can technically be solved by improving documentation, but I'm of the mind that doing only that is the worst solution. info being a Vec is fragile and typo-prone, and there should be better ways to interact with it than as a Vec.

Ideally, keeping ErrorKind as a fieldless enum, info's type would change to an enum with fields that store the relevant information for each kind of error. Some of the methods described above could still be implemented for convenience.

However, I acknowledge that this would be a significant breaking change that would take more than a few renames for users of clap to fix. Solutions like making info a method that generates a Vec<String> from data in this enum or providing both info and the enum in another field aren't entirely compelling either.

Additional Context

I am very willing to work with you fine folks to implement any of these changes, if approved. Have a good day!

@epage
Copy link
Owner Author

epage commented Dec 6, 2021

Comment by pksunkara
Wednesday Aug 11, 2021 at 18:56 GMT


@epage Do you think you can take care of this after the utf8 thing you are currently working on?

@epage
Copy link
Owner Author

epage commented Dec 6, 2021

Comment by pksunkara
Wednesday Aug 11, 2021 at 18:58 GMT


If we are breaking it, I would actually love the variants in ErrorKind to be tuples of fields that can contain the metadata of the error.

@epage
Copy link
Owner Author

epage commented Dec 6, 2021

Comment by epage
Wednesday Aug 11, 2021 at 19:04 GMT


After iterating with different error designs, I think our approach of a tag-only Kind with auxiliary data is the best approach (granted, how we expose that auxiliary data can be improved). The problem with enum fields is its brittle (can't add fields without breaking compat) and harder to check for just the kind (matches! helps some but not completely).

The error working group is working on a general API for querying information from errors. Something like that seems like the best long-term solution. Short term, we can add our own query methods if we really need this before the Error WG gets their feature landed.

@epage
Copy link
Owner Author

epage commented Dec 6, 2021

Comment by pksunkara
Wednesday Aug 11, 2021 at 19:06 GMT


Error WG gets their feature landed.

Which feature is that?

we can add our own query methods

In that case, shall I mark this as non 3.0 since it wont be breaking change?

@epage
Copy link
Owner Author

epage commented Dec 6, 2021

Comment by epage
Wednesday Aug 11, 2021 at 19:13 GMT


Error WG gets their feature landed.
Which feature is that?

Dynamic member access, rust-lang/rfcs#2895

The RFC is old but it looks like work has restarted on this. Just today they were talking about this on their Zulip channel.

In that case, shall I mark this as non 3.0 since it wont be breaking change?

Agreed

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

1 participant