Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ContextKind::Custom has no functionality #5044

Open
benluelo opened this issue Jul 24, 2023 · 5 comments
Open

ContextKind::Custom has no functionality #5044

benluelo opened this issue Jul 24, 2023 · 5 comments

Comments

@benluelo
Copy link

As per the title, ContextKind::Custom is currently useless as nothing ever reads it. I expected it to be used in a similar way to ContextKind::Usage, in that it could be attached to any ErrorKind and be printed as An opaque message to the user, but it just does nothing.

@epage
Copy link
Member

epage commented Jul 24, 2023

Huh, that must have slipped through the cracks in #3402 (real don't want to dig into the 35 commits to verify)

Our options

  • Reserve it for use for custom formatters
  • Define how it should be rendered in our errors
  • Remove it on the next breaking change

@benluelo
Copy link
Author

I think defining how it should be rendered would be the best option - it would be super helpful for adding additional context as to why a custom ValueParser failed ("cannot parse '0xinvalid' as hex", "file 'abc.xyz' should have .txt extension", etc)

@epage
Copy link
Member

epage commented Jul 25, 2023

If its for a random ValueParser, why not just include it in the main message?

How would it show up in the regular error formatting:

error: Some messge

Usage: ...

For more information, try `--help`

@benluelo
Copy link
Author

A few things:

  • I couldn't get ContextKind::Usage to work, but I just realized that it's because it expects a ContextValue::StyledStr. (documentation on what values are valid in what cases would be great!)

  • I would expect ContextKind::Usage to prepend Usage: to the provided ContextValue::StyledStr, considering it's doc comment of "A usage string", which it does not(?)

  • Even with the above, I would still expect a way to append a general "context" to the error message, along with the usage. Is it expected that this is all done in the usage message, manually formatted?

@epage
Copy link
Member

epage commented Jul 25, 2023

I couldn't get ContextKind::Usage to work, but I just realized that it's because it expects a ContextValue::StyledStr. (documentation on what values are valid in what cases would be great!)

While it would be great to have everything in clap to the same degree of quality, the practical aspect is that this area isn't as much of a focus as this is fairly off the beaten path. So long as we provide an escape hatch to work, that is sufficient.

If there is interest from someone improving the docs in a way that fits within the docs and doesn't detract from higher priority workflows, it would be welcome.

I would expect ContextKind::Usage to prepend Usage: to the provided ContextValue::StyledStr, considering it's doc comment of "A usage string", which it does not(?)

We can consider this for 5.0 though it should have its own Issue

Even with the above, I would still expect a way to append a general "context" to the error message, along with the usage. Is it expected that this is all done in the usage message, manually formatted?

What do you mean?

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

No branches or pull requests

2 participants