feat: Support visible aliases for possible values #4344
Conversation
Show visible aliases in help and in generated manpages. Does not show aliases in error messages yet.
joshtriplett
force-pushed
the
possible-value-visible-aliases
branch
from
cbf8b8c
to
936aea3
Compare
| @@ -5,7 +5,7 @@ A simple to use, efficient, and full-featured Command Line Argument Parser | |||
| Usage: 04_01_enum_derive[EXE] <MODE> | |||
|
|
|||
| Arguments: | |||
| <MODE> What mode to run the program in [possible values: fast, slow] | |||
| <MODE> What mode to run the program in [possible values: fast (alias: quick), slow] | |||
There was a problem hiding this comment.
Feel like this is too many layers of parenthetical statements.
We should either flatten them or reserve them for long help.
I lean towards flattening.
There was a problem hiding this comment.
@epage I think it would be confusing to put all the aliases into a single flat list, without indicating what value they're aliases for.
No objections to only showing them in long help, though.
There was a problem hiding this comment.
As this is more being driven by your needs, I'll defer to your preference until we get wider feedback
There was a problem hiding this comment.
@epage FWIW, while I have a need for visible aliases, I have no particular preferences regarding how to display them, as long as they're displayed and it's clear what they're an alias for. I do think the brief help may just be too short to reasonably show them.
| } else { | ||
| None | ||
| } | ||
| } | ||
|
|
||
| /// Get the names of all visible aliases, but wrapped in quotes if they contain whitespace | ||
| pub fn get_quoted_visible_aliases(&self) -> Vec<std::borrow::Cow<'_, str>> { |
There was a problem hiding this comment.
This feels too specialized to make pub
There was a problem hiding this comment.
@epage clap_mangen needs to call it.
There was a problem hiding this comment.
clap_mangen is choosing to format quoted aliases the same way as the --help. I don't see that as a hard requirement to be driving code sharing and rather only couple code together when its driven by requirements
There was a problem hiding this comment.
@epage What would you suggest instead? I don't want to leave them out of the manpages entirely.
| /// # ; | ||
| /// ``` | ||
| #[must_use] | ||
| pub fn visible_alias(mut self, name: impl IntoResettable<Str>) -> Self { |
There was a problem hiding this comment.
Does not show aliases in error messages yet.
Whats holding that back?
There was a problem hiding this comment.
@epage Primarily, the amount of additional complexity required to pass groups of aliases through the error-context infrastructure and update all the places creating those. Secondarily, trying to figure out how to handle the "did you mean".
There was a problem hiding this comment.
So long as we hide visible aliases in the short help, we can forego them in the errors.
For the suggestion,
- If we consolidate the two possible value
ValueParsers, we could probably switch the "did you mean" up a layer into those which could give greater control over it - However, if we suggest a value that was not shown in the error, that might be confusing.
I can go either way
btw can you update the commit with some of these "for now" decisions so its easier for someone to discover them when making changes in the future? People might discover the commit but its then a smaller set of people who might explore the PR comments.
|
I'm going to close this for now, I don't think I have the bandwidth to pursue it at the moment and I won't need it for a while. |
Show visible aliases in help and in generated manpages.
Does not show aliases in error messages yet.