From 3d65fd7cc4f6e0db0c1c31b895feabf2be66cb0a Mon Sep 17 00:00:00 2001 From: Doru Date: Sat, 28 Jan 2023 10:50:12 -0300 Subject: [PATCH] Expose filtering by file type in glob (#7834) # Description Add flags for filtering the output of `glob` by file type. I find myself occasionally wanting to do this, and getting a file's [file_type](https://docs.rs/wax/latest/wax/struct.WalkEntry.html#method.file_type) is presumably fast to do as it doesn't have to go through the fallible metadata method. The design of the signature does concern me; it's not as readable as a filter or "include" type list would be. They have to be filtered one by one, which can be annoying if you only want files `-D -S`, or only want folders `-F -S`, or only want symlinks `--butwhy?`. I considered SyntaxShape::Keyword for this but I'll just defer to comments on this PR if they pop up. I'd also like to bring up performance since including these flags technically incurs a `.filter` penalty on all glob calls, which could be optimized out if we added a branch for the no-filters case. But in reality I'd expect the file system to be the bottleneck and the flags to be pretty branch predictor friendly, so eh # User-Facing Changes Three new flags when using `glob` and a slightly more cluttered help page. No breaking changes, I hope. # 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 -A clippy::needless_collect` to check that you're using the standard code style - `cargo test --workspace` to check that all tests pass # 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. --- crates/nu-command/src/filesystem/glob.rs | 31 +++++++++ crates/nu-command/tests/commands/glob.rs | 81 ++++++++++++++++++++++++ 2 files changed, 112 insertions(+) diff --git a/crates/nu-command/src/filesystem/glob.rs b/crates/nu-command/src/filesystem/glob.rs index 08dfbc726..29f6bb682 100644 --- a/crates/nu-command/src/filesystem/glob.rs +++ b/crates/nu-command/src/filesystem/glob.rs @@ -26,6 +26,21 @@ impl Command for Glob { "directory depth to search", Some('d'), ) + .switch( + "no-dir", + "Whether to filter out directories from the returned paths", + Some('D'), + ) + .switch( + "no-file", + "Whether to filter out files from the returned paths", + Some('F'), + ) + .switch( + "no-symlink", + "Whether to filter out symlinks from the returned paths", + Some('S'), + ) .category(Category::FileSystem) } @@ -81,6 +96,11 @@ impl Command for Glob { example: "glob <[a-d]:1,10>", result: None, }, + Example { + description: "Search for folders that begin with an uppercase ASCII letter, ignoring files and symlinks", + example: r#"glob "[A-Z]*" --no-file --no-symlink"#, + result: None, + }, ] } @@ -100,6 +120,10 @@ impl Command for Glob { let glob_pattern: Spanned = call.req(engine_state, stack, 0)?; let depth = call.get_flag(engine_state, stack, "depth")?; + let no_dirs = call.has_flag("no-dir"); + let no_files = call.has_flag("no-file"); + let no_symlinks = call.has_flag("no-symlink"); + if glob_pattern.item.is_empty() { return Err(ShellError::GenericError( "glob pattern must not be empty".to_string(), @@ -139,6 +163,13 @@ impl Command for Glob { }, ) .flatten() + .filter(|entry| { + let file_type = entry.file_type(); + + !(no_dirs && file_type.is_dir() + || no_files && file_type.is_file() + || no_symlinks && file_type.is_symlink()) + }) .map(|entry| Value::String { val: entry.into_path().to_string_lossy().to_string(), span, diff --git a/crates/nu-command/tests/commands/glob.rs b/crates/nu-command/tests/commands/glob.rs index 7a5e628e9..23bdf52fe 100644 --- a/crates/nu-command/tests/commands/glob.rs +++ b/crates/nu-command/tests/commands/glob.rs @@ -37,3 +37,84 @@ fn nonempty_glob_lists_matching_paths() { assert_eq!(actual.out, "3"); }) } + +#[test] +fn glob_subdirs() { + Playground::setup("glob_subdirs", |dirs, sandbox| { + sandbox.with_files(vec![ + EmptyFile("yehuda.txt"), + EmptyFile("jonathan.txt"), + EmptyFile("andres.txt"), + ]); + sandbox.mkdir("children"); + sandbox.within("children").with_files(vec![ + EmptyFile("timothy.txt"), + EmptyFile("tiffany.txt"), + EmptyFile("trish.txt"), + ]); + + let actual = nu!( + cwd: dirs.test(), + pipeline("glob '**/*' | length"), + ); + + assert_eq!( + actual.out, "8", + "count must be 8 due to 6 files and 2 folders, including the cwd" + ); + }) +} + +#[test] +fn glob_subdirs_ignore_dirs() { + Playground::setup("glob_subdirs_ignore_directories", |dirs, sandbox| { + sandbox.with_files(vec![ + EmptyFile("yehuda.txt"), + EmptyFile("jonathan.txt"), + EmptyFile("andres.txt"), + ]); + sandbox.mkdir("children"); + sandbox.within("children").with_files(vec![ + EmptyFile("timothy.txt"), + EmptyFile("tiffany.txt"), + EmptyFile("trish.txt"), + ]); + + let actual = nu!( + cwd: dirs.test(), + pipeline("glob '**/*' -D | length"), + ); + + assert_eq!( + actual.out, "6", + "directory count must be 6, ignoring the cwd and the children folders" + ); + }) +} + +#[test] +fn glob_ignore_files() { + Playground::setup("glob_ignore_files", |dirs, sandbox| { + sandbox.with_files(vec![ + EmptyFile("yehuda.txt"), + EmptyFile("jonathan.txt"), + EmptyFile("andres.txt"), + ]); + sandbox.mkdir("children"); + sandbox.within("children").with_files(vec![ + EmptyFile("timothy.txt"), + EmptyFile("tiffany.txt"), + EmptyFile("trish.txt"), + ]); + + let actual = nu!( + cwd: dirs.test(), + pipeline("glob '*' -F | length"), + ); + + assert_eq!( + actual.out, "1", + "should only find one folder; ignoring cwd, files, subfolders" + ); + }) +}