Skip to content
Draft
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
16 changes: 16 additions & 0 deletions clap_complete/src/engine/candidate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ pub struct CompletionCandidate {
tag: Option<StyledStr>,
display_order: Option<usize>,
hidden: bool,
nospace: bool,
}

impl CompletionCandidate {
Expand Down Expand Up @@ -60,6 +61,16 @@ impl CompletionCandidate {
self
}

/// Suppress trailing space after this candidate
///
/// This is useful for completions like directory paths (ending in `/`),
/// `--flag=` values, and delimiter-separated values where a trailing space
/// would be incorrect.
Comment on lines +66 to +68
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is useful when there are more completion candidates if this completion is selected, like ...

pub fn nospace(mut self, nospace: bool) -> Self {
self.nospace = nospace;
self
}

/// Add a prefix to the value of completion candidate
///
/// This is generally used for post-process by [`complete`][crate::engine::complete()] for
Expand Down Expand Up @@ -104,6 +115,11 @@ impl CompletionCandidate {
pub fn is_hide_set(&self) -> bool {
self.hidden
}

/// Get whether trailing space should be suppressed
pub fn is_nospace_set(&self) -> bool {
self.nospace
}
}

impl<S: Into<OsString>> From<S> for CompletionCandidate {
Expand Down
31 changes: 24 additions & 7 deletions clap_complete/src/engine/complete.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ fn complete_option(
completions.extend(
complete_arg_value(value.to_str().ok_or(value), arg, current_dir)
.into_iter()
.map(|comp| comp.add_prefix(format!("--{flag}="))),
.map(|comp| comp.add_prefix(format!("--{flag}=")).nospace(true)),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why is this unconditionally adding nospace?

);
}
} else {
Expand Down Expand Up @@ -308,7 +308,12 @@ fn complete_option(
.into_iter()
.map(|comp| {
let sep = if has_equal { "=" } else { "" };
comp.add_prefix(format!("-{leading_flags}{sep}"))
let comp = comp.add_prefix(format!("-{leading_flags}{sep}"));
if has_equal {
comp.nospace(true)
} else {
comp
}
Comment on lines +311 to +316
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why?

}),
);
} else {
Expand Down Expand Up @@ -393,7 +398,7 @@ fn complete_arg_value(
if let Some(prefix) = prefix {
values = values
.into_iter()
.map(|comp| comp.add_prefix(prefix))
.map(|comp| comp.add_prefix(prefix).nospace(true))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why?

.collect();
}
values = values
Expand Down Expand Up @@ -478,9 +483,14 @@ fn longs_and_visible_aliases(p: &clap::Command) -> Vec<CompletionCandidate> {
p.get_arguments()
.filter_map(|a| {
a.get_long_and_visible_aliases().map(|longs| {
longs
.into_iter()
.map(|s| populate_arg_candidate(CompletionCandidate::new(format!("--{s}")), a))
longs.into_iter().map(|s| {
if a.is_require_equals_set() {
populate_arg_candidate(CompletionCandidate::new(format!("--{s}=")), a)
.nospace(true)
} else {
populate_arg_candidate(CompletionCandidate::new(format!("--{s}")), a)
Comment on lines +487 to +491
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Doesn't this conflict with your requires_equals PR?

I also think I prefer the approach taken in that PR.

}
})
})
})
.flatten()
Expand All @@ -495,7 +505,14 @@ fn hidden_longs_aliases(p: &clap::Command) -> Vec<CompletionCandidate> {
.filter_map(|a| {
a.get_aliases().map(|longs| {
longs.into_iter().map(|s| {
populate_arg_candidate(CompletionCandidate::new(format!("--{s}")), a).hide(true)
if a.is_require_equals_set() {
populate_arg_candidate(CompletionCandidate::new(format!("--{s}=")), a)
.hide(true)
.nospace(true)
} else {
populate_arg_candidate(CompletionCandidate::new(format!("--{s}")), a)
.hide(true)
}
})
})
})
Expand Down
3 changes: 2 additions & 1 deletion clap_complete/src/engine/custom.rs
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,8 @@ pub(crate) fn complete_path(
let mut suggestion = prefix.join(&raw_file_name);
suggestion.push(""); // Ensure trailing `/`
let candidate = CompletionCandidate::new(suggestion.as_os_str().to_owned())
.hide(is_hidden(&raw_file_name));
.hide(is_hidden(&raw_file_name))
.nospace(true);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we limit this to !is_wanted? I'm guessing not because something may look like a valid candidate but the user may instead want a directory inside of the current one.

Granted, besides sort order, we don't have a good way of communicating which directories match is_wanted and which don't. Maybe the way to deal with that is to provide a way to add a description for wanted paths.


if is_wanted(&entry.path()) {
completions.push(candidate);
Expand Down
46 changes: 32 additions & 14 deletions clap_complete/src/env/shells.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,11 @@ _clap_complete_NAME() {
) )
if [[ $? != 0 ]]; then
unset COMPREPLY
elif [[ $_CLAP_COMPLETE_SPACE == false ]] && [[ "${COMPREPLY-}" =~ [=/:]$ ]]; then
compopt -o nospace
elif [[ $_CLAP_COMPLETE_SPACE == false ]]; then
if [[ ${#COMPREPLY[@]} -gt 0 ]] && [[ "${COMPREPLY[-1]}" == "_CLAP_COMPLETE_NOSPACE" ]]; then
unset 'COMPREPLY[-1]'
compopt -o nospace
fi
fi
}
if [[ "${BASH_VERSINFO[0]}" -eq 4 && "${BASH_VERSINFO[1]}" -ge 4 || "${BASH_VERSINFO[0]}" -gt 4 ]]; then
Expand Down Expand Up @@ -90,12 +93,18 @@ fi
let ifs: Option<String> = std::env::var("_CLAP_IFS").ok().and_then(|i| i.parse().ok());
let completions = crate::engine::complete(cmd, args, index, current_dir)?;

let has_nospace = completions.iter().any(|c| c.is_nospace_set());

for (i, candidate) in completions.iter().enumerate() {
if i != 0 {
write!(buf, "{}", ifs.as_deref().unwrap_or("\n"))?;
}
write!(buf, "{}", candidate.get_value().to_string_lossy())?;
}
if has_nospace && !completions.is_empty() {
write!(buf, "{}", ifs.as_deref().unwrap_or("\n"))?;
write!(buf, "_CLAP_COMPLETE_NOSPACE")?;
}
Comment on lines +96 to +107
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This makes it so if any candidate doesn't have a space, none of them do.

Instead, if nospace is supported, could we always append a space and have bash always add nospace?

Ok(())
}
}
Expand Down Expand Up @@ -160,7 +169,14 @@ set edit:completion:arg-completer[BIN] = { |@words|
var index = (count $words)
set index = (- $index 1)

put (env _CLAP_IFS="\n" _CLAP_COMPLETE_INDEX=(to-string $index) VAR="elvish" COMPLETER -- $@words) | to-lines
env _CLAP_IFS="\n" _CLAP_COMPLETE_INDEX=(to-string $index) VAR="elvish" COMPLETER -- $@words | from-lines | each { |line|
if (str:has-prefix $line "\x1f") {
var value = (str:trim-prefix $line "\x1f")
edit:complex-candidate $value &code-suffix=''
} else {
put $line
}
}
}
"#
.replace("COMPLETER", &completer)
Expand Down Expand Up @@ -188,6 +204,9 @@ set edit:completion:arg-completer[BIN] = { |@words|
if i != 0 {
write!(buf, "{}", ifs.as_deref().unwrap_or("\n"))?;
}
if candidate.is_nospace_set() {
write!(buf, "\x1f")?;
}
write!(buf, "{}", candidate.get_value().to_string_lossy())?;
}
Ok(())
Expand Down Expand Up @@ -376,24 +395,18 @@ function _clap_dynamic_completer_NAME() {
)}")

if [[ -n $completions ]]; then
local -a dirs=()
local -a nospace=()
local -a other=()
local completion
for completion in $completions; do
local value="${completion%%:*}"
if [[ "$value" == */ ]]; then
local dir_no_slash="${value%/}"
if [[ "$completion" == *:* ]]; then
local desc="${completion#*:}"
dirs+=("$dir_no_slash:$desc")
else
dirs+=("$dir_no_slash")
fi
if [[ "$completion" == $'\x1f'* ]]; then
completion="${completion:1}"
nospace+=("$completion")
else
other+=("$completion")
fi
done
[[ -n $dirs ]] && _describe 'values' dirs -S '/' -r '/'
[[ -n $nospace ]] && _describe 'values' nospace -S ''
Comment on lines +398 to +409
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Again, this is doing it all or nothing

[[ -n $other ]] && _describe 'values' other
fi
}
Expand Down Expand Up @@ -431,6 +444,11 @@ compdef _clap_dynamic_completer_NAME BIN"#
if i != 0 {
write!(buf, "{}", ifs.as_deref().unwrap_or("\n"))?;
}
// Prefix nospace candidates with \x1f so the registration script can
// split them into a separate group with -S ''
if candidate.is_nospace_set() {
write!(buf, "\x1f")?;
}
write!(
buf,
"{}",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,11 @@ _clap_complete_exhaustive() {
) )
if [[ $? != 0 ]]; then
unset COMPREPLY
elif [[ $_CLAP_COMPLETE_SPACE == false ]] && [[ "${COMPREPLY-}" =~ [=/:]$ ]]; then
compopt -o nospace
elif [[ $_CLAP_COMPLETE_SPACE == false ]]; then
if [[ ${#COMPREPLY[@]} -gt 0 ]] && [[ "${COMPREPLY[-1]}" == "_CLAP_COMPLETE_NOSPACE" ]]; then
unset 'COMPREPLY[-1]'
compopt -o nospace
fi
fi
}
if [[ "${BASH_VERSINFO[0]}" -eq 4 && "${BASH_VERSINFO[1]}" -ge 4 || "${BASH_VERSINFO[0]}" -gt 4 ]]; then
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,13 @@ set edit:completion:arg-completer[exhaustive] = { |@words|
var index = (count $words)
set index = (- $index 1)

put (env _CLAP_IFS="\n" _CLAP_COMPLETE_INDEX=(to-string $index) COMPLETE="elvish" exhaustive -- $@words) | to-lines
env _CLAP_IFS="\n" _CLAP_COMPLETE_INDEX=(to-string $index) COMPLETE="elvish" exhaustive -- $@words | from-lines | each { |line|
if (str:has-prefix $line "\x1f") {
var value = (str:trim-prefix $line "\x1f")
edit:complex-candidate $value &code-suffix=''
} else {
put $line
}
}
}


Original file line number Diff line number Diff line change
Expand Up @@ -11,24 +11,18 @@ function _clap_dynamic_completer_exhaustive() {
)}")

if [[ -n $completions ]]; then
local -a dirs=()
local -a nospace=()
local -a other=()
local completion
for completion in $completions; do
local value="${completion%%:*}"
if [[ "$value" == */ ]]; then
local dir_no_slash="${value%/}"
if [[ "$completion" == *:* ]]; then
local desc="${completion#*:}"
dirs+=("$dir_no_slash:$desc")
else
dirs+=("$dir_no_slash")
fi
if [[ "$completion" == $'\x1f'* ]]; then
completion="${completion:1}"
nospace+=("$completion")
else
other+=("$completion")
fi
done
[[ -n $dirs ]] && _describe 'values' dirs -S '/' -r '/'
[[ -n $nospace ]] && _describe 'values' nospace -S ''
[[ -n $other ]] && _describe 'values' other
fi
}
Expand Down
21 changes: 12 additions & 9 deletions clap_complete/tests/snapshots/register_minimal.bash
Original file line number Diff line number Diff line change
@@ -1,24 +1,27 @@

_clap_complete_my_app() {
local IFS=$'/013'
local IFS=$'\013'
local _CLAP_COMPLETE_INDEX=${COMP_CWORD}
local _CLAP_COMPLETE_COMP_TYPE=${COMP_TYPE}
if compopt +o nospace 2> /dev/null; then
local _CLAP_COMPLETE_SPACE=false
else
local _CLAP_COMPLETE_SPACE=true
fi
COMPREPLY=( $( /
IFS="$IFS" /
_CLAP_COMPLETE_INDEX="$_CLAP_COMPLETE_INDEX" /
_CLAP_COMPLETE_COMP_TYPE="$_CLAP_COMPLETE_COMP_TYPE" /
_CLAP_COMPLETE_SPACE="$_CLAP_COMPLETE_SPACE" /
"my-app" complete bash -- "${COMP_WORDS[@]}" /
COMPREPLY=( $( \
IFS="$IFS" \
_CLAP_COMPLETE_INDEX="$_CLAP_COMPLETE_INDEX" \
_CLAP_COMPLETE_COMP_TYPE="$_CLAP_COMPLETE_COMP_TYPE" \
_CLAP_COMPLETE_SPACE="$_CLAP_COMPLETE_SPACE" \
"my-app" complete bash -- "${COMP_WORDS[@]}" \
) )
if [[ $? != 0 ]]; then
unset COMPREPLY
elif [[ $_CLAP_COMPLETE_SPACE == false ]] && [[ "${COMPREPLY-}" =~ [=/:]$ ]]; then
compopt -o nospace
elif [[ $_CLAP_COMPLETE_SPACE == false ]]; then
if [[ ${#COMPREPLY[@]} -gt 0 ]] && [[ "${COMPREPLY[-1]}" == "_CLAP_COMPLETE_NOSPACE" ]]; then
unset 'COMPREPLY[-1]'
compopt -o nospace
fi
fi
}
if [[ "${BASH_VERSINFO[0]}" -eq 4 && "${BASH_VERSINFO[1]}" -ge 4 || "${BASH_VERSINFO[0]}" -gt 4 ]]; then
Expand Down
Loading
Loading