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.

Sign up for GitHub

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

fix(bash): Fix bash completion for suggestions that contain special characters. #2126

Closed
wants to merge 8 commits into from
Closed

fix(bash): Fix bash completion for suggestions that contain special characters. #2126

Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
d131fa6
fix(bash): Fix bash completion for suggestions that contain special c…
JeffFaer Feb 28, 2024
497d36c
fix: We don't need an extra empty character when we're doing proper q…
JeffFaer Apr 20, 2024
76d900b
Don't quote different completion types.
JeffFaer May 1, 2024
9771cb0
address review comments
JeffFaer Dec 5, 2024
718f1d8
Handle wordbreaks better.
JeffFaer Dec 5, 2024
df206d9
remove line
JeffFaer Dec 5, 2024
7a9725b
Only quote the last arg.
JeffFaer Dec 6, 2024
2fc6dad
Automatically escape all special characters as soon as completion wou…
JeffFaer Dec 30, 2024
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
79 changes: 57 additions & 22 deletions bash_completionsV2.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,19 +59,15 @@ __%[1]s_get_completion_results() {
# Prepare the command to request completions for the program.
# Calling ${words[0]} instead of directly %[1]s allows handling aliases
args=("${words[@]:1}")
requestComp="${words[0]} %[2]s ${args[*]}"
requestComp="${words[0]} %[2]s"
if [[ "${#args[@]}" -gt 0 ]]; then
requestComp+="$(printf " %%q" "${args[@]}")"
fi

lastParam=${words[$((${#words[@]}-1))]}
lastChar=${lastParam:$((${#lastParam}-1)):1}
__%[1]s_debug "lastParam ${lastParam}, lastChar ${lastChar}"

if [[ -z ${cur} && ${lastChar} != = ]]; then
# If the last parameter is complete (there is a space following it)
# We add an extra empty parameter so we can indicate this to the go method.
__%[1]s_debug "Adding extra empty parameter"
requestComp="${requestComp} ''"
fi

# When completing a flag with an = (e.g., %[1]s -n=<TAB>)
# bash focuses on the part after the =, so we need to remove
# the flag part from $cur
Expand Down Expand Up @@ -173,8 +169,9 @@ __%[1]s_process_completion_results() {
__%[1]s_handle_completion_types
fi

__%[1]s_handle_special_char "$cur" :
__%[1]s_handle_special_char "$cur" =
__%[1]s_handle_wordbreaks "$cur"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

At surface value, this line looked wrong to me because I didn't think it was correct to handle wordbreaks like this when the user requested menu-completion. But after some manual testing I'm pleasantly surprised to find that this is correct, and menu-completion does still expect the same behavior as "regular" completion.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I didn't understand why this change was needed so for safety, I did not take it.


__%[1]s_debug "The final COMPREPLY: $(printf "%%s\n" "${COMPREPLY[@]}")"

# Print the activeHelp statements before we finish
if ((${#activeHelp[*]} != 0)); then
Expand Down Expand Up @@ -224,20 +221,28 @@ __%[1]s_handle_completion_types() {
# completions at once on the command-line we must remove the descriptions.
# https://github.com/spf13/cobra/issues/1508
local tab=$'\t' comp
while IFS='' read -r comp; do
for comp in "${completions[@]}"; do
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure why this was needed so I didn't take it.

[[ -z $comp ]] && continue
# Strip any description
comp=${comp%%%%$tab*}
# Only consider the completions that match
if [[ $comp == "$cur"* ]]; then
Copy link
Collaborator

Choose a reason for hiding this comment

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

We need to do the escaping before we filter on "$cur" so that when the user typed bash1\ s it still matches the (escaped) completion

COMPREPLY+=("$comp")
fi
done < <(printf "%%s\n" "${completions[@]}")
fi
done

IFS=$'\n' read -ra COMPREPLY -d '' < <(printf "%%q\n" "${COMPREPLY[@]}")
;;

*)
# Type: complete (normal completion)
__%[1]s_handle_standard_completion_case

# If there is a single completion left, escape the completion
if ((${#COMPREPLY[@]} == 1)); then
COMPREPLY[0]="$(printf "%%q" "${COMPREPLY[0]}")"
fi

;;
esac
}
Expand All @@ -247,7 +252,13 @@ __%[1]s_handle_standard_completion_case() {

# Short circuit to optimize if we don't have descriptions
if [[ "${completions[*]}" != *$tab* ]]; then
IFS=$'\n' read -ra COMPREPLY -d '' < <(compgen -W "${completions[*]}" -- "$cur")
# compgen's -W option respects shell quoting, so we need to escape.
local compgen_words="$(printf "%%q\n" "${completions[@]}")"
# compgen appears to respect shell quoting _after_ checking whether
# they have the right prefix, so we also need to quote cur.
local compgen_cur="$(printf "%%q" "${cur}")"
IFS=$'\n' read -ra COMPREPLY -d '' < <(IFS=$'\n'; compgen -W "${compgen_words}" -- "${compgen_cur}")

return 0
fi

Expand All @@ -271,21 +282,30 @@ __%[1]s_handle_standard_completion_case() {
__%[1]s_debug "COMPREPLY[0]: ${COMPREPLY[0]}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

In the loop above, we need to do the escaping before we filter on "$cur" so that when the user typed bash1\ s it still matches the (escaped) completion

comp="${COMPREPLY[0]%%%%$tab*}"
__%[1]s_debug "Removed description from single completion, which is now: ${comp}"
COMPREPLY[0]=$comp
COMPREPLY[0]="${comp}"
else # Format the descriptions
__%[1]s_format_comp_descriptions $longest
fi
}

__%[1]s_handle_special_char()
__%[1]s_handle_wordbreaks()
{
if ((${#COMPREPLY[@]} == 0)); then
return;
fi

local comp="$1"
local char=$2
if [[ "$comp" == *${char}* && "$COMP_WORDBREAKS" == *${char}* ]]; then
local word=${comp%%"${comp##*${char}}"}
local idx=${#COMPREPLY[*]}
while ((--idx >= 0)); do
COMPREPLY[idx]=${COMPREPLY[idx]#"$word"}
local i prefix
for ((i=0; i < ${#comp}; i++)); do
local char="${comp:$i:1}"
if [[ "${COMP_WORDBREAKS}" == *"${char}"* ]]; then
prefix="${comp::$i+1}"
fi
done

if [[ -n "${prefix}" ]]; then
for ((i=0; i < ${#COMPREPLY[@]}; i++)); do
COMPREPLY[i]=${COMPREPLY[i]#$prefix}
done
fi
}
Expand Down Expand Up @@ -342,6 +362,21 @@ __start_%[1]s()

COMPREPLY=()

# Omit wordbreaks that would need to be escaped.
local wordbreaks i
for ((i=0; i < ${#COMP_WORDBREAKS}; i++)); do
local char="${COMP_WORDBREAKS:$i:1}"
if [[ $'\n\t ' == *"${char}"* ]]; then
wordbreaks+="${char}"
continue
fi
if [[ "${char}" == "$(printf "%%q" "${char}")" ]]; then
wordbreaks+="${char}"
continue
fi
done
COMP_WORDBREAKS="${wordbreaks}"

# Call _init_completion from the bash-completion package
# to prepare the arguments properly
if declare -F _init_completion >/dev/null 2>&1; then
Expand Down
Loading