Public/nushell
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
b06f31d3c6
nushell/crates/nu-command/tests/commands
History
YizhePKU 6c649809d3
Rewrite run_external.rs (#12921) 
This PR is a complete rewrite of `run_external.rs`. The main goal of the
rewrite is improving readability, but it also fixes some bugs related to
argument handling and the PATH variable (fixes
https://github.com/nushell/nushell/issues/6011).

I'll discuss some technical details to make reviewing easier.

## Argument handling

Quoting arguments for external commands is hard. Like, *really* hard.
We've had more than a dozen issues and PRs dedicated to quoting
arguments (see Appendix) but the current implementation is still buggy.

Here's a demonstration of the buggy behavior:

```nu
let foo = "'bar'"
^touch $foo            # This creates a file named `bar`, but it should be `'bar'`
^touch ...[ "'bar'" ]  # Same
```

I'll describe how this PR deals with argument handling.

First, we'll introduce the concept of **bare strings**. Bare strings are
**string literals** that are either **unquoted** or **quoted by
backticks** [^1]. Strings within a list literal are NOT considered bare
strings, even if they are unquoted or quoted by backticks.

When a bare string is used as an argument to external process, we need
to perform tilde-expansion, glob-expansion, and inner-quotes-removal, in
that order. "Inner-quotes-removal" means transforming from
`--option="value"` into `--option=value`.

## `.bat` files and CMD built-ins

On Windows, `.bat` files and `.cmd` files are considered executable, but
they need `CMD.exe` as the interpreter. The Rust standard library
supports running `.bat` files directly and will spawn `CMD.exe` under
the hood (see
[documentation](https://doc.rust-lang.org/std/process/index.html#windows-argument-splitting)).
However, other extensions are not supported [^2].

Nushell also supports a selected number of CMD built-ins. The problem
with CMD is that it uses a different set of quoting rules. Correctly
quoting for CMD requires using
[Command::raw_arg()](https://doc.rust-lang.org/std/os/windows/process/trait.CommandExt.html#tymethod.raw_arg)
and manually quoting CMD special characters, on top of quoting from the
Nushell side. ~~I decided that this is too complex and chose to reject
special characters in CMD built-ins instead [^3]. Hopefully this will
not affact real-world use cases.~~ I've implemented escaping that works
reasonably well.

## `which-support` feature

The `which` crate is now a hard dependency of `nu-command`, making the
`which-support` feature essentially useless. The `which` crate is
already a hard dependency of `nu-cli`, and we should consider removing
the `which-support` feature entirely.

## Appendix

Here's a list of quoting-related issues and PRs in rough chronological
order.

* https://github.com/nushell/nushell/issues/4609
* https://github.com/nushell/nushell/issues/4631
* https://github.com/nushell/nushell/issues/4601
  * https://github.com/nushell/nushell/pull/5846
* https://github.com/nushell/nushell/issues/5978
  * https://github.com/nushell/nushell/pull/6014
* https://github.com/nushell/nushell/issues/6154
  * https://github.com/nushell/nushell/pull/6161
* https://github.com/nushell/nushell/issues/6399
  * https://github.com/nushell/nushell/pull/6420
  * https://github.com/nushell/nushell/pull/6426
* https://github.com/nushell/nushell/issues/6465
* https://github.com/nushell/nushell/issues/6559
  * https://github.com/nushell/nushell/pull/6560

[^1]: The idea that backtick-quoted strings act like bare strings was
introduced by Kubouch and briefly mentioned in [the language
reference](https://www.nushell.sh/lang-guide/chapters/strings_and_text.html#backtick-quotes).

[^2]: The documentation also said "running .bat scripts in this way may
be removed in the future and so should not be relied upon", which is
another reason to move away from this. But again, quoting for CMD is
hard.

[^3]: If anyone wants to try, the best resource I found on the topic is
[this](https://daviddeley.com/autohotkey/parameters/parameters.htm).
2024-05-23 02:05:27 +00:00
..
assignment Match ++= capabilities with ++ (#11130) 2023-12-07 05:46:37 +08:00
bytes Add string/binary type color to ByteStream (#12897) 2024-05-20 00:35:32 +00:00
conversions add --signed flag for binary into int conversions (#11902) 2024-02-27 15:05:26 +00:00
database Avoid taking unnecessary ownership of intermediates (#12740) 2024-05-04 00:53:15 +00:00
date fix format date based on users locale (#11908) 2024-02-20 11:08:49 -06:00
debug Force timeit to not capture stdout (#12465) 2024-04-10 13:31:29 +00:00
hash_ Bump base64 to 0.22.1 (#12757) 2024-05-04 15:56:16 +03:00
math to json -r not removing whitespaces fix (#11948) 2024-03-20 22:14:31 +01:00
move_ Avoid taking unnecessary ownership of intermediates (#12740) 2024-05-04 00:53:15 +00:00
network Disable flaky network tests (#12010) 2024-02-28 16:28:33 +00:00
path Avoid taking unnecessary ownership of intermediates (#12740) 2024-05-04 00:53:15 +00:00
platform refactor: move du from platform to filesystem (#11852) 2024-02-15 06:55:21 +08:00
query
random remove size command in favor of str stats (#10784) 2023-11-17 06:49:19 +08:00
skip Add string/binary type color to ByteStream (#12897) 2024-05-20 00:35:32 +00:00
str_ fix range semantic in detect_columns, str substring, str index-of (#12894) 2024-05-22 20:00:58 +03:00
take Add string/binary type color to ByteStream (#12897) 2024-05-20 00:35:32 +00:00
url Allow 'url join' to print username without password (#11697) 2024-01-31 16:52:23 -06:00
alias.rs Avoid taking unnecessary ownership of intermediates (#12740) 2024-05-04 00:53:15 +00:00
all.rs Rewrite run_external.rs (#12921) 2024-05-23 02:05:27 +00:00
any.rs Rewrite run_external.rs (#12921) 2024-05-23 02:05:27 +00:00
append.rs
break_.rs
cal.rs to json -r not removing whitespaces fix (#11948) 2024-03-20 22:14:31 +01:00
cd.rs Implement PWD recovery (#12779) 2024-05-10 11:06:33 -05:00
compact.rs Remove file I/O from tests that don't need it (#11182) 2023-11-29 23:21:34 +01:00
complete.rs Rewrite run_external.rs (#12921) 2024-05-23 02:05:27 +00:00
config_env_default.rs Command: Add config env/nu --default to print defaults (#10480) 2023-09-25 08:00:59 -05:00
config_nu_default.rs Command: Add config env/nu --default to print defaults (#10480) 2023-09-25 08:00:59 -05:00
continue_.rs
debug_info.rs Make debug info lazy (#10728) 2023-10-24 12:48:05 -05:00
def.rs Remove --flag: bool support (#11541) 2024-01-25 14:16:49 +08:00
default.rs Refactor first and last (#12478) 2024-04-13 14:58:54 +00:00
detect_columns.rs fix range semantic in detect_columns, str substring, str index-of (#12894) 2024-05-22 20:00:58 +03:00
do_.rs allow define it as a variable inside closure (#12888) 2024-05-17 00:03:13 +00:00
drop.rs Refactor drop columns to fix issues (#10903) 2023-11-09 13:51:46 +01:00
du.rs Avoid taking unnecessary ownership of intermediates (#12740) 2024-05-04 00:53:15 +00:00
each.rs Prevent each from swallowing errors when eval_block returns a ListStream (#12412) 2024-05-01 17:24:54 -05:00
echo.rs
empty.rs
error_make.rs Refactor error make (#10923) 2023-11-03 10:09:33 -05:00
every.rs Avoid taking unnecessary ownership of intermediates (#12740) 2024-05-04 00:53:15 +00:00
exec.rs Isolate tests from user config (#12437) 2024-04-10 06:27:46 +08:00
export_def.rs
fill.rs
filter.rs Fix return in filter closure eval (#12292) 2024-03-26 17:50:36 +01:00
find.rs to json -r not removing whitespaces fix (#11948) 2024-03-20 22:14:31 +01:00
first.rs Add string/binary type color to ByteStream (#12897) 2024-05-20 00:35:32 +00:00
flatten.rs Remove file I/O from tests that don't need it (#11182) 2023-11-29 23:21:34 +01:00
for_.rs
format.rs Avoid taking unnecessary ownership of intermediates (#12740) 2024-05-04 00:53:15 +00:00
generate.rs rename unfold to generate (#10770) 2023-10-19 09:30:34 -05:00
get.rs Avoid taking unnecessary ownership of intermediates (#12740) 2024-05-04 00:53:15 +00:00
glob.rs Avoid taking unnecessary ownership of intermediates (#12740) 2024-05-04 00:53:15 +00:00
griddle.rs make grid throw an error when not enough columns (#12672) 2024-04-26 06:33:00 -05:00
group_by.rs Make group-by return errors in closure (#12508) 2024-04-16 21:52:21 +02:00
headers.rs
help.rs Avoid taking unnecessary ownership of intermediates (#12740) 2024-05-04 00:53:15 +00:00
histogram.rs Remove file I/O from tests that don't need it (#11182) 2023-11-29 23:21:34 +01:00
ignore.rs Change the ignore command to use drain() instead of collecting a value (#12120) 2024-03-08 02:18:26 -05:00
insert.rs Remove lazy records (#12682) 2024-05-03 08:36:10 +08:00
inspect.rs
interleave.rs Isolate tests from user config (#12437) 2024-04-10 06:27:46 +08:00
into_datetime.rs
into_filesize.rs Fix negative value file size for "into filesize" (issue #12396) (#12443) 2024-04-07 16:50:11 +00:00
into_int.rs add --signed flag for binary into int conversions (#11902) 2024-02-27 15:05:26 +00:00
join.rs
last.rs Add string/binary type color to ByteStream (#12897) 2024-05-20 00:35:32 +00:00
length.rs
let_.rs add raw-string literal support (#9956) 2024-05-02 09:36:37 -04:00
lines.rs
loop_.rs
ls.rs Allow ls works inside dir with [] brackets (#12625) 2024-05-06 14:01:32 +08:00
match_.rs Allow comments in match blocks (#11717) 2024-02-08 07:22:42 +08:00
merge.rs Remove file I/O from tests that don't need it (#11182) 2023-11-29 23:21:34 +01:00
mktemp.rs Add mktemp command (#11005) 2023-11-17 19:30:53 -06:00
mod.rs Add string/binary type color to ByteStream (#12897) 2024-05-20 00:35:32 +00:00
mut_.rs add raw-string literal support (#9956) 2024-05-02 09:36:37 -04:00
nu_check.rs Avoid taking unnecessary ownership of intermediates (#12740) 2024-05-04 00:53:15 +00:00
open.rs Remove dataframes crate and feature (#12889) 2024-05-20 17:22:08 +00:00
par_each.rs
parse.rs Avoid taking unnecessary ownership of intermediates (#12740) 2024-05-04 00:53:15 +00:00
prepend.rs Avoid taking unnecessary ownership of intermediates (#12740) 2024-05-04 00:53:15 +00:00
print.rs
range.rs Avoid taking unnecessary ownership of intermediates (#12740) 2024-05-04 00:53:15 +00:00
redirection.rs Avoid taking unnecessary ownership of intermediates (#12740) 2024-05-04 00:53:15 +00:00
reduce.rs Improve with-env robustness (#12523) 2024-04-16 19:08:58 +08:00
reject.rs Remove list of cell path support for select and reject (#11859) 2024-02-15 07:49:48 -06:00
rename.rs Remove file I/O from tests that don't need it (#11182) 2023-11-29 23:21:34 +01:00
return_.rs Isolate tests from user config (#12437) 2024-04-10 06:27:46 +08:00
reverse.rs
rm.rs Avoid taking unnecessary ownership of intermediates (#12740) 2024-05-04 00:53:15 +00:00
roll.rs
rotate.rs Fix panic in rotate; Add safe record creation function (#11718) 2024-02-03 13:23:16 +02:00
run_external.rs Avoid taking unnecessary ownership of intermediates (#12740) 2024-05-04 00:53:15 +00:00
save.rs collect: don't require a closure (#12788) 2024-05-17 18:46:03 +02:00
select.rs Add sys subcommands (#12747) 2024-05-06 23:20:27 +00:00
semicolon.rs
seq_char.rs
seq_date.rs Fix panic in seq date (#11871) 2024-02-17 10:51:20 +02:00
seq.rs
sort_by.rs to json -r not removing whitespaces fix (#11948) 2024-03-20 22:14:31 +01:00
sort.rs
source_env.rs Rewrite run_external.rs (#12921) 2024-05-23 02:05:27 +00:00
split_by.rs Avoid taking unnecessary ownership of intermediates (#12740) 2024-05-04 00:53:15 +00:00
split_column.rs Avoid taking unnecessary ownership of intermediates (#12740) 2024-05-04 00:53:15 +00:00
split_row.rs Avoid taking unnecessary ownership of intermediates (#12740) 2024-05-04 00:53:15 +00:00
table.rs Minimize future false positive typos (#12751) 2024-05-04 15:00:44 +00:00
tee.rs Add tee command for operating on copies of streams (#11928) 2024-02-28 17:08:31 -06:00
terminal.rs Add is-terminal to determine if stdin/out/err are a terminal (#10970) 2023-11-21 20:48:39 -06:00
to_text.rs
touch.rs Avoid taking unnecessary ownership of intermediates (#12740) 2024-05-04 00:53:15 +00:00
transpose.rs
try_.rs Make exit code available in catch block (#12648) 2024-04-26 16:35:08 +00:00
ucp.rs Avoid taking unnecessary ownership of intermediates (#12740) 2024-05-04 00:53:15 +00:00
ulimit.rs FreeBSD compatibility patches (#11869) 2024-02-17 20:04:59 +01:00
umkdir.rs Fix #12391: mkdir uses process startup directory instead of current script directory (#12394) 2024-04-04 14:23:10 +02:00
uname.rs Initial implementation for uutils uname (#11684) 2024-03-25 16:51:50 -05:00
uniq_by.rs Remove file I/O from tests that don't need it (#11182) 2023-11-29 23:21:34 +01:00
uniq.rs Remove file I/O from tests that don't need it (#11182) 2023-11-29 23:21:34 +01:00
update.rs Remove lazy records (#12682) 2024-05-03 08:36:10 +08:00
upsert.rs Remove lazy records (#12682) 2024-05-03 08:36:10 +08:00
use_.rs Rewrite run_external.rs (#12921) 2024-05-23 02:05:27 +00:00
where_.rs Fix ignored clippy lints (#12160) 2024-03-11 19:46:04 +01:00
which.rs
while_.rs
with_env.rs Improve with-env robustness (#12523) 2024-04-16 19:08:58 +08:00
wrap.rs Remove file I/O from tests that don't need it (#11182) 2023-11-29 23:21:34 +01:00
zip.rs Avoid taking unnecessary ownership of intermediates (#12740) 2024-05-04 00:53:15 +00:00