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 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 is a mild breaking change: if a variable expansion contains spaces or
other shell metacharacters, "eval" will do the wrong thing.  I expect the
risk to be minor.
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 substituions, in the hope that this matches
user expectations better.

CHANGELOG.rst

@@ -15,6 +15,7 @@ We have tried to keep these to a minimum, but in some cases it is unavoidable.
 - ``functions --handlers`` will now list handlers in a different order.
   Now it is definition order, first to last, where before it was last to first.
   This was never specifically defined, and we recommend not relying on a specific order (:issue:`9944`).
+- ``commandline --tokenize`` learned to expand tokens, removing the need to run "eval" on its output (:issue:`???`).
 
 Notable improvements and fixes
 ------------------------------

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,8 @@
 use super::prelude::*;
-use crate::common::{unescape_string, UnescapeFlags, UnescapeStringStyle};
+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, EXPANSION_LIMIT_DEFAULT};
 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 +12,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 +90,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 +101,34 @@ 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;
             }
+            if expand_string(
+                tok.text_of(&token).to_owned(),
+                &mut args,
+                ExpandFlags::SKIP_CMDSUBST,
+                &OperationContext::foreground(
+                    parser.shared(),
+                    Box::new(no_cancel),
+                    EXPANSION_LIMIT_DEFAULT,
+                ),
+                None,
+            ) != ExpandResultCode::ok
+            {
+                return;
+            };
+        }
+        for arg in args {
+            streams.out.appendln(arg.completion);
         }
-
-        streams.out.append(out);
     } else {
         if cut_at_cursor {
             streams.out.append(&buffer[range.start..range.start + pos]);
@@ -476,6 +486,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;