From a92f9c5bae293e108c505ff4a1b2969a7c43695e Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Tue, 24 Jan 2023 00:19:41 +0100 Subject: [PATCH 1/3] hide duplicate symlinks from the picker --- book/src/configuration.md | 2 ++ helix-term/src/commands.rs | 11 +++++++---- helix-term/src/lib.rs | 26 ++++++++++++++++++++++++++ helix-term/src/ui/mod.rs | 19 ++++++++----------- helix-view/src/editor.rs | 3 +++ 5 files changed, 46 insertions(+), 15 deletions(-) diff --git a/book/src/configuration.md b/book/src/configuration.md index ab229f772a1c..3c6aee6840f7 100644 --- a/book/src/configuration.md +++ b/book/src/configuration.md @@ -150,6 +150,8 @@ All git related options are only enabled in a git repository. | Key | Description | Default | |--|--|---------| |`hidden` | Enables ignoring hidden files. | true +|`follow-links` | Follow symlinks instead of ignoring them | true +|`deduplicate-links` | Ignore symlinks that point at files already shown in the picker | true |`parents` | Enables reading ignore files from parent directories. | true |`ignore` | Enables reading `.ignore` files. | true |`git-ignore` | Enables reading `.gitignore` files. | true diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index e773a2c789d8..906bf87b05bc 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -45,6 +45,7 @@ use movement::Movement; use crate::{ args, compositor::{self, Component, Compositor}, + filter_entry, job::Callback, keymap::ReverseKeymap, ui::{self, overlay::overlayed, FilePicker, Picker, Popup, Prompt, PromptEvent}, @@ -1913,6 +1914,11 @@ fn global_search(cx: &mut Context) { let search_root = std::env::current_dir() .expect("Global search error: Failed to get current dir"); + let dedup_symlinks = file_picker_config.deduplicate_links; + let absolute_root = search_root + .canonicalize() + .unwrap_or_else(|_| search_root.clone()); + WalkBuilder::new(search_root) .hidden(file_picker_config.hidden) .parents(file_picker_config.parents) @@ -1922,10 +1928,7 @@ fn global_search(cx: &mut Context) { .git_global(file_picker_config.git_global) .git_exclude(file_picker_config.git_exclude) .max_depth(file_picker_config.max_depth) - // We always want to ignore the .git directory, otherwise if - // `ignore` is turned off above, we end up with a lot of noise - // in our picker. - .filter_entry(|entry| entry.file_name() != ".git") + .filter_entry(move |entry| filter_entry(entry, &absolute_root, dedup_symlinks)) .build_parallel() .run(|| { let mut searcher = searcher.clone(); diff --git a/helix-term/src/lib.rs b/helix-term/src/lib.rs index a945b20dedaf..742c8a98c6fa 100644 --- a/helix-term/src/lib.rs +++ b/helix-term/src/lib.rs @@ -10,6 +10,9 @@ pub mod health; pub mod job; pub mod keymap; pub mod ui; +use std::path::Path; + +use ignore::DirEntry; pub use keymap::macros::*; #[cfg(not(windows))] @@ -22,3 +25,26 @@ fn true_color() -> bool { fn true_color() -> bool { true } + +/// Function used for filtering dir entries in the various file pickers. +/// +/// We always want to ignore the .git directory, otherwise if +/// `ignore` is turned off, we end up with a lot of noise +/// in our picker. +/// We also ignore symlinks that point inside the current directory +/// if `dedup_links` is enabled. +fn filter_entry(entry: &DirEntry, root: &Path, dedup_links: bool) -> bool { + if entry.file_name() != ".git" { + return false; + } + + if dedup_links && entry.path_is_symlink() { + return entry + .path() + .canonicalize() + .ok() + .map_or(false, |path| !path.starts_with(&root)); + } + + true +} diff --git a/helix-term/src/ui/mod.rs b/helix-term/src/ui/mod.rs index eb4807581e8c..f74a840d7930 100644 --- a/helix-term/src/ui/mod.rs +++ b/helix-term/src/ui/mod.rs @@ -14,6 +14,7 @@ mod statusline; mod text; use crate::compositor::{Component, Compositor}; +use crate::filter_entry; use crate::job::{self, Callback}; pub use completion::Completion; pub use editor::EditorView; @@ -162,6 +163,9 @@ pub fn file_picker(root: PathBuf, config: &helix_view::editor::Config) -> FilePi let now = Instant::now(); + let deadup_symlinks = config.file_picker.deduplicate_links; + let absolute_root = root.canonicalize().unwrap_or_else(|_| root.clone()); + let mut walk_builder = WalkBuilder::new(&root); walk_builder .hidden(config.file_picker.hidden) @@ -172,10 +176,7 @@ pub fn file_picker(root: PathBuf, config: &helix_view::editor::Config) -> FilePi .git_global(config.file_picker.git_global) .git_exclude(config.file_picker.git_exclude) .max_depth(config.file_picker.max_depth) - // We always want to ignore the .git directory, otherwise if - // `ignore` is turned off above, we end up with a lot of noise - // in our picker. - .filter_entry(|entry| entry.file_name() != ".git"); + .filter_entry(move |entry| filter_entry(entry, &absolute_root, deadup_symlinks)); // We want to exclude files that the editor can't handle yet let mut type_builder = TypesBuilder::new(); @@ -194,15 +195,11 @@ pub fn file_picker(root: PathBuf, config: &helix_view::editor::Config) -> FilePi // We want files along with their modification date for sorting let files = walk_builder.build().filter_map(|entry| { let entry = entry.ok()?; - // This is faster than entry.path().is_dir() since it uses cached fs::Metadata fetched by ignore/walkdir - let is_dir = entry.file_type().map_or(false, |ft| ft.is_dir()); - if is_dir { - // Will give a false positive if metadata cannot be read (eg. permission error) - None - } else { - Some(entry.into_path()) + if !entry.file_type()?.is_file() { + return None; } + Some(entry.into_path()) }); // Cap the number of files if we aren't in a git project, preventing diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index eef4a3f99e91..881dff4b37b3 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -171,6 +171,8 @@ pub struct FilePickerConfig { /// Enables following symlinks. /// Whether to follow symbolic links in file picker and file or directory completions. Defaults to true. pub follow_symlinks: bool, + /// Hides symlinks that point into the current directory. Default to true + pub deduplicate_links: bool, /// Enables reading ignore files from parent directories. Defaults to true. pub parents: bool, /// Enables reading `.ignore` files. @@ -195,6 +197,7 @@ impl Default for FilePickerConfig { Self { hidden: true, follow_symlinks: true, + deduplicate_links: true, parents: true, ignore: true, git_ignore: true, From 87f87a0cf819097e786b36a4d26c8f40ec7b669f Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Wed, 25 Jan 2023 14:24:25 +0100 Subject: [PATCH 2/3] Apply suggestions from code review Co-authored-by: g-re-g <123515925+g-re-g@users.noreply.github.com> --- helix-term/src/commands.rs | 6 ++++-- helix-term/src/lib.rs | 17 ++++++++--------- helix-term/src/ui/mod.rs | 12 ++++++------ helix-view/src/editor.rs | 2 +- 4 files changed, 19 insertions(+), 18 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 906bf87b05bc..733f93dee31b 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -45,7 +45,7 @@ use movement::Movement; use crate::{ args, compositor::{self, Component, Compositor}, - filter_entry, + filter_picker_entry, job::Callback, keymap::ReverseKeymap, ui::{self, overlay::overlayed, FilePicker, Picker, Popup, Prompt, PromptEvent}, @@ -1928,7 +1928,9 @@ fn global_search(cx: &mut Context) { .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_entry(entry, &absolute_root, dedup_symlinks)) + .filter_entry(move |entry| { + filter_picker_entry(entry, &absolute_root, dedup_symlinks) + }) .build_parallel() .run(|| { let mut searcher = searcher.clone(); diff --git a/helix-term/src/lib.rs b/helix-term/src/lib.rs index 742c8a98c6fa..f0bc9129a139 100644 --- a/helix-term/src/lib.rs +++ b/helix-term/src/lib.rs @@ -27,18 +27,17 @@ fn true_color() -> bool { } /// Function used for filtering dir entries in the various file pickers. -/// -/// We always want to ignore the .git directory, otherwise if -/// `ignore` is turned off, we end up with a lot of noise -/// in our picker. -/// We also ignore symlinks that point inside the current directory -/// if `dedup_links` is enabled. -fn filter_entry(entry: &DirEntry, root: &Path, dedup_links: bool) -> bool { - if entry.file_name() != ".git" { +fn filter_picker_entry(entry: &DirEntry, root: &Path, dedup_symlinks: bool) -> bool { + // We always want to ignore the .git directory, otherwise if + // `ignore` is turned off, we end up with a lot of noise + // in our picker. + if entry.file_name() == ".git" { return false; } - if dedup_links && entry.path_is_symlink() { + // We also ignore symlinks that point inside the current directory + // if `dedup_links` is enabled. + if dedup_symlinks && entry.path_is_symlink() { return entry .path() .canonicalize() diff --git a/helix-term/src/ui/mod.rs b/helix-term/src/ui/mod.rs index f74a840d7930..b66fb9f42a48 100644 --- a/helix-term/src/ui/mod.rs +++ b/helix-term/src/ui/mod.rs @@ -14,7 +14,7 @@ mod statusline; mod text; use crate::compositor::{Component, Compositor}; -use crate::filter_entry; +use crate::filter_picker_entry; use crate::job::{self, Callback}; pub use completion::Completion; pub use editor::EditorView; @@ -163,7 +163,7 @@ pub fn file_picker(root: PathBuf, config: &helix_view::editor::Config) -> FilePi let now = Instant::now(); - let deadup_symlinks = config.file_picker.deduplicate_links; + 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); @@ -176,7 +176,7 @@ pub fn file_picker(root: PathBuf, config: &helix_view::editor::Config) -> FilePi .git_global(config.file_picker.git_global) .git_exclude(config.file_picker.git_exclude) .max_depth(config.file_picker.max_depth) - .filter_entry(move |entry| filter_entry(entry, &absolute_root, deadup_symlinks)); + .filter_entry(move |entry| filter_picker_entry(entry, &absolute_root, dedup_symlinks)); // We want to exclude files that the editor can't handle yet let mut type_builder = TypesBuilder::new(); @@ -196,10 +196,10 @@ pub fn file_picker(root: PathBuf, config: &helix_view::editor::Config) -> FilePi let files = walk_builder.build().filter_map(|entry| { let entry = entry.ok()?; // This is faster than entry.path().is_dir() since it uses cached fs::Metadata fetched by ignore/walkdir - if !entry.file_type()?.is_file() { - return None; + if entry.file_type()?.is_file() { + return Some(entry.into_path()); } - Some(entry.into_path()) + None }); // Cap the number of files if we aren't in a git project, preventing diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 881dff4b37b3..7dacf273dbe9 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -171,7 +171,7 @@ pub struct FilePickerConfig { /// Enables following symlinks. /// Whether to follow symbolic links in file picker and file or directory completions. Defaults to true. pub follow_symlinks: bool, - /// Hides symlinks that point into the current directory. Default to true + /// Hides symlinks that point into the current directory. Defaults to true. pub deduplicate_links: bool, /// Enables reading ignore files from parent directories. Defaults to true. pub parents: bool, From 100ae2a1e1b1f3960422538936deb71e5970dfa0 Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Wed, 1 Feb 2023 23:39:04 +0100 Subject: [PATCH 3/3] minor stylistic fix Co-authored-by: Michael Davis --- helix-term/src/ui/mod.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/helix-term/src/ui/mod.rs b/helix-term/src/ui/mod.rs index b66fb9f42a48..74076552513e 100644 --- a/helix-term/src/ui/mod.rs +++ b/helix-term/src/ui/mod.rs @@ -197,9 +197,10 @@ pub fn file_picker(root: PathBuf, config: &helix_view::editor::Config) -> FilePi let entry = entry.ok()?; // This is faster than entry.path().is_dir() since it uses cached fs::Metadata fetched by ignore/walkdir if entry.file_type()?.is_file() { - return Some(entry.into_path()); + Some(entry.into_path()) + } else { + None } - None }); // Cap the number of files if we aren't in a git project, preventing