extern/nushell
1
0
Fork 1
mirror of https://github.com/nushell/nushell.git synced 2025-07-07 18:07:02 +02:00
Code Issues Packages Projects Releases Wiki Activity

Allow filesystem commands to access files with glob metachars in name (#10694)

Browse Source 
(squashed version of #10557, clean commit history and review thread)

Fixes #10571, also potentially: #10364, #10211, #9558, #9310,


# Description
Changes processing of arguments to filesystem commands that are source
paths or globs.
Applies to `cp, cp-old, mv, rm, du` but not `ls` (because it uses a
different globbing interface) or `glob` (because it uses a different
globbing library).

The core of the change is to lookup the argument first as a file and
only glob if it is not. That way,
a path containing glob metacharacters can be referenced without glob
quoting, though it will have to be single quoted to avoid nushell
parsing.

Before: A file path that looks like a glob is not matched by the glob
specified as a (source) argument and takes some thinking about to
access. You might say the glob pattern shadows a file with the same
spelling.
```
> ls a*
╭───┬────────┬──────┬──────┬────────────────╮
│ # │  name  │ type │ size │    modified    │
├───┼────────┼──────┼──────┼────────────────┤
│ 0 │ a[bc]d │ file │  0 B │ 34 seconds ago │
│ 1 │ abd    │ file │  0 B │ now            │
│ 2 │ acd    │ file │  0 B │ now            │
╰───┴────────┴──────┴──────┴────────────────╯

> cp --verbose 'a[bc]d' dest
copied /home/bobhy/src/rust/work/r4/abd to /home/bobhy/src/rust/work/r4/dest/abd
copied /home/bobhy/src/rust/work/r4/acd to /home/bobhy/src/rust/work/r4/dest/acd

> ## Note -- a[bc]d *not* copied, and seemingly hard to access.
> cp --verbose 'a\[bc\]d' dest
Error:   × No matches found
   ╭─[entry #33:1:1]
 1 │ cp --verbose 'a\[bc\]d' dest
   ·              ─────┬────
   ·                   ╰── no matches found
   ╰────

> #.. but is accessible with enough glob quoting.
> cp --verbose 'a[[]bc[]]d' dest
copied /home/bobhy/src/rust/work/r4/a[bc]d to /home/bobhy/src/rust/work/r4/dest/a[bc]d
```
Before_2: if file has glob metachars but isn't a valid pattern, user
gets a confusing error:

```
> touch 'a[b'
> cp 'a[b' dest
Error:   × Pattern syntax error near position 30: invalid range pattern
   ╭─[entry #13:1:1]
 1 │ cp 'a[b' dest
   ·    ──┬──
   ·      ╰── invalid pattern
   ╰────
```

After: Args to cp, mv, etc. are tried first as literal files, and only
as globs if not found to be files.

```
> cp --verbose 'a[bc]d' dest
copied /home/bobhy/src/rust/work/r4/a[bc]d to /home/bobhy/src/rust/work/r4/dest/a[bc]d
> cp --verbose '[a][bc]d' dest
copied /home/bobhy/src/rust/work/r4/abd to /home/bobhy/src/rust/work/r4/dest/abd
copied /home/bobhy/src/rust/work/r4/acd to /home/bobhy/src/rust/work/r4/dest/acd
```
After_2: file with glob metachars but invalid pattern just works.
(though Windows does not allow file name to contain `*`.).

```
> cp --verbose 'a[b' dest
copied /home/bobhy/src/rust/work/r4/a[b to /home/bobhy/src/rust/work/r4/dest/a[b
```

So, with this fix, a file shadows a glob pattern with the same spelling.
If you have such a file and really want to use the glob pattern, you
will have to glob quote some of the characters in the pattern. I think
that's less confusing to the user: if ls shows a file with a weird name,
s/he'll still be able to copy, rename or delete it.

# User-Facing Changes
Could break some existing scripts. If user happened to have a file with
a globbish name but was using a glob pattern with the same spelling, the
new version will process the file and not expand the glob.

# Tests + Formatting
<!--
Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to
check that you're using the standard code style
- `cargo test --workspace` to check that all tests pass (on Windows make
sure to [enable developer
mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging))
- `cargo run -- -c "use std testing; testing run-tests --path
crates/nu-std"` to run the tests for the standard library

> **Note**
> from `nushell` you can also use the `toolkit` as follows
> ```bash
> use toolkit.nu # or use an `env_change` hook to activate it
automatically
> toolkit check pr
> ```
-->

# 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.
-->

---------

Co-authored-by: Darren Schroeder <343840+fdncred@users.noreply.github.com>
This commit is contained in:
Bob Hyman
2023-10-18 11:31:15 -07:00
committed by GitHub
parent 88a87158c2
commit 09b3dab35d
13 changed files with 400 additions and 145 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@ -4,6 +4,7 @@ use std::path::PathBuf;
use std::sync::atomic::AtomicBool;
use std::sync::Arc;
use nu_cmd_base::arg_glob;
use nu_engine::env::current_dir;
use nu_engine::CallExt;
use nu_path::{canonicalize_with, expand_path_with};
@ -19,13 +20,6 @@ use super::util::try_interaction;
use crate::filesystem::util::FileStructure;
use crate::progress_bar;
const GLOB_PARAMS: nu_glob::MatchOptions = nu_glob::MatchOptions {
case_sensitive: true,
require_literal_separator: false,
require_literal_leading_dot: false,
recursive_match_hidden_dir: true,
};
#[derive(Clone)]
pub struct Cp;
@ -81,12 +75,6 @@ impl Command for Cp {
_input: PipelineData,
) -> Result<PipelineData, ShellError> {
let src: Spanned<String> = call.req(engine_state, stack, 0)?;
let src = {
Spanned {
item: nu_utils::strip_ansi_string_unlikely(src.item),
span: src.span,
}
};
let dst: Spanned<String> = call.req(engine_state, stack, 1)?;
let recursive = call.has_flag("recursive");
let verbose = call.has_flag("verbose");
@ -95,7 +83,6 @@ impl Command for Cp {
let update_mode = call.has_flag("update");
let current_dir_path = current_dir(engine_state, stack)?;
let source = current_dir_path.join(src.item.as_str());
let destination = current_dir_path.join(dst.item.as_str());
let path_last_char = destination.as_os_str().to_string_lossy().chars().last();
@ -110,7 +97,7 @@ impl Command for Cp {
let span = call.head;
// Get an iterator with all the source files.
let sources: Vec<_> = match nu_glob::glob_with(&source.to_string_lossy(), GLOB_PARAMS) {
let sources: Vec<_> = match arg_glob(&src, &current_dir_path) {
Ok(files) => files.collect(),
Err(e) => {
return Err(ShellError::GenericError(
@ -124,13 +111,7 @@ impl Command for Cp {
};
if sources.is_empty() {
return Err(ShellError::GenericError(
"No matches found".into(),
"no matches found".into(),
Some(src.span),
None,
Vec::new(),
));
return Err(ShellError::FileNotFound(src.span));
}
if sources.len() > 1 && !destination.is_dir() {
@ -189,9 +170,7 @@ impl Command for Cp {
}
let res = if src == dst {
let message = format!(
"src {source:?} and dst {destination:?} are identical(not copied)"
);
let message = format!("src and dst identical: {:?} (not copied)", src);
return Err(ShellError::GenericError(
"Copy aborted".into(),

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

@ -1,6 +1,7 @@
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;
@ -10,13 +11,6 @@ use nu_protocol::{
Spanned, SyntaxShape, Type, Value,
};
const GLOB_PARAMS: nu_glob::MatchOptions = nu_glob::MatchOptions {
case_sensitive: true,
require_literal_separator: false,
require_literal_leading_dot: false,
recursive_match_hidden_dir: true,
};
#[derive(Clone)]
pub struct Mv;
@ -70,12 +64,6 @@ impl Command for Mv {
) -> Result<PipelineData, ShellError> {
// TODO: handle invalid directory or insufficient permissions when moving
let spanned_source: Spanned<String> = call.req(engine_state, stack, 0)?;
let spanned_source = {
Spanned {
item: nu_utils::strip_ansi_string_unlikely(spanned_source.item),
span: spanned_source.span,
}
};
let spanned_destination: Spanned<String> = call.req(engine_state, stack, 1)?;
let verbose = call.has_flag("verbose");
let interactive = call.has_flag("interactive");
@ -88,17 +76,11 @@ impl Command for Mv {
let source = path.join(spanned_source.item.as_str());
let destination = path.join(spanned_destination.item.as_str());
let mut sources = nu_glob::glob_with(&source.to_string_lossy(), GLOB_PARAMS)
.map_or_else(|_| Vec::new(), Iterator::collect);
let mut sources =
arg_glob(&spanned_source, &path).map_or_else(|_| Vec::new(), Iterator::collect);
if sources.is_empty() {
return Err(ShellError::GenericError(
"File(s) not found".into(),
"could not find any files matching this glob pattern".into(),
Some(spanned_source.span),
None,
Vec::new(),
));
return Err(ShellError::FileNotFound(spanned_source.span));
}
// We have two possibilities.

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

@ -7,6 +7,7 @@ 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_protocol::ast::Call;
@ -16,13 +17,6 @@ use nu_protocol::{
Spanned, SyntaxShape, Type, Value,
};
const GLOB_PARAMS: nu_glob::MatchOptions = nu_glob::MatchOptions {
case_sensitive: true,
require_literal_separator: false,
require_literal_leading_dot: false,
recursive_match_hidden_dir: true,
};
const TRASH_SUPPORTED: bool = cfg!(all(
feature = "trash-support",
not(any(target_os = "android", target_os = "ios"))
@ -49,8 +43,8 @@ impl Command for Rm {
.input_output_types(vec![(Type::Nothing, Type::Nothing)])
.required(
"filename",
SyntaxShape::Filepath,
"the path of the file you want to remove",
SyntaxShape::GlobPattern,
"the file or files you want to remove",
)
.switch(
"trash",
@ -252,13 +246,7 @@ fn rm(
}
let path = currentdir_path.join(&target.item);
match nu_glob::glob_with(
&path.to_string_lossy(),
nu_glob::MatchOptions {
require_literal_leading_dot: true,
..GLOB_PARAMS
},
) {
match arg_glob_leading_dot(&target, &currentdir_path) {
Ok(files) => {
for file in files {
match file {

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

@ -1,5 +1,6 @@
use nu_engine::CallExt;
use nu_path::expand_to_real_path;
use nu_cmd_base::arg_glob;
use nu_engine::{current_dir, CallExt};
use nu_glob::GlobResult;
use nu_protocol::{
ast::Call,
engine::{Command, EngineState, Stack},
@ -10,12 +11,6 @@ use uu_cp::{BackupMode, UpdateMode};
// TODO: related to uucore::error::set_exit_code(EXIT_ERR)
// const EXIT_ERR: i32 = 1;
const GLOB_PARAMS: nu_glob::MatchOptions = nu_glob::MatchOptions {
case_sensitive: true,
require_literal_separator: false,
require_literal_leading_dot: false,
recursive_match_hidden_dir: true,
};
#[cfg(not(target_os = "windows"))]
const PATH_SEPARATOR: &str = "/";
@ -154,20 +149,22 @@ impl Command for UCp {
Vec::new(),
));
};
// paths now contains the sources
let sources: Vec<Vec<PathBuf>> = paths
.iter()
.map(|p| {
// Need to expand too make it work with globbing
let expanded_src = expand_to_real_path(&p.item);
match nu_glob::glob_with(&expanded_src.to_string_lossy(), GLOB_PARAMS) {
Ok(files) => {
let f = files.filter_map(Result::ok).collect::<Vec<PathBuf>>();
if f.is_empty() {
return Err(ShellError::FileNotFound(p.span));
}
let any_source_is_dir = f.iter().any(|f| matches!(f, f if f.is_dir()));
if any_source_is_dir && !recursive {
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>>();
if exp_files.is_empty() {
return Err(ShellError::FileNotFound(p.span));
};
let mut app_vals: Vec<PathBuf> = Vec::new();
for v in exp_files {
match v {
Ok(path) => {
if !recursive && path.is_dir() {
return Err(ShellError::GenericError(
"could_not_copy_directory".into(),
"resolves to a directory (not copied)".into(),
@ -175,22 +172,20 @@ impl Command for UCp {
Some("Directories must be copied using \"--recursive\"".into()),
Vec::new(),
));
}
Ok(f)
};
app_vals.push(path)
}
Err(e) => {
return Err(ShellError::ErrorExpandingGlob(
format!("error {} in path {}", e.error(), e.path().display()),
p.span,
));
}
Err(e) => Err(ShellError::GenericError(
e.to_string(),
"invalid pattern".to_string(),
Some(p.span),
None,
Vec::new(),
)),
}
})
.collect::<Result<Vec<Vec<PathBuf>>, ShellError>>()?;
}
sources.append(&mut app_vals);
}
let sources = sources.into_iter().flatten().collect::<Vec<PathBuf>>();
let options = uu_cp::Options {
overwrite,
reflink_mode,