Remove nu-glob's dependency on nu-protocol (#15349)

# Description

This PR solves a circular dependency issue (`nu-test-support` needs
`nu-glob` which needs `nu-protocol` which needs `nu-test-support`). This
was done by making the glob functions that any type that implements
`Interruptible` to remove the dependency on `Signals`.

# After Submitting

Make `Paths.next()` a O(1) operation so that cancellation/interrupt
handling can be moved to the caller (e.g., by wrapping the `Paths`
iterator in a cancellation iterator).

Cargo.lock

@@ -3795,7 +3795,6 @@ name = "nu-glob"
 version = "0.103.1"
 dependencies = [
  "doc-comment",
- "nu-protocol",
 ]
 
 [[package]]
@@ -3967,6 +3966,7 @@ dependencies = [
  "miette",
  "nix 0.29.0",
  "nu-derive-value",
+ "nu-glob",
  "nu-path",
  "nu-system",
  "nu-test-support",

crates/nu-command/src/filesystem/utouch.rs

@@ -158,17 +158,15 @@ impl Command for UTouch {
                     continue;
                 }
 
-                let mut expanded_globs = glob(
+                let mut expanded_globs =
-                    &file_path.to_string_lossy(),
+                    glob(&file_path.to_string_lossy(), engine_state.signals().clone())
-                    Some(engine_state.signals().clone()),
+                        .unwrap_or_else(|_| {
-                )
+                            panic!(
-                .unwrap_or_else(|_| {
+                                "Failed to process file path: {}",
-                    panic!(
+                                &file_path.to_string_lossy()
-                        "Failed to process file path: {}",
+                            )
-                        &file_path.to_string_lossy()
+                        })
-                    )
+                        .peekable();
-                })
-                .peekable();
 
                 if expanded_globs.peek().is_none() {
                     let file_name = file_path.file_name().unwrap_or_else(|| {

crates/nu-glob/Cargo.toml

@@ -14,7 +14,6 @@ categories = ["filesystem"]
 bench = false
 
 [dependencies]
-nu-protocol = { path = "../nu-protocol", version = "0.103.1", default-features = false }
 
 [dev-dependencies]
 doc-comment = "0.3"

crates/nu-glob/src/lib.rs

@@ -27,9 +27,9 @@
 //! To print all jpg files in `/media/` and all of its subdirectories.
 //!
 //! ```rust,no_run
-//! use nu_glob::glob;
+//! use nu_glob::{glob, Uninterruptible};
 //!
-//! for entry in glob("/media/**/*.jpg", None).expect("Failed to read glob pattern") {
+//! for entry in glob("/media/**/*.jpg", Uninterruptible).expect("Failed to read glob pattern") {
 //!     match entry {
 //!         Ok(path) => println!("{:?}", path.display()),
 //!         Err(e) => println!("{:?}", e),
@@ -42,9 +42,7 @@
 //! instead of printing them.
 //!
 //! ```rust,no_run
-//! use nu_glob::glob_with;
+//! use nu_glob::{glob_with, MatchOptions, Uninterruptible};
-//! use nu_glob::MatchOptions;
-//! use nu_protocol::Signals;
 //!
 //! let options = MatchOptions {
 //!     case_sensitive: false,
@@ -52,7 +50,7 @@
 //!     require_literal_leading_dot: false,
 //!     recursive_match_hidden_dir: true,
 //! };
-//! for entry in glob_with("local/*a*", options, Signals::empty()).unwrap() {
+//! for entry in glob_with("local/*a*", options, Uninterruptible).unwrap() {
 //!     if let Ok(path) = entry {
 //!         println!("{:?}", path.display())
 //!     }
@@ -73,7 +71,6 @@ extern crate doc_comment;
 #[cfg(test)]
 doctest!("../README.md");
 
-use nu_protocol::Signals;
 use std::cmp;
 use std::cmp::Ordering;
 use std::error::Error;
@@ -88,6 +85,29 @@ use MatchResult::{EntirePatternDoesntMatch, Match, SubPatternDoesntMatch};
 use PatternToken::AnyExcept;
 use PatternToken::{AnyChar, AnyRecursiveSequence, AnySequence, AnyWithin, Char};
 
+/// A trait for types that can be periodically polled to check whether to cancel an operation.
+pub trait Interruptible {
+    /// Returns whether the current operation should be cancelled.
+    fn interrupted(&self) -> bool;
+}
+
+impl<I: Interruptible> Interruptible for &I {
+    #[inline]
+    fn interrupted(&self) -> bool {
+        (*self).interrupted()
+    }
+}
+
+/// A no-op implementor of [`Interruptible`] that always returns `false` for [`interrupted`](Interruptible::interrupted).
+pub struct Uninterruptible;
+
+impl Interruptible for Uninterruptible {
+    #[inline]
+    fn interrupted(&self) -> bool {
+        false
+    }
+}
+
 /// An iterator that yields `Path`s from the filesystem that match a particular
 /// pattern.
 ///
@@ -98,16 +118,16 @@ use PatternToken::{AnyChar, AnyRecursiveSequence, AnySequence, AnyWithin, Char};
 ///
 /// See the `glob` function for more details.
 #[derive(Debug)]
-pub struct Paths {
+pub struct Paths<I = Uninterruptible> {
     dir_patterns: Vec<Pattern>,
     require_dir: bool,
     options: MatchOptions,
     todo: Vec<Result<(PathBuf, usize), GlobError>>,
     scope: Option<PathBuf>,
-    signals: Signals,
+    interrupt: I,
 }
 
-impl Paths {
+impl Paths<Uninterruptible> {
     /// An iterator representing a single path.
     pub fn single(path: &Path, relative_to: &Path) -> Self {
         Paths {
@@ -116,7 +136,7 @@ impl Paths {
             options: MatchOptions::default(),
             todo: vec![Ok((path.to_path_buf(), 0))],
             scope: Some(relative_to.into()),
-            signals: Signals::empty(),
+            interrupt: Uninterruptible,
         }
     }
 }
@@ -133,7 +153,7 @@ impl Paths {
 ///
 /// When iterating, each result is a `GlobResult` which expresses the
 /// possibility that there was an `IoError` when attempting to read the contents
-/// of the matched path.  In other words, each item returned by the iterator
+/// of the matched path. In other words, each item returned by the iterator
 /// will either be an `Ok(Path)` if the path matched, or an `Err(GlobError)` if
 /// the path (partially) matched _but_ its contents could not be read in order
 /// to determine if its contents matched.
@@ -146,9 +166,9 @@ impl Paths {
 /// `kittens.jpg`, `puppies.jpg` and `hamsters.gif`:
 ///
 /// ```rust,no_run
-/// use nu_glob::glob;
+/// use nu_glob::{glob, Uninterruptible};
 ///
-/// for entry in glob("/media/pictures/*.jpg", None).unwrap() {
+/// for entry in glob("/media/pictures/*.jpg", Uninterruptible).unwrap() {
 ///     match entry {
 ///         Ok(path) => println!("{:?}", path.display()),
 ///
@@ -170,20 +190,16 @@ impl Paths {
 /// `filter_map`:
 ///
 /// ```rust
-/// use nu_glob::glob;
+/// use nu_glob::{glob, Uninterruptible};
 /// use std::result::Result;
 ///
-/// for path in glob("/media/pictures/*.jpg", None).unwrap().filter_map(Result::ok) {
+/// for path in glob("/media/pictures/*.jpg", Uninterruptible).unwrap().filter_map(Result::ok) {
 ///     println!("{}", path.display());
 /// }
 /// ```
 /// Paths are yielded in alphabetical order.
-pub fn glob(pattern: &str, signals: Option<Signals>) -> Result<Paths, PatternError> {
+pub fn glob<I: Interruptible>(pattern: &str, interrupt: I) -> Result<Paths<I>, PatternError> {
-    glob_with(
+    glob_with(pattern, MatchOptions::default(), interrupt)
-        pattern,
-        MatchOptions::default(),
-        signals.unwrap_or(Signals::empty()),
-    )
 }
 
 /// Return an iterator that produces all the `Path`s that match the given
@@ -199,11 +215,11 @@ pub fn glob(pattern: &str, signals: Option<Signals>) -> Result<Paths, PatternErr
 /// passed to this function.
 ///
 /// Paths are yielded in alphabetical order.
-pub fn glob_with(
+pub fn glob_with<I: Interruptible>(
     pattern: &str,
     options: MatchOptions,
-    signals: Signals,
+    interrupt: I,
-) -> Result<Paths, PatternError> {
+) -> Result<Paths<I>, PatternError> {
     #[cfg(windows)]
     fn check_windows_verbatim(p: &Path) -> bool {
         match p.components().next() {
@@ -265,7 +281,7 @@ pub fn glob_with(
             options,
             todo: Vec::new(),
             scope: None,
-            signals,
+            interrupt,
         });
     }
 
@@ -297,7 +313,7 @@ pub fn glob_with(
         options,
         todo,
         scope: Some(scope),
-        signals,
+        interrupt,
     })
 }
 
@@ -308,13 +324,13 @@ pub fn glob_with(
 /// This is provided primarily for testability, so multithreaded test runners can
 /// test pattern matches in different test directories at the same time without
 /// having to append the parent to the pattern under test.
-pub fn glob_with_parent(
+pub fn glob_with_parent<I: Interruptible>(
     pattern: &str,
     options: MatchOptions,
     parent: &Path,
-    signals: Signals,
+    interrupt: I,
-) -> Result<Paths, PatternError> {
+) -> Result<Paths<I>, PatternError> {
-    match glob_with(pattern, options, signals) {
+    match glob_with(pattern, options, interrupt) {
         Ok(mut p) => {
             p.scope = match p.scope {
                 None => Some(parent.to_path_buf()),
@@ -408,7 +424,7 @@ fn is_dir(p: &Path) -> bool {
 /// such as failing to read a particular directory's contents.
 pub type GlobResult = Result<PathBuf, GlobError>;
 
-impl Iterator for Paths {
+impl<I: Interruptible> Iterator for Paths<I> {
     type Item = GlobResult;
 
     fn next(&mut self) -> Option<GlobResult> {
@@ -429,7 +445,7 @@ impl Iterator for Paths {
                         0,
                         &scope,
                         self.options,
-                        &self.signals,
+                        &self.interrupt,
                     );
                 }
             }
@@ -487,7 +503,7 @@ impl Iterator for Paths {
                         next,
                         &path,
                         self.options,
-                        &self.signals,
+                        &self.interrupt,
                     );
 
                     if next == self.dir_patterns.len() - 1 {
@@ -539,7 +555,7 @@ impl Iterator for Paths {
                         idx + 1,
                         &path,
                         self.options,
-                        &self.signals,
+                        &self.interrupt,
                     );
                 }
             }
@@ -929,7 +945,7 @@ fn fill_todo(
     idx: usize,
     path: &Path,
     options: MatchOptions,
-    signals: &Signals,
+    interrupt: &impl Interruptible,
 ) {
     // convert a pattern that's just many Char(_) to a string
     fn pattern_as_str(pattern: &Pattern) -> Option<String> {
@@ -951,7 +967,7 @@ fn fill_todo(
             // . or .. globs since these never show up as path components.
             todo.push(Ok((next_path, !0)));
         } else {
-            fill_todo(todo, patterns, idx + 1, &next_path, options, signals);
+            fill_todo(todo, patterns, idx + 1, &next_path, options, interrupt);
         }
     };
 
@@ -982,7 +998,7 @@ fn fill_todo(
         None if is_dir => {
             let dirs = fs::read_dir(path).and_then(|d| {
                 d.map(|e| {
-                    if signals.interrupted() {
+                    if interrupt.interrupted() {
                         return Err(io::Error::from(io::ErrorKind::Interrupted));
                     }
                     e.map(|e| {
@@ -1141,13 +1157,13 @@ impl Default for MatchOptions {
 
 #[cfg(test)]
 mod test {
-    use crate::{Paths, PatternError};
+    use crate::{Paths, PatternError, Uninterruptible};
 
     use super::{glob as glob_with_signals, MatchOptions, Pattern};
     use std::path::Path;
 
     fn glob(pattern: &str) -> Result<Paths, PatternError> {
-        glob_with_signals(pattern, None)
+        glob_with_signals(pattern, Uninterruptible)
     }
 
     #[test]

crates/nu-lsp/src/workspace.rs

@@ -9,6 +9,7 @@ use lsp_types::{
     TextDocumentPositionParams, TextEdit, Uri, WorkspaceEdit, WorkspaceFolder,
 };
 use miette::{miette, IntoDiagnostic, Result};
+use nu_glob::Uninterruptible;
 use nu_protocol::{
     engine::{EngineState, StateWorkingSet},
     Span,
@@ -42,7 +43,7 @@ fn find_nu_scripts_in_folder(folder_uri: &Uri) -> Result<nu_glob::Paths> {
         return Err(miette!("\nworkspace folder does not exist."));
     }
     let pattern = format!("{}/**/*.nu", path.to_string_lossy());
-    nu_glob::glob(&pattern, None).into_diagnostic()
+    nu_glob::glob(&pattern, Uninterruptible).into_diagnostic()
 }
 
 impl LanguageServer {

crates/nu-protocol/Cargo.toml

@@ -17,6 +17,7 @@ workspace = true
 
 [dependencies]
 nu-utils = { path = "../nu-utils", version = "0.103.1", default-features = false }
+nu-glob = { path = "../nu-glob", version = "0.103.1" }
 nu-path = { path = "../nu-path", version = "0.103.1" }
 nu-system = { path = "../nu-system", version = "0.103.1" }
 nu-derive-value = { path = "../nu-derive-value", version = "0.103.1" }

crates/nu-protocol/src/pipeline/signals.rs

@@ -1,11 +1,11 @@
 use crate::{ShellError, Span};
+use nu_glob::Interruptible;
+use serde::{Deserialize, Serialize};
 use std::sync::{
     atomic::{AtomicBool, Ordering},
     Arc,
 };
 
-use serde::{Deserialize, Serialize};
-
 /// Used to check for signals to suspend or terminate the execution of Nushell code.
 ///
 /// For now, this struct only supports interruption (ctrl+c or SIGINT).
@@ -84,6 +84,13 @@ impl Signals {
     }
 }
 
+impl Interruptible for Signals {
+    #[inline]
+    fn interrupted(&self) -> bool {
+        self.interrupted()
+    }
+}
+
 /// The types of things that can be signaled. It's anticipated this will change as we learn more
 /// about how we'd like signals to be handled.
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]

crates/nu-test-support/src/playground/play.rs

@@ -1,6 +1,6 @@
 use super::Director;
 use crate::fs::{self, Stub};
-use nu_glob::glob;
+use nu_glob::{glob, Uninterruptible};
 #[cfg(not(target_arch = "wasm32"))]
 use nu_path::Path;
 use nu_path::{AbsolutePath, AbsolutePathBuf};
@@ -231,7 +231,7 @@ impl Playground<'_> {
     }
 
     pub fn glob_vec(pattern: &str) -> Vec<std::path::PathBuf> {
-        let glob = glob(pattern, None);
+        let glob = glob(pattern, Uninterruptible);
 
         glob.expect("invalid pattern")
             .map(|path| {