From 87f87a0cf819097e786b36a4d26c8f40ec7b669f Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Wed, 25 Jan 2023 14:24:25 +0100 Subject: [PATCH] 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,