Mirrors/nushell
2
0
Fork 0
mirror of https://github.com/nushell/nushell synced 2024-12-27 05:23:11 +00:00
Code Issues Projects Releases Packages Wiki Activity

Fix run_external::expand_glob() to return paths that are PWD-relative but reflect the original intent (#13028)

Browse source 
# Description

Fix #13021

This changes the `expand_glob()` function to use
`nu_engine::glob_from()` so that absolute paths are actually preserved,
rather than being made relative to the provided parent. This preserves
the intent of whoever wrote the original path/glob, and also makes it so
that tilde always produces absolute paths.

I also made `expand_glob()` handle Ctrl-C so that it can be interrupted.

cc @YizhePKU

# Tests + Formatting
No additional tests here... but that might be a good idea.
This commit is contained in:
Devyn Cairns 2024-06-03 00:38:55 -07:00 committed by GitHub
parent b50903cf58
commit be8c1dc006
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
1 changed files with 99 additions and 44 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

143
crates/nu-command/src/system/run_external.rs
View file

@ -4,7 +4,7 @@ use nu_protocol::{
ast::{Expr, Expression},
did_you_mean,
process::ChildProcess,
ByteStream, OutDest,
ByteStream, NuGlob, OutDest,
};
use nu_system::ForegroundChild;
use nu_utils::IgnoreCaseExt;
@ -13,7 +13,7 @@ use std::{
io::Write,
path::{Path, PathBuf},
process::Stdio,
sync::Arc,
sync::{atomic::AtomicBool, Arc},
thread,
};
@ -155,6 +155,9 @@ impl Command for External {
}
};
// Log the command we're about to run in case it's useful for debugging purposes.
log::trace!("run-external spawning: {command:?}");
// Spawn the child process. On Unix, also put the child process to
// foreground if we're in an interactive session.
#[cfg(windows)]
@ -232,6 +235,7 @@ pub fn eval_arguments_from_call(
stack: &mut Stack,
call: &Call,
) -> Result<Vec<Spanned<String>>, ShellError> {
let ctrlc = &engine_state.ctrlc;
let cwd = engine_state.cwd(Some(stack))?;
let mut args: Vec<Spanned<String>> = vec![];
for (expr, spread) in call.rest_iter(1) {
@ -240,7 +244,7 @@ pub fn eval_arguments_from_call(
// glob-expansion, and inner-quotes-removal, in that order.
for arg in eval_argument(engine_state, stack, expr, spread)? {
let tilde_expanded = expand_tilde(&arg);
for glob_expanded in expand_glob(&tilde_expanded, &cwd, expr.span)? {
for glob_expanded in expand_glob(&tilde_expanded, &cwd, expr.span, ctrlc)? {
let inner_quotes_removed = remove_inner_quotes(&glob_expanded);
args.push(inner_quotes_removed.into_owned().into_spanned(expr.span));
}
@ -321,37 +325,75 @@ fn expand_tilde(arg: &str) -> String {
///
/// Note: This matches the default behavior of Bash, but is known to be
/// error-prone. We might want to change this behavior in the future.
fn expand_glob(arg: &str, cwd: &Path, span: Span) -> Result<Vec<String>, ShellError> {
let Ok(paths) = nu_glob::glob_with_parent(arg, nu_glob::MatchOptions::default(), cwd) else {
fn expand_glob(
arg: &str,
cwd: &Path,
span: Span,
interrupt: &Option<Arc<AtomicBool>>,
) -> Result<Vec<String>, ShellError> {
const GLOB_CHARS: &[char] = &['*', '?', '['];
// Don't expand something that doesn't include the GLOB_CHARS
if !arg.contains(GLOB_CHARS) {
return Ok(vec![arg.into()]);
}
// We must use `nu_engine::glob_from` here, in order to ensure we get paths from the correct
// dir
let glob = NuGlob::Expand(arg.to_owned()).into_spanned(span);
let Ok((_prefix, paths)) = nu_engine::glob_from(&glob, cwd, span, None) else {
// If an error occurred, return the original input
return Ok(vec![arg.into()]);
};
let mut result = vec![];
for path in paths {
let path = path.map_err(|err| ShellError::IOErrorSpanned {
msg: format!("{}: {:?}", err.path().display(), err.error()),
span,
})?;
// Strip PWD from the resulting paths if possible.
let path_stripped = if let Ok(remainder) = path.strip_prefix(cwd) {
// If stripping PWD results in an empty path, return `.` instead.
if remainder.components().next().is_none() {
Path::new(".")
} else {
remainder
// If the first component of the original `arg` string path was '.', that should be preserved
let relative_to_dot = Path::new(arg).starts_with(".");
let paths = paths
// Skip over glob failures. These are usually just inaccessible paths.
.flat_map(|path_result| match path_result {
Ok(path) => Some(path),
Err(err) => {
// But internally log them just in case we need to debug this.
log::warn!("Error in run_external::expand_glob(): {}", err);
None
}
} else {
&path
};
let path_string = path_stripped.to_string_lossy().to_string();
result.push(path_string);
}
})
// Make the paths relative to the cwd
.map(|path| {
path.strip_prefix(cwd)
.map(|path| path.to_owned())
.unwrap_or(path)
})
// Add './' to relative paths if the original pattern had it
.map(|path| {
if relative_to_dot && path.is_relative() {
Path::new(".").join(path)
} else {
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())
// Abandon if ctrl-c is pressed
.map(|path| {
if !nu_utils::ctrl_c::was_pressed(interrupt) {
Ok(path)
} else {
Err(ShellError::InterruptedByUser { span: Some(span) })
}
})
.collect::<Result<Vec<String>, ShellError>>()?;
if result.is_empty() {
result.push(arg.to_string());
if !paths.is_empty() {
Ok(paths)
} else {
// If we failed to match, return the original input
Ok(vec![arg.into()])
}
Ok(result)
}
/// Transforms `--option="value"` into `--option=value`. `value` can be quoted
@ -607,6 +649,7 @@ fn escape_cmd_argument(arg: &Spanned<String>) -> Result<Cow<'_, str>, ShellError
mod test {
use super::*;
use nu_protocol::ast::ListItem;
use nu_test_support::{fs::Stub, playground::Playground};
#[test]
fn test_remove_quotes() {
@ -669,26 +712,38 @@ mod test {
#[test]
fn test_expand_glob() {
let tempdir = tempfile::tempdir().unwrap();
let cwd = tempdir.path();
std::fs::File::create(cwd.join("a.txt")).unwrap();
std::fs::File::create(cwd.join("b.txt")).unwrap();
Playground::setup("test_expand_glob", |dirs, play| {
play.with_files(&[Stub::EmptyFile("a.txt"), Stub::EmptyFile("b.txt")]);
let actual = expand_glob("*.txt", cwd, Span::unknown()).unwrap();
let expected = &["a.txt", "b.txt"];
assert_eq!(actual, expected);
let cwd = dirs.test();
let actual = expand_glob("'*.txt'", cwd, Span::unknown()).unwrap();
let expected = &["'*.txt'"];
assert_eq!(actual, expected);
let actual = expand_glob("*.txt", cwd, Span::unknown(), &None).unwrap();
let expected = &["a.txt", "b.txt"];
assert_eq!(actual, expected);
let actual = expand_glob(cwd.to_str().unwrap(), cwd, Span::unknown()).unwrap();
let expected = &["."];
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(),
];
assert_eq!(actual, expected);
let actual = expand_glob("[*.txt", cwd, Span::unknown()).unwrap();
let expected = &["[*.txt"];
assert_eq!(actual, expected);
let actual = expand_glob("'*.txt'", cwd, Span::unknown(), &None).unwrap();
let expected = &["'*.txt'"];
assert_eq!(actual, expected);
let actual = expand_glob(".", cwd, Span::unknown(), &None).unwrap();
let expected = &["."];
assert_eq!(actual, expected);
let actual = expand_glob("./a.txt", cwd, Span::unknown(), &None).unwrap();
let expected = &["./a.txt"];
assert_eq!(actual, expected);
let actual = expand_glob("[*.txt", cwd, Span::unknown(), &None).unwrap();
let expected = &["[*.txt"];
assert_eq!(actual, expected);
})
}
#[test]