Hide duplicate symlinks from the picker (#5658)

* hide duplicate symlinks from the picker

* Apply suggestions from code review

Co-authored-by: g-re-g <123515925+g-re-g@users.noreply.github.com>

* minor stylistic fix

Co-authored-by: Michael Davis <mcarsondavis@gmail.com>

---------

Co-authored-by: g-re-g <123515925+g-re-g@users.noreply.github.com>
Co-authored-by: Michael Davis <mcarsondavis@gmail.com>

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

helix-term/src/commands.rs

@@ -49,6 +49,7 @@ use movement::Movement;
 use crate::{
     args,
     compositor::{self, Component, Compositor},
+    filter_picker_entry,
     job::Callback,
     keymap::ReverseKeymap,
     ui::{self, overlay::overlayed, FilePicker, Picker, Popup, Prompt, PromptEvent},
@@ -2013,6 +2014,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)
@@ -2022,10 +2028,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)
-                    // We always want to ignore the .git directory, otherwise if
+                    .filter_entry(move |entry| {
-                    // `ignore` is turned off above, we end up with a lot of noise
+                        filter_picker_entry(entry, &absolute_root, dedup_symlinks)
-                    // in our picker.
+                    })
-                    .filter_entry(|entry| entry.file_name() != ".git")
                     .build_parallel()
                     .run(|| {
                         let mut searcher = searcher.clone();

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,25 @@ fn true_color() -> bool {
 fn true_color() -> bool {
     true
 }
+
+/// Function used for filtering dir entries in the various file pickers.
+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;
+    }
+
+    // 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()
+            .ok()
+            .map_or(false, |path| !path.starts_with(&root));
+    }
+
+    true
+}

helix-term/src/ui/mod.rs

@@ -15,6 +15,7 @@ mod statusline;
 mod text;
 
 use crate::compositor::{Component, Compositor};
+use crate::filter_picker_entry;
 use crate::job::{self, Callback};
 pub use completion::Completion;
 pub use editor::EditorView;
@@ -163,6 +164,9 @@ pub fn file_picker(root: PathBuf, config: &helix_view::editor::Config) -> FilePi
 
     let now = Instant::now();
 
+    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);
     walk_builder
         .hidden(config.file_picker.hidden)
@@ -173,10 +177,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
+        .filter_entry(move |entry| filter_picker_entry(entry, &absolute_root, dedup_symlinks));
-        // `ignore` is turned off above, we end up with a lot of noise
-        // in our picker.
-        .filter_entry(|entry| entry.file_name() != ".git");
 
     // We want to exclude files that the editor can't handle yet
     let mut type_builder = TypesBuilder::new();
@@ -195,14 +196,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 entry.file_type()?.is_file() {
-        if is_dir {
-            // Will give a false positive if metadata cannot be read (eg. permission error)
-            None
-        } else {
             Some(entry.into_path())
+        } else {
+            None
         }
     });
 

helix-view/src/editor.rs

@@ -173,6 +173,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. Defaults to true.
+    pub deduplicate_links: bool,
     /// Enables reading ignore files from parent directories. Defaults to true.
     pub parents: bool,
     /// Enables reading `.ignore` files.
@@ -197,6 +199,7 @@ impl Default for FilePickerConfig {
         Self {
             hidden: true,
             follow_symlinks: true,
+            deduplicate_links: true,
             parents: true,
             ignore: true,
             git_ignore: true,