Public/nushell
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

more tests, and more fixes for tests

Browse Source
This commit is contained in:
Devyn Cairns 2024-06-06 16:29:04 -07:00
parent 624cadae52
commit fdaf393beb
No known key found for this signature in database
5 changed files with 123 additions and 88 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

45
crates/nu-command/src/system/run_external.rs
View File

@ -1,5 +1,6 @@
use nu_cmd_base::hook::eval_hook;
use nu_engine::{command_prelude::*, env_to_strings, get_eval_expression};
use nu_path::{dots::expand_ndots, expand_tilde};
use nu_protocol::{
ast::{Expr, Expression},
did_you_mean,
@ -9,6 +10,7 @@ use nu_protocol::{
use nu_system::ForegroundChild;
use nu_utils::IgnoreCaseExt;
use std::{
ffi::{OsStr, OsString},
io::Write,
path::{Path, PathBuf},
process::Stdio,
@ -76,7 +78,7 @@ impl Command for External {
let expanded_name = if is_bare_string(name_expr) {
expand_tilde(&name.item)
} else {
name.item.clone()
Path::new(&name.item).to_owned()
};
// Determine the PATH to be used and then use `which` to find it - though this has no
@ -228,10 +230,10 @@ pub fn eval_arguments_from_call(
engine_state: &EngineState,
stack: &mut Stack,
call: &Call,
) -> Result<Vec<Spanned<String>>, ShellError> {
) -> Result<Vec<Spanned<OsString>>, ShellError> {
let ctrlc = &engine_state.ctrlc;
let cwd = engine_state.cwd(Some(stack))?;
let mut args: Vec<Spanned<String>> = vec![];
let mut args: Vec<Spanned<OsString>> = vec![];
for (expr, spread) in call.rest_iter(1) {
if is_bare_string(expr) {
// If `expr` is a bare string, perform glob expansion.
@ -244,7 +246,7 @@ pub fn eval_arguments_from_call(
}
} else {
for arg in eval_argument(engine_state, stack, expr, spread)? {
args.push(arg.into_spanned(expr.span));
args.push(OsString::from(arg).into_spanned(expr.span));
}
}
}
@ -306,12 +308,6 @@ fn is_bare_string(expr: &Expression) -> bool {
)
}
/// Performs tilde expansion on `arg`. Returns the original string if `arg`
/// doesn't start with tilde.
fn expand_tilde(arg: &str) -> String {
nu_path::expand_tilde(arg).to_string_lossy().to_string()
}
/// Performs glob expansion on `arg`. If the expansion found no matches or the pattern
/// is not a valid glob, then this returns the original string as the expansion result.
///
@ -322,12 +318,14 @@ fn expand_glob(
cwd: &Path,
span: Span,
interrupt: &Option<Arc<AtomicBool>>,
) -> Result<Vec<String>, ShellError> {
) -> Result<Vec<OsString>, ShellError> {
const GLOB_CHARS: &[char] = &['*', '?', '['];
// Don't expand something that doesn't include the GLOB_CHARS
// For an argument that doesn't include the GLOB_CHARS, just do the `expand_tilde`
// and `expand_ndots` expansion
if !arg.contains(GLOB_CHARS) {
return Ok(vec![arg.into()]);
let path = expand_ndots(expand_tilde(arg));
return Ok(vec![path.into()]);
}
// We must use `nu_engine::glob_from` here, in order to ensure we get paths from the correct
@ -365,11 +363,7 @@ fn expand_glob(
path
}
})
// Convert the paths returned to UTF-8 strings.
//
// FIXME: this fails to return the correct results for non-UTF-8 paths, but we don't support
// those in Nushell yet.
.map(|path| path.to_string_lossy().into_owned())
.map(OsString::from)
// Abandon if ctrl-c is pressed
.map(|path| {
if !nu_utils::ctrl_c::was_pressed(interrupt) {
@ -378,7 +372,7 @@ fn expand_glob(
Err(ShellError::InterruptedByUser { span: Some(span) })
}
})
.collect::<Result<Vec<String>, ShellError>>()?;
.collect::<Result<Vec<OsString>, ShellError>>()?;
if !paths.is_empty() {
Ok(paths)
@ -556,7 +550,7 @@ pub fn command_not_found(
/// Note: the `which.rs` crate always uses PATHEXT from the environment. As
/// such, changing PATHEXT within Nushell doesn't work without updating the
/// actual environment of the Nushell process.
pub fn which(name: &str, paths: &str, cwd: &Path) -> Option<PathBuf> {
pub fn which(name: impl AsRef<OsStr>, paths: &str, cwd: &Path) -> Option<PathBuf> {
#[cfg(windows)]
let paths = format!("{};{}", cwd.display(), paths);
which::which_in(name, Some(paths), cwd).ok()
@ -678,9 +672,9 @@ mod test {
assert_eq!(actual, expected);
let actual = expand_glob("./*.txt", cwd, Span::unknown(), &None).unwrap();
let expected = vec![
Path::new(".").join("a.txt").to_string_lossy().into_owned(),
Path::new(".").join("b.txt").to_string_lossy().into_owned(),
let expected: Vec<OsString> = vec![
Path::new(".").join("a.txt").into(),
Path::new(".").join("b.txt").into(),
];
assert_eq!(actual, expected);
@ -699,6 +693,11 @@ mod test {
let actual = expand_glob("[*.txt", cwd, Span::unknown(), &None).unwrap();
let expected = &["[*.txt"];
assert_eq!(actual, expected);
let actual = expand_glob("~/foo.txt", cwd, Span::unknown(), &None).unwrap();
let home = dirs_next::home_dir().expect("failed to get home dir");
let expected: Vec<OsString> = vec![home.join("foo.txt").into()];
assert_eq!(actual, expected);
})
}

38
crates/nu-command/tests/commands/run_external.rs
View File

@ -221,6 +221,44 @@ fn external_command_escape_args() {
})
}
#[test]
#[cfg_attr(
not(target_os = "linux"),
ignore = "only runs on Linux, where controlling the HOME var is reliable"
)]
fn external_command_expand_tilde() {
Playground::setup("external command expand tilde", |dirs, _| {
// Make a copy of the nu executable that we can use
let mut src = std::fs::File::open(nu_test_support::fs::binaries().join("nu"))
.expect("failed to open nu");
let mut dst = std::fs::File::create_new(dirs.test().join("test_nu"))
.expect("failed to create test_nu file");
std::io::copy(&mut src, &mut dst).expect("failed to copy data for nu binary");
// Make test_nu have the same permissions so that it's executable
dst.set_permissions(
src.metadata()
.expect("failed to get nu metadata")
.permissions(),
)
.expect("failed to set permissions on test_nu");
// Close the files
drop(dst);
drop(src);
let actual = nu!(
envs: vec![
("HOME".to_string(), dirs.test().to_string_lossy().into_owned()),
],
r#"
^~/test_nu --testbin cococo hello
"#
);
assert_eq!(actual.out, "hello");
})
}
#[test]
fn external_arg_expand_tilde() {
Playground::setup("external arg expand tilde", |dirs, _| {

11
crates/nu-parser/src/parser.rs
View File

@ -237,7 +237,7 @@ fn parse_external_string(working_set: &mut StateWorkingSet, span: Span) -> Expre
parse_raw_string(working_set, span)
} else if contents
.iter()
.any(|b| matches!(b, b'"' | b'\'' | b'`' | b'(' | b')'))
.any(|b| matches!(b, b'"' | b'\'' | b'(' | b')'))
{
enum State {
Bare {
@ -250,6 +250,11 @@ fn parse_external_string(working_set: &mut StateWorkingSet, span: Span) -> Expre
depth: i32,
},
}
// Find the spans of parts of the string that can be parsed as their own strings for
// concatenation.
//
// By passing each of these parts to `parse_string()`, we can eliminate the quotes and also
// handle string interpolation.
let make_span = |from: usize, index: usize| Span {
start: span.start + from,
end: span.start + index,
@ -259,7 +264,7 @@ fn parse_external_string(working_set: &mut StateWorkingSet, span: Span) -> Expre
for (index, &ch) in contents.iter().enumerate() {
match &mut state {
State::Bare { from } => match ch {
b'"' | b'\'' | b'`' => {
b'"' | b'\'' => {
// Push bare string
if index != *from {
spans.push(make_span(*from, index));
@ -281,7 +286,7 @@ fn parse_external_string(working_set: &mut StateWorkingSet, span: Span) -> Expre
escaped,
depth,
} => match ch {
b'"' if !*escaped => {
ch if ch == *quote_char && !*escaped => {
// Count if there are more than `depth` quotes remaining
if contents[index..]
.iter()

111
crates/nu-parser/tests/test_parser.rs
View File

@ -713,6 +713,32 @@ pub fn parse_call_missing_req_flag() {
r"foo\external-call",
"bare word with backslash and caret"
)]
#[case("`foo external call`", "foo external call", "backtick quote")]
#[case(
"^`foo external call`",
"foo external call",
"backtick quote with caret"
)]
#[case(
"`foo/external call`",
"foo/external call",
"backtick quote with forward slash"
)]
#[case(
"^`foo/external call`",
"foo/external call",
"backtick quote with forward slash and caret"
)]
#[case(
r"`foo\external call`",
r"foo\external call",
"backtick quote with backslash"
)]
#[case(
r"^`foo\external call`",
r"foo\external call",
"backtick quote with backslash and caret"
)]
pub fn test_external_call_head_glob(
#[case] input: &str,
#[case] expected: &str,
@ -827,32 +853,6 @@ pub fn test_external_call_head_raw_string(
r#"foo\external call"#,
"double quote with backslash and caret"
)]
#[case("`foo external call`", "foo external call", "backtick quote")]
#[case(
"^`foo external call`",
"foo external call",
"backtick quote with caret"
)]
#[case(
"`foo/external call`",
"foo/external call",
"backtick quote with forward slash"
)]
#[case(
"^`foo/external call`",
"foo/external call",
"backtick quote with forward slash and caret"
)]
#[case(
r"`foo\external call`",
r"foo\external call",
"backtick quote with backslash"
)]
#[case(
r"^`foo\external call`",
r"foo\external call",
"backtick quote with backslash and caret"
)]
pub fn test_external_call_head_string(
#[case] input: &str,
#[case] expected: &str,
@ -948,6 +948,21 @@ pub fn test_external_call_head_interpolated_string(
r"foo\external-call",
"bare word with backslash"
)]
#[case(
"^foo `foo external call`",
"foo external call",
"backtick quote with caret"
)]
#[case(
"^foo `foo/external call`",
"foo/external call",
"backtick quote with forward slash"
)]
#[case(
r"^foo `foo\external call`",
r"foo\external call",
"backtick quote with backslash"
)]
pub fn test_external_call_arg_glob(#[case] input: &str, #[case] expected: &str, #[case] tag: &str) {
let engine_state = EngineState::new();
let mut working_set = StateWorkingSet::new(&engine_state);
@ -1054,55 +1069,27 @@ pub fn test_external_call_arg_raw_string(
}
#[rstest]
#[case(
"^foo 'foo external call'",
"foo external call",
"single quote with caret"
)]
#[case("^foo 'foo external call'", "foo external call", "single quote")]
#[case(
"^foo 'foo/external call'",
"foo/external call",
"single quote with forward slash and caret"
"single quote with forward slash"
)]
#[case(
r"^foo 'foo\external call'",
r"foo\external call",
"single quote with backslash and caret"
)]
#[case(
r#"^foo "foo external call""#,
r#"foo external call"#,
"double quote with caret"
"single quote with backslash"
)]
#[case(r#"^foo "foo external call""#, r#"foo external call"#, "double quote")]
#[case(
r#"^foo "foo/external call""#,
r#"foo/external call"#,
"double quote with forward slash and caret"
"double quote with forward slash"
)]
#[case(
r#"^foo "foo\\external call""#,
r#"foo\external call"#,
"double quote with backslash and caret"
)]
#[case(
"^foo `foo external call`",
"foo external call",
"backtick quote with caret"
)]
#[case(
"^foo `foo/external call`",
"foo/external call",
"backtick quote with forward slash"
)]
#[case(
"^foo `foo/external call`",
"foo/external call",
"backtick quote with forward slash and caret"
)]
#[case(
r"^foo `foo\external call`",
r"foo\external call",
"backtick quote with backslash and caret"
"double quote with backslash"
)]
pub fn test_external_call_arg_string(
#[case] input: &str,
@ -1138,11 +1125,11 @@ pub fn test_external_call_arg_string(
assert_eq!(expected, string, "{tag}: incorrect arg");
}
other => {
panic!("Unexpected expression in command arg position: {other:?}")
panic!("{tag}: Unexpected expression in command arg position: {other:?}")
}
},
other @ ExternalArgument::Spread(..) => {
panic!("Unexpected external spread argument in command arg position: {other:?}")
panic!("{tag}: Unexpected external spread argument in command arg position: {other:?}")
}
}
}

6
crates/nu-test-support/src/macros.rs
View File

@ -245,6 +245,7 @@ use tempfile::tempdir;
pub struct NuOpts {
pub cwd: Option<String>,
pub locale: Option<String>,
pub envs: Option<Vec<(String, String)>>,
pub collapse_output: Option<bool>,
}
@ -278,6 +279,11 @@ pub fn nu_run_test(opts: NuOpts, commands: impl AsRef<str>, with_std: bool) -> O
command
.env(nu_utils::locale::LOCALE_OVERRIDE_ENV_VAR, locale)
.env(NATIVE_PATH_ENV_VAR, paths_joined);
if let Some(envs) = opts.envs {
command.envs(envs);
}
// Ensure that the user's config doesn't interfere with the tests
command.arg("--no-config-file");
if !with_std {