holm/nushell
1
1
Fork 0
forked from extern/nushell
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Unify glob behavior on open, rm, cp-old, mv, umv, cp and du commands (#11621)

Browse Source 
# Description
This pr is a follow up to
[#11569](https://github.com/nushell/nushell/pull/11569#issuecomment-1902279587)
> Revert the logic in https://github.com/nushell/nushell/pull/10694 and
apply the logic in this pr to mv, cp, rv will require a larger change, I
need to think how to achieve the bahavior

And sorry @bobhy for reverting some of your changes.

This pr is going to unify glob behavior on the given commands:
* open
* rm
* cp-old
* mv
* umv
* cp
* du

So they have the same behavior to `ls`, which is:
If given parameter is quoted by single quote(`'`) or double quote(`"`),
don't auto-expand the glob pattern. If not quoted, auto-expand the glob
pattern.

Fixes: #9558  Fixes: #10211 Fixes: #9310 Fixes: #10364 

# TODO
But there is one thing remains: if we give a variable to the command, it
will always auto-expand the glob pattern, e.g:
```nushell
let path = "a[123]b"
rm $path
```
I don't think it's expected. But I also think user might want to
auto-expand the glob pattern in variables.

So I'll introduce a new command called `glob escape`, then if user
doesn't want to auto-expand the glob pattern, he can just do this: `rm
($path | glob escape)`

# User-Facing Changes
<!-- List of all changes that impact the user experience here. This
helps us keep track of breaking changes. -->

# Tests + Formatting
Done

# After Submitting
<!-- If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.
-->

## NOTE
This pr changes the semantic of `GlobPattern`, before this pr, it will
`expand path` after evaluated, this makes `nu_engine::glob_from` have no
chance to glob things right if a path contains glob pattern.

e.g: [#9310
](https://github.com/nushell/nushell/issues/9310#issuecomment-1886824030)
#10211

I think changing the semantic is fine, because it makes glob works if
path contains something like '*'.

It maybe a breaking change if a custom command's argument are annotated
by `: glob`.
This commit is contained in:
WindSoilder
2024-01-26 21:57:35 +08:00
committed by GitHub
parent e43d893ea3
commit d646903161
28 changed files with 318 additions and 431 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
crates/nu-command/src/filesystem/ls.rs
View File

@ -41,7 +41,7 @@ impl Command for Ls {
.input_output_types(vec![(Type::Nothing, Type::Table(vec![]))])
// LsGlobPattern is similar to string, it won't auto-expand
// and we use it to track if the user input is quoted.
.optional("pattern", SyntaxShape::LsGlobPattern, "The glob pattern to use.")
.optional("pattern", SyntaxShape::GlobPattern, "The glob pattern to use.")
.switch("all", "Show hidden files", Some('a'))
.switch(
"long",
@ -165,7 +165,8 @@ impl Command for Ls {
};
let hidden_dir_specified = is_hidden_dir(&path);
// when it's quoted, we need to escape our glob pattern
// when it's quoted, we need to escape our glob pattern(but without the last extra
// start which may be added under given directory)
// so we can do ls for a file or directory like `a[123]b`
let path = if quoted {
let p = path.display().to_string();
@ -185,7 +186,8 @@ impl Command for Ls {
};
let glob_path = Spanned {
item: path.clone(),
// It needs to be un-quoted, the relative logic is handled previously
item: NuPath::UnQuoted(path.clone()),
span: p_tag,
};

16
crates/nu-command/src/filesystem/mv.rs
View File

@ -1,14 +1,13 @@
use std::path::{Path, PathBuf};
use super::util::try_interaction;
use nu_cmd_base::arg_glob;
use nu_engine::env::current_dir;
use nu_engine::CallExt;
use nu_protocol::ast::Call;
use nu_protocol::engine::{Command, EngineState, Stack};
use nu_protocol::{
Category, Example, IntoInterruptiblePipelineData, PipelineData, ShellError, Signature, Span,
Spanned, SyntaxShape, Type, Value,
Category, Example, IntoInterruptiblePipelineData, NuPath, PipelineData, ShellError, Signature,
Span, Spanned, SyntaxShape, Type, Value,
};
#[derive(Clone)]
@ -63,7 +62,8 @@ impl Command for Mv {
_input: PipelineData,
) -> Result<PipelineData, ShellError> {
// TODO: handle invalid directory or insufficient permissions when moving
let spanned_source: Spanned<String> = call.req(engine_state, stack, 0)?;
let mut spanned_source: Spanned<NuPath> = call.req(engine_state, stack, 0)?;
spanned_source.item = spanned_source.item.strip_ansi_string_unlikely();
let spanned_destination: Spanned<String> = call.req(engine_state, stack, 1)?;
let verbose = call.has_flag(engine_state, stack, "verbose")?;
let interactive = call.has_flag(engine_state, stack, "interactive")?;
@ -73,11 +73,11 @@ impl Command for Mv {
let ctrlc = engine_state.ctrlc.clone();
let path = current_dir(engine_state, stack)?;
let source = path.join(spanned_source.item.as_str());
let destination = path.join(spanned_destination.item.as_str());
let mut sources =
arg_glob(&spanned_source, &path).map_or_else(|_| Vec::new(), Iterator::collect);
let mut sources = nu_engine::glob_from(&spanned_source, &path, call.head, None)
.map(|p| p.1)
.map_or_else(|_| Vec::new(), Iterator::collect);
if sources.is_empty() {
return Err(ShellError::FileNotFound {
@ -94,7 +94,7 @@ impl Command for Mv {
//
// Second, the destination doesn't exist, so we can only rename a single source. Otherwise
// it's an error.
let source = path.join(spanned_source.item.as_ref());
if destination.exists() && !force && !destination.is_dir() && !source.is_dir() {
return Err(ShellError::GenericError {
error: "Destination file already exists".into(),

29
crates/nu-command/src/filesystem/open.rs
View File

@ -4,8 +4,8 @@ use nu_protocol::ast::Call;
use nu_protocol::engine::{Command, EngineState, Stack};
use nu_protocol::util::BufferedReader;
use nu_protocol::{
Category, DataSource, Example, IntoInterruptiblePipelineData, PipelineData, PipelineMetadata,
RawStream, ShellError, Signature, Spanned, SyntaxShape, Type, Value,
Category, DataSource, Example, IntoInterruptiblePipelineData, NuPath, PipelineData,
PipelineMetadata, RawStream, ShellError, Signature, Spanned, SyntaxShape, Type, Value,
};
use std::io::BufReader;
@ -42,10 +42,10 @@ impl Command for Open {
fn signature(&self) -> nu_protocol::Signature {
Signature::build("open")
.input_output_types(vec![(Type::Nothing, Type::Any), (Type::String, Type::Any)])
.optional("filename", SyntaxShape::Filepath, "The filename to use.")
.optional("filename", SyntaxShape::GlobPattern, "The filename to use.")
.rest(
"filenames",
SyntaxShape::Filepath,
SyntaxShape::GlobPattern,
"Optional additional files to open.",
)
.switch("raw", "open file as raw binary", Some('r'))
@ -63,8 +63,8 @@ impl Command for Open {
let call_span = call.head;
let ctrlc = engine_state.ctrlc.clone();
let cwd = current_dir(engine_state, stack)?;
let req_path = call.opt::<Spanned<String>>(engine_state, stack, 0)?;
let mut path_params = call.rest::<Spanned<String>>(engine_state, stack, 1)?;
let req_path = call.opt::<Spanned<NuPath>>(engine_state, stack, 0)?;
let mut path_params = call.rest::<Spanned<NuPath>>(engine_state, stack, 1)?;
// FIXME: JT: what is this doing here?
@ -87,19 +87,20 @@ impl Command for Open {
}
};
path_params.insert(0, filename);
path_params.insert(
0,
Spanned {
item: NuPath::UnQuoted(filename.item),
span: filename.span,
},
);
}
let mut output = vec![];
for path in path_params.into_iter() {
for mut path in path_params.into_iter() {
//FIXME: `open` should not have to do this
let path = {
Spanned {
item: nu_utils::strip_ansi_string_unlikely(path.item),
span: path.span,
}
};
path.item = path.item.strip_ansi_string_unlikely();
let arg_span = path.span;
// let path_no_whitespace = &path.item.trim_end_matches(|x| matches!(x, '\x09'..='\x0d'));

37
crates/nu-command/src/filesystem/rm.rs
View File

@ -7,14 +7,15 @@ use std::path::PathBuf;
use super::util::try_interaction;
use nu_cmd_base::arg_glob_leading_dot;
use nu_engine::env::current_dir;
use nu_engine::CallExt;
use nu_glob::MatchOptions;
use nu_path::expand_path_with;
use nu_protocol::ast::Call;
use nu_protocol::engine::{Command, EngineState, Stack};
use nu_protocol::{
Category, Example, IntoInterruptiblePipelineData, PipelineData, ShellError, Signature, Span,
Spanned, SyntaxShape, Type, Value,
Category, Example, IntoInterruptiblePipelineData, NuPath, PipelineData, ShellError, Signature,
Span, Spanned, SyntaxShape, Type, Value,
};
const TRASH_SUPPORTED: bool = cfg!(all(
@ -134,7 +135,7 @@ fn rm(
let ctrlc = engine_state.ctrlc.clone();
let mut targets: Vec<Spanned<String>> = call.rest(engine_state, stack, 0)?;
let mut targets: Vec<Spanned<NuPath>> = call.rest(engine_state, stack, 0)?;
let mut unique_argument_check = None;
@ -157,12 +158,19 @@ fn rm(
for (idx, path) in targets.clone().into_iter().enumerate() {
if let Some(ref home) = home {
if &path.item == home {
if expand_path_with(path.item.as_ref(), &currentdir_path)
.to_string_lossy()
.as_ref()
== home.as_str()
{
unique_argument_check = Some(path.span);
}
}
let corrected_path = Spanned {
item: nu_utils::strip_ansi_string_unlikely(path.item),
item: match path.item {
NuPath::Quoted(s) => NuPath::Quoted(nu_utils::strip_ansi_string_unlikely(s)),
NuPath::UnQuoted(s) => NuPath::UnQuoted(nu_utils::strip_ansi_string_unlikely(s)),
},
span: path.span,
};
let _ = std::mem::replace(&mut targets[idx], corrected_path);
@ -233,7 +241,8 @@ fn rm(
let mut all_targets: HashMap<PathBuf, Span> = HashMap::new();
for target in targets {
if currentdir_path.to_string_lossy() == target.item
let path = expand_path_with(target.item.as_ref(), &currentdir_path);
if currentdir_path.to_string_lossy() == path.to_string_lossy()
|| currentdir_path.starts_with(format!("{}{}", target.item, std::path::MAIN_SEPARATOR))
{
return Err(ShellError::GenericError {
@ -245,10 +254,18 @@ fn rm(
});
}
let path = currentdir_path.join(&target.item);
match arg_glob_leading_dot(&target, &currentdir_path) {
// let path = currentdir_path.join(target.item.as_ref());
match nu_engine::glob_from(
&target,
&currentdir_path,
call.head,
Some(MatchOptions {
require_literal_leading_dot: true,
..Default::default()
}),
) {
Ok(files) => {
for file in files {
for file in files.1 {
match file {
Ok(f) => {
if !target_exists {

40
crates/nu-command/src/filesystem/ucp.rs
View File

@ -1,6 +1,5 @@
use nu_cmd_base::arg_glob;
use nu_engine::{current_dir, CallExt};
use nu_glob::GlobResult;
use nu_protocol::NuPath;
use nu_protocol::{
ast::Call,
engine::{Command, EngineState, Stack},
@ -62,7 +61,7 @@ impl Command for UCp {
None
)
.switch("debug", "explain how a file is copied. Implies -v", None)
.rest("paths", SyntaxShape::Filepath, "Copy SRC file/s to DEST.")
.rest("paths", SyntaxShape::GlobPattern, "Copy SRC file/s to DEST.")
.allow_variants_without_examples(true)
.category(Category::FileSystem)
}
@ -146,14 +145,7 @@ impl Command for UCp {
let reflink_mode = uu_cp::ReflinkMode::Auto;
#[cfg(not(any(target_os = "linux", target_os = "android", target_os = "macos")))]
let reflink_mode = uu_cp::ReflinkMode::Never;
let paths: Vec<Spanned<String>> = call.rest(engine_state, stack, 0)?;
let mut paths: Vec<Spanned<String>> = paths
.into_iter()
.map(|p| Spanned {
item: nu_utils::strip_ansi_string_unlikely(p.item),
span: p.span,
})
.collect();
let mut paths: Vec<Spanned<NuPath>> = call.rest(engine_state, stack, 0)?;
if paths.is_empty() {
return Err(ShellError::GenericError {
error: "Missing file operand".into(),
@ -167,15 +159,20 @@ impl Command for UCp {
if paths.len() == 1 {
return Err(ShellError::GenericError {
error: "Missing destination path".into(),
msg: format!("Missing destination path operand after {}", paths[0].item),
msg: format!(
"Missing destination path operand after {}",
paths[0].item.as_ref()
),
span: Some(paths[0].span),
help: None,
inner: vec![],
});
}
let target = paths.pop().expect("Should not be reached?");
let target_path = PathBuf::from(&target.item);
if target.item.ends_with(PATH_SEPARATOR) && !target_path.is_dir() {
let target_path = PathBuf::from(&nu_utils::strip_ansi_string_unlikely(
target.item.to_string(),
));
if target.item.as_ref().ends_with(PATH_SEPARATOR) && !target_path.is_dir() {
return Err(ShellError::GenericError {
error: "is not a directory".into(),
msg: "is not a directory".into(),
@ -190,8 +187,12 @@ impl Command for UCp {
let cwd = current_dir(engine_state, stack)?;
let mut sources: Vec<PathBuf> = Vec::new();
for p in paths {
let exp_files = arg_glob(&p, &cwd)?.collect::<Vec<GlobResult>>();
for mut p in paths {
p.item = p.item.strip_ansi_string_unlikely();
let exp_files: Vec<Result<PathBuf, ShellError>> =
nu_engine::glob_from(&p, &cwd, call.head, None)
.map(|f| f.1)?
.collect();
if exp_files.is_empty() {
return Err(ShellError::FileNotFound { span: p.span });
};
@ -212,12 +213,7 @@ impl Command for UCp {
};
app_vals.push(path)
}
Err(e) => {
return Err(ShellError::ErrorExpandingGlob {
msg: format!("error {} in path {}", e.error(), e.path().display()),
span: p.span,
});
}
Err(e) => return Err(e),
}
}
sources.append(&mut app_vals);

53
crates/nu-command/src/filesystem/umv.rs
View File

@ -1,12 +1,10 @@
use nu_cmd_base::arg_glob;
use nu_engine::current_dir;
use nu_engine::CallExt;
use nu_glob::GlobResult;
use nu_path::{expand_path_with, expand_to_real_path};
use nu_protocol::ast::Call;
use nu_protocol::engine::{Command, EngineState, Stack};
use nu_protocol::{
Category, Example, PipelineData, ShellError, Signature, Spanned, SyntaxShape, Type,
Category, Example, NuPath, PipelineData, ShellError, Signature, Spanned, SyntaxShape, Type,
};
use std::ffi::OsString;
use std::path::PathBuf;
@ -58,7 +56,7 @@ impl Command for UMv {
.switch("no-clobber", "do not overwrite an existing file", Some('n'))
.rest(
"paths",
SyntaxShape::Filepath,
SyntaxShape::GlobPattern,
"Rename SRC to DST, or move SRC to DIR.",
)
.allow_variants_without_examples(true)
@ -84,14 +82,8 @@ impl Command for UMv {
uu_mv::OverwriteMode::Force
};
let paths: Vec<Spanned<String>> = call.rest(engine_state, stack, 0)?;
let paths: Vec<Spanned<String>> = paths
.into_iter()
.map(|p| Spanned {
item: nu_utils::strip_ansi_string_unlikely(p.item),
span: p.span,
})
.collect();
let cwd = current_dir(engine_state, stack)?;
let mut paths: Vec<Spanned<NuPath>> = call.rest(engine_state, stack, 0)?;
if paths.is_empty() {
return Err(ShellError::GenericError {
error: "Missing file operand".into(),
@ -102,9 +94,13 @@ impl Command for UMv {
});
}
if paths.len() == 1 {
// expand path for better error message
return Err(ShellError::GenericError {
error: "Missing destination path".into(),
msg: format!("Missing destination path operand after {}", paths[0].item),
msg: format!(
"Missing destination path operand after {}",
expand_path_with(paths[0].item.as_ref(), cwd).to_string_lossy()
),
span: Some(paths[0].span),
help: None,
inner: Vec::new(),
@ -112,11 +108,18 @@ impl Command for UMv {
}
// Do not glob target
let sources = &paths[..paths.len() - 1];
let cwd = current_dir(engine_state, stack)?;
let spanned_target = paths.pop().ok_or(ShellError::NushellFailedSpanned {
msg: "Missing file operand".into(),
label: "Missing file operand".into(),
span: call.head,
})?;
let mut files: Vec<PathBuf> = Vec::new();
for p in sources {
let exp_files = arg_glob(p, &cwd)?.collect::<Vec<GlobResult>>();
for mut p in paths {
p.item = p.item.strip_ansi_string_unlikely();
let exp_files: Vec<Result<PathBuf, ShellError>> =
nu_engine::glob_from(&p, &cwd, call.head, None)
.map(|f| f.1)?
.collect();
if exp_files.is_empty() {
return Err(ShellError::FileNotFound { span: p.span });
};
@ -126,12 +129,7 @@ impl Command for UMv {
Ok(path) => {
app_vals.push(path);
}
Err(e) => {
return Err(ShellError::ErrorExpandingGlob {
msg: format!("error {} in path {}", e.error(), e.path().display()),
span: p.span,
});
}
Err(e) => return Err(e),
}
}
files.append(&mut app_vals);
@ -146,12 +144,9 @@ impl Command for UMv {
}
// Add back the target after globbing
let spanned_target = paths.last().ok_or(ShellError::NushellFailedSpanned {
msg: "Missing file operand".into(),
label: "Missing file operand".into(),
span: call.head,
})?;
let expanded_target = expand_to_real_path(spanned_target.item.clone());
let expanded_target = expand_to_real_path(nu_utils::strip_ansi_string_unlikely(
spanned_target.item.to_string(),
));
let abs_target_path = expand_path_with(expanded_target, &cwd);
files.push(abs_target_path.clone());
let files = files

1
crates/nu-command/src/formats/from/nuon.rs
View File

@ -243,7 +243,6 @@ fn convert_to_value(
span: expr.span,
}),
Expr::GlobPattern(val, _) => Ok(Value::string(val, span)),
Expr::LsGlobPattern(val, _) => Ok(Value::string(val, span)),
Expr::ImportPattern(..) => Err(ShellError::OutsideSpannedLabeledError {
src: original_text.to_string(),
error: "Error when loading".into(),

34
crates/nu-command/src/platform/du.rs
View File

@ -1,12 +1,11 @@
use crate::{DirBuilder, DirInfo, FileInfo};
use nu_cmd_base::arg_glob;
use nu_engine::{current_dir, CallExt};
use nu_glob::{GlobError, Pattern};
use nu_glob::Pattern;
use nu_protocol::{
ast::Call,
engine::{Command, EngineState, Stack},
Category, Example, IntoInterruptiblePipelineData, PipelineData, ShellError, Signature, Span,
Spanned, SyntaxShape, Type, Value,
Category, Example, IntoInterruptiblePipelineData, NuPath, PipelineData, ShellError, Signature,
Span, Spanned, SyntaxShape, Type, Value,
};
use serde::Deserialize;
@ -15,7 +14,7 @@ pub struct Du;
#[derive(Deserialize, Clone, Debug)]
pub struct DuArgs {
path: Option<Spanned<String>>,
path: Option<Spanned<NuPath>>,
all: bool,
deref: bool,
exclude: Option<Spanned<String>>,
@ -116,28 +115,26 @@ impl Command for Du {
let include_files = args.all;
let mut paths = match args.path {
Some(p) => arg_glob(&p, &current_dir)?,
Some(p) => nu_engine::glob_from(&p, &current_dir, call.head, None),
// The * pattern should never fail.
None => arg_glob(
None => nu_engine::glob_from(
&Spanned {
item: "*".into(),
item: NuPath::UnQuoted("*".into()),
span: Span::unknown(),
},
&current_dir,
)?,
call.head,
None,
),
}
.map(|f| f.1)?
.filter(move |p| {
if include_files {
true
} else {
match p {
Ok(f) if f.is_dir() => true,
Err(e) if e.path().is_dir() => true,
_ => false,
}
matches!(p, Ok(f) if f.is_dir())
}
})
.map(|v| v.map_err(glob_err_into));
});
let all = args.all;
let deref = args.deref;
@ -182,11 +179,6 @@ impl Command for Du {
}
}
fn glob_err_into(e: GlobError) -> ShellError {
let e = e.into_error();
ShellError::from(e)
}
#[cfg(test)]
mod tests {
use super::Du;

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

@ -2,6 +2,7 @@ use nu_cmd_base::hook::eval_hook;
use nu_engine::env_to_strings;
use nu_engine::eval_expression;
use nu_engine::CallExt;
use nu_protocol::NuPath;
use nu_protocol::{
ast::{Call, Expr},
did_you_mean,
@ -735,7 +736,12 @@ fn trim_expand_and_apply_arg(
}
let cwd = PathBuf::from(cwd);
if arg.item.contains('*') && run_glob_expansion {
if let Ok((prefix, matches)) = nu_engine::glob_from(&arg, &cwd, arg.span, None) {
// we need to run glob expansion, so it's unquoted.
let path = Spanned {
item: NuPath::UnQuoted(arg.item.clone()),
span: arg.span,
};
if let Ok((prefix, matches)) = nu_engine::glob_from(&path, &cwd, arg.span, None) {
let matches: Vec<_> = matches.collect();
// FIXME: do we want to special-case this further? We might accidentally expand when they don't