From 2f08d8d8b320c5599838d2ea9f09ad37ac6c799d Mon Sep 17 00:00:00 2001 From: mo8it Date: Sat, 24 Feb 2024 23:34:00 +0100 Subject: [PATCH 01/15] Deduplicate building a walk builder from config --- Cargo.lock | 1 + Cargo.toml | 1 + helix-term/Cargo.toml | 2 +- helix-term/src/commands.rs | 20 +++++--------------- helix-term/src/ui/mod.rs | 12 ++---------- helix-view/Cargo.toml | 1 + helix-view/src/editor.rs | 20 ++++++++++++++++++++ 7 files changed, 31 insertions(+), 26 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2b8a25c85dff..1662c949e9af 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1443,6 +1443,7 @@ dependencies = [ "helix-stdx", "helix-tui", "helix-vcs", + "ignore", "libc", "log", "once_cell", diff --git a/Cargo.toml b/Cargo.toml index 91f6e7cae498..ff750b0a6f2c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -39,6 +39,7 @@ package.helix-term.opt-level = 2 [workspace.dependencies] tree-sitter = { version = "0.20", git = "https://github.com/helix-editor/tree-sitter", rev = "660481dbf71413eba5a928b0b0ab8da50c1109e0" } nucleo = "0.2.0" +ignore = "0.4" [workspace.package] version = "23.10.0" diff --git a/helix-term/Cargo.toml b/helix-term/Cargo.toml index 1e21ec1610cd..85486309453f 100644 --- a/helix-term/Cargo.toml +++ b/helix-term/Cargo.toml @@ -51,7 +51,7 @@ log = "0.4" # File picker nucleo.workspace = true -ignore = "0.4" +ignore.workspace = true # markdown doc rendering pulldown-cmark = { version = "0.10", default-features = false } # file type detection diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 51a1ede9b7e0..fab73594539c 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -74,7 +74,7 @@ use url::Url; use grep_regex::RegexMatcherBuilder; use grep_searcher::{sinks, BinaryDetection, SearcherBuilder}; -use ignore::{DirEntry, WalkBuilder, WalkState}; +use ignore::{DirEntry, WalkState}; pub type OnKeyCallback = Box; @@ -2281,26 +2281,16 @@ fn global_search(cx: &mut Context) { .canonicalize() .unwrap_or_else(|_| search_root.clone()); let injector_ = injector.clone(); + let mut walk_builder = file_picker_config.walk_builder(search_root); std::thread::spawn(move || { let searcher = SearcherBuilder::new() .binary_detection(BinaryDetection::quit(b'\x00')) .build(); - let mut walk_builder = WalkBuilder::new(search_root); - - walk_builder - .hidden(file_picker_config.hidden) - .parents(file_picker_config.parents) - .ignore(file_picker_config.ignore) - .follow_links(file_picker_config.follow_symlinks) - .git_ignore(file_picker_config.git_ignore) - .git_global(file_picker_config.git_global) - .git_exclude(file_picker_config.git_exclude) - .max_depth(file_picker_config.max_depth) - .filter_entry(move |entry| { - filter_picker_entry(entry, &absolute_root, dedup_symlinks) - }); + walk_builder.filter_entry(move |entry| { + filter_picker_entry(entry, &absolute_root, dedup_symlinks) + }); walk_builder .add_custom_ignore_filename(helix_loader::config_dir().join("ignore")); diff --git a/helix-term/src/ui/mod.rs b/helix-term/src/ui/mod.rs index 0873116cb464..7fd9149e288a 100644 --- a/helix-term/src/ui/mod.rs +++ b/helix-term/src/ui/mod.rs @@ -156,7 +156,7 @@ pub fn regex_prompt( } pub fn file_picker(root: PathBuf, config: &helix_view::editor::Config) -> Picker { - use ignore::{types::TypesBuilder, WalkBuilder}; + use ignore::types::TypesBuilder; use std::time::Instant; let now = Instant::now(); @@ -164,17 +164,9 @@ pub fn file_picker(root: PathBuf, config: &helix_view::editor::Config) -> Picker let dedup_symlinks = config.file_picker.deduplicate_links; let absolute_root = root.canonicalize().unwrap_or_else(|_| root.clone()); - let mut walk_builder = WalkBuilder::new(&root); + let mut walk_builder = config.file_picker.walk_builder(&root); walk_builder - .hidden(config.file_picker.hidden) - .parents(config.file_picker.parents) - .ignore(config.file_picker.ignore) - .follow_links(config.file_picker.follow_symlinks) - .git_ignore(config.file_picker.git_ignore) - .git_global(config.file_picker.git_global) - .git_exclude(config.file_picker.git_exclude) .sort_by_file_name(|name1, name2| name1.cmp(name2)) - .max_depth(config.file_picker.max_depth) .filter_entry(move |entry| filter_picker_entry(entry, &absolute_root, dedup_symlinks)); walk_builder.add_custom_ignore_filename(helix_loader::config_dir().join("ignore")); diff --git a/helix-view/Cargo.toml b/helix-view/Cargo.toml index b1b444f90ae4..88ed81e431b3 100644 --- a/helix-view/Cargo.toml +++ b/helix-view/Cargo.toml @@ -48,6 +48,7 @@ log = "~0.4" parking_lot = "0.12.1" +ignore.workspace = true [target.'cfg(windows)'.dependencies] clipboard-win = { version = "5.1", features = ["std"] } diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index fffbe6207f06..f3d5527eeb34 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -17,6 +17,7 @@ use helix_vcs::DiffProviderRegistry; use futures_util::stream::select_all::SelectAll; use futures_util::{future, StreamExt}; use helix_lsp::Call; +use ignore::WalkBuilder; use tokio_stream::wrappers::UnboundedReceiverStream; use std::{ @@ -212,6 +213,25 @@ impl Default for FilePickerConfig { } } +impl FilePickerConfig { + pub fn walk_builder

(&self, path: P) -> WalkBuilder + where + P: AsRef, + { + let mut builder = WalkBuilder::new(path); + builder + .hidden(self.hidden) + .follow_links(self.follow_symlinks) + .parents(self.parents) + .ignore(self.ignore) + .git_ignore(self.git_ignore) + .git_global(self.git_global) + .git_exclude(self.git_exclude) + .max_depth(self.max_depth); + builder + } +} + #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "kebab-case", default, deny_unknown_fields)] pub struct Config { From 6c43c2a7bfaab8c9c0084f2d06d13b6b0ca76863 Mon Sep 17 00:00:00 2001 From: mo8it Date: Sun, 25 Feb 2024 14:36:29 +0100 Subject: [PATCH 02/15] Add globset to helix-term --- Cargo.lock | 1 + Cargo.toml | 1 + helix-core/Cargo.toml | 2 +- helix-lsp/Cargo.toml | 2 +- helix-term/Cargo.toml | 1 + 5 files changed, 5 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1662c949e9af..f7d14270a24c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1360,6 +1360,7 @@ dependencies = [ "crossterm", "fern", "futures-util", + "globset", "grep-regex", "grep-searcher", "helix-core", diff --git a/Cargo.toml b/Cargo.toml index ff750b0a6f2c..64e33d731653 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -40,6 +40,7 @@ package.helix-term.opt-level = 2 tree-sitter = { version = "0.20", git = "https://github.com/helix-editor/tree-sitter", rev = "660481dbf71413eba5a928b0b0ab8da50c1109e0" } nucleo = "0.2.0" ignore = "0.4" +globset = "0.4.14" [workspace.package] version = "23.10.0" diff --git a/helix-core/Cargo.toml b/helix-core/Cargo.toml index 0b0dd7452864..6e29e8c48023 100644 --- a/helix-core/Cargo.toml +++ b/helix-core/Cargo.toml @@ -52,7 +52,7 @@ textwrap = "0.16.1" nucleo.workspace = true parking_lot = "0.12" -globset = "0.4.14" +globset.workspace = true [dev-dependencies] quickcheck = { version = "1", default-features = false } diff --git a/helix-lsp/Cargo.toml b/helix-lsp/Cargo.toml index 4284b0520930..5995cab59a97 100644 --- a/helix-lsp/Cargo.toml +++ b/helix-lsp/Cargo.toml @@ -21,7 +21,7 @@ helix-parsec = { path = "../helix-parsec" } anyhow = "1.0" futures-executor = "0.3" futures-util = { version = "0.3", features = ["std", "async-await"], default-features = false } -globset = "0.4.14" +globset.workspace = true log = "0.4" lsp-types = { version = "0.95" } serde = { version = "1.0", features = ["derive"] } diff --git a/helix-term/Cargo.toml b/helix-term/Cargo.toml index 85486309453f..3f3596020ee7 100644 --- a/helix-term/Cargo.toml +++ b/helix-term/Cargo.toml @@ -72,6 +72,7 @@ grep-regex = "0.1.12" grep-searcher = "0.1.13" [target.'cfg(not(windows))'.dependencies] # https://github.com/vorner/signal-hook/issues/100 +globset.workspace = true signal-hook-tokio = { version = "0.3", features = ["futures-v0_3"] } libc = "0.2.153" From f3c9dd6caa6c67e018d5f8524ac8441a8a63e017 Mon Sep 17 00:00:00 2001 From: mo8it Date: Sun, 25 Feb 2024 14:38:20 +0100 Subject: [PATCH 03/15] Add globbing support to the open command --- helix-term/src/commands/typed.rs | 81 +++++++++++++++++++++++++++++--- 1 file changed, 74 insertions(+), 7 deletions(-) diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index 3d7ea3fc8b36..7f90c12ea74b 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -5,9 +5,11 @@ use crate::job::Job; use super::*; +use globset::{Glob, GlobBuilder, GlobSet}; use helix_core::fuzzy::fuzzy_match; use helix_core::indent::MAX_INDENT; use helix_core::{encoding, line_ending, shellwords::Shellwords}; +use helix_stdx::env::current_working_dir; use helix_view::document::DEFAULT_LANGUAGE_NAME; use helix_view::editor::{Action, CloseError, ConfigEvent}; use serde_json::Value; @@ -108,16 +110,16 @@ fn open(cx: &mut compositor::Context, args: &[Cow], event: PromptEvent) -> } ensure!(!args.is_empty(), "wrong argument count"); - for arg in args { - let (path, pos) = args::parse_file(arg); - let path = helix_stdx::path::expand_tilde(path); - // If the path is a directory, open a file picker on that directory and update the status - // message - if let Ok(true) = std::fs::canonicalize(&path).map(|p| p.is_dir()) { + + fn open(cx: &mut compositor::Context, path: Cow, pos: Position) -> anyhow::Result<()> { + // If the path is a directory, open a file picker on that directory and update + // the status message + if path.is_dir() { + let path = path.into_owned(); let callback = async move { let call: job::Callback = job::Callback::EditorCompositor(Box::new( move |editor: &mut Editor, compositor: &mut Compositor| { - let picker = ui::file_picker(path.into_owned(), &editor.config()); + let picker = ui::file_picker(path, &editor.config()); compositor.push(Box::new(overlaid(picker))); }, )); @@ -133,6 +135,71 @@ fn open(cx: &mut compositor::Context, args: &[Cow], event: PromptEvent) -> // does not affect opening a buffer without pos align_view(doc, view, Align::Center); } + Ok(()) + } + + fn glob(path: &Path) -> anyhow::Result { + let path_str = path.to_str().context("invalid unicode")?; + GlobBuilder::new(path_str) + .literal_separator(true) + .empty_alternates(true) + .build() + .context("invalid glob") + } + + for arg in args { + let (path, pos) = args::parse_file(arg); + let path = helix_stdx::path::canonicalize(path); + + if path.exists() { + // Shortcut for opening a path without globbing + open(cx, Cow::Owned(path), pos)?; + continue; + } + + let mut glob_set_builder = GlobSet::builder(); + glob_set_builder.add(glob(&path)?); + + let mut root = None; + let mut comps = path.components(); + + // Iterate over all parents + while comps.next_back().is_some() { + let parent = comps.as_path(); + if parent.as_os_str().is_empty() { + // No parents left + break; + } + + // Add each parent as a glob for filtering in the recursive walker + glob_set_builder.add(glob(parent)?); + + if root.is_none() && parent.exists() { + // Found the first parent that exists + root = Some(parent.to_path_buf()); + } + } + + let glob_set = glob_set_builder.build().context("invalid glob")?; + let root = root.unwrap_or_else(current_working_dir); + + let mut walk = cx + .editor + .config() + .file_picker + .walk_builder(root) + .filter_entry(move |entry| glob_set.is_match(entry.path())) + .build(); + + // Ignore the root directory which is yielded first + if walk.next().is_none() { + continue; + } + + for entry in walk { + let entry = entry.context("recursive walking failed")?; + open(cx, Cow::Borrowed(entry.path()), pos)?; + } } Ok(()) } From f1c837b6a04f58d9d0aec5aad97196e5168eb0b1 Mon Sep 17 00:00:00 2001 From: mo8it Date: Sun, 25 Feb 2024 14:57:12 +0100 Subject: [PATCH 04/15] globset is not only for Windows --- helix-term/Cargo.toml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/helix-term/Cargo.toml b/helix-term/Cargo.toml index 3f3596020ee7..ad1366a59c8f 100644 --- a/helix-term/Cargo.toml +++ b/helix-term/Cargo.toml @@ -71,8 +71,9 @@ serde = { version = "1.0", features = ["derive"] } grep-regex = "0.1.12" grep-searcher = "0.1.13" -[target.'cfg(not(windows))'.dependencies] # https://github.com/vorner/signal-hook/issues/100 globset.workspace = true + +[target.'cfg(not(windows))'.dependencies] # https://github.com/vorner/signal-hook/issues/100 signal-hook-tokio = { version = "0.3", features = ["futures-v0_3"] } libc = "0.2.153" From 768b8399b08ac43582e5371fdd20b1fd88401e8f Mon Sep 17 00:00:00 2001 From: mo8it Date: Thu, 29 Feb 2024 22:47:35 +0100 Subject: [PATCH 05/15] Don't open dirs when globbing and set an upper limit --- helix-term/src/commands/typed.rs | 74 +++++++++++++++++++------------- 1 file changed, 45 insertions(+), 29 deletions(-) diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index 7f90c12ea74b..1ac058e7832a 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -15,6 +15,10 @@ use helix_view::editor::{Action, CloseError, ConfigEvent}; use serde_json::Value; use ui::completers::{self, Completer}; +// The maximum number of files to open with globbing to avoid freezing while trying to open too many files. +// It is kind of arbitrary and can be raised or offered as a configuration option. +const GLOBBING_MAX_N_FILES: usize = 32; + #[derive(Clone)] pub struct TypableCommand { pub name: &'static str, @@ -111,30 +115,13 @@ fn open(cx: &mut compositor::Context, args: &[Cow], event: PromptEvent) -> ensure!(!args.is_empty(), "wrong argument count"); - fn open(cx: &mut compositor::Context, path: Cow, pos: Position) -> anyhow::Result<()> { - // If the path is a directory, open a file picker on that directory and update - // the status message - if path.is_dir() { - let path = path.into_owned(); - let callback = async move { - let call: job::Callback = job::Callback::EditorCompositor(Box::new( - move |editor: &mut Editor, compositor: &mut Compositor| { - let picker = ui::file_picker(path, &editor.config()); - compositor.push(Box::new(overlaid(picker))); - }, - )); - Ok(call) - }; - cx.jobs.callback(callback); - } else { - // Otherwise, just open the file - let _ = cx.editor.open(&path, Action::Replace)?; - let (view, doc) = current!(cx.editor); - let pos = Selection::point(pos_at_coords(doc.text().slice(..), pos, true)); - doc.set_selection(view.id, pos); - // does not affect opening a buffer without pos - align_view(doc, view, Align::Center); - } + fn open_file(cx: &mut compositor::Context, path: &Path, pos: Position) -> anyhow::Result<()> { + let _ = cx.editor.open(path, Action::Replace)?; + let (view, doc) = current!(cx.editor); + let pos = Selection::point(pos_at_coords(doc.text().slice(..), pos, true)); + doc.set_selection(view.id, pos); + // does not affect opening a buffer without pos + align_view(doc, view, Align::Center); Ok(()) } @@ -150,10 +137,24 @@ fn open(cx: &mut compositor::Context, args: &[Cow], event: PromptEvent) -> for arg in args { let (path, pos) = args::parse_file(arg); let path = helix_stdx::path::canonicalize(path); - - if path.exists() { + if let Ok(metadata) = path.metadata() { + // Path exists // Shortcut for opening a path without globbing - open(cx, Cow::Owned(path), pos)?; + if metadata.is_dir() { + // If the path is a directory, open a file picker on that directory + let callback = async move { + let call: job::Callback = job::Callback::EditorCompositor(Box::new( + move |editor: &mut Editor, compositor: &mut Compositor| { + let picker = ui::file_picker(path, &editor.config()); + compositor.push(Box::new(overlaid(picker))); + }, + )); + Ok(call) + }; + cx.jobs.callback(callback); + } else { + open_file(cx, &path, pos)?; + } continue; } @@ -191,14 +192,29 @@ fn open(cx: &mut compositor::Context, args: &[Cow], event: PromptEvent) -> .filter_entry(move |entry| glob_set.is_match(entry.path())) .build(); - // Ignore the root directory which is yielded first + // Call `next` to ignore the root directory which is yielded first if walk.next().is_none() { continue; } + let mut to_open = Vec::with_capacity(GLOBBING_MAX_N_FILES); for entry in walk { let entry = entry.context("recursive walking failed")?; - open(cx, Cow::Borrowed(entry.path()), pos)?; + let Some(file_type) = entry.file_type() else { + // Entry is stdin + continue; + }; + // Don't open directories when globbing + if file_type.is_dir() { + continue; + } + if to_open.len() == GLOBBING_MAX_N_FILES { + bail!("tried to open more than {GLOBBING_MAX_N_FILES} files at once"); + } + to_open.push(entry.into_path()); + } + for path in to_open { + open_file(cx, &path, pos)?; } } Ok(()) From de3ef339591c8496ea425728505c2f36c35531d7 Mon Sep 17 00:00:00 2001 From: mo8it Date: Sat, 23 Mar 2024 17:16:59 +0100 Subject: [PATCH 06/15] Open a new file if nothing was found after globbing --- helix-term/src/commands/typed.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index 41cacde445f1..54fb91e00730 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -213,6 +213,12 @@ fn open(cx: &mut compositor::Context, args: &[Cow], event: PromptEvent) -> } to_open.push(entry.into_path()); } + + if to_open.is_empty() { + open_file(cx, &path, pos)?; + continue; + } + for path in to_open { open_file(cx, &path, pos)?; } From 08f4ab66409a824b0bc50b74495e12a5c65c638e Mon Sep 17 00:00:00 2001 From: mo8it Date: Sat, 23 Mar 2024 17:35:06 +0100 Subject: [PATCH 07/15] Add comment --- helix-term/src/commands/typed.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index 54fb91e00730..eecbcddb63db 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -215,6 +215,8 @@ fn open(cx: &mut compositor::Context, args: &[Cow], event: PromptEvent) -> } if to_open.is_empty() { + // Nothing found to open after globbing. + // Open a new file. open_file(cx, &path, pos)?; continue; } From 76ab99730d83bdce62d139f3510f100a5bf99362 Mon Sep 17 00:00:00 2001 From: mo8it Date: Thu, 9 May 2024 18:32:35 +0200 Subject: [PATCH 08/15] Raise the max number of files while globbing --- helix-term/src/commands/typed.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index eecbcddb63db..72ed4eab8961 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -16,8 +16,7 @@ use serde_json::Value; use ui::completers::{self, Completer}; // The maximum number of files to open with globbing to avoid freezing while trying to open too many files. -// It is kind of arbitrary and can be raised or offered as a configuration option. -const GLOBBING_MAX_N_FILES: usize = 32; +const GLOBBING_MAX_N_FILES: usize = 64; #[derive(Clone)] pub struct TypableCommand { From 5be6c211c542ec9de0086971709b4643b37bb57a Mon Sep 17 00:00:00 2001 From: mo8it Date: Thu, 9 May 2024 18:34:15 +0200 Subject: [PATCH 09/15] Don't use inner functions --- helix-term/src/commands/typed.rs | 39 ++++++++++++++++---------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index 72ed4eab8961..ec3b64790f9e 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -107,6 +107,25 @@ fn force_quit( Ok(()) } +fn open_file(cx: &mut compositor::Context, path: &Path, pos: Position) -> anyhow::Result<()> { + let _ = cx.editor.open(path, Action::Replace)?; + let (view, doc) = current!(cx.editor); + let pos = Selection::point(pos_at_coords(doc.text().slice(..), pos, true)); + doc.set_selection(view.id, pos); + // does not affect opening a buffer without pos + align_view(doc, view, Align::Center); + Ok(()) +} + +fn glob(path: &Path) -> anyhow::Result { + let path_str = path.to_str().context("invalid unicode")?; + GlobBuilder::new(path_str) + .literal_separator(true) + .empty_alternates(true) + .build() + .context("invalid glob") +} + fn open(cx: &mut compositor::Context, args: &[Cow], event: PromptEvent) -> anyhow::Result<()> { if event != PromptEvent::Validate { return Ok(()); @@ -114,25 +133,6 @@ fn open(cx: &mut compositor::Context, args: &[Cow], event: PromptEvent) -> ensure!(!args.is_empty(), "wrong argument count"); - fn open_file(cx: &mut compositor::Context, path: &Path, pos: Position) -> anyhow::Result<()> { - let _ = cx.editor.open(path, Action::Replace)?; - let (view, doc) = current!(cx.editor); - let pos = Selection::point(pos_at_coords(doc.text().slice(..), pos, true)); - doc.set_selection(view.id, pos); - // does not affect opening a buffer without pos - align_view(doc, view, Align::Center); - Ok(()) - } - - fn glob(path: &Path) -> anyhow::Result { - let path_str = path.to_str().context("invalid unicode")?; - GlobBuilder::new(path_str) - .literal_separator(true) - .empty_alternates(true) - .build() - .context("invalid glob") - } - for arg in args { let (path, pos) = args::parse_file(arg); let path = helix_stdx::path::canonicalize(path); @@ -224,6 +224,7 @@ fn open(cx: &mut compositor::Context, args: &[Cow], event: PromptEvent) -> open_file(cx, &path, pos)?; } } + Ok(()) } From bb4fef67bd9a2c806444712ece2ab4267621dd1d Mon Sep 17 00:00:00 2001 From: mo8it Date: Fri, 10 May 2024 00:51:32 +0200 Subject: [PATCH 10/15] Don't ignore any regular files when globbing --- helix-term/src/commands/typed.rs | 100 ++++++++++++++++++------------- 1 file changed, 59 insertions(+), 41 deletions(-) diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index ec3b64790f9e..652c3aa70446 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -1,11 +1,12 @@ use std::fmt::Write; +use std::fs; use std::ops::Deref; use crate::job::Job; use super::*; -use globset::{Glob, GlobBuilder, GlobSet}; +use globset::{Glob, GlobBuilder, GlobSetBuilder}; use helix_core::fuzzy::fuzzy_match; use helix_core::indent::MAX_INDENT; use helix_core::{encoding, line_ending, shellwords::Shellwords}; @@ -136,10 +137,12 @@ fn open(cx: &mut compositor::Context, args: &[Cow], event: PromptEvent) -> for arg in args { let (path, pos) = args::parse_file(arg); let path = helix_stdx::path::canonicalize(path); - if let Ok(metadata) = path.metadata() { + + // Shortcut for opening an existing path without globbing + if let Ok(metadata) = fs::metadata(&path) { // Path exists - // Shortcut for opening a path without globbing - if metadata.is_dir() { + let file_type = metadata.file_type(); + if file_type.is_dir() { // If the path is a directory, open a file picker on that directory let callback = async move { let call: job::Callback = job::Callback::EditorCompositor(Box::new( @@ -151,71 +154,86 @@ fn open(cx: &mut compositor::Context, args: &[Cow], event: PromptEvent) -> Ok(call) }; cx.jobs.callback(callback); - } else { + } else if file_type.is_file() { open_file(cx, &path, pos)?; + } else { + bail!("{} is not a regular file", path.display()); } continue; } - let mut glob_set_builder = GlobSet::builder(); - glob_set_builder.add(glob(&path)?); + let file_glob_set = GlobSetBuilder::new() + .add(glob(&path)?) + .build() + .context("invalid glob")?; + let mut parent_dirs_glob_set_builder = GlobSetBuilder::new(); let mut root = None; let mut comps = path.components(); // Iterate over all parents while comps.next_back().is_some() { let parent = comps.as_path(); - if parent.as_os_str().is_empty() { - // No parents left - break; - } - // Add each parent as a glob for filtering in the recursive walker - glob_set_builder.add(glob(parent)?); + parent_dirs_glob_set_builder.add(glob(parent)?); if root.is_none() && parent.exists() { // Found the first parent that exists root = Some(parent.to_path_buf()); + break; } } - let glob_set = glob_set_builder.build().context("invalid glob")?; - let root = root.unwrap_or_else(current_working_dir); - - let mut walk = cx - .editor - .config() - .file_picker - .walk_builder(root) - .filter_entry(move |entry| glob_set.is_match(entry.path())) - .build(); + let parent_dirs_glob_set = parent_dirs_glob_set_builder + .build() + .context("invalid glob")?; - // Call `next` to ignore the root directory which is yielded first - if walk.next().is_none() { + let root = root.unwrap_or_else(current_working_dir); + let Ok(dir_reader) = fs::read_dir(root) else { continue; - } + }; + let mut dir_reader_stack = Vec::with_capacity(32); + dir_reader_stack.push(dir_reader); let mut to_open = Vec::with_capacity(GLOBBING_MAX_N_FILES); - for entry in walk { - let entry = entry.context("recursive walking failed")?; - let Some(file_type) = entry.file_type() else { - // Entry is stdin - continue; - }; - // Don't open directories when globbing - if file_type.is_dir() { - continue; - } - if to_open.len() == GLOBBING_MAX_N_FILES { - bail!("tried to open more than {GLOBBING_MAX_N_FILES} files at once"); + + // Recursion with a "stack" (on the heap) instead of a recursive function + 'outer: while let Some(dir_reader) = dir_reader_stack.last_mut() { + for entry in dir_reader { + let Ok(entry) = entry else { + continue; + }; + + let path = entry.path(); + let Ok(metadata) = fs::metadata(&path) else { + continue; + }; + let file_type = metadata.file_type(); + + if file_type.is_dir() { + if !parent_dirs_glob_set.is_match(&path) { + continue; + } + let Ok(dir_reader) = fs::read_dir(path) else { + continue; + }; + dir_reader_stack.push(dir_reader); + continue 'outer; + } + + if file_type.is_file() && file_glob_set.is_match(&path) { + if to_open.len() == GLOBBING_MAX_N_FILES { + bail!("tried to open more than {GLOBBING_MAX_N_FILES} files at once"); + } + to_open.push(path); + } + // Can't be a symlink because `fs::metadata` traverses symlinks } - to_open.push(entry.into_path()); + dir_reader_stack.pop(); } if to_open.is_empty() { - // Nothing found to open after globbing. - // Open a new file. + // Nothing found to open after globbing. Open a new file open_file(cx, &path, pos)?; continue; } From 3d1716fe1a147ef31a7dd01fa0afd06c39668be0 Mon Sep 17 00:00:00 2001 From: mo8it Date: Fri, 10 May 2024 01:04:09 +0200 Subject: [PATCH 11/15] Format --- helix-term/src/commands/typed.rs | 2 +- helix-view/src/editor.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index 494123d1d20b..807f3c7f7530 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -8,9 +8,9 @@ use crate::job::Job; use super::*; use globset::{Glob, GlobBuilder, GlobSetBuilder}; -use helix_core::{encoding, line_ending, shellwords::Shellwords}; use helix_core::fuzzy::fuzzy_match; use helix_core::indent::MAX_INDENT; +use helix_core::{encoding, line_ending, shellwords::Shellwords}; use helix_stdx::env::current_working_dir; use helix_view::document::{read_to_string, DEFAULT_LANGUAGE_NAME}; use helix_view::editor::{Action, CloseError, ConfigEvent}; diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index f22f418962a8..68f3ddddf84b 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -14,8 +14,8 @@ use crate::{ use dap::StackFrame; use helix_vcs::DiffProviderRegistry; -use futures_util::{future, StreamExt}; use futures_util::stream::select_all::SelectAll; +use futures_util::{future, StreamExt}; use helix_lsp::{Call, LanguageServerId}; use ignore::WalkBuilder; use tokio_stream::wrappers::UnboundedReceiverStream; From fc7a1e75155482d930ee42c1a84be8804a581e2e Mon Sep 17 00:00:00 2001 From: mo8it Date: Fri, 10 May 2024 02:17:05 +0200 Subject: [PATCH 12/15] A root must be found if the path is not / --- helix-term/src/commands/typed.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index 807f3c7f7530..fa3a9439e55e 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -178,18 +178,18 @@ fn open(cx: &mut compositor::Context, args: &[Cow], event: PromptEvent) -> // Add each parent as a glob for filtering in the recursive walker parent_dirs_glob_set_builder.add(glob(parent)?); - if root.is_none() && parent.exists() { + if parent.exists() { // Found the first parent that exists - root = Some(parent.to_path_buf()); + root = Some(parent); break; } } + let root = root.context("invalid glob")?; let parent_dirs_glob_set = parent_dirs_glob_set_builder .build() .context("invalid glob")?; - let root = root.unwrap_or_else(current_working_dir); let Ok(dir_reader) = fs::read_dir(root) else { continue; }; From a3f44f4aa0d8b788f0b9692793b102c3e36a2449 Mon Sep 17 00:00:00 2001 From: mo8it Date: Fri, 10 May 2024 02:20:42 +0200 Subject: [PATCH 13/15] Remove unused import --- helix-term/src/commands/typed.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index fa3a9439e55e..70899177612c 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -11,7 +11,6 @@ use globset::{Glob, GlobBuilder, GlobSetBuilder}; use helix_core::fuzzy::fuzzy_match; use helix_core::indent::MAX_INDENT; use helix_core::{encoding, line_ending, shellwords::Shellwords}; -use helix_stdx::env::current_working_dir; use helix_view::document::{read_to_string, DEFAULT_LANGUAGE_NAME}; use helix_view::editor::{Action, CloseError, ConfigEvent}; use serde_json::Value; From 404548464a8bc16f05ed940ca86366a87295cca0 Mon Sep 17 00:00:00 2001 From: mo8it Date: Fri, 10 May 2024 18:24:56 +0200 Subject: [PATCH 14/15] Don't write your own walker :) --- helix-term/src/commands/typed.rs | 109 +++++++++++++++---------------- 1 file changed, 53 insertions(+), 56 deletions(-) diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index 70899177612c..8e2d98267227 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -2,17 +2,20 @@ use std::fmt::Write; use std::fs; use std::io::BufReader; use std::ops::Deref; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::Mutex; use crate::job::Job; use super::*; -use globset::{Glob, GlobBuilder, GlobSetBuilder}; +use globset::{GlobBuilder, GlobSetBuilder}; use helix_core::fuzzy::fuzzy_match; use helix_core::indent::MAX_INDENT; use helix_core::{encoding, line_ending, shellwords::Shellwords}; use helix_view::document::{read_to_string, DEFAULT_LANGUAGE_NAME}; use helix_view::editor::{Action, CloseError, ConfigEvent}; +use ignore::WalkBuilder; use serde_json::Value; use ui::completers::{self, Completer}; @@ -118,15 +121,6 @@ fn open_file(cx: &mut compositor::Context, path: &Path, pos: Position) -> anyhow Ok(()) } -fn glob(path: &Path) -> anyhow::Result { - let path_str = path.to_str().context("invalid unicode")?; - GlobBuilder::new(path_str) - .literal_separator(true) - .empty_alternates(true) - .build() - .context("invalid glob") -} - fn open(cx: &mut compositor::Context, args: &[Cow], event: PromptEvent) -> anyhow::Result<()> { if event != PromptEvent::Validate { return Ok(()); @@ -162,20 +156,25 @@ fn open(cx: &mut compositor::Context, args: &[Cow], event: PromptEvent) -> continue; } - let file_glob_set = GlobSetBuilder::new() - .add(glob(&path)?) + let glob = GlobBuilder::new(path.to_str().context("invalid unicode")?) + .literal_separator(true) + .empty_alternates(true) + .build() + .context("invalid glob")?; + // Using a glob set instead of `compile_matcher` because the single matcher is always + // a regex matcher. A glob set tries other strategies first which can be more efficient. + // Example: `**/FILENAME` only compares the base name instead of matching with regex. + let glob_set = GlobSetBuilder::new() + .add(glob) .build() .context("invalid glob")?; - let mut parent_dirs_glob_set_builder = GlobSetBuilder::new(); let mut root = None; let mut comps = path.components(); // Iterate over all parents while comps.next_back().is_some() { let parent = comps.as_path(); - // Add each parent as a glob for filtering in the recursive walker - parent_dirs_glob_set_builder.add(glob(parent)?); if parent.exists() { // Found the first parent that exists @@ -185,53 +184,51 @@ fn open(cx: &mut compositor::Context, args: &[Cow], event: PromptEvent) -> } let root = root.context("invalid glob")?; - let parent_dirs_glob_set = parent_dirs_glob_set_builder - .build() - .context("invalid glob")?; - - let Ok(dir_reader) = fs::read_dir(root) else { - continue; - }; - let mut dir_reader_stack = Vec::with_capacity(32); - dir_reader_stack.push(dir_reader); - - let mut to_open = Vec::with_capacity(GLOBBING_MAX_N_FILES); - - // Recursion with a "stack" (on the heap) instead of a recursive function - 'outer: while let Some(dir_reader) = dir_reader_stack.last_mut() { - for entry in dir_reader { - let Ok(entry) = entry else { - continue; - }; - - let path = entry.path(); - let Ok(metadata) = fs::metadata(&path) else { - continue; - }; - let file_type = metadata.file_type(); + let to_open = Mutex::new(Vec::with_capacity(GLOBBING_MAX_N_FILES)); + let exceeded_max_n_files = AtomicBool::new(false); + + WalkBuilder::new(root) + // Traversing symlinks makes the time explode. + // Not even sure if non-trivial cycles are detected. + // Because we don't have a timeout, we should better ignore symlinks. + .follow_links(false) + .standard_filters(false) + .build_parallel() + .run(|| { + Box::new(|entry| { + if exceeded_max_n_files.load(Ordering::Relaxed) { + return WalkState::Quit; + } - if file_type.is_dir() { - if !parent_dirs_glob_set.is_match(&path) { - continue; + let Ok(entry) = entry else { + return WalkState::Continue; + }; + if !glob_set.is_match(entry.path()) { + return WalkState::Continue; } - let Ok(dir_reader) = fs::read_dir(path) else { - continue; + let Ok(metadata) = entry.metadata() else { + return WalkState::Continue; }; - dir_reader_stack.push(dir_reader); - continue 'outer; - } - if file_type.is_file() && file_glob_set.is_match(&path) { - if to_open.len() == GLOBBING_MAX_N_FILES { - bail!("tried to open more than {GLOBBING_MAX_N_FILES} files at once"); + if metadata.is_file() { + let Ok(mut to_open) = to_open.lock() else { + return WalkState::Quit; + }; + if to_open.len() == GLOBBING_MAX_N_FILES { + exceeded_max_n_files.store(true, Ordering::Relaxed); + return WalkState::Quit; + } + to_open.push(entry.into_path()); } - to_open.push(path); - } - // Can't be a symlink because `fs::metadata` traverses symlinks - } - dir_reader_stack.pop(); - } + WalkState::Continue + }) + }); + + if exceeded_max_n_files.load(Ordering::Relaxed) { + bail!("tried to open more than {GLOBBING_MAX_N_FILES} files at once"); + } + let to_open = to_open.into_inner().context("walker thread panicked")?; if to_open.is_empty() { // Nothing found to open after globbing. Open a new file open_file(cx, &path, pos)?; From af96b9ba2143df1cd7f7a4a0dff8302bbf17fd9a Mon Sep 17 00:00:00 2001 From: mo8it Date: Fri, 10 May 2024 18:48:28 +0200 Subject: [PATCH 15/15] Open file directly if not a glob --- helix-term/src/commands/typed.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index 8e2d98267227..64d179a521ef 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -156,7 +156,14 @@ fn open(cx: &mut compositor::Context, args: &[Cow], event: PromptEvent) -> continue; } - let glob = GlobBuilder::new(path.to_str().context("invalid unicode")?) + let path_str = path.to_str().context("invalid unicode")?; + if !path_str.as_bytes().iter().any(|c| b"*?{}[]".contains(c)) { + // Not a glob, open the file to avoid unneeded walking of huge directories + open_file(cx, &path, pos)?; + continue; + } + + let glob = GlobBuilder::new(path_str) .literal_separator(true) .empty_alternates(true) .build()