Add Visible Aliases for PossibleValue #5480
Conversation
naftulikay
force-pushed
the
feature/derive-visible-aliases
branch
5 times, most recently
from
f984aa5 to
b267197
Compare
|
As a heads up, I'll be traveling for the next week or so and so it might take a bit before I get to this |
naftulikay
force-pushed
the
feature/derive-visible-aliases
branch
from
b267197 to
10d27be
Compare
Before this change, it was not possible to have *visible* aliases for PossibleValues, all aliases were hidden in help output. This change adds a new API for adding visible aliases and modifies the help generation scheme to display these visible aliases.
naftulikay
force-pushed
the
feature/derive-visible-aliases
branch
from
10d27be to
5c6f832
Compare
| aliases: Vec<Str>, // (name, visible) | ||
| visible_aliases: Vec<Str>, |
There was a problem hiding this comment.
Heh, that comment wasn't accurate when it was copied here but it shows that aliases are generally stored in a single Vec, rather than in parallel ones. Let's do the same here.
| /// | ||
| /// If there are no visible aliases, this will simply emit the formatted name, if there are visible aliases, these | ||
| /// will be appended like this: `name [aliases: a, b, c]`. | ||
| pub fn render_help_prefix_long(&self, literal: &Style) -> String { |
There was a problem hiding this comment.
- This doesn't seem like it needs to be public
- This is coupled to the help rendering and should be there
- This returns a
Stringwhich should only be used for unstyled text. Instead, this should return aStyledStr
| /// Add a _visible_ alias to this [PossibleValue]. | ||
| /// | ||
| /// Unlike [PossibleValue::alias], these aliases will show in help output. |
| #[must_use] | ||
| pub fn visible_alias(mut self, name: impl IntoResettable<Str>) -> Self { |
There was a problem hiding this comment.
This needs a test. Ideally, the test is added in a prior commit, with a regular alias, and should pass. In this commit, it would then change to a visible alias with the expected output updated.
Being this particular makes it very easy to show anyone (1) what is supported today, (2) what this new feature looks like, and (3) how the behavior is different than what is there today.
One of the big design questions that needs discussion in #4416 is what the help output should be like for this. Note that the design conversation should happen there.
| #[must_use] | ||
| pub fn visible_alias(mut self, name: impl IntoResettable<Str>) -> Self { |
There was a problem hiding this comment.
Search for aliases_command and add cases that use this (and invisible aliases)
Again, modify the tests in a prior commit (pass) and then switch tests from regular aliases to visible aliases.
|
Hmm, when I marked #4416 as ready for implementation, I had overlooked that we needed a design conversation on the help output first. I ended up starting this review before I realized it. I would recommend we focus the conversation on the help output and have it in #4416 before focusing too much more on the implementation. |
| /// | ||
| /// Unlike [PossibleValue::aliases], these aliases will show in help output. | ||
| #[must_use] | ||
| pub fn visible_aliases(mut self, names: impl IntoIterator<Item = impl Into<Str>>) -> Self { |
Before this change, it was not possible to have visible aliases for PossibleValues, all aliases were hidden in help output. This change adds a new API for adding visible aliases and modifies the help generation scheme to display these visible aliases.
Tracking issue: #4416