builtin commandline: --tokenize to expand strings, to avoid need for …

…eval

Issue fish-shell#10194 reports Cobra completions do

    set -l args (commandline -opc)
    eval $args[1] __complete $args[2..] (commandline -ct | string escape)

The intent behind "eval" is to expand variables and tildes in "$args".
Fair enough. Several of our own completions do the same, see below.

The problem with "commandline -o" + "eval" is that the former already removes
quotes that are significant for "eval". This becomes a problem if $args
contains quoted () or {}, for example this command will wrongly execute a
command substituion:

    git --work-tree='(launch-missiles)' <TAB>

It is possible to escape the string the tokens before running eval, but
then there will be no expansion of variables etc.  The problem is that
"commandline -o" only unescapes tokens so they end up in a weird state
in-between source and expanded version.

Remove the need for "eval" by making "commandline -o" expand things like
variables and braces I don't think there is any use case where not expanding is
better. This enables custom completion scripts to be aware of shell variables
without eval, see the added test for completions to "make -C $var/some/dir ".

Some completion scripts use eval without escaping the tokens.  This is
already broken in some ways but this changes adds more cases of breakage :
when a variable expansion contains spaces or other shell metacharacters,
"eval" will do the wrong thing.
There are some third-party uses of "eval (commandline -opc)".  Besides Cobra
that's at least [pip](pypa/pip#9727).

The new expansion skips command substitutions, in the hope that this matches
user expectation better than running them. They are passed through as-is
(instead of cancelling or expanding to nothing) to make custom completion
scripts work well in the common case.

CHANGELOG.rst

@@ -5,6 +5,9 @@ fish 3.8.0 (released ???)
 
 Notable backwards-incompatible changes
 --------------------------------------
+
+- ``commandline --tokenize`` learned to expand tokens, removing the need to run "eval" on its output (:issue:`10212`).
+
 fish is being (once you are reading this hopefully "has been") ported to rust, which unfortunately involves a few backwards-incompatible changes.
 We have tried to keep these to a minimum, but in some cases it is unavoidable.
 

doc_src/cmds/commandline.rst

@@ -80,7 +80,8 @@ The following options change the way ``commandline`` prints the current commandl
     or ``commandline -co; commandline -ct`` for short.
 
 **-o** or **--tokenize**
-    Tokenize the selection and print one string-type token per line.
+    Perform argument expansion on the selection and print one argument per line.
+    Command substituions are not expanded but forwarded as-is.
 
 If ``commandline`` is called during a call to complete a given string using ``complete -C STRING``, ``commandline`` will consider the specified string to be the current contents of the command line.
 

share/completions/ant.fish

@@ -8,7 +8,7 @@ function __fish_complete_ant_targets -d "Print list of targets from build.xml an
         for token in $argv[2..-1]
             switch $prev
                 case -buildfile -file -f
-                    set buildfile (eval echo $token)
+                    set buildfile echo $token
             end
             set prev $token
         end

share/completions/blender.fish

@@ -10,7 +10,7 @@ function __blender_echo_input_file_name
         string match -r '.*\\.blend[0-9]*$' |
         tail --lines=1)
     # Using eval to expand ~ and variables specified on the commandline.
-    eval echo $path
+    echo $path
 end
 
 function __blender_list_scenes

share/completions/git.fish

@@ -16,8 +16,7 @@ function __fish_git
         end
     end
     # Using 'command git' to avoid interactions for aliases from git to (e.g.) hub
-    # Using eval to expand ~ and variables specified on the commandline.
-    eval command git $global_args \$saved_args 2>/dev/null
+    command git $global_args $saved_args 2>/dev/null
 end
 
 # Print an optspec for argparse to handle git's options that are independent of any subcommand.
@@ -2462,11 +2461,11 @@ complete -c git -n __fish_git_needs_command -a '(__fish_git_custom_commands)' -d
 function __fish_git_complete_custom_command -a subcommand
     set -l cmd (commandline -opc)
     set -e cmd[1] # Drop "git".
-    set -l subcommand_args
+    set -lx subcommand_args
     if argparse -s (__fish_git_global_optspecs) -- $cmd
         set subcommand_args $argv[2..] # Drop the subcommand.
     end
-    complete -C "git-$subcommand $subcommand_args "(commandline -ct)
+    complete -C "git-$subcommand \$subcommand_args "(commandline -ct)
 end
 
 # source git-* commands' autocompletion file if exists

share/completions/ninja.fish

@@ -2,8 +2,7 @@ function __fish_ninja
     set -l saved_args $argv
     set -l dir .
     if argparse -i C/dir= -- (commandline -opc)
-        # Using eval to expand ~ and variables specified on the commandline.
-        eval command ninja -C$_flag_C \$saved_args
+        command ninja -C$_flag_C $saved_args
     end
 end
 

share/completions/unzip.fish

@@ -39,8 +39,7 @@ if unzip -v 2>/dev/null | string match -eq Debian
     complete -c unzip -n "__fish_is_nth_token 1" -k -xa '(__fish_complete_suffix .zip .jar .aar)'
 
     # Files thereafter are either files to include or exclude from the operation
-    set -l zipfile
-    complete -c unzip -n 'not __fish_is_nth_token 1' -xa '(unzip -l (eval set zipfile (__fish_first_token); echo $zipfile) 2>/dev/null | string replace -r --filter ".*:\S+\s+(.*)" "\$1")'
+    complete -c unzip -n 'not __fish_is_nth_token 1' -xa '(unzip -l (__fish_first_token) 2>/dev/null | string replace -r --filter ".*:\S+\s+(.*)" "\$1")'
 
 else
 

share/completions/watchexec.fish

@@ -1,7 +1,7 @@
 function __fish_watchexec_print_remaining_args
     set -l spec w/watch= c/clear='?' o/on-busy-update= r/restart s/signal= stop-signal= stop-timeout= d/debounce= stdin-quit no-vcs-ignore no-project-ignore no-global-ignore no-default-ignore no-discover-ignore p/postpone delay-run= poll= shell= n no-environment emit-events-to= E/env= no-process-group N/notify project-origin= workdir= e/exts= f/filter= filter-file= i/ignore= ignore-file= fs-events= no-meta print-events v/verbose log-file= manual h/help V/version
 
-    set argv (commandline -opc) (commandline -ct)
+    set argv (commandline -opc | string escape) (commandline -ct)
     set -e argv[1]
 
     argparse -s $spec -- $argv 2>/dev/null

share/completions/which.fish

@@ -16,4 +16,4 @@ else # OSX
     complete -c which -s s -d "Print no output, only return 0 if found"
 end
 
-complete -c which -a "(complete -C (printf %s\n (commandline -ot)))" -x
+complete -c which -a "(complete -C (commandline -t))" -x

share/functions/__fish_complete_subcommand.fish

@@ -18,7 +18,7 @@ function __fish_complete_subcommand -d "Complete subcommand" --no-scope-shadowin
     set -l options_with_param $argv
 
     if not string length -q -- $subcommand
-        set -l cmd (commandline -cop) (commandline -ct)
+        set -l cmd (commandline -cop | string escape) (commandline -ct)
         while set -q cmd[1]
             set -l token $cmd[1]
             set -e cmd[1]

share/functions/__fish_preview_current_file.fish

@@ -5,20 +5,22 @@ function __fish_preview_current_file --description "Open the file at the cursor
     # commandline -t will never return an empty list. However, the token
     # could comprise multiple lines, so join them into a single string.
     set -l file (commandline -t | string collect)
+    set -l prefix eval set
 
     if test -z $file
         # $backslash will parsed as regex which may need additional escaping.
         set -l backslash '\\\\'
         not status test-feature regex-easyesc && set backslash $backslash$backslash
         set file (string replace -ra -- '([ ;#^<>&|()"\'])' "$backslash\$1" (commandline -oc)[-1])
+        set prefix set
     end
 
     set -q file[1] || return
 
     # strip -option= from token if present
     set file (string replace -r -- '^-[^\s=]*=' '' $file | string collect)
 
-    eval set -l files $file || return # Bail if $file does not tokenize.
+    $prefix -l files $file || return # Bail if $file does not tokenize.
 
     if set -q files[1] && test -f $files[1]
         $pager $files

src/builtins/commandline.rs

@@ -1,7 +1,9 @@
 use super::prelude::*;
-use crate::common::{unescape_string, UnescapeFlags, UnescapeStringStyle};
+use crate::complete::{CompleteFlags, Completion};
+use crate::expand::{expand_string, ExpandFlags, ExpandResultCode};
 use crate::input::input_function_get_code;
 use crate::input_common::{CharEvent, ReadlineCmd};
+use crate::operation_context::{no_cancel, OperationContext};
 use crate::parse_constants::ParserTestErrorBits;
 use crate::parse_util::{
     parse_util_detect_errors, parse_util_job_extent, parse_util_lineno, parse_util_process_extent,
@@ -11,9 +13,8 @@ use crate::proc::is_interactive_session;
 use crate::reader::{
     commandline_get_state, commandline_set_buffer, reader_handle_command, reader_queue_ch,
 };
-use crate::tokenizer::TokenType;
-use crate::tokenizer::Tokenizer;
 use crate::tokenizer::TOK_ACCEPT_UNFINISHED;
+use crate::tokenizer::{TokenType, Tokenizer};
 use crate::wchar::prelude::*;
 use crate::wcstringutil::join_strings;
 use crate::wgetopt::{wgetopter_t, wopt, woption, woption_argument_t};
@@ -90,6 +91,7 @@ fn replace_part(
 /// \param buffer the original command line buffer
 /// \param cursor_pos the position of the cursor in the command line
 fn write_part(
+    parser: &Parser,
     range: Range<usize>,
     cut_at_cursor: bool,
     tokenize: bool,
@@ -100,25 +102,47 @@ fn write_part(
     let pos = cursor_pos - range.start;
 
     if tokenize {
-        let mut out = WString::new();
         let buff = &buffer[range];
         let mut tok = Tokenizer::new(buff, TOK_ACCEPT_UNFINISHED);
+        let mut args = vec![];
         while let Some(token) = tok.next() {
             if cut_at_cursor && token.end() >= pos {
                 break;
             }
-
-            if token.type_ == TokenType::string {
-                let tmp = tok.text_of(&token);
-                let unescaped =
-                    unescape_string(tmp, UnescapeStringStyle::Script(UnescapeFlags::INCOMPLETE))
-                        .unwrap();
-                out.push_utfstr(&unescaped);
-                out.push('\n');
+            if token.type_ != TokenType::string {
+                continue;
             }
-        }
 
-        streams.out.append(out);
+            const COMMANDLINE_TOKENS_MAX_EXPANSION: usize = 512;
+
+            let token_text = tok.text_of(&token);
+
+            match expand_string(
+                token_text.to_owned(),
+                &mut args,
+                ExpandFlags::SKIP_CMDSUBST,
+                &OperationContext::foreground(
+                    parser.shared(),
+                    Box::new(no_cancel),
+                    COMMANDLINE_TOKENS_MAX_EXPANSION,
+                ),
+                None,
+            )
+            .result
+            {
+                ExpandResultCode::error | ExpandResultCode::wildcard_no_match => {
+                    // Hit expansion limit, forward the unexpanded string.
+                    args.push(Completion::from_completion(token_text.to_owned()));
+                }
+                ExpandResultCode::cancel => {
+                    return;
+                }
+                ExpandResultCode::ok => (),
+            };
+        }
+        for arg in args {
+            streams.out.appendln(arg.completion);
+        }
     } else {
         if cut_at_cursor {
             streams.out.append(&buffer[range.start..range.start + pos]);
@@ -476,6 +500,7 @@ pub fn commandline(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr])
 
     if positional_args == 0 {
         write_part(
+            parser,
             range,
             cut_at_cursor,
             tokenize,

src/expand.rs

@@ -34,6 +34,8 @@ bitflags! {
     pub struct ExpandFlags : u16 {
         /// Fail expansion if there is a command substitution.
         const FAIL_ON_CMDSUBST = 1 << 0;
+        /// Skip command substitutions.
+        const SKIP_CMDSUBST = 1 << 14;
         /// Skip variable expansion.
         const SKIP_VARIABLES = 1 << 1;
         /// Skip wildcard expansion.
@@ -1309,6 +1311,12 @@ impl<'a, 'b, 'c> Expander<'a, 'b, 'c> {
     }
 
     fn stage_cmdsubst(&mut self, input: WString, out: &mut CompletionReceiver) -> ExpandResult {
+        if self.flags.contains(ExpandFlags::SKIP_CMDSUBST) {
+            if !out.add(input) {
+                return append_overflow_error(self.errors, None);
+            }
+            return ExpandResult::ok();
+        }
         if self.flags.contains(ExpandFlags::FAIL_ON_CMDSUBST) {
             let mut cursor = 0;
             let mut start = 0;

tests/checks/complete.fish

@@ -552,3 +552,28 @@ rm -r $tmpdir
 complete -C'complete --command=mktemp' | string replace -rf '=mktemp\t.*' '=mktemp'
 # (one "--command=" is okay, we used to get "--command=--command="
 # CHECK: --command=mktemp
+
+## Test token expansion in commandline -o
+
+complete complete_make -f -a '(argparse C/directory= -- (commandline -opc)[2..];
+                               echo Completing targets in directory $_flag_C)'
+var=path/to complete -C'complete_make -C "$var/build-directory" '
+# CHECK: Completing targets in directory path/to/build-directory
+var1=path complete -C'var2=to complete_make -C "$var1/$var2/other-build-directory" '
+# CHECK: Completing targets in directory path/to/other-build-directory
+
+complete complete_existing_argument -f -a '(commandline -opc)[2..]'
+var=a_value complete -C'complete_existing_argument "1  2" $var \'quoted (foo bar)\' unquoted(baz qux) '
+# CHECK: 1  2
+# CHECK: a_value
+# CHECK: quoted (foo bar)
+# CHECK: unquoted(baz qux)
+
+complete complete_argument_count -f -a '(set -l args (commandline -opc)[2..]
+                                        echo (count $args) arguments, first argument is $args[1])'
+list=arg(seq 10) begin
+    complete -C'complete_argument_count $list$list '
+    # CHECK: 100 arguments, first argument is arg1arg1
+    complete -C'complete_argument_count $list$list$list '
+    # CHECK: 1 arguments, first argument is $list$list$list
+end