From d131fa6e8884350ba8932a0ea9e4e9cc7ff11d10 Mon Sep 17 00:00:00 2001 From: Jeffrey Faer Date: Tue, 27 Feb 2024 23:11:42 -0700 Subject: [PATCH 1/7] fix(bash): Fix bash completion for suggestions that contain special characters. Special characters include whitepace, so this is more general than #1743. This should also fix #1740. I added some test cases to cobra-completion-testing. This PR makes them pass. It also doesn't trip any of the performance regression tests. I'm happy to submit those tests as a PR as well. - https://github.com/JeffFaer/cobra-completion-testing/tree/special_characters - https://github.com/JeffFaer/cobra-completion-testing/commit/52254c1a25b9816cf31a617cf9667e61a1bd2259 Signed-off-by: Jeffrey Faer --- bash_completionsV2.go | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/bash_completionsV2.go b/bash_completionsV2.go index 1cce5c3..cf3a361 100644 --- a/bash_completionsV2.go +++ b/bash_completionsV2.go @@ -59,7 +59,10 @@ __%[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} @@ -224,15 +227,24 @@ __%[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 + local matches=() + for comp in "${completions[@]}"; do [[ -z $comp ]] && continue # Strip any description comp=${comp%%%%$tab*} # Only consider the completions that match if [[ $comp == "$cur"* ]]; then - COMPREPLY+=("$comp") - fi - done < <(printf "%%s\n" "${completions[@]}") + # Strictly speaking we could append directly to COMPREPLY here. + # But there's a pretty big performance hit involved with + # creating one subshell to printf %%q for each completion that + # matches. Instead, batch all the matches up so we can quote + # them all at once in a single printf call. + matches+=( "$comp" ) + fi + done + while IFS='' read -r comp; do + COMPREPLY+=( "$comp" ) + done < <(printf "%%q\n" "${matches[@]}") ;; *) @@ -247,7 +259,12 @@ __%[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 @@ -271,7 +288,7 @@ __%[1]s_handle_standard_completion_case() { __%[1]s_debug "COMPREPLY[0]: ${COMPREPLY[0]}" comp="${COMPREPLY[0]%%%%$tab*}" __%[1]s_debug "Removed description from single completion, which is now: ${comp}" - COMPREPLY[0]=$comp + COMPREPLY[0]="$(printf "%%q" "${comp}")" else # Format the descriptions __%[1]s_format_comp_descriptions $longest fi From 497d36cfc1b51ab8ce525a7d35195d45c9ced40e Mon Sep 17 00:00:00 2001 From: Jeffrey Faer Date: Fri, 19 Apr 2024 23:25:37 -0600 Subject: [PATCH 2/7] fix: We don't need an extra empty character when we're doing proper quoting. Signed-off-by: Jeffrey Faer --- bash_completionsV2.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/bash_completionsV2.go b/bash_completionsV2.go index cf3a361..98c5696 100644 --- a/bash_completionsV2.go +++ b/bash_completionsV2.go @@ -68,13 +68,6 @@ __%[1]s_get_completion_results() { 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=) # bash focuses on the part after the =, so we need to remove # the flag part from $cur From 76d900bc72e0691dc0c911070a88714a8528c61d Mon Sep 17 00:00:00 2001 From: Jeffrey Faer Date: Wed, 1 May 2024 12:54:23 -0600 Subject: [PATCH 3/7] Don't quote different completion types. They're not going through compgen so they don't need it. Signed-off-by: Jeffrey Faer --- bash_completionsV2.go | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/bash_completionsV2.go b/bash_completionsV2.go index 98c5696..79846bb 100644 --- a/bash_completionsV2.go +++ b/bash_completionsV2.go @@ -220,24 +220,15 @@ __%[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 - local matches=() for comp in "${completions[@]}"; do [[ -z $comp ]] && continue # Strip any description comp=${comp%%%%$tab*} # Only consider the completions that match if [[ $comp == "$cur"* ]]; then - # Strictly speaking we could append directly to COMPREPLY here. - # But there's a pretty big performance hit involved with - # creating one subshell to printf %%q for each completion that - # matches. Instead, batch all the matches up so we can quote - # them all at once in a single printf call. - matches+=( "$comp" ) + COMPREPLY+=( "$comp" ) fi done - while IFS='' read -r comp; do - COMPREPLY+=( "$comp" ) - done < <(printf "%%q\n" "${matches[@]}") ;; *) From 9771cb0949938723af21df1e14c6269314e8567e Mon Sep 17 00:00:00 2001 From: Jeffrey Faer Date: Wed, 4 Dec 2024 21:39:24 -0700 Subject: [PATCH 4/7] address review comments Signed-off-by: Jeffrey Faer --- bash_completionsV2.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/bash_completionsV2.go b/bash_completionsV2.go index 79846bb..b39cce5 100644 --- a/bash_completionsV2.go +++ b/bash_completionsV2.go @@ -226,9 +226,11 @@ __%[1]s_handle_completion_types() { comp=${comp%%%%$tab*} # Only consider the completions that match if [[ $comp == "$cur"* ]]; then - COMPREPLY+=( "$comp" ) + COMPREPLY+=("$comp") fi done + + IFS=$'\n' read -ra COMPREPLY -d '' < <(printf "%%q\n" "${COMPREPLY[@]}") ;; *) @@ -249,6 +251,12 @@ __%[1]s_handle_standard_completion_case() { # 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}") + + # If there is a single completion left, escape the completion + if ((${#COMPREPLY[*]} == 1)); then + COMPREPLY[0]=$(printf %%q "${COMPREPLY[0]}") + fi + return 0 fi From 718f1d85e550fb5b69c4f1016caf7d5435e4a861 Mon Sep 17 00:00:00 2001 From: Jeffrey Faer Date: Wed, 4 Dec 2024 22:55:38 -0700 Subject: [PATCH 5/7] Handle wordbreaks better. Signed-off-by: Jeffrey Faer --- bash_completionsV2.go | 56 +++++++++++++++++++++++++++++++------------ 1 file changed, 41 insertions(+), 15 deletions(-) diff --git a/bash_completionsV2.go b/bash_completionsV2.go index b39cce5..d087ae7 100644 --- a/bash_completionsV2.go +++ b/bash_completionsV2.go @@ -169,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" + + __%[1]s_debug "The final COMPREPLY: $(printf "%%s\n" "${COMPREPLY[@]}")" # Print the activeHelp statements before we finish if ((${#activeHelp[*]} != 0)); then @@ -236,6 +237,12 @@ __%[1]s_handle_completion_types() { *) # 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 } @@ -252,11 +259,6 @@ __%[1]s_handle_standard_completion_case() { local compgen_cur="$(printf "%%q" "${cur}")" IFS=$'\n' read -ra COMPREPLY -d '' < <(IFS=$'\n'; compgen -W "${compgen_words}" -- "${compgen_cur}") - # If there is a single completion left, escape the completion - if ((${#COMPREPLY[*]} == 1)); then - COMPREPLY[0]=$(printf %%q "${COMPREPLY[0]}") - fi - return 0 fi @@ -280,21 +282,30 @@ __%[1]s_handle_standard_completion_case() { __%[1]s_debug "COMPREPLY[0]: ${COMPREPLY[0]}" comp="${COMPREPLY[0]%%%%$tab*}" __%[1]s_debug "Removed description from single completion, which is now: ${comp}" - COMPREPLY[0]="$(printf "%%q" "${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 } @@ -351,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 From df206d9f382523479eb4bac108738783be076b87 Mon Sep 17 00:00:00 2001 From: Jeffrey Faer Date: Wed, 4 Dec 2024 23:04:36 -0700 Subject: [PATCH 6/7] remove line Signed-off-by: Jeffrey Faer --- bash_completionsV2.go | 1 - 1 file changed, 1 deletion(-) diff --git a/bash_completionsV2.go b/bash_completionsV2.go index d087ae7..ff4df32 100644 --- a/bash_completionsV2.go +++ b/bash_completionsV2.go @@ -258,7 +258,6 @@ __%[1]s_handle_standard_completion_case() { # 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 From 7a9725b5878c945ad342d754eeae476834096110 Mon Sep 17 00:00:00 2001 From: Jeffrey Faer Date: Thu, 5 Dec 2024 21:33:06 -0700 Subject: [PATCH 7/7] Only quote the last arg. The other ones should already have been quoted by the completion script. Signed-off-by: Jeffrey Faer --- bash_completionsV2.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/bash_completionsV2.go b/bash_completionsV2.go index ff4df32..c57c06b 100644 --- a/bash_completionsV2.go +++ b/bash_completionsV2.go @@ -61,7 +61,10 @@ __%[1]s_get_completion_results() { args=("${words[@]:1}") requestComp="${words[0]} %[2]s" if [[ "${#args[@]}" -gt 0 ]]; then - requestComp+="$(printf " %%q" "${args[@]}")" + # Previous args should already be escaped... + requestComp+=" ${args[*]::${#args[@]}-1}" + # ...but the current arg might not yet be escaped. + requestComp+=" $(printf "%%q" "${args[${#args[@]}-1]}")" fi lastParam=${words[$((${#words[@]}-1))]}