From 62bd6fe08b92873f8f01a1445087e4cdf07880ff Mon Sep 17 00:00:00 2001 From: Yash Thakur <45539777+ysthakur@users.noreply.github.com> Date: Wed, 1 Jan 2025 19:04:17 -0500 Subject: [PATCH] Create nu_glob::is_glob function (#14717) # Description Adds an `is_glob` function to the nu-glob crate that takes a string pattern and returns whether or not it's a glob that would be expanded by nu-glob. Right now, this just means checking if it contains `*`, `?`, or `[`. Previously, this same code was duplicated in the following places: - `ls`: Determining whether to read a folder's contents or expand a glob - `run_external.rs` in nu-command: Arguments to externals only have n-dots and tilde expansion applied if they weren't globs - `glob_from` in nu-engine: - `glob_from` can get the prefix in a simpler way for non-globs - If the canonical path for a non-glob path contains glob metacharacters, it needs to be escaped - `completion_common.rs` in nu-cli: File/folder completions containing glob metacharacters need to be wrapped in quotes All of these locations can use `nu_glob::is_glob` now instead of rolling their own checks. This does mean that nu-cli now has a dependency on nu-glob. # User-Facing Changes Users of nu-glob will now be able to check if a given pattern is a glob expanded by nu-glob. For users of Nushell, completion suggestions for files containing `]` will no longer be wrapped in quotes if they contain no other glob metacharacters. This is because unmatched `]`s are ignored by nu-glob, but we used to consider such file completions contaminated anyway. # Tests + Formatting This is a very basic function, so I just added some doctests. # After Submitting This is meant to be used in https://github.com/nushell/nushell/pull/14674. --- Cargo.lock | 1 + crates/nu-cli/Cargo.toml | 1 + .../src/completions/completion_common.rs | 3 +-- crates/nu-command/src/filesystem/ls.rs | 4 +--- crates/nu-command/src/system/run_external.rs | 6 ++---- crates/nu-engine/src/glob_from.rs | 10 ++++----- crates/nu-glob/src/lib.rs | 21 +++++++++++++++++++ 7 files changed, 31 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c9b4436c5e..65d98c9fb1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3649,6 +3649,7 @@ dependencies = [ "nu-color-config", "nu-command", "nu-engine", + "nu-glob", "nu-parser", "nu-path", "nu-plugin-engine", diff --git a/crates/nu-cli/Cargo.toml b/crates/nu-cli/Cargo.toml index 00b146dba1..fc9b838af7 100644 --- a/crates/nu-cli/Cargo.toml +++ b/crates/nu-cli/Cargo.toml @@ -20,6 +20,7 @@ tempfile = { workspace = true } [dependencies] nu-cmd-base = { path = "../nu-cmd-base", version = "0.101.1" } nu-engine = { path = "../nu-engine", version = "0.101.1", features = ["os"] } +nu-glob = { path = "../nu-glob", version = "0.101.1" } nu-path = { path = "../nu-path", version = "0.101.1" } nu-parser = { path = "../nu-parser", version = "0.101.1" } nu-plugin-engine = { path = "../nu-plugin-engine", version = "0.101.1", optional = true } diff --git a/crates/nu-cli/src/completions/completion_common.rs b/crates/nu-cli/src/completions/completion_common.rs index 4ab0f8f509..6212196627 100644 --- a/crates/nu-cli/src/completions/completion_common.rs +++ b/crates/nu-cli/src/completions/completion_common.rs @@ -286,8 +286,7 @@ pub fn complete_item( // Fix files or folders with quotes or hashes pub fn escape_path(path: String, dir: bool) -> String { // make glob pattern have the highest priority. - let glob_contaminated = path.contains(['[', '*', ']', '?']); - if glob_contaminated { + if nu_glob::is_glob(path.as_str()) { return if path.contains('\'') { // decide to use double quote, also need to escape `"` in path // or else users can't do anything with completed path either. diff --git a/crates/nu-command/src/filesystem/ls.rs b/crates/nu-command/src/filesystem/ls.rs index 2df34af855..68f35c5294 100644 --- a/crates/nu-command/src/filesystem/ls.rs +++ b/crates/nu-command/src/filesystem/ls.rs @@ -33,8 +33,6 @@ struct Args { call_span: Span, } -const GLOB_CHARS: &[char] = &['*', '?', '[']; - impl Command for Ls { fn name(&self) -> &str { "ls" @@ -291,7 +289,7 @@ fn ls_for_one_pattern( { return Ok(Value::test_nothing().into_pipeline_data()); } - just_read_dir = !(pat.item.is_expand() && pat.item.as_ref().contains(GLOB_CHARS)); + just_read_dir = !(pat.item.is_expand() && nu_glob::is_glob(pat.item.as_ref())); } // it's absolute path if: diff --git a/crates/nu-command/src/system/run_external.rs b/crates/nu-command/src/system/run_external.rs index 304b0fff8d..af597d459c 100644 --- a/crates/nu-command/src/system/run_external.rs +++ b/crates/nu-command/src/system/run_external.rs @@ -340,11 +340,9 @@ fn expand_glob( span: Span, signals: &Signals, ) -> Result, ShellError> { - const GLOB_CHARS: &[char] = &['*', '?', '[']; - - // For an argument that doesn't include the GLOB_CHARS, just do the `expand_tilde` + // For an argument that isn't a glob, just do the `expand_tilde` // and `expand_ndots` expansion - if !arg.contains(GLOB_CHARS) { + if !nu_glob::is_glob(arg) { let path = expand_ndots_safe(expand_tilde(arg)); return Ok(vec![path.into()]); } diff --git a/crates/nu-engine/src/glob_from.rs b/crates/nu-engine/src/glob_from.rs index 2847a3c5b4..376b3c2baf 100644 --- a/crates/nu-engine/src/glob_from.rs +++ b/crates/nu-engine/src/glob_from.rs @@ -6,8 +6,6 @@ use std::{ path::{Component, Path, PathBuf}, }; -const GLOB_CHARS: &[char] = &['*', '?', '[']; - /// This function is like `nu_glob::glob` from the `glob` crate, except it is relative to a given cwd. /// /// It returns a tuple of two values: the first is an optional prefix that the expanded filenames share. @@ -29,7 +27,7 @@ pub fn glob_from( ShellError, > { let no_glob_for_pattern = matches!(pattern.item, NuGlob::DoNotExpand(_)); - let (prefix, pattern) = if pattern.item.as_ref().contains(GLOB_CHARS) { + let (prefix, pattern) = if nu_glob::is_glob(pattern.item.as_ref()) { // Pattern contains glob, split it let mut p = PathBuf::new(); let path = PathBuf::from(&pattern.item.as_ref()); @@ -38,7 +36,7 @@ pub fn glob_from( for c in components { if let Component::Normal(os) = c { - if os.to_string_lossy().contains(GLOB_CHARS) { + if nu_glob::is_glob(os.to_string_lossy().as_ref()) { break; } } @@ -73,8 +71,8 @@ pub fn glob_from( (path.parent().map(|parent| parent.to_path_buf()), path) } else { let path = if let Ok(p) = canonicalize_with(path.clone(), cwd) { - if p.to_string_lossy().contains(GLOB_CHARS) { - // our path might contains GLOB_CHARS too + if nu_glob::is_glob(p.to_string_lossy().as_ref()) { + // our path might contain glob metacharacters too. // in such case, we need to escape our path to make // glob work successfully PathBuf::from(nu_glob::Pattern::escape(&p.to_string_lossy())) diff --git a/crates/nu-glob/src/lib.rs b/crates/nu-glob/src/lib.rs index 602500fa1a..30eec9f9e9 100644 --- a/crates/nu-glob/src/lib.rs +++ b/crates/nu-glob/src/lib.rs @@ -313,6 +313,27 @@ pub fn glob_with_parent( } } +const GLOB_CHARS: &[char] = &['*', '?', '[']; + +/// Returns true if the given pattern is a glob, false if it's merely text to be +/// matched exactly. +/// +/// ```rust +/// assert!(nu_glob::is_glob("foo/*")); +/// assert!(nu_glob::is_glob("foo/**/bar")); +/// assert!(nu_glob::is_glob("foo?")); +/// assert!(nu_glob::is_glob("foo[A]")); +/// +/// assert!(!nu_glob::is_glob("foo")); +/// // nu_glob will ignore an unmatched ']' +/// assert!(!nu_glob::is_glob("foo]")); +/// // nu_glob doesn't expand {} +/// assert!(!nu_glob::is_glob("foo.{txt,png}")); +/// ``` +pub fn is_glob(pattern: &str) -> bool { + pattern.contains(GLOB_CHARS) +} + /// A glob iteration error. /// /// This is typically returned when a particular path cannot be read