From 0af9b732ab85c060c1e9f205577cb802bea72ce4 Mon Sep 17 00:00:00 2001
From: nick <nick@paco.to>
Date: Wed, 13 Mar 2024 18:10:20 -0400
Subject: [PATCH] refined exclusion rules

---
 src/args.rs      | 35 +++++++++++++++++++++++++++--------
 src/directory.rs | 11 ++++++++++-
 2 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/src/args.rs b/src/args.rs
index c8023b7..063c29c 100644
--- a/src/args.rs
+++ b/src/args.rs
@@ -3,7 +3,7 @@ use glob::Pattern;
 
 use crate::unit::Unit;
 
-use std::fs::Metadata;
+use std::fs::{symlink_metadata, Metadata};
 use std::path::{Component, Path};
 use std::slice::Iter;
 use std::iter::once_with;
@@ -90,7 +90,6 @@ pub struct Args {
     #[arg(
         short='X', long,
         help = "exclude from search and printing",
-        default_values_t = once_with(|| Pattern::new("").unwrap()),
         value_parser = parse_glob,
         value_delimiter = ',',
         action = ArgAction::Append,
@@ -115,6 +114,9 @@ pub struct Args {
     path: Vec<String>,
 }
 impl Args {
+    /// utility method to chuck default values on the end
+    /// it feels like I should be able to do this with
+    /// clever `clap` macros but I don't know how
     pub fn post_process(mut self) -> Self {
         if self.base_two {
             self.unit = Unit::Kibi;
@@ -134,7 +136,14 @@ impl Args {
     }
 
     pub fn should_exclude(&self, path: &Path, file: &Metadata) -> bool {
-        if !self.follow_links && file.is_symlink() {
+        if !self.follow_links
+        && file.is_symlink()
+        // `.` counts as a symlink. excluding . is usually not
+        // useful, so even if this is a symlink when we're not
+        // supposed to be following them, if this opens with a
+        // CurDir component then we shouldn't exclude
+        && !matches!(path.components().nth(0), Some(Component::CurDir))
+        {
             return true
         }
 
@@ -179,21 +188,31 @@ impl Args {
 
 fn validate_path(s: &str) -> Result<String, String> {
     // try to access it's metadata, since that is what is used
-    // to get its length
-    std::fs::metadata(s)
+    // to get its length. using symlink because that's what's
+    // used in the actual program
+    symlink_metadata(s)
         .map(|_| s.to_string())
         .map_err(|e| e.to_string())
 }
 
 fn parse_glob(s: &str) -> Result<Pattern, String> {
-    Pattern::new(s).map_err(|_| format!("invalid glob: {s}"))
+    Pattern::new(s).map_err(|globerr| format!("invalid glob \"{s}\": {}", globerr.msg))
 }
 
 fn any_pattern_matches_any_component(patterns: &[Pattern], path: &Path) -> bool {
     for pat in patterns {
         for cmp in path.components() {
-            let Component::Normal(cmp) = cmp else { continue };
-            let Some(s) = cmp.to_str() else { continue };
+            let Component::Normal(cmp) = cmp else {
+                // this seems wacky, but only normal components
+                // are useful
+                continue
+            };
+            let Some(s) = cmp.to_str() else {
+                // this is a code smell
+                // I don't believe it, but I can't think
+                // of anything worthwhile to do
+                continue
+            };
             if pat.matches(s) {
                 return true
             }
diff --git a/src/directory.rs b/src/directory.rs
index fefcb63..bee7a52 100644
--- a/src/directory.rs
+++ b/src/directory.rs
@@ -28,7 +28,16 @@ impl Directory {
 
     pub fn new< P: AsRef<Path> >(path: P, args: &Args) -> Result<Option<Self>> {
         let path = path.as_ref();
-        let name = path.file_name()
+        // NOTE: I go back and forth on canonicalize()ing all the time.
+        // I feel like it changes every commit. The performance loss seems
+        // to be negligible, even when I do crazy things like `hb -p /`, which
+        // is the most I can currently do.
+        let name = match (path.canonicalize(), args.persistant()) {
+            (Ok(path), _) => path,
+            (Err(_), true) => return Ok(None),
+            (Err(e), false) => return Err(e.into()),
+        }
+            .file_name()
             .map_or_else(|| OsString::from("/"), ToOwned::to_owned)
             .into();