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

Add Visible Aliases for PossibleValue #5480

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 62 additions & 16 deletions clap_builder/src/builder/possible_value.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use anstyle::Style;

use crate::builder::IntoResettable;
use crate::builder::Str;
use crate::builder::StyledStr;
Expand Down Expand Up @@ -37,6 +39,7 @@ pub struct PossibleValue {
name: Str,
help: Option<StyledStr>,
aliases: Vec<Str>, // (name, visible)
visible_aliases: Vec<Str>,
Comment on lines 41 to +42
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

hide: bool,
}

Expand Down Expand Up @@ -130,6 +133,20 @@ impl PossibleValue {
self
}

/// Add a _visible_ alias to this [PossibleValue].
///
/// Unlike [PossibleValue::alias], these aliases will show in help output.
Comment on lines +136 to +138
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs an example

#[must_use]
pub fn visible_alias(mut self, name: impl IntoResettable<Str>) -> Self {
Comment on lines +139 to +140
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +139 to +140
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

if let Some(name) = name.into_resettable().into_option() {
self.visible_aliases.push(name);
} else {
self.visible_aliases.clear();
}

self
}

/// Sets multiple *hidden* aliases for this argument value.
///
/// # Examples
Expand All @@ -146,6 +163,16 @@ impl PossibleValue {
self.aliases.extend(names.into_iter().map(|a| a.into()));
self
}

/// Add multiple _visible_ aliases to this [PossibleValue].
///
/// 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 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll need to add support for this to the dynamic completion system.

See #5585 and a follow up PR that will be linked to from #3951

self.visible_aliases
.extend(names.into_iter().map(|a| a.into()));
self
}
}

/// Reflection
Expand All @@ -156,6 +183,38 @@ impl PossibleValue {
self.name.as_str()
}

/// Get the visible aliases for this argument
pub fn get_visible_aliases(&self) -> &Vec<Str> {
&self.visible_aliases
}

/// Render help text for this [PossibleValue] with all of its visible aliases included.
///
/// 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 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • This doesn't seem like it needs to be public
  • This is coupled to the help rendering and should be there
  • This returns a String which should only be used for unstyled text. Instead, this should return a StyledStr

let name = self.get_name();
let aliases = if self.get_visible_aliases().is_empty() {
"".to_string()
} else {
// [aliases: a, b, c]
format!(
"[aliases: {}]",
self.get_visible_aliases()
.iter()
.map(|s| { format!("{}{s}{}", literal.render(), literal.render_reset()) })
.collect::<Vec<_>>()
.join(", ")
)
};

format!(
"{}{name}{}{aliases}",
literal.render(),
literal.render_reset()
)
}

/// Get the help specified for this argument, if any
#[inline]
pub fn get_help(&self) -> Option<&StyledStr> {
Expand All @@ -173,26 +232,13 @@ impl PossibleValue {
!self.hide && self.help.is_some()
}

/// Get the name if argument value is not hidden, `None` otherwise,
/// but wrapped in quotes if it contains whitespace
#[cfg(feature = "help")]
pub(crate) fn get_visible_quoted_name(&self) -> Option<std::borrow::Cow<'_, str>> {
if !self.hide {
Some(if self.name.contains(char::is_whitespace) {
format!("{:?}", self.name).into()
} else {
self.name.as_str().into()
})
} else {
None
}
}

/// Returns all valid values of the argument value.
///
/// Namely the name and all aliases.
pub fn get_name_and_aliases(&self) -> impl Iterator<Item = &str> + '_ {
std::iter::once(self.get_name()).chain(self.aliases.iter().map(|s| s.as_str()))
std::iter::once(self.get_name())
.chain(self.aliases.iter().map(|s| s.as_str()))
.chain(self.visible_aliases.iter().map(|s| s.as_str()))
}

/// Tests if the value is valid for this argument value
Expand Down
23 changes: 12 additions & 11 deletions clap_builder/src/output/help_template.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@ use std::borrow::Cow;
use std::cmp;
use std::usize;

use crate::builder::{Arg, Command};
// Internal
use crate::builder::PossibleValue;
use crate::builder::Str;
use crate::builder::StyledStr;
use crate::builder::Styles;
use crate::builder::{Arg, Command};
use crate::output::display_width;
use crate::output::wrap;
use crate::output::Usage;
Expand Down Expand Up @@ -689,10 +689,11 @@ impl<'cmd, 'writer> HelpTemplate<'cmd, 'writer> {
let possible_vals = arg.get_possible_values();
if !possible_vals.is_empty() {
debug!("HelpTemplate::help: Found possible vals...{possible_vals:?}");

let longest = possible_vals
.iter()
.filter(|f| !f.is_hide_set())
.map(|f| display_width(f.get_name()))
.map(|f| display_width(f.render_help_prefix_long(literal).as_str()))
.max()
.expect("Only called with possible value");

Expand All @@ -705,19 +706,15 @@ impl<'cmd, 'writer> HelpTemplate<'cmd, 'writer> {
}
self.writer.push_str("Possible values:");
for pv in possible_vals.iter().filter(|pv| !pv.is_hide_set()) {
let name = pv.get_name();
let prefix = pv.render_help_prefix_long(literal);

let mut descr = StyledStr::new();
let _ = write!(
&mut descr,
"{}{name}{}",
literal.render(),
literal.render_reset()
);

let _ = write!(&mut descr, "{prefix}",);
if let Some(help) = pv.get_help() {
debug!("HelpTemplate::help: Possible Value help");
// To align help messages
let padding = longest - display_width(name);
let padding = longest - display_width(prefix.as_str());
let _ = write!(&mut descr, ": {:padding$}", "");
descr.push_styled(help);
}
Expand Down Expand Up @@ -850,7 +847,11 @@ impl<'cmd, 'writer> HelpTemplate<'cmd, 'writer> {

let pvs = possible_vals
.iter()
.filter_map(PossibleValue::get_visible_quoted_name)
.filter(|v| !v.is_hide_set())
.flat_map(|v| {
std::iter::once(v.get_name())
.chain(v.get_visible_aliases().iter().map(|s| s.as_str()))
})
.collect::<Vec<_>>()
.join(", ");

Expand Down
Loading