From c5a14bb8ff87155a6731e9e5ec4f6a9a3c9470b6 Mon Sep 17 00:00:00 2001 From: Solomon Date: Fri, 28 Feb 2025 11:36:39 -0700 Subject: [PATCH] check signals in `nu-glob` and `ls` (#15140) Fixes #10144 # User-Facing Changes Long running glob expansions and `ls` runs (e.g. `ls /**/*`) can now be interrupted with ctrl-c. --- Cargo.lock | 1 + crates/nu-command/src/filesystem/du.rs | 13 +++-- crates/nu-command/src/filesystem/ls.rs | 22 ++++++-- crates/nu-command/src/filesystem/open.rs | 21 +++---- crates/nu-command/src/filesystem/rm.rs | 1 + crates/nu-command/src/filesystem/ucp.rs | 2 +- crates/nu-command/src/filesystem/umv.rs | 2 +- crates/nu-command/src/filesystem/utouch.rs | 19 ++++--- crates/nu-command/src/system/run_external.rs | 29 ++++++---- crates/nu-engine/src/glob_from.rs | 5 +- crates/nu-glob/Cargo.toml | 3 + crates/nu-glob/src/lib.rs | 56 +++++++++++++++---- crates/nu-lsp/src/workspace.rs | 2 +- crates/nu-test-support/src/playground/play.rs | 2 +- 14 files changed, 120 insertions(+), 58 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f79236ece3..aeb46f31b8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3784,6 +3784,7 @@ name = "nu-glob" version = "0.102.1" dependencies = [ "doc-comment", + "nu-protocol", ] [[package]] diff --git a/crates/nu-command/src/filesystem/du.rs b/crates/nu-command/src/filesystem/du.rs index d0a2d3d5f3..e4b145065c 100644 --- a/crates/nu-command/src/filesystem/du.rs +++ b/crates/nu-command/src/filesystem/du.rs @@ -118,7 +118,7 @@ impl Command for Du { min_size, }; Ok( - du_for_one_pattern(args, ¤t_dir, tag, engine_state.signals())? + du_for_one_pattern(args, ¤t_dir, tag, engine_state.signals().clone())? .into_pipeline_data(tag, engine_state.signals().clone()), ) } @@ -137,7 +137,7 @@ impl Command for Du { args, ¤t_dir, tag, - engine_state.signals(), + engine_state.signals().clone(), )?) } @@ -163,9 +163,8 @@ fn du_for_one_pattern( args: DuArgs, current_dir: &Path, span: Span, - signals: &Signals, + signals: Signals, ) -> Result + Send, ShellError> { - let signals_clone = signals.clone(); let exclude = args.exclude.map_or(Ok(None), move |x| { Pattern::new(x.item.as_ref()) .map(Some) @@ -176,7 +175,8 @@ fn du_for_one_pattern( })?; let paths = match args.path { - Some(p) => nu_engine::glob_from(&p, current_dir, span, None), + Some(p) => nu_engine::glob_from(&p, current_dir, span, None, signals.clone()), + // The * pattern should never fail. None => nu_engine::glob_from( &Spanned { @@ -186,6 +186,7 @@ fn du_for_one_pattern( current_dir, span, None, + signals.clone(), ), } .map(|f| f.1)?; @@ -206,7 +207,7 @@ fn du_for_one_pattern( Ok(paths.filter_map(move |p| match p { Ok(a) => { if a.is_dir() { - match DirInfo::new(a, ¶ms, max_depth, span, &signals_clone) { + match DirInfo::new(a, ¶ms, max_depth, span, &signals) { Ok(v) => Some(Value::from(v)), Err(_) => None, } diff --git a/crates/nu-command/src/filesystem/ls.rs b/crates/nu-command/src/filesystem/ls.rs index 19418b1a70..e80d49f846 100644 --- a/crates/nu-command/src/filesystem/ls.rs +++ b/crates/nu-command/src/filesystem/ls.rs @@ -285,7 +285,10 @@ fn ls_for_one_pattern( nu_path::expand_path_with(pat.item.as_ref(), &cwd, pat.item.is_expand()); // Avoid checking and pushing "*" to the path when directory (do not show contents) flag is true if !directory && tmp_expanded.is_dir() { - if read_dir(tmp_expanded, p_tag, use_threads)?.next().is_none() { + if read_dir(tmp_expanded, p_tag, use_threads, signals.clone())? + .next() + .is_none() + { return Ok(Value::test_nothing().into_pipeline_data()); } just_read_dir = !(pat.item.is_expand() && nu_glob::is_glob(pat.item.as_ref())); @@ -304,7 +307,10 @@ fn ls_for_one_pattern( // Avoid pushing "*" to the default path when directory (do not show contents) flag is true if directory { (NuGlob::Expand(".".to_string()), false) - } else if read_dir(cwd.clone(), p_tag, use_threads)?.next().is_none() { + } else if read_dir(cwd.clone(), p_tag, use_threads, signals.clone())? + .next() + .is_none() + { return Ok(Value::test_nothing().into_pipeline_data()); } else { (NuGlob::Expand("*".to_string()), false) @@ -317,7 +323,7 @@ fn ls_for_one_pattern( let path = pattern_arg.into_spanned(p_tag); let (prefix, paths) = if just_read_dir { let expanded = nu_path::expand_path_with(path.item.as_ref(), &cwd, path.item.is_expand()); - let paths = read_dir(expanded.clone(), p_tag, use_threads)?; + let paths = read_dir(expanded.clone(), p_tag, use_threads, signals.clone())?; // just need to read the directory, so prefix is path itself. (Some(expanded), paths) } else { @@ -330,11 +336,13 @@ fn ls_for_one_pattern( }; Some(glob_options) }; - glob_from(&path, &cwd, call_span, glob_options)? + glob_from(&path, &cwd, call_span, glob_options, signals.clone())? }; let mut paths_peek = paths.peekable(); - if paths_peek.peek().is_none() { + let no_matches = paths_peek.peek().is_none(); + signals.check(call_span)?; + if no_matches { return Err(ShellError::GenericError { error: format!("No matches found for {:?}", path.item), msg: "Pattern, file or folder not found".into(), @@ -959,17 +967,21 @@ fn read_dir( f: PathBuf, span: Span, use_threads: bool, + signals: Signals, ) -> Result> + Send>, ShellError> { + let signals_clone = signals.clone(); let items = f .read_dir() .map_err(|err| IoError::new(err.kind(), span, f.clone()))? .map(move |d| { + signals_clone.check(span)?; d.map(|r| r.path()) .map_err(|err| IoError::new(err.kind(), span, f.clone())) .map_err(ShellError::from) }); if !use_threads { let mut collected = items.collect::>(); + signals.check(span)?; collected.sort_by(|a, b| match (a, b) { (Ok(a), Ok(b)) => a.cmp(b), (Ok(_), Err(_)) => Ordering::Greater, diff --git a/crates/nu-command/src/filesystem/open.rs b/crates/nu-command/src/filesystem/open.rs index 27dbf5723a..ccb477fd43 100644 --- a/crates/nu-command/src/filesystem/open.rs +++ b/crates/nu-command/src/filesystem/open.rs @@ -95,16 +95,17 @@ impl Command for Open { let arg_span = path.span; // let path_no_whitespace = &path.item.trim_end_matches(|x| matches!(x, '\x09'..='\x0d')); - for path in nu_engine::glob_from(&path, &cwd, call_span, None) - .map_err(|err| match err { - ShellError::Io(mut err) => { - err.kind = err.kind.not_found_as(NotFound::File); - err.span = arg_span; - err.into() - } - _ => err, - })? - .1 + for path in + nu_engine::glob_from(&path, &cwd, call_span, None, engine_state.signals().clone()) + .map_err(|err| match err { + ShellError::Io(mut err) => { + err.kind = err.kind.not_found_as(NotFound::File); + err.span = arg_span; + err.into() + } + _ => err, + })? + .1 { let path = path?; let path = Path::new(&path); diff --git a/crates/nu-command/src/filesystem/rm.rs b/crates/nu-command/src/filesystem/rm.rs index 91daf99d8c..303606353f 100644 --- a/crates/nu-command/src/filesystem/rm.rs +++ b/crates/nu-command/src/filesystem/rm.rs @@ -260,6 +260,7 @@ fn rm( require_literal_leading_dot: true, ..Default::default() }), + engine_state.signals().clone(), ) { Ok(files) => { for file in files.1 { diff --git a/crates/nu-command/src/filesystem/ucp.rs b/crates/nu-command/src/filesystem/ucp.rs index a65cbece66..56eea910ad 100644 --- a/crates/nu-command/src/filesystem/ucp.rs +++ b/crates/nu-command/src/filesystem/ucp.rs @@ -193,7 +193,7 @@ impl Command for UCp { for mut p in paths { p.item = p.item.strip_ansi_string_unlikely(); let exp_files: Vec> = - nu_engine::glob_from(&p, &cwd, call.head, None) + nu_engine::glob_from(&p, &cwd, call.head, None, engine_state.signals().clone()) .map(|f| f.1)? .collect(); if exp_files.is_empty() { diff --git a/crates/nu-command/src/filesystem/umv.rs b/crates/nu-command/src/filesystem/umv.rs index c76ab9f376..9c3c713245 100644 --- a/crates/nu-command/src/filesystem/umv.rs +++ b/crates/nu-command/src/filesystem/umv.rs @@ -134,7 +134,7 @@ impl Command for UMv { for mut p in paths { p.item = p.item.strip_ansi_string_unlikely(); let exp_files: Vec> = - nu_engine::glob_from(&p, &cwd, call.head, None) + nu_engine::glob_from(&p, &cwd, call.head, None, engine_state.signals().clone()) .map(|f| f.1)? .collect(); if exp_files.is_empty() { diff --git a/crates/nu-command/src/filesystem/utouch.rs b/crates/nu-command/src/filesystem/utouch.rs index d887a5c411..53798ebc81 100644 --- a/crates/nu-command/src/filesystem/utouch.rs +++ b/crates/nu-command/src/filesystem/utouch.rs @@ -158,14 +158,17 @@ impl Command for UTouch { continue; } - let mut expanded_globs = glob(&file_path.to_string_lossy()) - .unwrap_or_else(|_| { - panic!( - "Failed to process file path: {}", - &file_path.to_string_lossy() - ) - }) - .peekable(); + let mut expanded_globs = glob( + &file_path.to_string_lossy(), + Some(engine_state.signals().clone()), + ) + .unwrap_or_else(|_| { + panic!( + "Failed to process file path: {}", + &file_path.to_string_lossy() + ) + }) + .peekable(); if expanded_globs.peek().is_none() { let file_name = file_path.file_name().unwrap_or_else(|| { diff --git a/crates/nu-command/src/system/run_external.rs b/crates/nu-command/src/system/run_external.rs index c350eff025..b5299979f7 100644 --- a/crates/nu-command/src/system/run_external.rs +++ b/crates/nu-command/src/system/run_external.rs @@ -382,9 +382,14 @@ pub fn eval_external_arguments( match arg { // Expand globs passed to run-external Value::Glob { val, no_expand, .. } if !no_expand => args.extend( - expand_glob(&val, cwd.as_std_path(), span, engine_state.signals())? - .into_iter() - .map(|s| s.into_spanned(span)), + expand_glob( + &val, + cwd.as_std_path(), + span, + engine_state.signals().clone(), + )? + .into_iter() + .map(|s| s.into_spanned(span)), ), other => args .push(OsString::from(coerce_into_string(engine_state, other)?).into_spanned(span)), @@ -415,7 +420,7 @@ fn expand_glob( arg: &str, cwd: &Path, span: Span, - signals: &Signals, + signals: Signals, ) -> Result, ShellError> { // For an argument that isn't a glob, just do the `expand_tilde` // and `expand_ndots` expansion @@ -427,7 +432,7 @@ fn expand_glob( // We must use `nu_engine::glob_from` here, in order to ensure we get paths from the correct // dir let glob = NuGlob::Expand(arg.to_owned()).into_spanned(span); - if let Ok((prefix, matches)) = nu_engine::glob_from(&glob, cwd, span, None) { + if let Ok((prefix, matches)) = nu_engine::glob_from(&glob, cwd, span, None, signals.clone()) { let mut result: Vec = vec![]; for m in matches { @@ -740,30 +745,30 @@ mod test { let cwd = dirs.test().as_std_path(); - let actual = expand_glob("*.txt", cwd, Span::unknown(), &Signals::empty()).unwrap(); + let actual = expand_glob("*.txt", cwd, Span::unknown(), Signals::empty()).unwrap(); let expected = &["a.txt", "b.txt"]; assert_eq!(actual, expected); - let actual = expand_glob("./*.txt", cwd, Span::unknown(), &Signals::empty()).unwrap(); + let actual = expand_glob("./*.txt", cwd, Span::unknown(), Signals::empty()).unwrap(); assert_eq!(actual, expected); - let actual = expand_glob("'*.txt'", cwd, Span::unknown(), &Signals::empty()).unwrap(); + let actual = expand_glob("'*.txt'", cwd, Span::unknown(), Signals::empty()).unwrap(); let expected = &["'*.txt'"]; assert_eq!(actual, expected); - let actual = expand_glob(".", cwd, Span::unknown(), &Signals::empty()).unwrap(); + let actual = expand_glob(".", cwd, Span::unknown(), Signals::empty()).unwrap(); let expected = &["."]; assert_eq!(actual, expected); - let actual = expand_glob("./a.txt", cwd, Span::unknown(), &Signals::empty()).unwrap(); + let actual = expand_glob("./a.txt", cwd, Span::unknown(), Signals::empty()).unwrap(); let expected = &["./a.txt"]; assert_eq!(actual, expected); - let actual = expand_glob("[*.txt", cwd, Span::unknown(), &Signals::empty()).unwrap(); + let actual = expand_glob("[*.txt", cwd, Span::unknown(), Signals::empty()).unwrap(); let expected = &["[*.txt"]; assert_eq!(actual, expected); - let actual = expand_glob("~/foo.txt", cwd, Span::unknown(), &Signals::empty()).unwrap(); + let actual = expand_glob("~/foo.txt", cwd, Span::unknown(), Signals::empty()).unwrap(); let home = dirs::home_dir().expect("failed to get home dir"); let expected: Vec = vec![home.join("foo.txt").into()]; assert_eq!(actual, expected); diff --git a/crates/nu-engine/src/glob_from.rs b/crates/nu-engine/src/glob_from.rs index 1120499b43..b9d473913d 100644 --- a/crates/nu-engine/src/glob_from.rs +++ b/crates/nu-engine/src/glob_from.rs @@ -1,6 +1,6 @@ use nu_glob::MatchOptions; use nu_path::{canonicalize_with, expand_path_with}; -use nu_protocol::{shell_error::io::IoError, NuGlob, ShellError, Span, Spanned}; +use nu_protocol::{shell_error::io::IoError, NuGlob, ShellError, Signals, Span, Spanned}; use std::{ fs, path::{Component, Path, PathBuf}, @@ -19,6 +19,7 @@ pub fn glob_from( cwd: &Path, span: Span, options: Option, + signals: Signals, ) -> Result< ( Option, @@ -90,7 +91,7 @@ pub fn glob_from( let pattern = pattern.to_string_lossy().to_string(); let glob_options = options.unwrap_or_default(); - let glob = nu_glob::glob_with(&pattern, glob_options).map_err(|e| { + let glob = nu_glob::glob_with(&pattern, glob_options, signals).map_err(|e| { nu_protocol::ShellError::GenericError { error: "Error extracting glob pattern".into(), msg: e.to_string(), diff --git a/crates/nu-glob/Cargo.toml b/crates/nu-glob/Cargo.toml index 4ca8051930..ec38717371 100644 --- a/crates/nu-glob/Cargo.toml +++ b/crates/nu-glob/Cargo.toml @@ -13,6 +13,9 @@ categories = ["filesystem"] [lib] bench = false +[dependencies] +nu-protocol = { path = "../nu-protocol", version = "0.102.1", default-features = false } + [dev-dependencies] doc-comment = "0.3" diff --git a/crates/nu-glob/src/lib.rs b/crates/nu-glob/src/lib.rs index 04cab26a77..b733cc1bda 100644 --- a/crates/nu-glob/src/lib.rs +++ b/crates/nu-glob/src/lib.rs @@ -29,7 +29,7 @@ //! ```rust,no_run //! use nu_glob::glob; //! -//! for entry in glob("/media/**/*.jpg").expect("Failed to read glob pattern") { +//! for entry in glob("/media/**/*.jpg", None).expect("Failed to read glob pattern") { //! match entry { //! Ok(path) => println!("{:?}", path.display()), //! Err(e) => println!("{:?}", e), @@ -44,6 +44,7 @@ //! ```rust,no_run //! use nu_glob::glob_with; //! use nu_glob::MatchOptions; +//! use nu_protocol::Signals; //! //! let options = MatchOptions { //! case_sensitive: false, @@ -51,7 +52,7 @@ //! require_literal_leading_dot: false, //! recursive_match_hidden_dir: true, //! }; -//! for entry in glob_with("local/*a*", options).unwrap() { +//! for entry in glob_with("local/*a*", options, Signals::empty()).unwrap() { //! if let Ok(path) = entry { //! println!("{:?}", path.display()) //! } @@ -72,6 +73,7 @@ extern crate doc_comment; #[cfg(test)] doctest!("../README.md"); +use nu_protocol::Signals; use std::cmp; use std::cmp::Ordering; use std::error::Error; @@ -102,6 +104,7 @@ pub struct Paths { options: MatchOptions, todo: Vec>, scope: Option, + signals: Signals, } impl Paths { @@ -113,6 +116,7 @@ impl Paths { options: MatchOptions::default(), todo: vec![Ok((path.to_path_buf(), 0))], scope: Some(relative_to.into()), + signals: Signals::empty(), } } } @@ -144,7 +148,7 @@ impl Paths { /// ```rust,no_run /// use nu_glob::glob; /// -/// for entry in glob("/media/pictures/*.jpg").unwrap() { +/// for entry in glob("/media/pictures/*.jpg", None).unwrap() { /// match entry { /// Ok(path) => println!("{:?}", path.display()), /// @@ -169,13 +173,17 @@ impl Paths { /// use nu_glob::glob; /// use std::result::Result; /// -/// for path in glob("/media/pictures/*.jpg").unwrap().filter_map(Result::ok) { +/// for path in glob("/media/pictures/*.jpg", None).unwrap().filter_map(Result::ok) { /// println!("{}", path.display()); /// } /// ``` /// Paths are yielded in alphabetical order. -pub fn glob(pattern: &str) -> Result { - glob_with(pattern, MatchOptions::default()) +pub fn glob(pattern: &str, signals: Option) -> Result { + glob_with( + pattern, + MatchOptions::default(), + signals.unwrap_or(Signals::empty()), + ) } /// Return an iterator that produces all the `Path`s that match the given @@ -191,7 +199,11 @@ pub fn glob(pattern: &str) -> Result { /// passed to this function. /// /// Paths are yielded in alphabetical order. -pub fn glob_with(pattern: &str, options: MatchOptions) -> Result { +pub fn glob_with( + pattern: &str, + options: MatchOptions, + signals: Signals, +) -> Result { #[cfg(windows)] fn check_windows_verbatim(p: &Path) -> bool { match p.components().next() { @@ -253,6 +265,7 @@ pub fn glob_with(pattern: &str, options: MatchOptions) -> Result Result Result { - match glob_with(pattern, options) { + match glob_with(pattern, options, signals) { Ok(mut p) => { p.scope = match p.scope { None => Some(parent.to_path_buf()), @@ -408,7 +423,14 @@ impl Iterator for Paths { // if there's one prefilled result, take it, otherwise fill the todo buffer if self.todo.len() != 1 { - fill_todo(&mut self.todo, &self.dir_patterns, 0, &scope, self.options); + fill_todo( + &mut self.todo, + &self.dir_patterns, + 0, + &scope, + self.options, + &self.signals, + ); } } } @@ -465,6 +487,7 @@ impl Iterator for Paths { next, &path, self.options, + &self.signals, ); if next == self.dir_patterns.len() - 1 { @@ -516,6 +539,7 @@ impl Iterator for Paths { idx + 1, &path, self.options, + &self.signals, ); } } @@ -905,6 +929,7 @@ fn fill_todo( idx: usize, path: &Path, options: MatchOptions, + signals: &Signals, ) { // convert a pattern that's just many Char(_) to a string fn pattern_as_str(pattern: &Pattern) -> Option { @@ -926,7 +951,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); + fill_todo(todo, patterns, idx + 1, &next_path, options, signals); } }; @@ -957,6 +982,9 @@ fn fill_todo( None if is_dir => { let dirs = fs::read_dir(path).and_then(|d| { d.map(|e| { + if signals.interrupted() { + return Err(io::Error::from(io::ErrorKind::Interrupted)); + } e.map(|e| { if curdir { PathBuf::from( @@ -1113,9 +1141,15 @@ impl Default for MatchOptions { #[cfg(test)] mod test { - use super::{glob, MatchOptions, Pattern}; + use crate::{Paths, PatternError}; + + use super::{glob as glob_with_signals, MatchOptions, Pattern}; use std::path::Path; + fn glob(pattern: &str) -> Result { + glob_with_signals(pattern, None) + } + #[test] fn test_pattern_from_str() { assert!("a*b".parse::().unwrap().matches("a_b")); diff --git a/crates/nu-lsp/src/workspace.rs b/crates/nu-lsp/src/workspace.rs index 65095bfe0b..f7d99c4c41 100644 --- a/crates/nu-lsp/src/workspace.rs +++ b/crates/nu-lsp/src/workspace.rs @@ -42,7 +42,7 @@ fn find_nu_scripts_in_folder(folder_uri: &Uri) -> Result { return Err(miette!("\nworkspace folder does not exist.")); } let pattern = format!("{}/**/*.nu", path.to_string_lossy()); - nu_glob::glob(&pattern).into_diagnostic() + nu_glob::glob(&pattern, None).into_diagnostic() } impl LanguageServer { diff --git a/crates/nu-test-support/src/playground/play.rs b/crates/nu-test-support/src/playground/play.rs index cb66f2bcb5..60b6c7f9f5 100644 --- a/crates/nu-test-support/src/playground/play.rs +++ b/crates/nu-test-support/src/playground/play.rs @@ -231,7 +231,7 @@ impl Playground<'_> { } pub fn glob_vec(pattern: &str) -> Vec { - let glob = glob(pattern); + let glob = glob(pattern, None); glob.expect("invalid pattern") .map(|path| {