From 57ec686ea657badde2ea2250fb78cad116f16be3 Mon Sep 17 00:00:00 2001 From: Marc Khouzam Date: Sun, 13 Sep 2020 20:13:38 -0400 Subject: [PATCH 1/4] Fix fish for ShellDirectiveNoSpace and file comp For fish shell we achieve ShellDirectiveNoSpace by outputing a fake second completion with an extra character. However, this extra character was being added after the description string, instead of before. This commit fixes that. It also cleans up the script of useless code, now that fish completion details are better understood. Signed-off-by: Marc Khouzam --- fish_completions.go | 103 +++++++++++++++++++++++--------------------- 1 file changed, 54 insertions(+), 49 deletions(-) diff --git a/fish_completions.go b/fish_completions.go index 0b2ff7df7..f3910017f 100644 --- a/fish_completions.go +++ b/fish_completions.go @@ -28,9 +28,9 @@ function __%[1]s_debug end function __%[1]s_perform_completion - __%[1]s_debug "Starting __%[1]s_perform_completion with: $argv" + __%[1]s_debug "Starting __%[1]s_perform_completion" - set args (string split -- " " "$argv") + set args (string split -- " " (commandline -c)) set lastArg "$args[-1]" __%[1]s_debug "args: $args" @@ -84,31 +84,22 @@ function __%[1]s_perform_completion printf "%%s\n" "$directiveLine" end -# This function does three things: -# 1- Obtain the completions and store them in the global __%[1]s_comp_results -# 2- Set the __%[1]s_comp_do_file_comp flag if file completion should be performed -# and unset it otherwise -# 3- Return true if the completion results are not empty +# This function does two things: +# - Obtain the completions and store them in the global __%[1]s_comp_results +# - Return false if file completion should be performed function __%[1]s_prepare_completions + __%[1]s_debug "" + __%[1]s_debug "========= starting completion logic ==========" + # Start fresh - set --erase __%[1]s_comp_do_file_comp set --erase __%[1]s_comp_results - # Check if the command-line is already provided. This is useful for testing. - if not set --query __%[1]s_comp_commandLine - # Use the -c flag to allow for completion in the middle of the line - set __%[1]s_comp_commandLine (commandline -c) - end - __%[1]s_debug "commandLine is: $__%[1]s_comp_commandLine" - - set results (__%[1]s_perform_completion "$__%[1]s_comp_commandLine") - set --erase __%[1]s_comp_commandLine + set results (__%[1]s_perform_completion) __%[1]s_debug "Completion results: $results" if test -z "$results" __%[1]s_debug "No completion, probably due to a failure" # Might as well do file completion, in case it helps - set --global __%[1]s_comp_do_file_comp 1 return 1 end @@ -132,7 +123,6 @@ function __%[1]s_prepare_completions if test $compErr -eq 1 __%[1]s_debug "Received error directive: aborting." # Might as well do file completion, in case it helps - set --global __%[1]s_comp_do_file_comp 1 return 1 end @@ -141,7 +131,6 @@ function __%[1]s_prepare_completions if test $filefilter -eq 1; or test $dirfilter -eq 1 __%[1]s_debug "File extension filtering or directory filtering not supported" # Do full file completion instead - set --global __%[1]s_comp_do_file_comp 1 return 1 end @@ -150,32 +139,56 @@ function __%[1]s_prepare_completions __%[1]s_debug "nospace: $nospace, nofiles: $nofiles" - # Important not to quote the variable for count to work - set numComps (count $__%[1]s_comp_results) - __%[1]s_debug "numComps: $numComps" - - if test $numComps -eq 1; and test $nospace -ne 0 - # To support the "nospace" directive we trick the shell - # by outputting an extra, longer completion. - __%[1]s_debug "Adding second completion to perform nospace directive" - set --append __%[1]s_comp_results $__%[1]s_comp_results[1]. - end + # If we want to prevent a space, or if file completion is NOT disabled, + # we need to count the number of valid completions. + # To do so, we will filter on prefix as the completions we have received + # may not already be filtered so as to allow fish to match on different + # criteria than the prefix. + if test $nospace -ne 0; or test $nofiles -eq 0 + set prefix (commandline -t) + __%[1]s_debug "prefix: $prefix" + + set completions + for comp in $__%[1]s_comp_results + if test (string match -e -r -- "^$prefix" "$comp") + set -a completions $comp + end + end + set --global __%[1]s_comp_results $completions + __%[1]s_debug "Filtered completions are: $__%[1]s_comp_results" + + # Important not to quote the variable for count to work + set numComps (count $__%[1]s_comp_results) + __%[1]s_debug "numComps: $numComps" + + if test $numComps -eq 1; and test $nospace -ne 0 + # To support the "nospace" directive we trick the shell + # by outputting an extra, longer completion. + # We must first split on \t to get rid of the descriptions because + # the extra character we add to the fake second completion must be + # before the description. We don't need descriptions anyway since + # there is only a single real completion which the shell will expand + # immediately. + __%[1]s_debug "Adding second completion to perform nospace directive" + set split (string split --max 1 \t $__%[1]s_comp_results[1]) + set --global __%[1]s_comp_results $split[1] $split[1]. + __%[1]s_debug "Completions are now: $__%[1]s_comp_results" + end - if test $numComps -eq 0; and test $nofiles -eq 0 - __%[1]s_debug "Requesting file completion" - set --global __%[1]s_comp_do_file_comp 1 + if test $numComps -eq 0; and test $nofiles -eq 0 + # To be consistent with bash and zsh, we only trigger file + # completion when there are no other completions + __%[1]s_debug "Requesting file completion" + return 1 + end end - # If we don't want file completion, we must return true even if there - # are no completions found. This is because fish will perform the last - # completion command, even if its condition is false, if no other - # completion command was triggered - return (not set --query __%[1]s_comp_do_file_comp) + return 0 end # Since Fish completions are only loaded once the user triggers them, we trigger them ourselves # so we can properly delete any completions provided by another script. -# The space after the the program name is essential to trigger completion for the program +# The space after the program name is essential to trigger completion for the program # and not completion of the program name itself. complete --do-complete "%[2]s " > /dev/null 2>&1 # Using '> /dev/null 2>&1' since '&>' is not supported in older versions of fish. @@ -183,16 +196,8 @@ complete --do-complete "%[2]s " > /dev/null 2>&1 # Remove any pre-existing completions for the program since we will be handling all of them. complete -c %[2]s -e -# The order in which the below two lines are defined is very important so that __%[1]s_prepare_completions -# is called first. It is __%[1]s_prepare_completions that sets up the __%[1]s_comp_do_file_comp variable. -# -# This completion will be run second as complete commands are added FILO. -# It triggers file completion choices when __%[1]s_comp_do_file_comp is set. -complete -c %[2]s -n 'set --query __%[1]s_comp_do_file_comp' - -# This completion will be run first as complete commands are added FILO. -# The call to __%[1]s_prepare_completions will setup both __%[1]s_comp_results and __%[1]s_comp_do_file_comp. -# It provides the program's completion choices. +# The call to __%[1]s_prepare_completions will setup __%[1]s_comp_results +# which provides the program's completion choices. complete -c %[2]s -n '__%[1]s_prepare_completions' -f -a '$__%[1]s_comp_results' `, nameForVar, name, compCmd, From d6f02839b939e492649747d5b352bfc97f0de538 Mon Sep 17 00:00:00 2001 From: Marc Khouzam Date: Sun, 27 Dec 2020 13:49:22 -0500 Subject: [PATCH 2/4] Handle case when completion starts with a space Fixes #1303 Signed-off-by: Marc Khouzam --- fish_completions.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fish_completions.go b/fish_completions.go index f3910017f..99f91c99a 100644 --- a/fish_completions.go +++ b/fish_completions.go @@ -30,7 +30,7 @@ end function __%[1]s_perform_completion __%[1]s_debug "Starting __%[1]s_perform_completion" - set args (string split -- " " (commandline -c)) + set args (string split -- " " (string trim -l (commandline -c))) set lastArg "$args[-1]" __%[1]s_debug "args: $args" From 281d573652be1eb2cf1f27d6117bfcccfe7692c0 Mon Sep 17 00:00:00 2001 From: Marc Khouzam Date: Thu, 31 Dec 2020 16:41:02 -0500 Subject: [PATCH 3/4] Support fish completion with env vars in the path Fixes https://github.com/spf13/cobra/issues/1214 Fixes https://github.com/spf13/cobra/issues/1306 Signed-off-by: Marc Khouzam --- fish_completions.go | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/fish_completions.go b/fish_completions.go index 99f91c99a..55a7a2640 100644 --- a/fish_completions.go +++ b/fish_completions.go @@ -43,16 +43,13 @@ function __%[1]s_perform_completion end __%[1]s_debug "emptyArg: $emptyArg" - if not type -q "$args[1]" - # This can happen when "complete --do-complete %[2]s" is called when running this script. - __%[1]s_debug "Cannot find $args[1]. No completions." - return - end - set requestComp "$args[1] %[3]s $args[2..-1] $emptyArg" __%[1]s_debug "Calling $requestComp" - set results (eval $requestComp 2> /dev/null) + # Call the command as a sub-shell so that we can redirect any errors + # For example, if $requestComp has an unmatched quote + # https://github.com/spf13/cobra/issues/1214 + set results (fish -c "$requestComp" 2> /dev/null) # Some programs may output extra empty lines after the directive. # Let's ignore them or else it will break completion. @@ -66,6 +63,7 @@ function __%[1]s_perform_completion break end end + set comps $results[1..-2] set directiveLine $results[-1] From 9ab0247787edf05877e3a02d2fa30bf0c40411c9 Mon Sep 17 00:00:00 2001 From: Marc Khouzam Date: Sun, 24 Jan 2021 22:51:05 -0500 Subject: [PATCH 4/4] Update based on review 1- We use `set -l` for local variable to make sure there are no conflicts with global variables 2- We use `commandline -opc` which: a) splits the command line into tokens (-o) b) only considers the current command (-p) (e.g., echo hello; helm ) c) stops at the cursor (-c) 3- We extract the last arg with `commandline -ct` and escape it to handle the case where it is a space, or unmatched quote. 4- We avoid looping when filtering on prefix. 5- We don't add a fake comp for ShellCompDirectiveNoSpace when the completion ends with any of @=/:., as fish won't add a space Signed-off-by: Marc Khouzam --- fish_completions.go | 102 +++++++++++++++++++++----------------------- 1 file changed, 49 insertions(+), 53 deletions(-) diff --git a/fish_completions.go b/fish_completions.go index 55a7a2640..b0db323ac 100644 --- a/fish_completions.go +++ b/fish_completions.go @@ -21,7 +21,7 @@ func genFishComp(buf io.StringWriter, name string, includeDesc bool) { WriteStringAndCheck(buf, fmt.Sprintf("# fish completion for %-36s -*- shell-script -*-\n", name)) WriteStringAndCheck(buf, fmt.Sprintf(` function __%[1]s_debug - set file "$BASH_COMP_DEBUG_FILE" + set -l file "$BASH_COMP_DEBUG_FILE" if test -n "$file" echo "$argv" >> $file end @@ -30,26 +30,18 @@ end function __%[1]s_perform_completion __%[1]s_debug "Starting __%[1]s_perform_completion" - set args (string split -- " " (string trim -l (commandline -c))) - set lastArg "$args[-1]" + # Extract all args except the last one + set -l args (commandline -opc) + # Extract the last arg and escape it in case it is a space + set -l lastArg (string escape -- (commandline -ct)) __%[1]s_debug "args: $args" __%[1]s_debug "last arg: $lastArg" - set emptyArg "" - if test -z "$lastArg" - __%[1]s_debug "Setting emptyArg" - set emptyArg \"\" - end - __%[1]s_debug "emptyArg: $emptyArg" + set -l requestComp "$args[1] %[3]s $args[2..-1] $lastArg" - set requestComp "$args[1] %[3]s $args[2..-1] $emptyArg" __%[1]s_debug "Calling $requestComp" - - # Call the command as a sub-shell so that we can redirect any errors - # For example, if $requestComp has an unmatched quote - # https://github.com/spf13/cobra/issues/1214 - set results (fish -c "$requestComp" 2> /dev/null) + set -l results (eval $requestComp 2> /dev/null) # Some programs may output extra empty lines after the directive. # Let's ignore them or else it will break completion. @@ -64,12 +56,12 @@ function __%[1]s_perform_completion end end - set comps $results[1..-2] - set directiveLine $results[-1] + set -l comps $results[1..-2] + set -l directiveLine $results[-1] # For Fish, when completing a flag with an = (e.g., -n=) # completions must be prefixed with the flag - set flagPrefix (string match -r -- '-.*=' "$lastArg") + set -l flagPrefix (string match -r -- '-.*=' "$lastArg") __%[1]s_debug "Comps: $comps" __%[1]s_debug "DirectiveLine: $directiveLine" @@ -92,7 +84,7 @@ function __%[1]s_prepare_completions # Start fresh set --erase __%[1]s_comp_results - set results (__%[1]s_perform_completion) + set -l results (__%[1]s_perform_completion) __%[1]s_debug "Completion results: $results" if test -z "$results" @@ -101,39 +93,39 @@ function __%[1]s_prepare_completions return 1 end - set directive (string sub --start 2 $results[-1]) + set -l directive (string sub --start 2 $results[-1]) set --global __%[1]s_comp_results $results[1..-2] __%[1]s_debug "Completions are: $__%[1]s_comp_results" __%[1]s_debug "Directive is: $directive" - set shellCompDirectiveError %[4]d - set shellCompDirectiveNoSpace %[5]d - set shellCompDirectiveNoFileComp %[6]d - set shellCompDirectiveFilterFileExt %[7]d - set shellCompDirectiveFilterDirs %[8]d + set -l shellCompDirectiveError %[4]d + set -l shellCompDirectiveNoSpace %[5]d + set -l shellCompDirectiveNoFileComp %[6]d + set -l shellCompDirectiveFilterFileExt %[7]d + set -l shellCompDirectiveFilterDirs %[8]d if test -z "$directive" set directive 0 end - set compErr (math (math --scale 0 $directive / $shellCompDirectiveError) %% 2) + set -l compErr (math (math --scale 0 $directive / $shellCompDirectiveError) %% 2) if test $compErr -eq 1 __%[1]s_debug "Received error directive: aborting." # Might as well do file completion, in case it helps return 1 end - set filefilter (math (math --scale 0 $directive / $shellCompDirectiveFilterFileExt) %% 2) - set dirfilter (math (math --scale 0 $directive / $shellCompDirectiveFilterDirs) %% 2) + set -l filefilter (math (math --scale 0 $directive / $shellCompDirectiveFilterFileExt) %% 2) + set -l dirfilter (math (math --scale 0 $directive / $shellCompDirectiveFilterDirs) %% 2) if test $filefilter -eq 1; or test $dirfilter -eq 1 __%[1]s_debug "File extension filtering or directory filtering not supported" # Do full file completion instead return 1 end - set nospace (math (math --scale 0 $directive / $shellCompDirectiveNoSpace) %% 2) - set nofiles (math (math --scale 0 $directive / $shellCompDirectiveNoFileComp) %% 2) + set -l nospace (math (math --scale 0 $directive / $shellCompDirectiveNoSpace) %% 2) + set -l nofiles (math (math --scale 0 $directive / $shellCompDirectiveNoFileComp) %% 2) __%[1]s_debug "nospace: $nospace, nofiles: $nofiles" @@ -143,34 +135,34 @@ function __%[1]s_prepare_completions # may not already be filtered so as to allow fish to match on different # criteria than the prefix. if test $nospace -ne 0; or test $nofiles -eq 0 - set prefix (commandline -t) + set -l prefix (commandline -t | string escape --style=regex) __%[1]s_debug "prefix: $prefix" - set completions - for comp in $__%[1]s_comp_results - if test (string match -e -r -- "^$prefix" "$comp") - set -a completions $comp - end - end + set -l completions (string match -r -- "^$prefix.*" $__%[1]s_comp_results) set --global __%[1]s_comp_results $completions __%[1]s_debug "Filtered completions are: $__%[1]s_comp_results" # Important not to quote the variable for count to work - set numComps (count $__%[1]s_comp_results) + set -l numComps (count $__%[1]s_comp_results) __%[1]s_debug "numComps: $numComps" if test $numComps -eq 1; and test $nospace -ne 0 - # To support the "nospace" directive we trick the shell - # by outputting an extra, longer completion. - # We must first split on \t to get rid of the descriptions because - # the extra character we add to the fake second completion must be - # before the description. We don't need descriptions anyway since - # there is only a single real completion which the shell will expand - # immediately. - __%[1]s_debug "Adding second completion to perform nospace directive" - set split (string split --max 1 \t $__%[1]s_comp_results[1]) - set --global __%[1]s_comp_results $split[1] $split[1]. - __%[1]s_debug "Completions are now: $__%[1]s_comp_results" + # We must first split on \t to get rid of the descriptions to be + # able to check what the actual completion will be. + # We don't need descriptions anyway since there is only a single + # real completion which the shell will expand immediately. + set -l split (string split --max 1 \t $__%[1]s_comp_results[1]) + + # Fish won't add a space if the completion ends with any + # of the following characters: @=/:., + set -l lastChar (string sub -s -1 -- $split) + if not string match -r -q "[@=/:.,]" -- "$lastChar" + # In other cases, to support the "nospace" directive we trick the shell + # by outputting an extra, longer completion. + __%[1]s_debug "Adding second completion to perform nospace directive" + set --global __%[1]s_comp_results $split[1] $split[1]. + __%[1]s_debug "Completions are now: $__%[1]s_comp_results" + end end if test $numComps -eq 0; and test $nofiles -eq 0 @@ -186,10 +178,14 @@ end # Since Fish completions are only loaded once the user triggers them, we trigger them ourselves # so we can properly delete any completions provided by another script. -# The space after the program name is essential to trigger completion for the program -# and not completion of the program name itself. -complete --do-complete "%[2]s " > /dev/null 2>&1 -# Using '> /dev/null 2>&1' since '&>' is not supported in older versions of fish. +# Only do this if the program can be found, or else fish may print some errors; besides, +# the existing completions will only be loaded if the program can be found. +if type -q "%[2]s" + # The space after the program name is essential to trigger completion for the program + # and not completion of the program name itself. + # Also, we use '> /dev/null 2>&1' since '&>' is not supported in older versions of fish. + complete --do-complete "%[2]s " > /dev/null 2>&1 +end # Remove any pre-existing completions for the program since we will be handling all of them. complete -c %[2]s -e