From cc565fc16c69affeda4a7e8a1c8d7e419fc5c519 Mon Sep 17 00:00:00 2001 From: Kevin Ballard Date: Wed, 9 Jul 2014 18:21:06 -0700 Subject: [PATCH 01/35] Teach `command` builtin a -p/--path flag Give the `command` builtin a single flag, -p/--path, that causes it to print the full path that would be executed for the given command. --- builtin.cpp | 82 ++++++++++++++++++++++++++++++++++++++++++++- doc_src/command.txt | 12 +++++-- 2 files changed, 91 insertions(+), 3 deletions(-) diff --git a/builtin.cpp b/builtin.cpp index a7b173bb2..2d3eb749d 100644 --- a/builtin.cpp +++ b/builtin.cpp @@ -1053,6 +1053,86 @@ static int builtin_emit(parser_t &parser, wchar_t **argv) } +/** + Implementation of the builtin 'command'. Actual command running is handled by + the parser, this just processes the flags. +*/ +static int builtin_command(parser_t &parser, wchar_t **argv) +{ + int argc=builtin_count_args(argv); + int print_path=0; + + woptind=0; + + static const struct woption + long_options[] = + { + { L"path", no_argument, 0, 'p' }, + { L"help", no_argument, 0, 'h' }, + { 0, 0, 0, 0 } + }; + + while (1) + { + int opt_index = 0; + + int opt = wgetopt_long(argc, + argv, + L"ph", + long_options, + &opt_index); + if (opt == -1) + break; + + switch (opt) + { + case 0: + if (long_options[opt_index].flag != 0) + break; + append_format(stderr_buffer, + BUILTIN_ERR_UNKNOWN, + argv[0], + long_options[opt_index].name); + builtin_print_help(parser, argv[0], stderr_buffer); + return STATUS_BUILTIN_ERROR; + + case 'h': + builtin_print_help(parser, argv[0], stdout_buffer); + return STATUS_BUILTIN_OK; + + case 'p': + print_path=1; + break; + + case '?': + builtin_unknown_option(parser, argv[0], argv[woptind-1]); + return STATUS_BUILTIN_ERROR; + + } + + } + + if (!print_path) + { + builtin_print_help(parser, argv[0], stdout_buffer); + return STATUS_BUILTIN_ERROR; + } + + int found=0; + + for (int idx = woptind; argv[idx]; ++idx) + { + const wchar_t *command_name = argv[idx]; + wcstring path; + if (path_get_path(command_name, &path)) + { + append_format(stdout_buffer, L"%ls\n", path.c_str()); + ++found; + } + } + return found ? STATUS_BUILTIN_OK : STATUS_BUILTIN_ERROR; +} + /** A generic bultin that only supports showing a help message. This is only a placeholder that prints the help message. Useful for @@ -3703,7 +3783,7 @@ static const builtin_data_t builtin_datas[]= { L"builtin", &builtin_builtin, N_(L"Run a builtin command instead of a function") }, { L"case", &builtin_generic, N_(L"Conditionally execute a block of commands") }, { L"cd", &builtin_cd, N_(L"Change working directory") }, - { L"command", &builtin_generic, N_(L"Run a program instead of a function or builtin") }, + { L"command", &builtin_command, N_(L"Run a program instead of a function or builtin") }, { L"commandline", &builtin_commandline, N_(L"Set or get the commandline") }, { L"complete", &builtin_complete, N_(L"Edit command specific completions") }, { L"contains", &builtin_contains, N_(L"Search for a specified string in a list") }, diff --git a/doc_src/command.txt b/doc_src/command.txt index 19baccd46..e93f34b8e 100644 --- a/doc_src/command.txt +++ b/doc_src/command.txt @@ -1,12 +1,20 @@ \section command command - run a program \subsection command-synopsis Synopsis -command COMMANDNAME [OPTIONS...] +command [OPTIONS] COMMANDNAME [ARGS...] \subsection command-description Description \c command forces the shell to execute the program \c COMMANDNAME and ignore any functions or builtins with the same name. -\subsection command-example Example +The following options are available: +- \c -h or \c --help prints help and then exits. +- \c -p or \c --path returns the name of the disk file that would be executed, or nothing if no file with the specified name could be found in the $PATH. + +With the \c -p option, \c command treats every argument as a separate command to look up and sets the exit status to 0 if any of the specified commands were found, or 1 if no commands could be found. + +\subsection command-example Examples command ls causes fish to execute the \c ls program, even if an 'ls' function exists. + +command -p ls returns the path to the \c ls program. From 0933e5cab470125fc3757dc146173551a4cad177 Mon Sep 17 00:00:00 2001 From: Kevin Ballard Date: Wed, 9 Jul 2014 19:45:24 -0700 Subject: [PATCH 02/35] Fix typo in documentation for `type` builtin --- doc_src/type.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc_src/type.txt b/doc_src/type.txt index 9b5699f35..25bac5289 100644 --- a/doc_src/type.txt +++ b/doc_src/type.txt @@ -14,7 +14,7 @@ The following options are available: - \c -f or \c --no-functions suppresses function and builtin lookup. - \c -t or \c --type prints keyword, function, builtin, or file if \c NAME is a shell reserved word, function, builtin, or disk file, respectively. - \c -p or \c --path returns the name of the disk file that would be executed, or nothing if 'type -t name' would not return 'file'. -- \c -P or \c --force-path returns the name of the disk file that would be executed, or nothing no file with the specified name could be found in the $PATH. +- \c -P or \c --force-path returns the name of the disk file that would be executed, or nothing if no file with the specified name could be found in the $PATH. \c type sets the exit status to 0 if the specified command was found, and 1 if it could not be found. From bfd3a47380f65e24ff568efd1570338298d56200 Mon Sep 17 00:00:00 2001 From: Kevin Ballard Date: Wed, 9 Jul 2014 23:38:33 -0700 Subject: [PATCH 03/35] Fix `type` function to work better Stop using getopt to parse flags. It's far more expensive than necessary, and results in long flags not being parsed on OS X. This also allows args starting with - after the options list to be properly interpreted as a value to test. Print the error message to stderr as is appropriate. Use the new `command -p` functionality when the -a flag has not been provided (`command` does not have any equivalent to the -a flag), instead of using `which`. This is faster and also avoids any possible disagreement between `which` and what fish thinks is valid. Stop testing every path to see if it's executable, that test has already been done by `which` or `command -p`. The end result is `type -P ls` is roughly 250% faster, according to profiling, on my OS X machine. --- share/functions/type.fish | 114 ++++++++++++++------------------------ 1 file changed, 43 insertions(+), 71 deletions(-) diff --git a/share/functions/type.fish b/share/functions/type.fish index 4a4c77031..2ddcc622e 100644 --- a/share/functions/type.fish +++ b/share/functions/type.fish @@ -5,74 +5,48 @@ function type --description "Print the type of a command" set -l res 1 set -l mode normal set -l selection all - - # - # Get options - # - set -l options - set -l shortopt tpPafh - if not getopt -T > /dev/null - # GNU getopt - set -l longopt type,path,force-path,all,no-functions,help - set options -o $shortopt -l $longopt -- - # Verify options - if not getopt -n type $options $argv >/dev/null - return 1 - end - else - # Old getopt, used on OS X - set options $shortopt - # Verify options - if not getopt $options $argv >/dev/null - return 1 - end - end - # Do the real getopt invocation - set -l tmp (getopt $options $argv) + # Parse options + set -l names + if test (count $argv) -gt 0 + for i in (seq (count $argv)) + switch $argv[$i] + case -t --type + set mode type - # Break tmp up into an array - set -l opt - eval set opt $tmp - - for i in $opt - switch $i - case -t --type - set mode type + case -p --path + set mode path - case -p --path - set mode path + case -P --force-path + set mode path + set selection files - case -P --force-path - set mode path - set selection files + case -a --all + set selection multi - case -a --all - set selection multi + case -f --no-functions + set selection files - case -f --no-functions - set selection files + case -h --help + __fish_print_help type + return 0 - case -h --help - __fish_print_help type - return 0 - - case -- - break + case -- + set names $argv[$i..-1] + set -e names[1] + break + case '*' + set names $argv[$i..-1] + break + end end end # Check all possible types for the remaining arguments - for i in $argv - - switch $i - case '-*' - continue - end - + for i in $names # Found will be set to 1 if a match is found - set found 0 + set -l found 0 if test $selection != files @@ -119,32 +93,30 @@ function type --description "Print the type of a command" set -l paths if test $selection != multi - set paths (which $i ^/dev/null) + set paths (command -p $i) else set paths (which -a $i ^/dev/null) end for path in $paths - if test -x (echo $path) - set res 0 - set found 1 - switch $mode - case normal - printf (_ '%s is %s\n') $i $path + set res 0 + set found 1 + switch $mode + case normal + printf (_ '%s is %s\n') $i $path - case type - echo (_ 'file') + case type + echo (_ 'file') - case path - echo $path - end - if test $selection != multi - continue - end + case path + echo $path + end + if test $selection != multi + continue end end if test $found = 0 - printf (_ "%s: Could not find '%s'\n") type $i + printf (_ "%s: Could not find '%s'\n") type $i >&2 end end From 6b062b07b4758883c3d2b7bb28791a86b267ba0a Mon Sep 17 00:00:00 2001 From: Kevin Ballard Date: Thu, 10 Jul 2014 00:07:58 -0700 Subject: [PATCH 04/35] type: Separate the notion of multi and paths Track whether -a and -f have been supplied separately. That way both `type -a -f command` and `type -f -a command` behaves correctly, as does `type -a -f foo` where there are multiple executables named `foo` in the $PATH. --- share/functions/type.fish | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/share/functions/type.fish b/share/functions/type.fish index 2ddcc622e..9cadcabc3 100644 --- a/share/functions/type.fish +++ b/share/functions/type.fish @@ -4,6 +4,7 @@ function type --description "Print the type of a command" # Initialize set -l res 1 set -l mode normal + set -l multi no set -l selection all # Parse options @@ -22,7 +23,7 @@ function type --description "Print the type of a command" set selection files case -a --all - set selection multi + set multi yes case -f --no-functions set selection files @@ -65,7 +66,7 @@ function type --description "Print the type of a command" echo end - if test $selection != multi + if test $multi != yes continue end end @@ -84,7 +85,7 @@ function type --description "Print the type of a command" case path echo end - if test $selection != multi + if test $multi != yes continue end end @@ -92,7 +93,7 @@ function type --description "Print the type of a command" end set -l paths - if test $selection != multi + if test $multi != yes set paths (command -p $i) else set paths (which -a $i ^/dev/null) @@ -110,7 +111,7 @@ function type --description "Print the type of a command" case path echo $path end - if test $selection != multi + if test $multi != yes continue end end From 6f7a7459c1dadc88e793b558d5ce2668359e7bea Mon Sep 17 00:00:00 2001 From: Kevin Ballard Date: Thu, 10 Jul 2014 00:18:50 -0700 Subject: [PATCH 05/35] test: Add a new --quiet flag to suppress output The --quiet flag is useful when only the exit status matters. Fix the documentation for the -t flag to no longer claim that `type` can print "keyword", as it never does that. Stop printing a blank line for functions/builtins when the -p flag has been passed. It's just not useful. --- doc_src/type.txt | 3 ++- share/functions/type.fish | 24 +++++++++++++----------- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/doc_src/type.txt b/doc_src/type.txt index 25bac5289..8945102ca 100644 --- a/doc_src/type.txt +++ b/doc_src/type.txt @@ -12,9 +12,10 @@ The following options are available: - \c -h or \c --help prints help and then exits. - \c -a or \c --all prints all of possible definitions of the specified names. - \c -f or \c --no-functions suppresses function and builtin lookup. -- \c -t or \c --type prints keyword, function, builtin, or file if \c NAME is a shell reserved word, function, builtin, or disk file, respectively. +- \c -t or \c --type prints function, builtin, or file if \c NAME is a shell function, builtin, or disk file, respectively. - \c -p or \c --path returns the name of the disk file that would be executed, or nothing if 'type -t name' would not return 'file'. - \c -P or \c --force-path returns the name of the disk file that would be executed, or nothing if no file with the specified name could be found in the $PATH. +- \c -q or \c --quiet suppresses all output; this is useful when testing the exit status. \c type sets the exit status to 0 if the specified command was found, and 1 if it could not be found. diff --git a/share/functions/type.fish b/share/functions/type.fish index 9cadcabc3..c54997dd2 100644 --- a/share/functions/type.fish +++ b/share/functions/type.fish @@ -13,13 +13,19 @@ function type --description "Print the type of a command" for i in (seq (count $argv)) switch $argv[$i] case -t --type - set mode type + if test $mode != quiet + set mode type + end case -p --path - set mode path + if test $mode != quiet + set mode path + end case -P --force-path - set mode path + if test $mode != quiet + set mode path + end set selection files case -a --all @@ -28,6 +34,9 @@ function type --description "Print the type of a command" case -f --no-functions set selection files + case -q --quiet + set mode quiet + case -h --help __fish_print_help type return 0 @@ -61,10 +70,6 @@ function type --description "Print the type of a command" case type echo (_ 'function') - - case path - echo - end if test $multi != yes continue @@ -81,9 +86,6 @@ function type --description "Print the type of a command" case type echo (_ 'builtin') - - case path - echo end if test $multi != yes continue @@ -116,7 +118,7 @@ function type --description "Print the type of a command" end end - if test $found = 0 + if begin; test $found = 0; and test $mode != quiet; end printf (_ "%s: Could not find '%s'\n") type $i >&2 end From 533496e43ac4de68537b66412d54e880e03b6561 Mon Sep 17 00:00:00 2001 From: Kevin Ballard Date: Thu, 10 Jul 2014 00:26:58 -0700 Subject: [PATCH 06/35] Adopt the new type -q flag in the other functions --- share/functions/__fish_complete_cabal.fish | 2 +- share/functions/__fish_complete_vi.fish | 2 +- share/functions/__fish_config_interactive.fish | 4 ++-- share/functions/__fish_print_packages.fish | 14 +++++++------- share/functions/funced.fish | 2 +- share/functions/help.fish | 14 +++++++------- share/functions/ls.fish | 2 +- share/functions/open.fish | 4 ++-- share/functions/seq.fish | 2 +- 9 files changed, 23 insertions(+), 23 deletions(-) diff --git a/share/functions/__fish_complete_cabal.fish b/share/functions/__fish_complete_cabal.fish index b5b8c0c00..484710a84 100644 --- a/share/functions/__fish_complete_cabal.fish +++ b/share/functions/__fish_complete_cabal.fish @@ -1,5 +1,5 @@ function __fish_complete_cabal - if type -f cabal >/dev/null + if type -q -f cabal set cmd (commandline -poc) if test (count $cmd) -gt 1 cabal $cmd[2..-1] --list-options diff --git a/share/functions/__fish_complete_vi.fish b/share/functions/__fish_complete_vi.fish index 8a07ffd2e..51009d8b8 100644 --- a/share/functions/__fish_complete_vi.fish +++ b/share/functions/__fish_complete_vi.fish @@ -2,7 +2,7 @@ function __fish_complete_vi -d "Compleletions for vi and its aliases" --argument-names cmd set -l is_vim - if type $cmd > /dev/null + if type -q $cmd eval command $cmd --version >/dev/null ^/dev/null; and set -l is_vim vim end diff --git a/share/functions/__fish_config_interactive.fish b/share/functions/__fish_config_interactive.fish index 46b6ca248..1d3a1bea2 100644 --- a/share/functions/__fish_config_interactive.fish +++ b/share/functions/__fish_config_interactive.fish @@ -248,7 +248,7 @@ function __fish_config_interactive -d "Initializations that should be performed # First check if we are on OpenSUSE since SUSE's handler has no options # and expects first argument to be a command and second database # also check if there is command-not-found command. - if begin; test -f /etc/SuSE-release; and type -p command-not-found > /dev/null 2> /dev/null; end + if begin; test -f /etc/SuSE-release; and type -q -p command-not-found; end function __fish_command_not_found_handler --on-event fish_command_not_found /usr/bin/command-not-found $argv end @@ -263,7 +263,7 @@ function __fish_config_interactive -d "Initializations that should be performed /usr/lib/command-not-found -- $argv end # Ubuntu Feisty places this command in the regular path instead - else if type -p command-not-found > /dev/null 2> /dev/null + else if type -q -p command-not-found function __fish_command_not_found_handler --on-event fish_command_not_found command-not-found -- $argv end diff --git a/share/functions/__fish_print_packages.fish b/share/functions/__fish_print_packages.fish index 6755dd6ae..050fcac28 100644 --- a/share/functions/__fish_print_packages.fish +++ b/share/functions/__fish_print_packages.fish @@ -18,7 +18,7 @@ function __fish_print_packages end mkdir -m 700 -p $XDG_CACHE_HOME - if type -f apt-cache >/dev/null + if type -q -f apt-cache # Do not generate the cache as apparently sometimes this is slow. # http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=547550 apt-cache --no-generate pkgnames (commandline -tc) ^/dev/null | sed -e 's/$/'\t$package'/' @@ -28,7 +28,7 @@ function __fish_print_packages # Pkg is fast on FreeBSD and provides versioning info which we want for # installed packages if begin - type -f pkg > /dev/null + type -q -f pkg and test (uname) = "FreeBSD" end pkg query "%n-%v" @@ -36,7 +36,7 @@ function __fish_print_packages end # Caches for 5 minutes - if type -f pacman >/dev/null + if type -q -f pacman set cache_file $XDG_CACHE_HOME/.pac-cache.$USER if test -f $cache_file cat $cache_file @@ -53,7 +53,7 @@ function __fish_print_packages end # yum is slow, just like rpm, so go to the background - if type -f /usr/share/yum-cli/completion-helper.py >/dev/null + if type -q -f /usr/share/yum-cli/completion-helper.py # If the cache is less than six hours old, we do not recalculate it @@ -75,7 +75,7 @@ function __fish_print_packages # Rpm is too slow for this job, so we set it up to do completions # as a background job and cache the results. - if type -f rpm >/dev/null + if type -q -f rpm # If the cache is less than five minutes old, we do not recalculate it @@ -99,12 +99,12 @@ function __fish_print_packages # installed on the system packages is in completions/emerge.fish # eix is MUCH faster than emerge so use it if it is available - if type -f eix > /dev/null + if type -q -f eix eix --only-names "^"(commandline -tc) | cut -d/ -f2 return else # FIXME? Seems to be broken - if type -f emerge >/dev/null + if type -q -f emerge emerge -s \^(commandline -tc) |sgrep "^*" |cut -d\ -f3 |cut -d/ -f2 return end diff --git a/share/functions/funced.fish b/share/functions/funced.fish index ca2e27721..074e3c435 100644 --- a/share/functions/funced.fish +++ b/share/functions/funced.fish @@ -51,7 +51,7 @@ function funced --description 'Edit function definition' if test -n "$editor" set -l editor_cmd eval set editor_cmd $editor - if not type -f "$editor_cmd[1]" >/dev/null + if not type -q -f "$editor_cmd[1]" _ "funced: The value for \$EDITOR '$editor' could not be used because the command '$editor_cmd[1]' could not be found " set editor fish diff --git a/share/functions/help.fish b/share/functions/help.fish index 04992e35d..a04add53f 100644 --- a/share/functions/help.fish +++ b/share/functions/help.fish @@ -25,7 +25,7 @@ function help --description 'Show help for the fish shell' set -l graphical_browsers htmlview x-www-browser firefox galeon mozilla konqueror epiphany opera netscape rekonq google-chrome chromium-browser set -l text_browsers htmlview www-browser links elinks lynx w3m - if type "$BROWSER" >/dev/null + if type -q "$BROWSER" # User has manually set a preferred browser, so we respect that set fish_browser $BROWSER @@ -36,7 +36,7 @@ function help --description 'Show help for the fish shell' else # Check for a text-based browser. for i in $text_browsers - if type -f $i >/dev/null + if type -q -f $i set fish_browser $i break end @@ -46,7 +46,7 @@ function help --description 'Show help for the fish shell' # browser to use instead. if test "$DISPLAY" -a \( "$XAUTHORITY" = "$HOME/.Xauthority" -o "$XAUTHORITY" = "" \) for i in $graphical_browsers - if type -f $i >/dev/null + if type -q -f $i set fish_browser $i set fish_browser_bg 1 break @@ -55,17 +55,17 @@ function help --description 'Show help for the fish shell' end # If the OS appears to be Windows (graphical), try to use cygstart - if type cygstart > /dev/null + if type -q cygstart set fish_browser cygstart # If xdg-open is available, just use that - else if type xdg-open > /dev/null + else if type -q xdg-open set fish_browser xdg-open end # On OS X, we go through osascript by default if test (uname) = Darwin - if type osascript >/dev/null + if type -q osascript set fish_browser osascript end end @@ -92,7 +92,7 @@ function help --description 'Show help for the fish shell' case $help_topics set fish_help_page "index.html\#$fish_help_item" case "*" - if type -f $fish_help_item >/dev/null + if type -q -f $fish_help_item # Prefer to use fish's man pages, to avoid # the annoying useless "builtin" man page bash # installs on OS X diff --git a/share/functions/ls.fish b/share/functions/ls.fish index dd4fb3a67..4824e1c1d 100644 --- a/share/functions/ls.fish +++ b/share/functions/ls.fish @@ -13,7 +13,7 @@ if command ls --version 1>/dev/null 2>/dev/null end if not set -q LS_COLORS - if type -f dircolors >/dev/null + if type -q -f dircolors eval (dircolors -c | sed 's/>&\/dev\/null$//') end end diff --git a/share/functions/open.fish b/share/functions/open.fish index 63d562371..7a1e7bcec 100644 --- a/share/functions/open.fish +++ b/share/functions/open.fish @@ -14,11 +14,11 @@ if not test (uname) = Darwin end end - if type -f cygstart >/dev/null + if type -q -f cygstart for i in $argv cygstart $i end - else if type -f xdg-open >/dev/null + else if type -q -f xdg-open for i in $argv xdg-open $i end diff --git a/share/functions/seq.fish b/share/functions/seq.fish index 0f5f8534c..4229f0c5a 100644 --- a/share/functions/seq.fish +++ b/share/functions/seq.fish @@ -2,7 +2,7 @@ # test -x in /usr/bin/seq because that's where it usually is and # that's substantially cheaper than the type function -if begin ; not test -x /usr/bin/seq ; and not type -f seq > /dev/null; end +if begin ; not test -x /usr/bin/seq ; and not type -q -f seq; end # No seq command function seq --description "Print sequences of numbers" __fish_fallback_seq $argv From 29b3b6b31e3acbe2b5474db3b7c3841ba1ed17f9 Mon Sep 17 00:00:00 2001 From: Kevin Ballard Date: Thu, 10 Jul 2014 00:37:54 -0700 Subject: [PATCH 07/35] type: Stop claiming `grep` is a function Use `functions -q` instead of searching the `functiosn -na` list for the provided word. This may result in an automatically-loaded function being sourced, but that happens anyway with the default output. This change means the results of `test -q foo` can be relied upon to indicate whether `foo` can actually be invoked. Previosly, if `foo` was the name of an automatically-loaded function file but did not actually define a function `foo`, and there was no execuable `foo`, then `type -q foo` would lie and say `foo` can be invoked when it can't. --- share/functions/type.fish | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/share/functions/type.fish b/share/functions/type.fish index c54997dd2..06e94abbe 100644 --- a/share/functions/type.fish +++ b/share/functions/type.fish @@ -60,7 +60,7 @@ function type --description "Print the type of a command" if test $selection != files - if contains -- $i (functions -na) + if functions -q -- $i set res 0 set found 1 switch $mode From cfa13ed84c32ce3330041a54de392caed83f7ac5 Mon Sep 17 00:00:00 2001 From: Kevin Ballard Date: Thu, 10 Jul 2014 10:37:44 -0700 Subject: [PATCH 08/35] Update tests for new `type` behavior One of the tests was using `>/dev/null` to suppress the `type` output. That needs to be `^/dev/null` now, but instead just go ahead and use the new `-q` flag. --- tests/test7.in | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test7.in b/tests/test7.in index a3ae8360c..322982756 100644 --- a/tests/test7.in +++ b/tests/test7.in @@ -110,6 +110,6 @@ function fish_test_type_zzz true end # Should succeed -type fish_test_type_zzz >/dev/null ; echo $status +type -q fish_test_type_zzz ; echo $status # Should fail -type -f fish_test_type_zzz >/dev/null ; echo $status +type -q -f fish_test_type_zzz ; echo $status From 72e8489d50d749c86d5b57609bb0c4d83a03b41a Mon Sep 17 00:00:00 2001 From: Kevin Ballard Date: Thu, 10 Jul 2014 19:16:32 -0700 Subject: [PATCH 09/35] command: Rename -p/--path flag to -s/--search --- builtin.cpp | 7 ++++--- doc_src/command.txt | 8 +++++--- share/functions/type.fish | 2 +- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/builtin.cpp b/builtin.cpp index 2d3eb749d..6f7cc33c6 100644 --- a/builtin.cpp +++ b/builtin.cpp @@ -1067,7 +1067,7 @@ static int builtin_command(parser_t &parser, wchar_t **argv) static const struct woption long_options[] = { - { L"path", no_argument, 0, 'p' }, + { L"search", no_argument, 0, 's' }, { L"help", no_argument, 0, 'h' }, { 0, 0, 0, 0 } }; @@ -1078,7 +1078,7 @@ static int builtin_command(parser_t &parser, wchar_t **argv) int opt = wgetopt_long(argc, argv, - L"ph", + L"svh", long_options, &opt_index); if (opt == -1) @@ -1100,7 +1100,8 @@ static int builtin_command(parser_t &parser, wchar_t **argv) builtin_print_help(parser, argv[0], stdout_buffer); return STATUS_BUILTIN_OK; - case 'p': + case 's': + case 'v': print_path=1; break; diff --git a/doc_src/command.txt b/doc_src/command.txt index e93f34b8e..cef08fdc5 100644 --- a/doc_src/command.txt +++ b/doc_src/command.txt @@ -9,12 +9,14 @@ The following options are available: - \c -h or \c --help prints help and then exits. -- \c -p or \c --path returns the name of the disk file that would be executed, or nothing if no file with the specified name could be found in the $PATH. +- \c -s or \c --search returns the name of the disk file that would be executed, or nothing if no file with the specified name could be found in the $PATH. -With the \c -p option, \c command treats every argument as a separate command to look up and sets the exit status to 0 if any of the specified commands were found, or 1 if no commands could be found. +With the \c -s option, \c command treats every argument as a separate command to look up and sets the exit status to 0 if any of the specified commands were found, or 1 if no commands could be found. + +For basic compatibility with POSIX command, the \c -v flag is recognized as an alias for -s. \subsection command-example Examples command ls causes fish to execute the \c ls program, even if an 'ls' function exists. -command -p ls returns the path to the \c ls program. +command -s ls returns the path to the \c ls program. diff --git a/share/functions/type.fish b/share/functions/type.fish index 06e94abbe..8992d6b22 100644 --- a/share/functions/type.fish +++ b/share/functions/type.fish @@ -96,7 +96,7 @@ function type --description "Print the type of a command" set -l paths if test $multi != yes - set paths (command -p $i) + set paths (command -s $i) else set paths (which -a $i ^/dev/null) end From 973dd6ffbdc189f22b634de0d684e92a9c160c9d Mon Sep 17 00:00:00 2001 From: Kevin Ballard Date: Sun, 13 Jul 2014 22:36:26 -0700 Subject: [PATCH 10/35] read: Support arrays, character splitting Enhance the `read` builtin to support creating an array with the --array flag. With --array, only a single variable name is allowed and the entire input is tokenized and placed into that variable as an array. Also add custom behavior if IFS is empty or unset. In that event, split the input on every character, instead of the previous behavior of doing no splitting at all. --- builtin.cpp | 83 +++++++++++++++++++++++++++++++++++++++++------ doc_src/read.txt | 6 ++++ tests/read.err | 0 tests/read.in | 72 ++++++++++++++++++++++++++++++++++++++++ tests/read.out | 27 +++++++++++++++ tests/read.status | 1 + tests/top.out | 1 + 7 files changed, 180 insertions(+), 10 deletions(-) create mode 100644 tests/read.err create mode 100644 tests/read.in create mode 100644 tests/read.out create mode 100644 tests/read.status diff --git a/builtin.cpp b/builtin.cpp index 6f7cc33c6..1420e33c1 100644 --- a/builtin.cpp +++ b/builtin.cpp @@ -2301,6 +2301,7 @@ static int builtin_read(parser_t &parser, wchar_t **argv) int exit_res=STATUS_BUILTIN_OK; const wchar_t *mode_name = READ_MODE_NAME; int shell = 0; + int array = 0; woptind=0; @@ -2345,6 +2346,10 @@ static int builtin_read(parser_t &parser, wchar_t **argv) L"shell", no_argument, 0, 's' } , + { + L"array", no_argument, 0, 'a' + } + , { L"help", no_argument, 0, 'h' } @@ -2359,7 +2364,7 @@ static int builtin_read(parser_t &parser, wchar_t **argv) int opt = wgetopt_long(argc, argv, - L"xglUup:c:hm:s", + L"xglUup:c:hm:sa", long_options, &opt_index); if (opt == -1) @@ -2414,6 +2419,10 @@ static int builtin_read(parser_t &parser, wchar_t **argv) shell = 1; break; + case 'a': + array = 1; + break; + case 'h': builtin_print_help(parser, argv[0], stdout_buffer); return STATUS_BUILTIN_OK; @@ -2446,6 +2455,14 @@ static int builtin_read(parser_t &parser, wchar_t **argv) return STATUS_BUILTIN_ERROR; } + if (array && woptind+1 != argc) + { + append_format(stderr_buffer, _(L"%ls: --array option requires a single variable name.\n"), argv[0]); + builtin_print_help(parser, argv[0], stderr_buffer); + + return STATUS_BUILTIN_ERROR; + } + /* Verify all variable names */ @@ -2580,18 +2597,64 @@ static int builtin_read(parser_t &parser, wchar_t **argv) wchar_t *state; env_var_t ifs = env_get_string(L"IFS"); - if (ifs.missing()) - ifs = L""; - nxt = wcstok(buff, (i 0) + { + wcstring chars(bufflen+(bufflen-1), ARRAY_SEP); + for (size_t j=0; j-u or --unexport prevents the variables from being exported to child processes (default behaviour). - -U or --universal causes the specified shell variable to be made universal. - -x or --export exports the variables to child processes. +- -a or --array stores the result as an array. \c read reads a single line of input from stdin, breaks it into tokens based on the IFS shell variable, and then assigns one token to each variable specified in VARIABLES. If there are more tokens than variables, the complete remainder is assigned to the last variable. +As a special case, if \c IFS is set to the empty string, each character of the +input is considered a separate token. + +If \c -a or \c --array is provided, only one variable name is allowed and the +tokens are stored as an array in this variable. See the documentation for \c set for more details on the scoping rules for variables. diff --git a/tests/read.err b/tests/read.err new file mode 100644 index 000000000..e69de29bb diff --git a/tests/read.in b/tests/read.in new file mode 100644 index 000000000..53f9873b6 --- /dev/null +++ b/tests/read.in @@ -0,0 +1,72 @@ +# +# Test read builtin and IFS +# + +count (echo one\ntwo) +set -l IFS \t +count (echo one\ntwo) +set -l IFS +count (echo one\ntwo) +set -le IFS + +function print_vars --no-scope-shadowing + set -l space + set -l IFS \n # ensure our command substitution works right + for var in $argv + echo -n $space (count $$var) \'$$var\' + set space '' + end + echo +end + +echo +echo 'hello there' | read -l one two +print_vars one two +echo 'hello there' | read -l one +print_vars one +echo '' | read -l one +print_vars one +echo '' | read -l one two +print_vars one two +echo 'test' | read -l one two three +print_vars one two three + +echo +set -l IFS +echo 'hello' | read -l one +print_vars one +echo 'hello' | read -l one two +print_vars one two +echo 'hello' | read -l one two three +print_vars one two three +echo '' | read -l one +print_vars one +echo 't' | read -l one two +print_vars one two +echo 't' | read -l one two three +print_vars one two three +echo ' t' | read -l one two +print_vars one two +set -le IFS + +echo +echo 'hello there' | read -la ary +print_vars ary +echo 'hello' | read -la ary +print_vars ary +echo 'this is a bunch of words' | read -la ary +print_vars ary +echo ' one two three' | read -la ary +print_vars ary +echo '' | read -la ary +print_vars ary + +echo +set -l IFS +echo 'hello' | read -la ary +print_vars ary +echo 'h' | read -la ary +print_vars ary +echo '' | read -la ary +print_vars ary +set -le IFS diff --git a/tests/read.out b/tests/read.out new file mode 100644 index 000000000..6d90fc0e7 --- /dev/null +++ b/tests/read.out @@ -0,0 +1,27 @@ +2 +2 +1 + +1 'hello' 1 'there' +1 'hello there' +1 '' +1 '' 1 '' +1 'test' 1 '' 1 '' + +1 'hello' +1 'h' 1 'ello' +1 'h' 1 'e' 1 'llo' +1 '' +1 't' 1 '' +1 't' 1 '' 1 '' +1 ' ' 1 't' + +2 'hello' 'there' +1 'hello' +6 'this' 'is' 'a' 'bunch' 'of' 'words' +3 'one' 'two' 'three' +0 + +5 'h' 'e' 'l' 'l' 'o' +1 'h' +0 diff --git a/tests/read.status b/tests/read.status new file mode 100644 index 000000000..573541ac9 --- /dev/null +++ b/tests/read.status @@ -0,0 +1 @@ +0 diff --git a/tests/top.out b/tests/top.out index 768526c66..f2873b153 100644 --- a/tests/top.out +++ b/tests/top.out @@ -1,5 +1,6 @@ Testing high level script functionality File printf.in tested ok +File read.in tested ok File test1.in tested ok File test2.in tested ok File test3.in tested ok From cce4265cef63132f7e398f7cf89a63d8078a9c2e Mon Sep 17 00:00:00 2001 From: Kevin Ballard Date: Sun, 13 Jul 2014 23:08:38 -0700 Subject: [PATCH 11/35] Fix `make test` to use local functions When running `make test` we want to use the local function definitions, not the ones installed on the system. The system config.fish will still insert the system definitions at the end, but at least ours will take precedence. --- Makefile.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile.in b/Makefile.in index 9fe0f8e05..4995dddcc 100644 --- a/Makefile.in +++ b/Makefile.in @@ -284,7 +284,7 @@ doc/refman.pdf: doc test: $(PROGRAMS) fish_tests ./fish_tests - cd tests; ../fish Date: Mon, 14 Jul 2014 11:19:59 -0700 Subject: [PATCH 12/35] type: Restore combined flags behavior Fix the parsing of `type` flags to handle combined short flags as appropriate, e.g. `type -qf ls`. --- share/functions/type.fish | 85 ++++++++++++++++++++++++--------------- 1 file changed, 53 insertions(+), 32 deletions(-) diff --git a/share/functions/type.fish b/share/functions/type.fish index 8992d6b22..e7bc7b288 100644 --- a/share/functions/type.fish +++ b/share/functions/type.fish @@ -11,44 +11,65 @@ function type --description "Print the type of a command" set -l names if test (count $argv) -gt 0 for i in (seq (count $argv)) - switch $argv[$i] - case -t --type - if test $mode != quiet - set mode type - end + set -l arg $argv[$i] + set -l needbreak 0 + while test -n $arg + set -l flag $arg + set arg '' + switch $flag + case '--*' + # do nothing; this just prevents it matching the next case + case '-??*' + # combined flags + set -l IFS + echo -n $flag | read _ flag arg + set flag -$flag + set arg -$arg + end + switch $flag + case -t --type + if test $mode != quiet + set mode type + end - case -p --path - if test $mode != quiet - set mode path - end + case -p --path + if test $mode != quiet + set mode path + end - case -P --force-path - if test $mode != quiet - set mode path - end - set selection files + case -P --force-path + if test $mode != quiet + set mode path + end + set selection files - case -a --all - set multi yes + case -a --all + set multi yes - case -f --no-functions - set selection files + case -f --no-functions + set selection files - case -q --quiet - set mode quiet + case -q --quiet + set mode quiet - case -h --help - __fish_print_help type - return 0 + case -h --help + __fish_print_help type + return 0 - case -- - set names $argv[$i..-1] - set -e names[1] - break + case -- + set names $argv[$i..-1] + set -e names[1] + set needbreak 1 + break - case '*' - set names $argv[$i..-1] - break + case '*' + set names $argv[$i..-1] + set needbreak 1 + break + end + end + if test $needbreak -eq 1 + break end end end @@ -96,9 +117,9 @@ function type --description "Print the type of a command" set -l paths if test $multi != yes - set paths (command -s $i) + set paths (command -s -- $i) else - set paths (which -a $i ^/dev/null) + set paths (which -a -- $i ^/dev/null) end for path in $paths set res 0 From 3981b644d68f6b6947b4a12810c2fa5e09da4e58 Mon Sep 17 00:00:00 2001 From: Kevin Ballard Date: Wed, 20 Aug 2014 21:19:08 -0700 Subject: [PATCH 13/35] Fix double expansions (`$$foo`) Double expansions of variables had the following issues: * `"$$foo"` threw an error no matter what the value of `$foo` was. * `set -l foo ''; echo $$foo` threw an error because of the expansion of `$foo` to `''`. With this change, double expansion always works properly. When double-expanding a multi-valued variable, in a double-quoted string the first word of the inner expansion is used for the outer expansion, and outside of a quoted string every word is used for the double-expansion in each of the arguments. > set -l foo bar baz > set -l bar one two > set -l baz three four > echo "$$foo" one two baz > echo $$foo one two three four --- expand.cpp | 57 ++++++++++++++++++++++++++++--------- expand.h | 5 ++++ tests/expansion.err | 0 tests/expansion.in | 64 ++++++++++++++++++++++++++++++++++++++++++ tests/expansion.out | 38 +++++++++++++++++++++++++ tests/expansion.status | 1 + tests/top.out | 1 + 7 files changed, 153 insertions(+), 13 deletions(-) create mode 100644 tests/expansion.err create mode 100644 tests/expansion.in create mode 100644 tests/expansion.out create mode 100644 tests/expansion.status diff --git a/expand.cpp b/expand.cpp index d8cf4a1ab..d75c65723 100644 --- a/expand.cpp +++ b/expand.cpp @@ -1086,10 +1086,15 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std:: while (1) { - if (!(in[stop_pos ])) + const wchar_t nc = in[stop_pos]; + if (!(nc)) break; - if (!(iswalnum(in[stop_pos]) || - (wcschr(L"_", in[stop_pos])!= 0))) + if (nc == VARIABLE_EXPAND_EMPTY) + { + stop_pos++; + break; + } + if (!(wcsvarchr(nc))) break; stop_pos++; @@ -1108,7 +1113,15 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std:: } var_tmp.append(in + start_pos, var_len); - env_var_t var_val = expand_var(var_tmp.c_str()); + env_var_t var_val; + if (var_len == 1 && var_tmp[0] == VARIABLE_EXPAND_EMPTY) + { + var_val = env_var_t::missing_var(); + } + else + { + var_val = expand_var(var_tmp.c_str()); + } if (! var_val.missing()) { @@ -1174,7 +1187,18 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std:: { in[i]=0; wcstring res = in; - res.push_back(INTERNAL_SEPARATOR); + if (i > 0) + { + if (in[i-1] != VARIABLE_EXPAND_SINGLE) + { + res.push_back(INTERNAL_SEPARATOR); + } + else if (var_item_list.empty() || var_item_list.front().empty()) + { + // first expansion is empty, but we need to recursively expand + res.push_back(VARIABLE_EXPAND_EMPTY); + } + } for (size_t j=0; j 0) - new_in.append(in, start_pos - 1); - - // at this point new_in.size() is start_pos - 1 - if (start_pos>1 && new_in[start_pos-2]!=VARIABLE_EXPAND) + if (i > 0) { - new_in.push_back(INTERNAL_SEPARATOR); + if (in[i-1] != VARIABLE_EXPAND) + { + new_in.push_back(INTERNAL_SEPARATOR); + } + else if (next.empty()) + { + new_in.push_back(VARIABLE_EXPAND_EMPTY); + } } new_in.append(next); new_in.append(in + stop_pos); @@ -1243,8 +1271,11 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std:: Expansion to single argument. */ wcstring res; - in[i] = 0; - res.append(in); + res.append(in, i); + if (i > 0 && in[i-1] == VARIABLE_EXPAND_SINGLE) + { + res.push_back(VARIABLE_EXPAND_EMPTY); + } res.append(in + stop_pos); is_ok &= expand_variables2(parser, res, out, i, errors); diff --git a/expand.h b/expand.h index 3956b1f16..14a4f4772 100644 --- a/expand.h +++ b/expand.h @@ -102,6 +102,11 @@ enum */ INTERNAL_SEPARATOR, + /** + Character representing an empty variable expansion. + Only used transitively while expanding variables. + */ + VARIABLE_EXPAND_EMPTY, } ; diff --git a/tests/expansion.err b/tests/expansion.err new file mode 100644 index 000000000..e69de29bb diff --git a/tests/expansion.in b/tests/expansion.in new file mode 100644 index 000000000..d362e7895 --- /dev/null +++ b/tests/expansion.in @@ -0,0 +1,64 @@ +# Test expansion of variables + +function show --description 'Prints argument count followed by arguments' + echo (count $argv) $argv +end + +set -l foo +show "$foo" +show $foo +show "prefix$foo" +show prefix$foo + +show "$$foo" +show $$foo +show "prefix$$foo" +show prefix$$foo + +set -l foo '' +show "$foo" +show $foo +show "prefix$foo" +show prefix$foo + +show "$$foo" +show $$foo +show "prefix$$foo" +show prefix$$foo + +set -l foo bar +set -l bar +show "$$foo" +show $$foo +show "prefix$$foo" +show prefix$$foo + +set -l bar baz +show "$$foo" +show $$foo +show "prefix$$foo" +show prefix$$foo + +set -l bar baz quux +show "$$foo" +show $$foo +show "prefix$$foo" +show prefix$$foo + +set -l foo bar fooer fooest +set -l fooer +set -l fooest +show "$$foo" +show $$foo +show "prefix$$foo" +show prefix$$foo + +set -l fooer '' +show $$foo +show prefix$$foo + +set -l foo bar '' fooest +show "$$foo" +show $$foo +show "prefix$$foo" +show prefix$$foo diff --git a/tests/expansion.out b/tests/expansion.out new file mode 100644 index 000000000..3e7631804 --- /dev/null +++ b/tests/expansion.out @@ -0,0 +1,38 @@ +1 +0 +1 prefix +0 +1 +0 +1 prefix +0 +1 +1 +1 prefix +1 prefix +1 +0 +1 prefix +0 +1 +0 +1 prefix +0 +1 baz +1 baz +1 prefixbaz +1 prefixbaz +1 baz quux +2 baz quux +1 prefixbaz quux +2 prefixbaz prefixquux +1 baz quux fooer fooest +2 baz quux +1 prefixbaz quux fooer fooest +2 prefixbaz prefixquux +3 baz quux +3 prefixbaz prefixquux prefix +1 baz quux fooest +2 baz quux +1 prefixbaz quux fooest +2 prefixbaz prefixquux diff --git a/tests/expansion.status b/tests/expansion.status new file mode 100644 index 000000000..573541ac9 --- /dev/null +++ b/tests/expansion.status @@ -0,0 +1 @@ +0 diff --git a/tests/top.out b/tests/top.out index 768526c66..acf52d424 100644 --- a/tests/top.out +++ b/tests/top.out @@ -1,4 +1,5 @@ Testing high level script functionality +File expansion.in tested ok File printf.in tested ok File test1.in tested ok File test2.in tested ok From cc49042294aceb3ab30396eb5731e4ac0cf601bc Mon Sep 17 00:00:00 2001 From: Kevin Ballard Date: Wed, 20 Aug 2014 22:01:24 -0700 Subject: [PATCH 14/35] Parse slices even for empty variables When a variable is parsed as being empty, parse out the slice and validate the indexes anyway, behaving for slicing purposes as if the variable had a single empty value. Besides providing errors when expected, this also fixes the following: set -l foo echo "$foo[1]" This used to print "[1]", now it properly prints nothing. --- expand.cpp | 35 ++++++++++++++++++++++++++++++++++- tests/expansion.err | 18 ++++++++++++++++++ tests/expansion.in | 12 ++++++++++++ tests/expansion.out | 4 ++++ 4 files changed, 68 insertions(+), 1 deletion(-) diff --git a/expand.cpp b/expand.cpp index d75c65723..7388cb18b 100644 --- a/expand.cpp +++ b/expand.cpp @@ -1155,10 +1155,10 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std:: for (size_t j=0; j var_item_list.size()) { + size_t var_src_pos = var_pos_list.at(j); /* The slice was parsed starting at stop_pos, so we have to add that to the error position */ append_syntax_error(errors, slice_start + var_src_pos, @@ -1255,6 +1255,39 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std:: } else { + // even with no value, we still need to parse out slice syntax + // Behave as though we had 1 value, so $foo[1] always works. + const size_t slice_start = stop_pos; + if (in[slice_start] == L'[') + { + wchar_t *slice_end; + + if (parse_slice(in + slice_start, &slice_end, var_idx_list, var_pos_list, 1)) + { + append_syntax_error(errors, + stop_pos, + L"Invalid index value"); + is_ok = 0; + return is_ok; + } + stop_pos = (slice_end-in); + + // validate that the parsed indexes are valid + for (size_t j=0; j Date: Wed, 20 Aug 2014 22:28:42 -0700 Subject: [PATCH 15/35] Highlight "$foo[1]" properly Preserve the highlighting applied to the slice brackets when coloring variables inside of double-quoted strings. --- highlight.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/highlight.cpp b/highlight.cpp index 8145e5f30..f560264c2 100644 --- a/highlight.cpp +++ b/highlight.cpp @@ -861,7 +861,11 @@ static void color_argument_internal(const wcstring &buffstr, std::vector Date: Wed, 20 Aug 2014 22:31:58 -0700 Subject: [PATCH 16/35] Fix highlighting of `"foo\"bar"` The backslash-escape wasn't being properly caught by the highlighter. Also remove the highlighting of `"\'"`, as `\'` is not a valid escape in double-quotes, and add highlighting for a backslash-escaped newline. --- highlight.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/highlight.cpp b/highlight.cpp index f560264c2..9d43f8e08 100644 --- a/highlight.cpp +++ b/highlight.cpp @@ -880,7 +880,7 @@ static void color_argument_internal(const wcstring &buffstr, std::vector Date: Wed, 20 Aug 2014 22:55:24 -0700 Subject: [PATCH 17/35] Color `"$foo[1"` as an error We can't color the whole argument as an error, since the tokenizer is responsible for that and doesn't care abou this case, but we can color the `$foo[` bit as an error. --- highlight.cpp | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/highlight.cpp b/highlight.cpp index 9d43f8e08..5369f3a5a 100644 --- a/highlight.cpp +++ b/highlight.cpp @@ -617,12 +617,26 @@ static size_t color_variable(const wchar_t *in, size_t in_len, std::vector slice_begin_idx); - colors[slice_begin_idx] = highlight_spec_operator; - colors[slice_end_idx] = highlight_spec_operator; + case 1: + { + size_t slice_begin_idx = slice_begin - in, slice_end_idx = slice_end - in; + assert(slice_end_idx > slice_begin_idx); + colors[slice_begin_idx] = highlight_spec_operator; + colors[slice_end_idx] = highlight_spec_operator; + break; + } + case -1: + { + // syntax error + // Normally the entire token is colored red for us, but inside a double-quoted string + // that doesn't happen. As such, color the variable + the slice start red. Coloring any + // more than that looks bad, unless we're willing to try and detect where the double-quoted + // string ends, and I'd rather not do that. + std::fill(colors, colors + idx + 1, (highlight_spec_t)highlight_spec_error); + break; + } } } return idx; From 2974025010cac7b7238c88c88216c1722e61f5c9 Mon Sep 17 00:00:00 2001 From: Kevin Ballard Date: Thu, 21 Aug 2014 00:26:14 -0700 Subject: [PATCH 18/35] Fix error span for invalid slice indexes The span now properly points at the token that was invalid, rather than the start of the slice. Also fix the span for `()[1]` and `()[d]`, which were previously reporting no source location at all. --- expand.cpp | 31 +++++++++++++++++++++---------- tests/expansion.err | 12 ++++++++++++ tests/expansion.in | 6 ++++++ 3 files changed, 39 insertions(+), 10 deletions(-) diff --git a/expand.cpp b/expand.cpp index 7388cb18b..5f96fb2f9 100644 --- a/expand.cpp +++ b/expand.cpp @@ -953,8 +953,11 @@ void expand_variable_error(parser_t &parser, const wcstring &token, size_t token /** Parse an array slicing specification + Returns 0 on success. + If a parse error occurs, returns the index of the bad token. + Note that 0 can never be a bad index because the string always starts with [. */ -static int parse_slice(const wchar_t *in, wchar_t **end_ptr, std::vector &idx, std::vector &source_positions, size_t array_size) +static size_t parse_slice(const wchar_t *in, wchar_t **end_ptr, std::vector &idx, std::vector &source_positions, size_t array_size) { wchar_t *end; @@ -981,7 +984,7 @@ static int parse_slice(const wchar_t *in, wchar_t **end_ptr, std::vector & tmp = wcstol(&in[pos], &end, 10); if ((errno) || (end == &in[pos])) { - return 1; + return pos; } // debug( 0, L"Push idx %d", tmp ); @@ -999,7 +1002,7 @@ static int parse_slice(const wchar_t *in, wchar_t **end_ptr, std::vector & long tmp1 = wcstol(&in[pos], &end, 10); if ((errno) || (end == &in[pos])) { - return 1; + return pos; } pos = end-in; @@ -1136,12 +1139,14 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std:: if (in[slice_start] == L'[') { wchar_t *slice_end; + size_t bad_pos; all_vars=0; - if (parse_slice(in + slice_start, &slice_end, var_idx_list, var_pos_list, var_item_list.size())) + bad_pos = parse_slice(in + slice_start, &slice_end, var_idx_list, var_pos_list, var_item_list.size()); + if (bad_pos != 0) { append_syntax_error(errors, - stop_pos, + stop_pos + bad_pos, L"Invalid index value"); is_ok = 0; break; @@ -1261,11 +1266,13 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std:: if (in[slice_start] == L'[') { wchar_t *slice_end; + size_t bad_pos; - if (parse_slice(in + slice_start, &slice_end, var_idx_list, var_pos_list, 1)) + bad_pos = parse_slice(in + slice_start, &slice_end, var_idx_list, var_pos_list, 1); + if (bad_pos != 0) { append_syntax_error(errors, - stop_pos, + stop_pos + bad_pos, L"Invalid index value"); is_ok = 0; return is_ok; @@ -1499,11 +1506,14 @@ static int expand_cmdsubst(parser_t &parser, const wcstring &input, std::vector< { std::vector slice_idx; std::vector slice_source_positions; + const wchar_t * const slice_begin = tail_begin; wchar_t *slice_end; + size_t bad_pos; - if (parse_slice(tail_begin, &slice_end, slice_idx, slice_source_positions, sub_res.size())) + bad_pos = parse_slice(slice_begin, &slice_end, slice_idx, slice_source_positions, sub_res.size()); + if (bad_pos != 0) { - append_syntax_error(errors, SOURCE_LOCATION_UNKNOWN, L"Invalid index value"); + append_syntax_error(errors, slice_begin - in + bad_pos, L"Invalid index value"); return 0; } else @@ -1515,8 +1525,9 @@ static int expand_cmdsubst(parser_t &parser, const wcstring &input, std::vector< long idx = slice_idx.at(i); if (idx < 1 || (size_t)idx > sub_res.size()) { + size_t pos = slice_source_positions.at(i); append_syntax_error(errors, - SOURCE_LOCATION_UNKNOWN, + slice_begin - in + pos, ARRAY_BOUNDS_ERR); return 0; } diff --git a/tests/expansion.err b/tests/expansion.err index 6346f23fb..e60c203d5 100644 --- a/tests/expansion.err +++ b/tests/expansion.err @@ -16,3 +16,15 @@ fish: show "$foo[2 1]" Array index out of bounds fish: show $foo[2 1] ^ +Invalid index value +fish: echo "$foo[d]" + ^ +Invalid index value +fish: echo $foo[d] + ^ +Array index out of bounds +fish: echo ()[1] + ^ +Invalid index value +fish: echo ()[d] + ^ diff --git a/tests/expansion.in b/tests/expansion.in index 8b48772ef..a22251272 100644 --- a/tests/expansion.in +++ b/tests/expansion.in @@ -74,3 +74,9 @@ show "$foo[1 2]" show $foo[1 2] show "$foo[2 1]" show $foo[2 1] + +echo "$foo[d]" +echo $foo[d] + +echo ()[1] +echo ()[d] From 9f725bee1f59df961e93e3d2392bc779281d7de7 Mon Sep 17 00:00:00 2001 From: Kevin Ballard Date: Sun, 13 Jul 2014 19:08:15 -0700 Subject: [PATCH 19/35] set: Print an error when setting `umask` to a bad value Repurpose the ENV_INVALID return value for env_set(), which wasn't currently used by anything. When a bad value is passed for the 'umask' key, return ENV_INVALID to signal this and print a good error message from the `set` builtin. This makes `set umask foo` properly produce an error. --- builtin_set.cpp | 2 +- env.cpp | 6 ++++-- env.h | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/builtin_set.cpp b/builtin_set.cpp index af257d160..eb4f5eda5 100644 --- a/builtin_set.cpp +++ b/builtin_set.cpp @@ -166,7 +166,7 @@ static int my_env_set(const wchar_t *key, const wcstring_list_t &val, int scope) case ENV_INVALID: { - append_format(stderr_buffer, _(L"%ls: Unknown error"), L"set"); + append_format(stderr_buffer, _(L"%ls: Tried to set the special variable '%ls' to an invalid value\n"), L"set", key); retcode=1; break; } diff --git a/env.cpp b/env.cpp index 88d61c857..91bc7ac80 100644 --- a/env.cpp +++ b/env.cpp @@ -655,10 +655,12 @@ int env_set(const wcstring &key, const wchar_t *val, env_mode_flags_t var_mode) if (!errno && (!*end) && (mask <= 0777) && (mask >= 0)) { umask(mask); + /* Do not actually create a umask variable, on env_get, it will be calculated dynamically */ + return 0; } } - /* Do not actually create a umask variable, on env_get, it will be calculated dynamically */ - return 0; + + return ENV_INVALID; } /* diff --git a/env.h b/env.h index 7afa9b9cd..8d4a41a9c 100644 --- a/env.h +++ b/env.h @@ -80,7 +80,7 @@ void env_init(const struct config_paths_t *paths = NULL); * ENV_PERM, can only be returned when setting as a user, e.g. ENV_USER is set. This means that the user tried to change a read-only variable. * ENV_SCOPE, the variable cannot be set in the given scope. This applies to readonly/electric variables set from the local or universal scopes, or set as exported. - * ENV_INVALID, the variable name or mode was invalid + * ENV_INVALID, the variable value was invalid. This applies only to special variables. */ int env_set(const wcstring &key, const wchar_t *val, env_mode_flags_t mode); From f33e6a053e55dfeb8f102145b5a61893c2781d82 Mon Sep 17 00:00:00 2001 From: Kevin Ballard Date: Sun, 13 Jul 2014 18:30:18 -0700 Subject: [PATCH 20/35] doc: Fix links in "Further help and development" Hyperlink the mailing list to the proper info page. Tweak the GitHub link to use https. --- doc_src/index.hdr.in | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc_src/index.hdr.in b/doc_src/index.hdr.in index bf48bc459..2a249da49 100644 --- a/doc_src/index.hdr.in +++ b/doc_src/index.hdr.in @@ -1413,9 +1413,9 @@ directory, or install them in /etc. If you have a question not answered by this documentation, there are several avenues for help: --# The official mailing list at fish-users@lists.sf.net +-# The official mailing list at fish-users@lists.sf.net -# The Internet Relay Chat channel, \c #fish on \c irc.oftc.net --# The project GitHub page +-# The project GitHub page If you have an improvement for fish, you can submit it via the mailing list or the GitHub page. From b9948ca2971255c291bebabdc910f4f97393f43d Mon Sep 17 00:00:00 2001 From: Kevin Ballard Date: Sun, 13 Jul 2014 18:42:17 -0700 Subject: [PATCH 21/35] doc: Fix docs on $HOME/$USER The docs claimed that the $HOME and $USER variables could only be changed by the root user. This is untrue. They can be changed by non-root users as well. --- doc_src/index.hdr.in | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc_src/index.hdr.in b/doc_src/index.hdr.in index 2a249da49..d4fa15e0f 100644 --- a/doc_src/index.hdr.in +++ b/doc_src/index.hdr.in @@ -1005,10 +1005,10 @@ values of most of these variables. - \c _, the name of the currently running command. - \c argv, an array of arguments to the shell or function. \c argv is only defined when inside a function call, or if fish was invoked with a list of arguments, like 'fish myscript.fish foo bar'. This variable can be changed by the user. - \c history, an array containing the last commands that were entered. -- \c HOME, the user's home directory. This variable can only be changed by the root user. +- \c HOME, the user's home directory. This variable can be changed by the user. - \c PWD, the current working directory. - \c status, the exit status of the last foreground job to exit. If the job was terminated through a signal, the exit status will be 128 plus the signal number. -- \c USER, the current username. This variable can only be changed by the root user. +- \c USER, the current username. This variable can be changed by the user. - \c CMD_DURATION, the runtime of the last command in milliseconds. The names of these variables are mostly derived from the csh family of From 20899f2df9b6cd09c5b511c3c655b747e82a42f0 Mon Sep 17 00:00:00 2001 From: Kevin Ballard Date: Sun, 13 Jul 2014 18:51:54 -0700 Subject: [PATCH 22/35] doc: Document how IFS affects command substitution IFS is used for more than just the read builtin. Setting it to the empty string also disables line-splitting in command substitution, and it's done this for the past 7 years. Some day we may have a better way to do this, but for now, document the current solution. --- doc_src/index.hdr.in | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/doc_src/index.hdr.in b/doc_src/index.hdr.in index d4fa15e0f..458930f84 100644 --- a/doc_src/index.hdr.in +++ b/doc_src/index.hdr.in @@ -603,7 +603,7 @@ command. If a parameter contains a set of parenthesis, the text enclosed by the parenthesis will be interpreted as a list of commands. On expansion, this list is executed, and substituted by the output. If the output is more than one line long, each line will be expanded to a new -parameter. +parameter. Setting \c IFS to the empty string will disable line splitting. The exit status of the last run command substitution is available in the status variable. @@ -620,6 +620,9 @@ The command for i in *.jpg; convert $i (basename $i .jpg).png; end will convert all JPEG files in the current directory to the PNG format using the \c convert program. +The command begin; set -l IFS; set data (cat data.txt); end +will set the \c data variable to the contents of 'data.txt' without +splitting it into an array. \subsection expand-brace Brace expansion @@ -1006,6 +1009,7 @@ values of most of these variables. - \c argv, an array of arguments to the shell or function. \c argv is only defined when inside a function call, or if fish was invoked with a list of arguments, like 'fish myscript.fish foo bar'. This variable can be changed by the user. - \c history, an array containing the last commands that were entered. - \c HOME, the user's home directory. This variable can be changed by the user. +- \c IFS, the internal field separator that is used for word splitting with the read builtin. Setting this to the empty string will also disable line splitting in command substitution. This variable can be changed by the user. - \c PWD, the current working directory. - \c status, the exit status of the last foreground job to exit. If the job was terminated through a signal, the exit status will be 128 plus the signal number. - \c USER, the current username. This variable can be changed by the user. From d63db59adeb84251c3ab5fe1b13981e44571bf31 Mon Sep 17 00:00:00 2001 From: Kevin Ballard Date: Sun, 13 Jul 2014 18:56:19 -0700 Subject: [PATCH 23/35] Clean up the IFS handling in command substitution Remove the useless ASCII test of the first byte of IFS. We don't split on the first character, we only use a non-empty IFS as a signal to split on newlines. --- exec.cpp | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/exec.cpp b/exec.cpp index f2d168a9f..dfd04231e 100644 --- a/exec.cpp +++ b/exec.cpp @@ -1548,16 +1548,7 @@ static int exec_subshell_internal(const wcstring &cmd, wcstring_list_t *lst, boo if (! ifs.missing_or_empty()) { - if (ifs.at(0) < 128) - { - sep = '\n';//ifs[0]; - } - else - { - sep = 0; - debug(0, L"Warning - invalid command substitution separator '%lc'. Please change the first character of IFS", ifs[0]); - } - + sep = '\n'; } is_subshell=1; From aaddccfdb158351705596a6c90622e6d84819ad4 Mon Sep 17 00:00:00 2001 From: Andy Lutomirski Date: Mon, 11 Aug 2014 17:50:56 -0700 Subject: [PATCH 24/35] webconfig: Use a constant-time token comparison This prevents a linear-time attack to recover the auth token. --- share/tools/web_config/webconfig.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/share/tools/web_config/webconfig.py b/share/tools/web_config/webconfig.py index 55ab372ef..8ba99bab6 100755 --- a/share/tools/web_config/webconfig.py +++ b/share/tools/web_config/webconfig.py @@ -680,6 +680,14 @@ class FishConfigHTTPRequestHandler(SimpleHTTPServer.SimpleHTTPRequestHandler): result.extend([r for r in sample_results if r]) return result + def secure_startswith(self, haystack, needle): + if len(haystack) < len(needle): + return False + bits = 0 + for x,y in zip(haystack, needle): + bits |= ord(x) ^ ord(y) + return bits == 0 + def font_size_for_ansi_prompt(self, prompt_demo_ansi): width = ansi_prompt_line_width(prompt_demo_ansi) # Pick a font size @@ -697,7 +705,7 @@ class FishConfigHTTPRequestHandler(SimpleHTTPServer.SimpleHTTPRequestHandler): p = self.path authpath = '/' + authkey - if p.startswith(authpath): + if self.secure_startswith(p, authpath): p = p[len(authpath):] else: return self.send_error(403) @@ -736,7 +744,7 @@ class FishConfigHTTPRequestHandler(SimpleHTTPServer.SimpleHTTPRequestHandler): p = self.path authpath = '/' + authkey - if p.startswith(authpath): + if self.secure_startswith(p, authpath): p = p[len(authpath):] else: return self.send_error(403) From 9079ec459cfecea2b1c31e806e2893d7801eed3a Mon Sep 17 00:00:00 2001 From: Andy Lutomirski Date: Mon, 11 Aug 2014 17:51:27 -0700 Subject: [PATCH 25/35] webconfig: fixes for token security * Use 16-byte tokens * Use os.urandom (random.getrandbits shouldn't be used for security) * Convert to hex correctly --- share/tools/web_config/webconfig.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/share/tools/web_config/webconfig.py b/share/tools/web_config/webconfig.py index 8ba99bab6..9e0b9ac00 100755 --- a/share/tools/web_config/webconfig.py +++ b/share/tools/web_config/webconfig.py @@ -26,7 +26,7 @@ if term: os.environ['TERM'] = term import subprocess -import re, socket, cgi, select, time, glob, random, string +import re, socket, cgi, select, time, glob, random, string, binascii try: import json except ImportError: @@ -859,7 +859,7 @@ where = os.path.dirname(sys.argv[0]) os.chdir(where) # Generate a 16-byte random key as a hexadecimal string -authkey = hex(random.getrandbits(16*4))[2:] +authkey = binascii.b2a_hex(os.urandom(16)) # Try to find a suitable port PORT = 8000 From 61ce9db4bac2da9f69487e828f627e4247ccc140 Mon Sep 17 00:00:00 2001 From: Kevin Ballard Date: Wed, 20 Aug 2014 18:19:23 -0700 Subject: [PATCH 26/35] Make the `alias` built-in function work better The new --wraps functionality was breaking aliases of the form `alias foo='bar baz'`. That is, aliases where the body is multiple words. Extract the first word of the body and use that instead. Use better errors for aliases with no name or no body. --- share/functions/alias.fish | 38 ++++++++++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/share/functions/alias.fish b/share/functions/alias.fish index 56beed128..2dc86b1b1 100644 --- a/share/functions/alias.fish +++ b/share/functions/alias.fish @@ -12,6 +12,7 @@ function alias --description "Legacy function for creating shellscript functions set -l name set -l body set -l prefix + set -l first_word switch (count $argv) case 0 @@ -21,8 +22,9 @@ function alias --description "Legacy function for creating shellscript functions # Some seds (e.g. on Mac OS X), don't support \n in the RHS # Use a literal newline instead # http://sed.sourceforge.net/sedfaq4.html#s4.1 - set -l tmp (echo $argv|sed -e "s/\([^=]\)=/\1\\ -/") + # The extra '' at the end is so $tmp[2] is guaranteed to work + set -l tmp (echo $argv|sed -e 's/\([^=]\{0,1\}\)=/\1\ +/') '' set name $tmp[1] set body $tmp[2] @@ -35,18 +37,34 @@ function alias --description "Legacy function for creating shellscript functions return 1 end + # sanity check + if test -z "$name" + printf ( _ "%s: Name cannot be empty\n") alias + return 1 + else if test -z "$body" + printf ( _ "%s: Body cannot be empty\n") alias + return 1 + end + + # Extract the first command from the body + switch $body + case \*\ \* \*\t\* + # note: strip leading spaces if present + set first_word (echo $body|sed -e 's/^[[:space:]]\{1,\}//;s/[[:space:]].*//;q') + case '*' + set first_word $body + end # Prevent the alias from immediately running into an infinite recursion if # $body starts with the same command as $name. - switch $body - case $name $name\ \* $name\t\* - if contains $name (builtin --names) - set prefix builtin - else - set prefix command - end + if test $first_word = $name + if contains $name (builtin --names) + set prefix builtin + else + set prefix command + end end - eval "function $name --wraps $body; $prefix $body \$argv; end" + eval "function $name --wraps $first_word; $prefix $body \$argv; end" end From 8aad53556dfc6aa969056a741c009c9f127c1f96 Mon Sep 17 00:00:00 2001 From: Kevin Ballard Date: Wed, 20 Aug 2014 18:40:14 -0700 Subject: [PATCH 27/35] Highlight pipe characters correctly According to `fish_config`'s colors page, the pipe operator `|` is supposed to be colored the same as a statement terminator. --- highlight.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/highlight.cpp b/highlight.cpp index 8145e5f30..287f1a3ba 100644 --- a/highlight.cpp +++ b/highlight.cpp @@ -1358,6 +1358,7 @@ const highlighter_t::color_array_t & highlighter_t::highlight() } break; + case parse_token_type_pipe: case parse_token_type_background: case parse_token_type_end: case symbol_optional_background: From d8b955294a9ca5e1ffd6f3345fc57ef941bd2b0b Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Fri, 22 Aug 2014 11:53:20 -0700 Subject: [PATCH 28/35] Add a test for syntax highlighting pipes --- fish_tests.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fish_tests.cpp b/fish_tests.cpp index bdf12af10..70337af12 100644 --- a/fish_tests.cpp +++ b/fish_tests.cpp @@ -3404,6 +3404,8 @@ static void test_highlighting(void) {L"ls", highlight_spec_command}, {L"param2", highlight_spec_param}, {L")", highlight_spec_operator}, + {L"|", highlight_spec_statement_terminator}, + {L"cat", highlight_spec_command}, {NULL, -1} }; From f549ada16c6d6165908c3e98f52bf64561808838 Mon Sep 17 00:00:00 2001 From: Kevin Ballard Date: Wed, 20 Aug 2014 18:43:17 -0700 Subject: [PATCH 29/35] Set up fish_{function,complete}_path properly In the base config.fish, fish_function_path and fish_complete_path have $__fish_datadir/{functions,completions} added to them if not already present. For some reason they were replacing the final path component instead of being added on to the end. --- share/config.fish | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/share/config.fish b/share/config.fish index 455d40bb2..94046a6c5 100644 --- a/share/config.fish +++ b/share/config.fish @@ -47,7 +47,7 @@ if not set -q fish_function_path end if not contains $__fish_datadir/functions $fish_function_path - set fish_function_path[-1] $__fish_datadir/functions + set fish_function_path $fish_function_path $__fish_datadir/functions end if not set -q fish_complete_path @@ -55,7 +55,7 @@ if not set -q fish_complete_path end if not contains $__fish_datadir/completions $fish_complete_path - set fish_complete_path[-1] $__fish_datadir/completions + set fish_complete_path $fish_complete_path $__fish_datadir/completions end # From d9bf53c6e563d22072bf2e18e8d6bc0027661709 Mon Sep 17 00:00:00 2001 From: Kevin Ballard Date: Wed, 20 Aug 2014 19:07:35 -0700 Subject: [PATCH 30/35] Show a non-zero status in the fish_config prompt When selecting a prompt with fish_config, render the prompt with a non-zero status so the user knows what it looks like. --- share/tools/web_config/webconfig.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/share/tools/web_config/webconfig.py b/share/tools/web_config/webconfig.py index 9e0b9ac00..d7400f78f 100755 --- a/share/tools/web_config/webconfig.py +++ b/share/tools/web_config/webconfig.py @@ -618,13 +618,13 @@ class FishConfigHTTPRequestHandler(SimpleHTTPServer.SimpleHTTPRequestHandler): def do_get_current_prompt(self): # Return the current prompt prompt_func, err = run_fish_cmd('functions fish_prompt') - result = self.do_get_prompt('builtin cd "' + initial_wd + '" ; fish_prompt', prompt_func.strip(), {'name': 'Current'}) + result = self.do_get_prompt('builtin cd "' + initial_wd + '" ; false ; fish_prompt', prompt_func.strip(), {'name': 'Current'}) return result def do_get_sample_prompt(self, text, extras_dict): # Return the prompt you get from the given text # extras_dict is a dictionary whose values get merged in - cmd = text + "\n cd \"" + initial_wd + "\" \n fish_prompt\n" + cmd = text + "\n builtin cd \"" + initial_wd + "\" \n false \n fish_prompt\n" return self.do_get_prompt(cmd, text.strip(), extras_dict) def parse_one_sample_prompt_hash(self, line, result_dict): From f9f773cc284c430bbf2fe1fd9613bbef4f3734be Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Fri, 22 Aug 2014 12:04:23 -0700 Subject: [PATCH 31/35] Comment on why we run 'false' in web_config.py --- share/tools/web_config/webconfig.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/share/tools/web_config/webconfig.py b/share/tools/web_config/webconfig.py index d7400f78f..89d180b7d 100755 --- a/share/tools/web_config/webconfig.py +++ b/share/tools/web_config/webconfig.py @@ -617,6 +617,7 @@ class FishConfigHTTPRequestHandler(SimpleHTTPServer.SimpleHTTPRequestHandler): def do_get_current_prompt(self): # Return the current prompt + # We run 'false' to demonstrate how the prompt shows the command status (#1624) prompt_func, err = run_fish_cmd('functions fish_prompt') result = self.do_get_prompt('builtin cd "' + initial_wd + '" ; false ; fish_prompt', prompt_func.strip(), {'name': 'Current'}) return result @@ -624,6 +625,7 @@ class FishConfigHTTPRequestHandler(SimpleHTTPServer.SimpleHTTPRequestHandler): def do_get_sample_prompt(self, text, extras_dict): # Return the prompt you get from the given text # extras_dict is a dictionary whose values get merged in + # We run 'false' to demonstrate how the prompt shows the command status (#1624) cmd = text + "\n builtin cd \"" + initial_wd + "\" \n false \n fish_prompt\n" return self.do_get_prompt(cmd, text.strip(), extras_dict) From 1f3a93a3afd81d842732ba96ecf11466e5be6255 Mon Sep 17 00:00:00 2001 From: Konrad Borowski Date: Fri, 22 Aug 2014 21:52:41 +0200 Subject: [PATCH 32/35] Fix the compilation under gcc 4.9.0. gcc interpretes C99's compound literals more strictly by invalid the compound literal on implicit to pointer cast (because of automatic storage duration, 6.5.2.5.6 in C99 standard draft). This fixes the issue by not using compound literals at all. --- builtin.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/builtin.cpp b/builtin.cpp index efe28f90e..da9429b17 100644 --- a/builtin.cpp +++ b/builtin.cpp @@ -2636,7 +2636,13 @@ static int builtin_read(parser_t &parser, wchar_t **argv) size_t j = 0; for (; i+1 < argc; ++i) { - env_set(argv[i], j < bufflen ? (wchar_t[2]){buff[j], 0} : L"", place); + if (j < bufflen) { + wchar_t buffer[2] = {buff[j], 0}; + env_set(argv[i], buffer, place); + } + else { + env_set(argv[i], L"", place); + } if (j < bufflen) ++j; } if (i < argc) env_set(argv[i], &buff[j], place); From f71b10df8cd69f4d0944624fd1bac61b2419be7e Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sun, 24 Aug 2014 14:27:58 -0700 Subject: [PATCH 33/35] Don't suggest after | & or in comments Fixes #1631 --- complete.cpp | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/complete.cpp b/complete.cpp index d7ecdf73e..71df3e983 100644 --- a/complete.cpp +++ b/complete.cpp @@ -1884,7 +1884,7 @@ void complete(const wcstring &cmd_with_subcmds, std::vector &comps //const wcstring prev_token(prev_begin, prev_token_len); parse_node_tree_t tree; - parse_tree_from_string(cmd, parse_flag_continue_after_error | parse_flag_accept_incomplete_tokens, &tree, NULL); + parse_tree_from_string(cmd, parse_flag_continue_after_error | parse_flag_accept_incomplete_tokens | parse_flag_include_comments, &tree, NULL); /* Find any plain statement that contains the position. We have to backtrack past spaces (#1261). So this will be at either the last space character, or after the end of the string */ size_t adjusted_pos = pos; @@ -1896,9 +1896,31 @@ void complete(const wcstring &cmd_with_subcmds, std::vector &comps const parse_node_t *plain_statement = tree.find_node_matching_source_location(symbol_plain_statement, adjusted_pos, NULL); if (plain_statement == NULL) { - /* Not part of a plain statement. This could be e.g. a for loop header, case expression, etc. Do generic file completions (#1309). If we had to backtrack, it means there was whitespace; don't do an autosuggestion in that case. */ - bool no_file = (flags & COMPLETION_REQUEST_AUTOSUGGESTION) && (adjusted_pos < pos); - completer.complete_param_expand(current_token, ! no_file); + /* Not part of a plain statement. This could be e.g. a for loop header, case expression, etc. Do generic file completions (#1309). If we had to backtrack, it means there was whitespace; don't do an autosuggestion in that case. Also don't do it if we are just after a pipe, semicolon, or & (#1631), or in a comment. + + Overall this logic is a total mess. A better approach would be to return the "possible next token" from the parse tree directly (this data is available as the first of the sequence of nodes without source locations at the very end of the parse tree). */ + bool do_file = true; + if (flags & COMPLETION_REQUEST_AUTOSUGGESTION) + { + if (adjusted_pos < pos) + { + do_file = false; + } + else if (pos > 0) + { + // If the previous character is in one of these types, we don't do file suggestions + parse_token_type_t bad_types[] = {parse_token_type_pipe, parse_token_type_end, parse_token_type_background, parse_special_type_comment}; + for (size_t i=0; i < sizeof bad_types / sizeof *bad_types; i++) + { + if (tree.find_node_matching_source_location(bad_types[i], pos - 1, NULL)) + { + do_file = false; + break; + } + } + } + } + completer.complete_param_expand(current_token, do_file); } else { From 9419191aa653da91474b8a65b56d44036d59484e Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sun, 24 Aug 2014 14:28:31 -0700 Subject: [PATCH 34/35] Clean up variable expansion, and properly handle embedded NULs --- expand.cpp | 123 ++++++++++++++++++-------------------- fish_tests.cpp | 156 ++++++++++++++++++++++++++++++++++++------------- wildcard.cpp | 33 +++++++---- wildcard.h | 4 +- 4 files changed, 199 insertions(+), 117 deletions(-) diff --git a/expand.cpp b/expand.cpp index 5f96fb2f9..c3cb95031 100644 --- a/expand.cpp +++ b/expand.cpp @@ -1049,21 +1049,24 @@ static size_t parse_slice(const wchar_t *in, wchar_t **end_ptr, std::vector &out, long last_idx, parse_error_list_t *errors); - -static int expand_variables2(parser_t &parser, const wcstring &instr, std::vector &out, long last_idx, parse_error_list_t *errors) +static int expand_variables(parser_t &parser, const wcstring &instr, std::vector &out, long last_idx, parse_error_list_t *errors) { - wchar_t *in = wcsdup(instr.c_str()); - int result = expand_variables_internal(parser, in, out, last_idx, errors); - free(in); - return result; -} - -static int expand_variables_internal(parser_t &parser, wchar_t * const in, std::vector &out, long last_idx, parse_error_list_t *errors) -{ - int is_ok= 1; - int empty=0; + // We permit last_idx to be beyond the end of the string if and only if the string is empty + assert(instr.empty() || (last_idx >= 0 && (size_t)last_idx < instr.size())); + + // Make this explicit + if (instr.empty()) + { + append_completion(out, instr); + return true; + } + + bool is_ok = true; + bool empty = false; + const size_t insize = instr.size(); wcstring var_tmp; @@ -1077,7 +1080,7 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std:: for (long i=last_idx; (i>=0) && is_ok && !empty; i--) { - const wchar_t c = in[i]; + const wchar_t c = instr.at(i); if ((c == VARIABLE_EXPAND) || (c == VARIABLE_EXPAND_SINGLE)) { long start_pos = i+1; @@ -1087,17 +1090,15 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std:: stop_pos = start_pos; - while (1) + while (stop_pos < insize) { - const wchar_t nc = in[stop_pos]; - if (!(nc)) - break; + const wchar_t nc = instr.at(stop_pos); if (nc == VARIABLE_EXPAND_EMPTY) { stop_pos++; break; } - if (!(wcsvarchr(nc))) + if (!wcsvarchr(nc)) break; stop_pos++; @@ -1109,13 +1110,13 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std:: if (var_len == 0) { - expand_variable_error(parser, in, stop_pos-1, -1, errors); + expand_variable_error(parser, instr, stop_pos-1, -1, errors); - is_ok = 0; + is_ok = false; break; } - var_tmp.append(in + start_pos, var_len); + var_tmp.append(instr, start_pos, var_len); env_var_t var_val; if (var_len == 1 && var_tmp[0] == VARIABLE_EXPAND_EMPTY) { @@ -1133,22 +1134,22 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std:: if (is_ok) { - tokenize_variable_array(var_val.c_str(), var_item_list); + tokenize_variable_array(var_val, var_item_list); const size_t slice_start = stop_pos; - if (in[slice_start] == L'[') + if (slice_start < insize && instr.at(slice_start) == L'[') { wchar_t *slice_end; size_t bad_pos; all_vars=0; - + const wchar_t *in = instr.c_str(); bad_pos = parse_slice(in + slice_start, &slice_end, var_idx_list, var_pos_list, var_item_list.size()); if (bad_pos != 0) { append_syntax_error(errors, stop_pos + bad_pos, L"Invalid index value"); - is_ok = 0; + is_ok = false; break; } stop_pos = (slice_end-in); @@ -1168,7 +1169,7 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std:: append_syntax_error(errors, slice_start + var_src_pos, ARRAY_BOUNDS_ERR); - is_ok=0; + is_ok = false; var_idx_list.resize(j); break; } @@ -1187,14 +1188,12 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std:: if (is_ok) { - if (is_single) { - in[i]=0; - wcstring res = in; + wcstring res(instr, 0, i); if (i > 0) { - if (in[i-1] != VARIABLE_EXPAND_SINGLE) + if (instr.at(i-1) != VARIABLE_EXPAND_SINGLE) { res.push_back(INTERNAL_SEPARATOR); } @@ -1215,15 +1214,16 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std:: res.append(next); } } - res.append(in + stop_pos); - is_ok &= expand_variables2(parser, res, out, i, errors); + assert(stop_pos <= insize); + res.append(instr, stop_pos, insize - stop_pos); + is_ok &= expand_variables(parser, res, out, i, errors); } else { for (size_t j=0; j 0) { - if (in[i-1] != VARIABLE_EXPAND) + if (instr.at(i-1) != VARIABLE_EXPAND) { new_in.push_back(INTERNAL_SEPARATOR); } @@ -1246,9 +1246,10 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std:: new_in.push_back(VARIABLE_EXPAND_EMPTY); } } + assert(stop_pos <= insize); new_in.append(next); - new_in.append(in + stop_pos); - is_ok &= expand_variables2(parser, new_in, out, i, errors); + new_in.append(instr, stop_pos, insize - stop_pos); + is_ok &= expand_variables(parser, new_in, out, i, errors); } } @@ -1263,8 +1264,9 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std:: // even with no value, we still need to parse out slice syntax // Behave as though we had 1 value, so $foo[1] always works. const size_t slice_start = stop_pos; - if (in[slice_start] == L'[') + if (slice_start < insize && instr.at(slice_start) == L'[') { + const wchar_t *in = instr.c_str(); wchar_t *slice_end; size_t bad_pos; @@ -1294,42 +1296,35 @@ static int expand_variables_internal(parser_t &parser, wchar_t * const in, std:: } } } - - /* - Expand a non-existing variable - */ + + /* Expand a non-existing variable */ if (c == VARIABLE_EXPAND) { - /* - Regular expansion, i.e. expand this argument to nothing - */ - empty = 1; + /* Regular expansion, i.e. expand this argument to nothing */ + empty = true; } else { - /* - Expansion to single argument. - */ + /* Expansion to single argument. */ wcstring res; - res.append(in, i); - if (i > 0 && in[i-1] == VARIABLE_EXPAND_SINGLE) + res.append(instr, 0, i); + if (i > 0 && instr.at(i-1) == VARIABLE_EXPAND_SINGLE) { res.push_back(VARIABLE_EXPAND_EMPTY); } - res.append(in + stop_pos); + assert(stop_pos <= insize); + res.append(instr, stop_pos, insize - stop_pos); - is_ok &= expand_variables2(parser, res, out, i, errors); + is_ok &= expand_variables(parser, res, out, i, errors); return is_ok; } } - - } } if (!empty) { - append_completion(out, in); + append_completion(out, instr); } return is_ok; @@ -1814,7 +1809,7 @@ int expand_string(const wcstring &input, std::vector &output, expa } else { - if (!expand_variables2(parser, next, *out, next.size() - 1, errors)) + if (!expand_variables(parser, next, *out, next.size() - 1, errors)) { return EXPAND_ERROR; } @@ -1878,12 +1873,10 @@ int expand_string(const wcstring &input, std::vector &output, expa for (i=0; i < in->size(); i++) { - wcstring next_str = in->at(i).completion; + wcstring next = in->at(i).completion; int wc_res; - remove_internal_separator(next_str, (EXPAND_SKIP_WILDCARDS & flags) ? true : false); - const wchar_t *next = next_str.c_str(); - + remove_internal_separator(next, (EXPAND_SKIP_WILDCARDS & flags) ? true : false); const bool has_wildcard = wildcard_has(next, 1); if (has_wildcard && (flags & EXECUTABLES_ONLY)) @@ -1893,12 +1886,12 @@ int expand_string(const wcstring &input, std::vector &output, expa else if (((flags & ACCEPT_INCOMPLETE) && (!(flags & EXPAND_SKIP_WILDCARDS))) || has_wildcard) { - const wchar_t *start, *rest; + wcstring start, rest; if (next[0] == '/') { start = L"/"; - rest = &next[1]; + rest = next.substr(1); } else { @@ -1943,7 +1936,7 @@ int expand_string(const wcstring &input, std::vector &output, expa { if (!(flags & ACCEPT_INCOMPLETE)) { - append_completion(*out, next_str); + append_completion(*out, next); } } } diff --git a/fish_tests.cpp b/fish_tests.cpp index 70337af12..744da79d9 100644 --- a/fish_tests.cpp +++ b/fish_tests.cpp @@ -1341,6 +1341,11 @@ static int expand_test(const wchar_t *in, int flags, ...) i++; } va_end(va); + + if (output.size() != i) + { + res = false; + } return res; @@ -1367,6 +1372,11 @@ static void test_expand() { err(L"Cannot skip wildcard expansion"); } + + if (!expand_test(L"/bin/l\\0", ACCEPT_INCOMPLETE, 0)) + { + err(L"Failed to handle null escape in expansion"); + } if (system("mkdir -p /tmp/fish_expand_test/")) err(L"mkdir failed"); if (system("touch /tmp/fish_expand_test/.foo")) err(L"touch failed"); @@ -1858,6 +1868,7 @@ static void test_colors() static void test_complete(void) { say(L"Testing complete"); + const wchar_t *name_strs[] = {L"Foo1", L"Foo2", L"Foo3", L"Bar1", L"Bar2", L"Bar3"}; size_t count = sizeof name_strs / sizeof *name_strs; const wcstring_list_t names(name_strs, name_strs + count); @@ -1937,7 +1948,47 @@ static void test_complete(void) completions.clear(); complete(L"echo \\$Foo", completions, COMPLETION_REQUEST_DEFAULT); do_test(completions.empty()); + + /* File completions */ + char saved_wd[PATH_MAX + 1] = {}; + getcwd(saved_wd, sizeof saved_wd); + if (system("mkdir -p '/tmp/complete_test/'")) err(L"mkdir failed"); + if (system("touch '/tmp/complete_test/testfile'")) err(L"touch failed"); + if (chdir("/tmp/complete_test/")) err(L"chdir failed"); + complete(L"cat te", completions, COMPLETION_REQUEST_DEFAULT); + do_test(completions.size() == 1); + do_test(completions.at(0).completion == L"stfile"); + completions.clear(); + complete(L"cat /tmp/complete_test/te", completions, COMPLETION_REQUEST_DEFAULT); + do_test(completions.size() == 1); + do_test(completions.at(0).completion == L"stfile"); + completions.clear(); + complete(L"echo sup > /tmp/complete_test/te", completions, COMPLETION_REQUEST_DEFAULT); + do_test(completions.size() == 1); + do_test(completions.at(0).completion == L"stfile"); + completions.clear(); + complete(L"echo sup > /tmp/complete_test/te", completions, COMPLETION_REQUEST_DEFAULT); + do_test(completions.size() == 1); + do_test(completions.at(0).completion == L"stfile"); + completions.clear(); + + // Zero escapes can cause problems. See #1631 + complete(L"cat foo\\0", completions, COMPLETION_REQUEST_DEFAULT); + do_test(completions.empty()); + completions.clear(); + complete(L"cat foo\\0bar", completions, COMPLETION_REQUEST_DEFAULT); + do_test(completions.empty()); + completions.clear(); + complete(L"cat \\0", completions, COMPLETION_REQUEST_DEFAULT); + do_test(completions.empty()); + completions.clear(); + complete(L"cat te\\0", completions, COMPLETION_REQUEST_DEFAULT); + do_test(completions.empty()); + completions.clear(); + if (chdir(saved_wd)) err(L"chdir failed"); + if (system("rm -Rf '/tmp/complete_test/'")) err(L"rm failed"); + complete_set_variable_names(NULL); /* Test wraps */ @@ -2004,7 +2055,7 @@ static void test_completion_insertions() TEST_1_COMPLETION(L"'foo^", L"bar", COMPLETE_REPLACES_TOKEN, false, L"bar ^"); } -static void perform_one_autosuggestion_test(const wcstring &command, const wcstring &wd, const wcstring &expected, long line) +static void perform_one_autosuggestion_special_test(const wcstring &command, const wcstring &wd, const wcstring &expected, long line) { wcstring suggestion; bool success = autosuggest_suggest_special(command, wd, suggestion); @@ -2034,57 +2085,81 @@ static void test_autosuggest_suggest_special() if (system("mkdir -p ~/test_autosuggest_suggest_special/")) err(L"mkdir failed"); //make sure tilde is handled const wcstring wd = L"/tmp/autosuggest_test/"; - perform_one_autosuggestion_test(L"cd /tmp/autosuggest_test/0", wd, L"cd /tmp/autosuggest_test/0foobar/", __LINE__); - perform_one_autosuggestion_test(L"cd \"/tmp/autosuggest_test/0", wd, L"cd \"/tmp/autosuggest_test/0foobar/\"", __LINE__); - perform_one_autosuggestion_test(L"cd '/tmp/autosuggest_test/0", wd, L"cd '/tmp/autosuggest_test/0foobar/'", __LINE__); - perform_one_autosuggestion_test(L"cd 0", wd, L"cd 0foobar/", __LINE__); - perform_one_autosuggestion_test(L"cd \"0", wd, L"cd \"0foobar/\"", __LINE__); - perform_one_autosuggestion_test(L"cd '0", wd, L"cd '0foobar/'", __LINE__); + perform_one_autosuggestion_special_test(L"cd /tmp/autosuggest_test/0", wd, L"cd /tmp/autosuggest_test/0foobar/", __LINE__); + perform_one_autosuggestion_special_test(L"cd \"/tmp/autosuggest_test/0", wd, L"cd \"/tmp/autosuggest_test/0foobar/\"", __LINE__); + perform_one_autosuggestion_special_test(L"cd '/tmp/autosuggest_test/0", wd, L"cd '/tmp/autosuggest_test/0foobar/'", __LINE__); + perform_one_autosuggestion_special_test(L"cd 0", wd, L"cd 0foobar/", __LINE__); + perform_one_autosuggestion_special_test(L"cd \"0", wd, L"cd \"0foobar/\"", __LINE__); + perform_one_autosuggestion_special_test(L"cd '0", wd, L"cd '0foobar/'", __LINE__); - perform_one_autosuggestion_test(L"cd /tmp/autosuggest_test/1", wd, L"cd /tmp/autosuggest_test/1foo\\ bar/", __LINE__); - perform_one_autosuggestion_test(L"cd \"/tmp/autosuggest_test/1", wd, L"cd \"/tmp/autosuggest_test/1foo bar/\"", __LINE__); - perform_one_autosuggestion_test(L"cd '/tmp/autosuggest_test/1", wd, L"cd '/tmp/autosuggest_test/1foo bar/'", __LINE__); - perform_one_autosuggestion_test(L"cd 1", wd, L"cd 1foo\\ bar/", __LINE__); - perform_one_autosuggestion_test(L"cd \"1", wd, L"cd \"1foo bar/\"", __LINE__); - perform_one_autosuggestion_test(L"cd '1", wd, L"cd '1foo bar/'", __LINE__); + perform_one_autosuggestion_special_test(L"cd /tmp/autosuggest_test/1", wd, L"cd /tmp/autosuggest_test/1foo\\ bar/", __LINE__); + perform_one_autosuggestion_special_test(L"cd \"/tmp/autosuggest_test/1", wd, L"cd \"/tmp/autosuggest_test/1foo bar/\"", __LINE__); + perform_one_autosuggestion_special_test(L"cd '/tmp/autosuggest_test/1", wd, L"cd '/tmp/autosuggest_test/1foo bar/'", __LINE__); + perform_one_autosuggestion_special_test(L"cd 1", wd, L"cd 1foo\\ bar/", __LINE__); + perform_one_autosuggestion_special_test(L"cd \"1", wd, L"cd \"1foo bar/\"", __LINE__); + perform_one_autosuggestion_special_test(L"cd '1", wd, L"cd '1foo bar/'", __LINE__); - perform_one_autosuggestion_test(L"cd /tmp/autosuggest_test/2", wd, L"cd /tmp/autosuggest_test/2foo\\ \\ bar/", __LINE__); - perform_one_autosuggestion_test(L"cd \"/tmp/autosuggest_test/2", wd, L"cd \"/tmp/autosuggest_test/2foo bar/\"", __LINE__); - perform_one_autosuggestion_test(L"cd '/tmp/autosuggest_test/2", wd, L"cd '/tmp/autosuggest_test/2foo bar/'", __LINE__); - perform_one_autosuggestion_test(L"cd 2", wd, L"cd 2foo\\ \\ bar/", __LINE__); - perform_one_autosuggestion_test(L"cd \"2", wd, L"cd \"2foo bar/\"", __LINE__); - perform_one_autosuggestion_test(L"cd '2", wd, L"cd '2foo bar/'", __LINE__); + perform_one_autosuggestion_special_test(L"cd /tmp/autosuggest_test/2", wd, L"cd /tmp/autosuggest_test/2foo\\ \\ bar/", __LINE__); + perform_one_autosuggestion_special_test(L"cd \"/tmp/autosuggest_test/2", wd, L"cd \"/tmp/autosuggest_test/2foo bar/\"", __LINE__); + perform_one_autosuggestion_special_test(L"cd '/tmp/autosuggest_test/2", wd, L"cd '/tmp/autosuggest_test/2foo bar/'", __LINE__); + perform_one_autosuggestion_special_test(L"cd 2", wd, L"cd 2foo\\ \\ bar/", __LINE__); + perform_one_autosuggestion_special_test(L"cd \"2", wd, L"cd \"2foo bar/\"", __LINE__); + perform_one_autosuggestion_special_test(L"cd '2", wd, L"cd '2foo bar/'", __LINE__); - perform_one_autosuggestion_test(L"cd /tmp/autosuggest_test/3", wd, L"cd /tmp/autosuggest_test/3foo\\\\bar/", __LINE__); - perform_one_autosuggestion_test(L"cd \"/tmp/autosuggest_test/3", wd, L"cd \"/tmp/autosuggest_test/3foo\\bar/\"", __LINE__); - perform_one_autosuggestion_test(L"cd '/tmp/autosuggest_test/3", wd, L"cd '/tmp/autosuggest_test/3foo\\bar/'", __LINE__); - perform_one_autosuggestion_test(L"cd 3", wd, L"cd 3foo\\\\bar/", __LINE__); - perform_one_autosuggestion_test(L"cd \"3", wd, L"cd \"3foo\\bar/\"", __LINE__); - perform_one_autosuggestion_test(L"cd '3", wd, L"cd '3foo\\bar/'", __LINE__); + perform_one_autosuggestion_special_test(L"cd /tmp/autosuggest_test/3", wd, L"cd /tmp/autosuggest_test/3foo\\\\bar/", __LINE__); + perform_one_autosuggestion_special_test(L"cd \"/tmp/autosuggest_test/3", wd, L"cd \"/tmp/autosuggest_test/3foo\\bar/\"", __LINE__); + perform_one_autosuggestion_special_test(L"cd '/tmp/autosuggest_test/3", wd, L"cd '/tmp/autosuggest_test/3foo\\bar/'", __LINE__); + perform_one_autosuggestion_special_test(L"cd 3", wd, L"cd 3foo\\\\bar/", __LINE__); + perform_one_autosuggestion_special_test(L"cd \"3", wd, L"cd \"3foo\\bar/\"", __LINE__); + perform_one_autosuggestion_special_test(L"cd '3", wd, L"cd '3foo\\bar/'", __LINE__); - perform_one_autosuggestion_test(L"cd /tmp/autosuggest_test/4", wd, L"cd /tmp/autosuggest_test/4foo\\'bar/", __LINE__); - perform_one_autosuggestion_test(L"cd \"/tmp/autosuggest_test/4", wd, L"cd \"/tmp/autosuggest_test/4foo'bar/\"", __LINE__); - perform_one_autosuggestion_test(L"cd '/tmp/autosuggest_test/4", wd, L"cd '/tmp/autosuggest_test/4foo\\'bar/'", __LINE__); - perform_one_autosuggestion_test(L"cd 4", wd, L"cd 4foo\\'bar/", __LINE__); - perform_one_autosuggestion_test(L"cd \"4", wd, L"cd \"4foo'bar/\"", __LINE__); - perform_one_autosuggestion_test(L"cd '4", wd, L"cd '4foo\\'bar/'", __LINE__); + perform_one_autosuggestion_special_test(L"cd /tmp/autosuggest_test/4", wd, L"cd /tmp/autosuggest_test/4foo\\'bar/", __LINE__); + perform_one_autosuggestion_special_test(L"cd \"/tmp/autosuggest_test/4", wd, L"cd \"/tmp/autosuggest_test/4foo'bar/\"", __LINE__); + perform_one_autosuggestion_special_test(L"cd '/tmp/autosuggest_test/4", wd, L"cd '/tmp/autosuggest_test/4foo\\'bar/'", __LINE__); + perform_one_autosuggestion_special_test(L"cd 4", wd, L"cd 4foo\\'bar/", __LINE__); + perform_one_autosuggestion_special_test(L"cd \"4", wd, L"cd \"4foo'bar/\"", __LINE__); + perform_one_autosuggestion_special_test(L"cd '4", wd, L"cd '4foo\\'bar/'", __LINE__); - perform_one_autosuggestion_test(L"cd /tmp/autosuggest_test/5", wd, L"cd /tmp/autosuggest_test/5foo\\\"bar/", __LINE__); - perform_one_autosuggestion_test(L"cd \"/tmp/autosuggest_test/5", wd, L"cd \"/tmp/autosuggest_test/5foo\\\"bar/\"", __LINE__); - perform_one_autosuggestion_test(L"cd '/tmp/autosuggest_test/5", wd, L"cd '/tmp/autosuggest_test/5foo\"bar/'", __LINE__); - perform_one_autosuggestion_test(L"cd 5", wd, L"cd 5foo\\\"bar/", __LINE__); - perform_one_autosuggestion_test(L"cd \"5", wd, L"cd \"5foo\\\"bar/\"", __LINE__); - perform_one_autosuggestion_test(L"cd '5", wd, L"cd '5foo\"bar/'", __LINE__); - - perform_one_autosuggestion_test(L"cd ~/test_autosuggest_suggest_specia", wd, L"cd ~/test_autosuggest_suggest_special/", __LINE__); + perform_one_autosuggestion_special_test(L"cd /tmp/autosuggest_test/5", wd, L"cd /tmp/autosuggest_test/5foo\\\"bar/", __LINE__); + perform_one_autosuggestion_special_test(L"cd \"/tmp/autosuggest_test/5", wd, L"cd \"/tmp/autosuggest_test/5foo\\\"bar/\"", __LINE__); + perform_one_autosuggestion_special_test(L"cd '/tmp/autosuggest_test/5", wd, L"cd '/tmp/autosuggest_test/5foo\"bar/'", __LINE__); + perform_one_autosuggestion_special_test(L"cd 5", wd, L"cd 5foo\\\"bar/", __LINE__); + perform_one_autosuggestion_special_test(L"cd \"5", wd, L"cd \"5foo\\\"bar/\"", __LINE__); + perform_one_autosuggestion_special_test(L"cd '5", wd, L"cd '5foo\"bar/'", __LINE__); + perform_one_autosuggestion_special_test(L"cd ~/test_autosuggest_suggest_specia", wd, L"cd ~/test_autosuggest_suggest_special/", __LINE__); + // A single quote should defeat tilde expansion - perform_one_autosuggestion_test(L"cd '~/test_autosuggest_suggest_specia'", wd, L"", __LINE__); + perform_one_autosuggestion_special_test(L"cd '~/test_autosuggest_suggest_specia'", wd, L"", __LINE__); if (system("rm -Rf '/tmp/autosuggest_test/'")) err(L"rm failed"); if (system("rm -Rf ~/test_autosuggest_suggest_special/")) err(L"rm failed"); } +static void perform_one_autosuggestion_should_ignore_test(const wcstring &command, const wcstring &wd, long line) +{ + completion_list_t comps; + complete(command, comps, COMPLETION_REQUEST_AUTOSUGGESTION); + do_test(comps.empty()); + if (! comps.empty()) + { + const wcstring &suggestion = comps.front().completion; + printf("line %ld: complete() expected to return nothing for %ls\n", line, command.c_str()); + printf(" instead got: %ls\n", suggestion.c_str()); + } +} + +static void test_autosuggestion_ignores() +{ + say(L"Testing scenarios that should produce no autosuggestions"); + const wcstring wd = L"/tmp/autosuggest_test/"; + // Do not do file autosuggestions immediately after certain statement terminators - see #1631 + perform_one_autosuggestion_should_ignore_test(L"echo PIPE_TEST|", wd, __LINE__); + perform_one_autosuggestion_should_ignore_test(L"echo PIPE_TEST&", wd, __LINE__); + perform_one_autosuggestion_should_ignore_test(L"echo PIPE_TEST#comment", wd, __LINE__); + perform_one_autosuggestion_should_ignore_test(L"echo PIPE_TEST;", wd, __LINE__); +} + static void test_autosuggestion_combining() { say(L"Testing autosuggestion combining"); @@ -3631,6 +3706,7 @@ int main(int argc, char **argv) if (should_test_function("universal")) test_universal_callbacks(); if (should_test_function("notifiers")) test_universal_notifiers(); if (should_test_function("completion_insertions")) test_completion_insertions(); + if (should_test_function("autosuggestion_ignores")) test_autosuggestion_ignores(); if (should_test_function("autosuggestion_combining")) test_autosuggestion_combining(); if (should_test_function("autosuggest_suggest_special")) test_autosuggest_suggest_special(); if (should_test_function("history")) history_tests_t::test_history(); diff --git a/wildcard.cpp b/wildcard.cpp index 3ec1a8e53..fde6ddc60 100644 --- a/wildcard.cpp +++ b/wildcard.cpp @@ -105,18 +105,14 @@ wildcards using **. /** Hashtable containing all descriptions that describe an executable */ static std::map suffix_map; - -bool wildcard_has(const wchar_t *str, bool internal) +// Implementation of wildcard_has. Needs to take the length to handle embedded nulls (#1631) +static bool wildcard_has_impl(const wchar_t *str, size_t len, bool internal) { - if (!str) - { - debug(2, L"Got null string on line %d of file %s", __LINE__, __FILE__); - return false; - } - + assert(str != NULL); + const wchar_t *end = str + len; if (internal) { - for (; *str; str++) + for (; str < end; str++) { if ((*str == ANY_CHAR) || (*str == ANY_STRING) || (*str == ANY_STRING_RECURSIVE)) return true; @@ -125,7 +121,7 @@ bool wildcard_has(const wchar_t *str, bool internal) else { wchar_t prev=0; - for (; *str; str++) + for (; str < end; str++) { if (((*str == L'*') || (*str == L'?')) && (prev != L'\\')) return true; @@ -136,6 +132,18 @@ bool wildcard_has(const wchar_t *str, bool internal) return false; } +bool wildcard_has(const wchar_t *str, bool internal) +{ + assert(str != NULL); + return wildcard_has_impl(str, wcslen(str), internal); +} + +bool wildcard_has(const wcstring &str, bool internal) +{ + return wildcard_has_impl(str.data(), str.size(), internal); +} + + /** Check whether the string str matches the wildcard string wc. @@ -1084,6 +1092,11 @@ int wildcard_expand(const wchar_t *wc, int wildcard_expand_string(const wcstring &wc, const wcstring &base_dir, expand_flags_t flags, std::vector &outputs) { + /* Hackish fix for 1631. We are about to call c_str(), which will produce a string truncated at any embedded nulls. We could fix this by passing around the size, etc. However embedded nulls are never allowed in a filename, so we just check for them and return 0 (no matches) if there is an embedded null. This isn't quite right, e.g. it will fail for \0?, but that is an edge case. */ + if (wc.find(L'\0') != wcstring::npos) + { + return 0; + } // PCA: not convinced this temporary variable is really necessary std::vector lst; int res = wildcard_expand(wc.c_str(), base_dir.c_str(), flags, lst); diff --git a/wildcard.h b/wildcard.h index 5a3e291a0..8c542221a 100644 --- a/wildcard.h +++ b/wildcard.h @@ -79,9 +79,9 @@ int wildcard_expand_string(const wcstring &wc, const wcstring &base_dir, expand_ */ bool wildcard_match(const wcstring &str, const wcstring &wc, bool leading_dots_fail_to_match = false); - /** Check if the specified string contains wildcards */ -bool wildcard_has(const wchar_t *str, bool internal); +bool wildcard_has(const wcstring &, bool internal); +bool wildcard_has(const wchar_t *, bool internal); /** Test wildcard completion From 02a07164f34462b8ce7c26594746ae587206a96e Mon Sep 17 00:00:00 2001 From: Kevin Ballard Date: Fri, 22 Aug 2014 09:29:37 -0700 Subject: [PATCH 35/35] Make `commandline -P` actually work `commandline --paging-mode` worked but the short flags list accidentally omitted the documented `-P`. --- builtin_commandline.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin_commandline.cpp b/builtin_commandline.cpp index 0af6f6565..0bc60dca2 100644 --- a/builtin_commandline.cpp +++ b/builtin_commandline.cpp @@ -326,7 +326,7 @@ static int builtin_commandline(parser_t &parser, wchar_t **argv) int opt = wgetopt_long(argc, argv, - L"abijpctwforhI:CLSs", + L"abijpctwforhI:CLSsP", long_options, &opt_index); if (opt == -1)