Create nu_glob::is_glob function (nushell#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 nushell#14674.
hjetmundsen · Jan 2, 2025 · 62bd6fe · 62bd6fe
1 parent f69b22f
commit 62bd6fe
Show file tree

Hide file tree

Showing 7 changed files with 31 additions and 15 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git 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
@@ -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
@@ -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
@@ -340,11 +340,9 @@ fn expand_glob(
     span: Span,
     signals: &Signals,
 ) -> Result<Vec<OsString>, 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
@@ -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
@@ -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