From 9f0f36618762a843619fe7688cb8ec04c891622c Mon Sep 17 00:00:00 2001
From: "andy.boot" <bootandy@gmail.com>
Date: Sat, 14 Jan 2023 18:55:57 +0000
Subject: [PATCH] Refactor: progress bar & file permissions

Move permissions flag into shared pdata object.

Fixed bug where permissions flag was not being set correctly
---
 src/dir_walker.rs | 33 +++++++++++----------------------
 src/main.rs       | 10 +++++-----
 src/progress.rs   |  7 +++----
 3 files changed, 19 insertions(+), 31 deletions(-)

diff --git a/src/dir_walker.rs b/src/dir_walker.rs
index 2665020..89f5ab4 100644
--- a/src/dir_walker.rs
+++ b/src/dir_walker.rs
@@ -2,10 +2,10 @@ use std::fs;
 use std::sync::Arc;
 
 use crate::node::Node;
-use crate::progress::ORDERING;
 use crate::progress::Operation;
 use crate::progress::PAtomicInfo;
 use crate::progress::ThreadSyncTrait;
+use crate::progress::ORDERING;
 use crate::utils::is_filtered_out_due_to_invert_regex;
 use crate::utils::is_filtered_out_due_to_regex;
 use rayon::iter::ParallelBridge;
@@ -13,8 +13,6 @@ use rayon::prelude::ParallelIterator;
 use regex::Regex;
 use std::path::PathBuf;
 
-use std::sync::atomic::AtomicBool;
-
 use std::collections::HashSet;
 
 use crate::node::build_node;
@@ -33,21 +31,19 @@ pub struct WalkData<'a> {
     pub progress_data: Option<Arc<PAtomicInfo>>,
 }
 
-pub fn walk_it(dirs: HashSet<PathBuf>, walk_data: WalkData) -> (Vec<Node>, bool) {
-    let permissions_flag = AtomicBool::new(false);
-
+pub fn walk_it(dirs: HashSet<PathBuf>, walk_data: WalkData) -> Vec<Node> {
     let mut inodes = HashSet::new();
     let top_level_nodes: Vec<_> = dirs
         .into_iter()
         .filter_map(|d| {
-            let node = walk(d, &permissions_flag, &walk_data, 0)?;
+            let node = walk(d, &walk_data, 0)?;
             if let Some(data) = &walk_data.progress_data {
                 data.state.store(Operation::PREPARING, ORDERING);
             }
             clean_inodes(node, &mut inodes, walk_data.use_apparent_size)
         })
         .collect();
-    (top_level_nodes, permissions_flag.into_inner())
+    top_level_nodes
 }
 
 // Remove files which have the same inode, we don't want to double count them.
@@ -127,12 +123,7 @@ fn ignore_file(entry: &DirEntry, walk_data: &WalkData) -> bool {
     (is_dot_file && walk_data.ignore_hidden) || is_ignored_path
 }
 
-fn walk(
-    dir: PathBuf,
-    permissions_flag: &AtomicBool,
-    walk_data: &WalkData,
-    depth: usize,
-) -> Option<Node> {
+fn walk(dir: PathBuf, walk_data: &WalkData, depth: usize) -> Option<Node> {
     let info_data = &walk_data.progress_data;
 
     if let Some(data) = info_data {
@@ -163,7 +154,7 @@ fn walk(
                     if !ignore_file(entry, walk_data) {
                         if let Ok(data) = entry.file_type() {
                             if data.is_dir() || (walk_data.follow_links && data.is_symlink()) {
-                                return walk(entry.path(), permissions_flag, walk_data, depth + 1);
+                                return walk(entry.path(), walk_data, depth + 1);
                             }
 
                             let node = build_node(
@@ -188,17 +179,15 @@ fn walk(
                             return node;
                         }
                     }
-                } else {
-                    permissions_flag.store(true, ORDERING);
+                } else if let Some(data) = &walk_data.progress_data {
+                    data.no_permissions.store(true, ORDERING)
                 }
                 None
             })
             .collect();
-    } else {
-        // Handle edge case where dust is called with a file instead of a directory
-        if !dir.exists() {
-            // TODO: Migrate permissions_flag to the new progress_data object
-            permissions_flag.store(true, ORDERING);
+    } else if !dir.is_file() {
+        if let Some(data) = &walk_data.progress_data {
+            data.no_permissions.store(true, ORDERING)
         }
     }
     build_node(
diff --git a/src/main.rs b/src/main.rs
index 9062a44..d8d92cb 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -14,6 +14,7 @@ use crate::cli::build_cli;
 use dir_walker::WalkData;
 use filter::AggregateData;
 use progress::PIndicator;
+use progress::ORDERING;
 use std::collections::HashSet;
 use std::io::BufRead;
 use std::process;
@@ -200,7 +201,7 @@ fn main() {
 
     let _rayon = init_rayon();
 
-    let (top_level_nodes, has_errors) = walk_it(simplified_dirs, walk_data);
+    let top_level_nodes = walk_it(simplified_dirs, walk_data);
 
     let tree = match summarize_file_types {
         true => get_all_file_types(&top_level_nodes, number_of_lines),
@@ -218,13 +219,12 @@ fn main() {
     };
 
     if let Some(info) = info_indicator {
+        if info.data.no_permissions.load(ORDERING) {
+            eprintln!("Did not have permissions for all directories");
+        }
         info.stop();
     }
 
-    if has_errors {
-        eprintln!("Did not have permissions for all directories");
-    }
-
     if let Some(root_node) = tree {
         draw_it(
             config.get_full_paths(&options),
diff --git a/src/progress.rs b/src/progress.rs
index 4b88f00..5cee580 100644
--- a/src/progress.rs
+++ b/src/progress.rs
@@ -53,6 +53,7 @@ pub struct PAtomicInfo {
     pub total_file_size: AtomicU64,
     pub state: AtomicU8,
     pub current_path: ThreadStringWrapper,
+    pub no_permissions: AtomicBool,
 }
 
 /* -------------------------------------------------------------------------- */
@@ -97,10 +98,8 @@ impl PIndicator {
                         let base = format_indicator_str(&data, progress_char_i, "Indexing");
 
                         let file_count = data.file_number.load(ORDERING);
-                        let size = human_readable_number(
-                            data.total_file_size.load(ORDERING),
-                            is_iso,
-                        );
+                        let size =
+                            human_readable_number(data.total_file_size.load(ORDERING), is_iso);
                         let file_str = format!("{} {} files", file_count, size);
                         format!("{} - {}", base, file_str)
                     }