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;