Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

glob with ../ prefix now works; #10504

Merged
merged 11 commits into from
Sep 29, 2023
26 changes: 22 additions & 4 deletions crates/nu-command/src/filesystem/glob.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ impl Command for Glob {
}

fn extra_usage(&self) -> &str {
r#"For more glob pattern help, please refer to https://github.com/olson-sean-k/wax"#
r#"For more glob pattern help, please refer to https://docs.rs/crate/wax/latest"#
}

fn run(
Expand All @@ -135,7 +135,6 @@ impl Command for Glob {
) -> Result<PipelineData, ShellError> {
let ctrlc = engine_state.ctrlc.clone();
let span = call.head;
let path = current_dir(engine_state, stack)?;
let glob_pattern: Spanned<String> = call.req(engine_state, stack, 0)?;
let depth = call.get_flag(engine_state, stack, "depth")?;
let no_dirs = call.has_flag("no-dir");
Expand Down Expand Up @@ -173,8 +172,8 @@ impl Command for Glob {
usize::MAX
};

let glob = match WaxGlob::new(&glob_pattern.item) {
Ok(p) => p,
let (prefix, glob) = match WaxGlob::new(&glob_pattern.item) {
Ok(p) => p.partition(),
Err(e) => {
return Err(ShellError::GenericError(
"error with glob pattern".to_string(),
Expand All @@ -186,6 +185,25 @@ impl Command for Glob {
}
};

let path = current_dir(engine_state, stack)?;
let path = match nu_path::canonicalize_with(prefix, path) {
Ok(path) => path,
Err(e) if e.to_string().contains("os error 2") =>
// path we're trying to glob doesn't exist,
{
std::path::PathBuf::new() // user should get empty list not an error
}
Err(e) => {
return Err(ShellError::GenericError(
"error in canonicalize".to_string(),
format!("{e}"),
Some(glob_pattern.span),
None,
Vec::new(),
))
}
};

Ok(if !not_patterns.is_empty() {
let np: Vec<&str> = not_patterns.iter().map(|s| s as &str).collect();
let glob_results = glob
Expand Down
55 changes: 55 additions & 0 deletions crates/nu-command/tests/commands/glob.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use nu_test_support::fs::Stub::EmptyFile;
use nu_test_support::nu;
use nu_test_support::playground::Playground;
use rstest::rstest;
use std::path::{Path, PathBuf};

#[test]
fn empty_glob_pattern_triggers_error() {
Expand Down Expand Up @@ -118,3 +120,56 @@ fn glob_ignore_files() {
);
})
}

// clone of fs::create_file_at removing the parent panic, whose purpose I do not grok.
pub fn create_file_at(full_path: impl AsRef<Path>) -> Result<(), std::io::Error> {
let full_path = full_path.as_ref();
std::fs::write(full_path, b"fake data")
}

// playground has root directory and subdirectories foo and foo/bar to play with
// specify all test files relative to root directory.
// OK to use fwd slash in paths, they're hacked to OS dir separator when needed (windows)
#[rstest]
#[case(".", r#"'*z'"#, &["ablez", "baker", "charliez"], &["ablez", "charliez"], "simple glob")]
#[case(".", r#"'qqq'"#, &["ablez", "baker", "charliez"], &[], "glob matches none")]
#[case("foo/bar", r#"'*[\]}]*'"#, &[r#"foo/bar/ab}le"#, "foo/bar/baker", r#"foo/bar/cha]rlie"#], &[r#"foo/bar/ab}le"#, r#"foo/bar/cha]rlie"#], "glob has quoted metachars")]
#[case("foo/bar", r#"'../*'"#, &["foo/able", "foo/bar/baker", "foo/charlie"], &["foo/able", "foo/bar", "foo/charlie"], "glob matches files in parent")]
#[case("foo", r#"'./{a,b}*'"#, &["foo/able", "foo/bar/baker", "foo/charlie"], &["foo/able", "foo/bar"], "glob with leading ./ matches peer files")]
fn glob_files_in_parent(
#[case] wd: &str,
#[case] glob: &str,
#[case] ini: &[&str],
#[case] exp: &[&str],
#[case] tag: &str,
) {
Playground::setup("glob_test", |dirs, sandbox| {
sandbox.within("foo").within("bar");
let working_directory = &dirs.test().join(wd);

for f in ini {
create_file_at(dirs.test().join(f)).expect("couldn't create file");
}

let actual = nu!(
cwd: working_directory,
r#"glob {} | sort | str join " ""#,
glob
);

let mut expected: Vec<String> = vec![];
for e in exp {
expected.push(
dirs.test()
.join(PathBuf::from(e)) // sadly, does *not" convert "foo/bar" to "foo\\bar" on Windows.
.to_string_lossy()
.to_string(),
);
}

let expected = expected
.join(" ")
.replace('/', std::path::MAIN_SEPARATOR_STR);
assert_eq!(actual.out, expected, "\n test: {}", tag);
});
}