Explorar o código

Redo Git implementation to allow --git --recurse

This is all a big commit because it took a lot more work than I thought it would! The commit basically moves Git repositories from being per-directory to living for the whole life of the program. This allows for several directories in the same repository to be listed in the same invocation; before, it would try to rediscover the repository each time! This is why two of the tests “broke”: it suddenly started working with --recurse.

The Dir type does now not use Git at all; because a Dir doesn’t have a Git, then a File doesn’t have one either, so the Git cache gets passed to the render functions which will put them in the Table to render them.
Benjamin Sago %!s(int64=8) %!d(string=hai) anos
pai
achega
45a807a14f
Modificáronse 12 ficheiros con 286 adicións e 231 borrados
  1. 30 26
      src/exa.rs
  2. 5 29
      src/fs/dir.rs
  3. 157 64
      src/fs/feature/git.rs
  4. 4 2
      src/fs/fields.rs
  5. 26 47
      src/fs/file.rs
  6. 1 1
      src/fs/mod.rs
  7. 1 1
      src/options/mod.rs
  8. 4 4
      src/output/details.rs
  9. 12 11
      src/output/grid_details.rs
  10. 30 30
      src/output/table.rs
  11. 7 7
      xtests/git_1_recurse
  12. 9 9
      xtests/git_2_recurse

+ 30 - 26
src/exa.rs

@@ -56,6 +56,11 @@ pub struct Exa<'args, 'w, W: Write + 'w> {
     /// List of the free command-line arguments that should correspond to file
     /// names (anything that isn’t an option).
     pub args: Vec<&'args OsStr>,
+
+    /// A global Git cache, if the option was passed in.
+    /// This has to last the lifetime of the program, because the user might
+    /// want to list several directories in the same repository.
+    pub git: Option<GitCache>,
 }
 
 /// The “real” environment variables type.
@@ -68,31 +73,41 @@ impl Vars for LiveVars {
     }
 }
 
+/// Create a Git cache populated with the arguments that are going to be
+/// listed before they’re actually listed, if the options demand it.
+fn git_options(options: &Options, args: &[&OsStr]) -> Option<GitCache> {
+    if options.should_scan_for_git() {
+        Some(args.iter().map(|os| PathBuf::from(os)).collect())
+    }
+    else {
+        None
+    }
+}
+
 impl<'args, 'w, W: Write + 'w> Exa<'args, 'w, W> {
     pub fn new<I>(args: I, writer: &'w mut W) -> Result<Exa<'args, 'w, W>, Misfire>
     where I: Iterator<Item=&'args OsString> {
-        Options::parse(args, &LiveVars).map(move |(options, args)| {
+        Options::parse(args, &LiveVars).map(move |(options, mut args)| {
             debug!("Dir action from arguments: {:#?}", options.dir_action);
             debug!("Filter from arguments: {:#?}", options.filter);
             debug!("View from arguments: {:#?}", options.view.mode);
-            Exa { options, writer, args }
+
+            // List the current directory by default, like ls.
+            // This has to be done here, otherwise git_options won’t see it.
+            if args.is_empty() {
+                args = vec![ OsStr::new(".") ];
+            }
+
+            let git = git_options(&options, &args);
+            Exa { options, writer, args, git }
         })
     }
 
     pub fn run(&mut self) -> IOResult<i32> {
-        use fs::DirOptions;
-
         let mut files = Vec::new();
         let mut dirs = Vec::new();
         let mut exit_status = 0;
 
-        // List the current directory by default, like ls.
-        if self.args.is_empty() {
-            self.args = vec![ OsStr::new(".") ];
-        }
-
-        let git = self.git_options(&*self.args);
-
         for file_path in &self.args {
             match File::new(PathBuf::from(file_path), None, None) {
                 Err(e) => {
@@ -101,7 +116,7 @@ impl<'args, 'w, W: Write + 'w> Exa<'args, 'w, W> {
                 },
                 Ok(f) => {
                     if f.is_directory() && !self.options.dir_action.treat_dirs_as_files() {
-                        match f.to_dir(DirOptions { git: git.as_ref() }) {
+                        match f.to_dir() {
                             Ok(d) => dirs.push(d),
                             Err(e) => writeln!(stderr(), "{:?}: {}", file_path, e)?,
                         }
@@ -126,18 +141,7 @@ impl<'args, 'w, W: Write + 'w> Exa<'args, 'w, W> {
         self.print_dirs(dirs, no_files, is_only_dir, exit_status)
     }
 
-    fn git_options(&self, args: &[&OsStr]) -> Option<GitCache> {
-        if self.options.should_scan_for_git() {
-            Some(args.iter().map(|os| PathBuf::from(os)).collect())
-        }
-        else {
-            None
-        }
-    }
-
     fn print_dirs(&mut self, dir_files: Vec<Dir>, mut first: bool, is_only_dir: bool, exit_status: i32) -> IOResult<i32> {
-        use fs::DirOptions;
-
         for dir in dir_files {
 
             // Put a gap between directories, or between the list of files and
@@ -172,7 +176,7 @@ impl<'args, 'w, W: Write + 'w> Exa<'args, 'w, W> {
 
                     let mut child_dirs = Vec::new();
                     for child_dir in children.iter().filter(|f| f.is_directory()) {
-                        match child_dir.to_dir(DirOptions { git: None }) {
+                        match child_dir.to_dir() {
                             Ok(d)  => child_dirs.push(d),
                             Err(e) => writeln!(stderr(), "{}: {}", child_dir.path.display(), e)?,
                         }
@@ -208,10 +212,10 @@ impl<'args, 'w, W: Write + 'w> Exa<'args, 'w, W> {
                     grid::Render { files, colours, style, opts }.render(self.writer)
                 }
                 Mode::Details(ref opts) => {
-                    details::Render { dir, files, colours, style, opts, filter: &self.options.filter, recurse: self.options.dir_action.recurse_options() }.render(self.writer)
+                    details::Render { dir, files, colours, style, opts, filter: &self.options.filter, recurse: self.options.dir_action.recurse_options() }.render(self.git.as_ref(), self.writer)
                 }
                 Mode::GridDetails(ref opts) => {
-                    grid_details::Render { dir, files, colours, style, grid: &opts.grid, details: &opts.details, filter: &self.options.filter, row_threshold: opts.row_threshold }.render(self.writer)
+                    grid_details::Render { dir, files, colours, style, grid: &opts.grid, details: &opts.details, filter: &self.options.filter, row_threshold: opts.row_threshold }.render(self.git.as_ref(), self.writer)
                 }
             }
         }

+ 5 - 29
src/fs/dir.rs

@@ -3,8 +3,7 @@ use std::fs;
 use std::path::{Path, PathBuf};
 use std::slice::Iter as SliceIter;
 
-use fs::feature::git::{Git, GitCache};
-use fs::{File, fields};
+use fs::File;
 
 
 /// A **Dir** provides a cached list of the file paths in a directory that's
@@ -20,14 +19,6 @@ pub struct Dir {
 
     /// The path that was read.
     pub path: PathBuf,
-
-    /// Holds a `Git` object if scanning for Git repositories is switched on,
-    /// and this directory happens to contain one.
-    git: Option<Git>,
-}
-
-pub struct DirOptions<'exa> {
-    pub git: Option<&'exa GitCache>
 }
 
 impl Dir {
@@ -40,15 +31,14 @@ impl Dir {
     /// The `read_dir` iterator doesn’t actually yield the `.` and `..`
     /// entries, so if the user wants to see them, we’ll have to add them
     /// ourselves after the files have been read.
-    pub fn read_dir(path: PathBuf, options: DirOptions) -> IOResult<Dir> {
+    pub fn read_dir(path: PathBuf) -> IOResult<Dir> {
         info!("Reading directory {:?}", &path);
 
         let contents: Vec<PathBuf> = try!(fs::read_dir(&path)?
-                                                 .map(|result| result.map(|entry| entry.path()))
-                                                 .collect());
+                                             .map(|result| result.map(|entry| entry.path()))
+                                             .collect());
 
-        let git = options.git.and_then(|cache| cache.get(&path));
-        Ok(Dir { contents, path, git })
+        Ok(Dir { contents, path })
     }
 
     /// Produce an iterator of IO results of trying to read all the files in
@@ -71,20 +61,6 @@ impl Dir {
     pub fn join(&self, child: &Path) -> PathBuf {
         self.path.join(child)
     }
-
-    /// Return whether there's a Git repository on or above this directory.
-    pub fn has_git_repo(&self) -> bool {
-        self.git.is_some()
-    }
-
-    /// Get a string describing the Git status of the given file.
-    pub fn git_status(&self, path: &Path, prefix_lookup: bool) -> fields::Git {
-        match (&self.git, prefix_lookup) {
-            (&Some(ref git), false)  => git.status(path),
-            (&Some(ref git), true)   => git.dir_status(path),
-            (&None, _)               => fields::Git::empty()
-        }
-    }
 }
 
 

+ 157 - 64
src/fs/feature/git.rs

@@ -1,6 +1,7 @@
 //! Getting the Git status of files and directories.
 
 use std::path::{Path, PathBuf};
+use std::sync::Mutex;
 
 use git2;
 
@@ -20,34 +21,16 @@ pub struct GitCache {
     misses: Vec<PathBuf>,
 }
 
-
-/// A **Git repository** is one we’ve discovered somewhere on the filesystem.
-pub struct GitRepo {
-
-    /// Most of the interesting Git stuff goes through this.
-    repo: git2::Repository,
-
-    /// The working directory of this repository.
-    /// This is used to check whether two repositories are the same.
-    workdir: PathBuf,
-
-    /// The path that was originally checked to discover this repository.
-    /// This is as important as the extra_paths (it gets checked first), but
-    /// is separate to avoid having to deal with a non-empty Vec.
-    original_path: PathBuf,
-
-    /// Any other paths that were checked only to result in this same
-    /// repository.
-    extra_paths: Vec<PathBuf>,
-}
-
-impl GitRepo {
-    fn has_workdir(&self, path: &Path) -> bool {
-        self.workdir == path
+impl GitCache {
+    pub fn has_anything_for(&self, index: &Path) -> bool {
+        true
     }
 
-    fn has_path(&self, path: &Path) -> bool {
-        self.original_path == path || self.extra_paths.iter().any(|e| e == path)
+    pub fn get(&self, index: &Path, prefix_lookup: bool) -> f::Git {
+        self.repos.iter()
+            .find(|e| e.has_path(index))
+            .map(|repo| repo.search(index, prefix_lookup))
+            .unwrap_or_default()
     }
 }
 
@@ -76,7 +59,7 @@ impl FromIterator<PathBuf> for GitCache {
                             continue;
                         }
 
-                        debug!("Creating new repo in cache");
+                        debug!("Discovered new Git repo");
                         git.repos.push(r);
                     },
                     Err(miss) => git.misses.push(miss),
@@ -88,10 +71,89 @@ impl FromIterator<PathBuf> for GitCache {
     }
 }
 
+
+
+
+/// A **Git repository** is one we’ve discovered somewhere on the filesystem.
+pub struct GitRepo {
+
+    /// The queryable contents of the repository: either a `git2` repo, or the
+    /// cached results from when we queried it last time.
+    contents: Mutex<GitContents>,
+
+    /// The working directory of this repository.
+    /// This is used to check whether two repositories are the same.
+    workdir: PathBuf,
+
+    /// The path that was originally checked to discover this repository.
+    /// This is as important as the extra_paths (it gets checked first), but
+    /// is separate to avoid having to deal with a non-empty Vec.
+    original_path: PathBuf,
+
+    /// Any other paths that were checked only to result in this same
+    /// repository.
+    extra_paths: Vec<PathBuf>,
+}
+
+/// A repository’s queried state.
+enum GitContents {
+
+    /// All the interesting Git stuff goes through this.
+    Before { repo: git2::Repository },
+
+    /// Temporary value used in `repo_to_statuses` so we can move the
+    /// repository out of the `Before` variant.
+    Processing,
+
+    /// The data we’ve extracted from the repository, but only after we’ve
+    /// actually done so.
+    After { statuses: Git }
+}
+
 impl GitRepo {
+
+    /// Searches through this repository for a path (to a file or directory,
+    /// depending on the prefix-lookup flag) and returns its Git status.
+    ///
+    /// Actually querying the `git2` repository for the mapping of paths to
+    /// Git statuses is only done once, and gets cached so we don't need to
+    /// re-query the entire repository the times after that.
+    ///
+    /// The temporary `Processing` enum variant is used after the `git2`
+    /// repository is moved out, but before the results have been moved in!
+    /// See https://stackoverflow.com/q/45985827/3484614
+    fn search(&self, index: &Path, prefix_lookup: bool) -> f::Git {
+        use self::GitContents::*;
+        use std::mem::replace;
+
+        let mut contents = self.contents.lock().unwrap();
+        if let After { ref statuses } = *contents {
+            debug!("Git repo {:?} has been found in cache", &self.workdir);
+            return statuses.status(index, prefix_lookup);
+        }
+
+        debug!("Querying Git repo {:?} for the first time", &self.workdir);
+        let repo = replace(&mut *contents, Processing).inner_repo();
+        let statuses = repo_to_statuses(repo, &self.workdir);
+        let result = statuses.status(index, prefix_lookup);
+        let _processing = replace(&mut *contents, After { statuses });
+        result
+    }
+
+    /// Whether this repository has the given working directory.
+    fn has_workdir(&self, path: &Path) -> bool {
+        self.workdir == path
+    }
+
+    /// Whether this repository cares about the given path at all.
+    fn has_path(&self, path: &Path) -> bool {
+        path.starts_with(&self.original_path) || self.extra_paths.iter().any(|e| path.starts_with(e))
+    }
+
+    /// Searches for a Git repository at any point above the given path.
+    /// Returns the original buffer if none is found.
     fn discover(path: PathBuf) -> Result<GitRepo, PathBuf> {
         info!("Searching for Git repository above {:?}", path);
-
         let repo = match git2::Repository::discover(&path) {
             Ok(r) => r,
             Err(e) => {
@@ -101,7 +163,10 @@ impl GitRepo {
         };
 
         match repo.workdir().map(|wd| wd.to_path_buf()) {
-            Some(workdir) => Ok(GitRepo { repo, workdir, original_path: path, extra_paths: Vec::new() }),
+            Some(workdir) => {
+                let contents = Mutex::new(GitContents::Before { repo });
+                Ok(GitRepo { contents, workdir, original_path: path, extra_paths: Vec::new() })
+            },
             None => {
                 warn!("Repository has no workdir?");
                 Err(path)
@@ -110,66 +175,94 @@ impl GitRepo {
     }
 }
 
-impl GitCache {
-
-    /// Gets a repository from the cache and scans it to get all its files’ statuses.
-    pub fn get(&self, index: &Path) -> Option<Git> {
-        let repo = match self.repos.iter().find(|e| e.has_path(index)) {
-            Some(r) => r,
-            None    => return None,
-        };
-
-        info!("Getting Git statuses for repo with workdir {:?}", &repo.workdir);
-        let iter = match repo.repo.statuses(None) {
-            Ok(es) => es,
-            Err(e) => {
-                error!("Error looking up Git statuses: {:?}", e);
-                return None;
-            }
-        };
 
-        let mut statuses = Vec::new();
-
-        for e in iter.iter() {
-            let path = repo.workdir.join(Path::new(e.path().unwrap()));
-            let elem = (path, e.status());
-            statuses.push(elem);
+impl GitContents {
+    /// Assumes that the repository hasn’t been queried, and extracts it
+    /// (consuming the value) if it has. This is needed because the entire
+    /// enum variant gets replaced when a repo is queried (see above).
+    fn inner_repo(self) -> git2::Repository {
+        if let GitContents::Before { repo } = self {
+            repo
+        }
+        else {
+            unreachable!("Tried to extract a non-Repository")
         }
+    }
+}
 
-        Some(Git { statuses })
+/// Iterates through a repository’s statuses, consuming it and returning the
+/// mapping of files to their Git status.
+/// We will have already used the working directory at this point, so it gets
+/// passed in rather than deriving it from the `Repository` again.
+fn repo_to_statuses(repo: git2::Repository, workdir: &Path) -> Git {
+    let mut statuses = Vec::new();
+
+    info!("Getting Git statuses for repo with workdir {:?}", workdir);
+    match repo.statuses(None) {
+        Ok(es) => {
+            for e in es.iter() {
+                let path = workdir.join(Path::new(e.path().unwrap()));
+                let elem = (path, e.status());
+                statuses.push(elem);
+            }
+        },
+        Err(e) => error!("Error looking up Git statuses: {:?}", e),
     }
+
+    Git { statuses }
 }
 
 
 /// Container of Git statuses for all the files in this folder’s Git repository.
-pub struct Git {
+struct Git {
     statuses: Vec<(PathBuf, git2::Status)>,
 }
 
 impl Git {
 
-    /// Get the status for the file at the given path, if present.
-    pub fn status(&self, path: &Path) -> f::Git {
-        let status = self.statuses.iter()
-                                  .find(|p| p.0.as_path() == path);
-        match status {
-            Some(&(_, s)) => f::Git { staged: index_status(s),           unstaged: working_tree_status(s) },
-            None          => f::Git { staged: f::GitStatus::NotModified, unstaged: f::GitStatus::NotModified }
-        }
+    /// Get either the file or directory status for the given path.
+    /// “Prefix lookup” means that it should report an aggregate status of all
+    /// paths starting with the given prefix (in other words, a directory).
+    fn status(&self, index: &Path, prefix_lookup: bool) -> f::Git {
+        if prefix_lookup { self.dir_status(index) }
+                    else { self.file_status(index) }
+    }
+
+    /// Get the status for the file at the given path.
+    fn file_status(&self, file: &Path) -> f::Git {
+        let path = reorient(file);
+        self.statuses.iter()
+            .find(|p| p.0.as_path() == path)
+            .map(|&(_, s)| f::Git { staged: index_status(s), unstaged: working_tree_status(s) })
+            .unwrap_or_default()
     }
 
     /// Get the combined status for all the files whose paths begin with the
     /// path that gets passed in. This is used for getting the status of
     /// directories, which don’t really have an ‘official’ status.
-    pub fn dir_status(&self, dir: &Path) -> f::Git {
+    fn dir_status(&self, dir: &Path) -> f::Git {
+        let path = reorient(dir);
         let s = self.statuses.iter()
-                             .filter(|p| p.0.starts_with(dir))
+                             .filter(|p| p.0.starts_with(&path))
                              .fold(git2::Status::empty(), |a, b| a | b.1);
 
         f::Git { staged: index_status(s), unstaged: working_tree_status(s) }
     }
 }
 
+/// Converts a path to an absolute path based on the current directory.
+/// Paths need to be absolute for them to be compared properly, otherwise
+/// you’d ask a repo about “./README.md” but it only knows about
+/// “/vagrant/REAMDE.md”, prefixed by the workdir.
+fn reorient(path: &Path) -> PathBuf {
+    use std::env::current_dir;
+    // I’m not 100% on this func tbh
+    match current_dir() {
+        Err(_)  => Path::new(".").join(&path),
+        Ok(dir) => dir.join(&path),
+    }
+}
+
 /// The character to display if the file has been modified, but not staged.
 fn working_tree_status(status: git2::Status) -> f::GitStatus {
     match status {

+ 4 - 2
src/fs/fields.rs

@@ -1,3 +1,4 @@
+
 //! Wrapper types for the values returned from `File`s.
 //!
 //! The methods of `File` that return information about the entry on the
@@ -206,10 +207,11 @@ pub struct Git {
     pub unstaged: GitStatus,
 }
 
-impl Git {
+use std::default::Default;
+impl Default for Git {
 
     /// Create a Git status for a file with nothing done to it.
-    pub fn empty() -> Git {
+    fn default() -> Git {
         Git { staged: GitStatus::NotModified, unstaged: GitStatus::NotModified }
     }
 }

+ 26 - 47
src/fs/file.rs

@@ -6,7 +6,7 @@ use std::io::Result as IOResult;
 use std::os::unix::fs::{MetadataExt, PermissionsExt, FileTypeExt};
 use std::path::{Path, PathBuf};
 
-use fs::dir::{Dir, DirOptions};
+use fs::dir::Dir;
 use fs::fields as f;
 
 
@@ -19,7 +19,7 @@ use fs::fields as f;
 /// start and hold on to all the information.
 pub struct File<'dir> {
 
-    /// The filename portion of this file's path, including the extension.
+    /// The filename portion of this files path, including the extension.
     ///
     /// This is used to compare against certain filenames (such as checking if
     /// it’s “Makefile” or something) and to highlight only the filename in
@@ -33,26 +33,27 @@ pub struct File<'dir> {
 
     /// The path that begat this file.
     ///
-    /// Even though the file's name is extracted, the path needs to be kept
-    /// around, as certain operations involve looking up the file's absolute
-    /// location (such as the Git status, or searching for compiled files).
+    /// Even though the file’s name is extracted, the path needs to be kept
+    /// around, as certain operations involve looking up the file’s absolute
+    /// location (such as searching for compiled files) or using its original
+    /// path (following a symlink).
     pub path: PathBuf,
 
-    /// A cached `metadata` call for this file.
+    /// A cached `metadata` (`stat`) call for this file.
     ///
     /// This too is queried multiple times, and is *not* cached by the OS, as
-    /// it could easily change between invocations - but exa is so short-lived
+    /// it could easily change between invocations  but exa is so short-lived
     /// it's better to just cache it.
     pub metadata: fs::Metadata,
 
-    /// A reference to the directory that contains this file, if present.
+    /// A reference to the directory that contains this file, if any.
     ///
     /// Filenames that get passed in on the command-line directly will have no
-    /// parent directory reference - although they technically have one on the
-    /// filesystem, we'll never need to look at it, so it'll be `None`.
+    /// parent directory reference  although they technically have one on the
+    /// filesystem, we’ll never need to look at it, so it’ll be `None`.
     /// However, *directories* that get passed in will produce files that
     /// contain a reference to it, which is used in certain operations (such
-    /// as looking up a file's Git status).
+    /// as looking up compiled files).
     pub parent_dir: Option<&'dir Dir>,
 }
 
@@ -88,11 +89,11 @@ impl<'dir> File<'dir> {
     /// Extract an extension from a file path, if one is present, in lowercase.
     ///
     /// The extension is the series of characters after the last dot. This
-    /// deliberately counts dotfiles, so the ".git" folder has the extension "git".
+    /// deliberately counts dotfiles, so the “.git” folder has the extension “git”.
     ///
     /// ASCII lowercasing is used because these extensions are only compared
     /// against a pre-compiled list of extensions which are known to only exist
-    /// within ASCII, so it's alright.
+    /// within ASCII, so its alright.
     fn ext(path: &Path) -> Option<String> {
         use std::ascii::AsciiExt;
 
@@ -110,24 +111,24 @@ impl<'dir> File<'dir> {
     }
 
     /// If this file is a directory on the filesystem, then clone its
-    /// `PathBuf` for use in one of our own `Dir` objects, and read a list of
+    /// `PathBuf` for use in one of our own `Dir` values, and read a list of
     /// its contents.
     ///
-    /// Returns an IO error upon failure, but this shouldn't be used to check
+    /// Returns an IO error upon failure, but this shouldnt be used to check
     /// if a `File` is a directory or not! For that, just use `is_directory()`.
-    pub fn to_dir(&self, options: DirOptions) -> IOResult<Dir> {
-        Dir::read_dir(self.path.clone(), options)
+    pub fn to_dir(&self) -> IOResult<Dir> {
+        Dir::read_dir(self.path.clone())
     }
 
-    /// Whether this file is a regular file on the filesystem - that is, not a
+    /// Whether this file is a regular file on the filesystem  that is, not a
     /// directory, a link, or anything else treated specially.
     pub fn is_file(&self) -> bool {
         self.metadata.is_file()
     }
 
     /// Whether this file is both a regular file *and* executable for the
-    /// current user. Executable files have different semantics than
-    /// executable directories, and so should be highlighted differently.
+    /// current user. An executable file has a different purpose from an
+    /// executable directory, so they should be highlighted differently.
     pub fn is_executable_file(&self) -> bool {
         let bit = modes::USER_EXECUTE;
         self.is_file() && (self.metadata.permissions().mode() & bit) == bit
@@ -159,7 +160,7 @@ impl<'dir> File<'dir> {
     }
 
 
-    /// Re-prefixes the path pointed to by this file, if it's a symlink, to
+    /// Re-prefixes the path pointed to by this file, if its a symlink, to
     /// make it an absolute path that can be accessed from whichever
     /// directory exa is being run from.
     fn reorient_target_path(&self, path: &Path) -> PathBuf {
@@ -190,8 +191,8 @@ impl<'dir> File<'dir> {
     pub fn link_target(&self) -> FileTarget<'dir> {
 
         // We need to be careful to treat the path actually pointed to by
-        // this file -- which could be absolute or relative -- to the path
-        // we actually look up and turn into a `File` -- which needs to be
+        // this file — which could be absolute or relative — to the path
+        // we actually look up and turn into a `File`  which needs to be
         // absolute to be accessible from any directory.
         debug!("Reading link {:?}", &self.path);
         let path = match fs::read_link(&self.path) {
@@ -216,11 +217,11 @@ impl<'dir> File<'dir> {
         }
     }
 
-    /// This file's number of hard links.
+    /// This files number of hard links.
     ///
     /// It also reports whether this is both a regular file, and a file with
     /// multiple links. This is important, because a file with multiple links
-    /// is uncommon, while you can come across directories and other types
+    /// is uncommon, while you come across directories and other types
     /// with multiple links much more often. Thus, it should get highlighted
     /// more attentively.
     pub fn links(&self) -> f::Links {
@@ -378,28 +379,6 @@ impl<'dir> File<'dir> {
     pub fn name_is_one_of(&self, choices: &[&str]) -> bool {
         choices.contains(&&self.name[..])
     }
-
-    /// This file's Git status as two flags: one for staged changes, and the
-    /// other for unstaged changes.
-    ///
-    /// This requires looking at the `git` field of this file's parent
-    /// directory, so will not work if this file has just been passed in on
-    /// the command line.
-    pub fn git_status(&self) -> f::Git {
-        use std::env::current_dir;
-
-        match self.parent_dir {
-            None    => f::Git { staged: f::GitStatus::NotModified, unstaged: f::GitStatus::NotModified },
-            Some(d) => {
-                let cwd = match current_dir() {
-                    Err(_)  => Path::new(".").join(&self.path),
-                    Ok(dir) => dir.join(&self.path),
-                };
-
-                d.git_status(&cwd, self.is_directory())
-            },
-        }
-    }
 }
 
 

+ 1 - 1
src/fs/mod.rs

@@ -1,5 +1,5 @@
 mod dir;
-pub use self::dir::{Dir, DirOptions, DotFilter};
+pub use self::dir::{Dir, DotFilter};
 
 mod file;
 pub use self::file::{File, FileTarget};

+ 1 - 1
src/options/mod.rs

@@ -149,7 +149,7 @@ impl Options {
     pub fn should_scan_for_git(&self) -> bool {
         match self.view.mode {
             Mode::Details(details::Options { table: Some(ref table), .. }) |
-            Mode::GridDetails(grid_details::Options { details: details::Options { table: Some(ref table), .. }, .. }) => table.extra_columns.should_scan_for_git(),
+            Mode::GridDetails(grid_details::Options { details: details::Options { table: Some(ref table), .. }, .. }) => table.extra_columns.git,
             _ => false,
         }
     }

+ 4 - 4
src/output/details.rs

@@ -69,6 +69,7 @@ use ansi_term::Style;
 use fs::{Dir, File};
 use fs::dir_action::RecurseOptions;
 use fs::filter::FileFilter;
+use fs::feature::git::GitCache;
 use fs::feature::xattr::{Attribute, FileAttributes};
 use style::Colours;
 use output::cell::TextCell;
@@ -139,11 +140,11 @@ impl<'a> AsRef<File<'a>> for Egg<'a> {
 
 
 impl<'a> Render<'a> {
-    pub fn render<W: Write>(self, w: &mut W) -> IOResult<()> {
+    pub fn render<W: Write>(self, git: Option<&'a GitCache>, w: &mut W) -> IOResult<()> {
         let mut rows = Vec::new();
 
         if let Some(ref table) = self.opts.table {
-            let mut table = Table::new(&table, self.dir, &self.colours);
+            let mut table = Table::new(&table, self.dir, git, &self.colours);
 
             if self.opts.header {
                 let header = table.header_row();
@@ -178,7 +179,6 @@ impl<'a> Render<'a> {
         use scoped_threadpool::Pool;
         use std::sync::{Arc, Mutex};
         use fs::feature::xattr;
-        use fs::DirOptions;
 
         let mut pool = Pool::new(num_cpus::get() as u32);
         let mut file_eggs = Vec::new();
@@ -241,7 +241,7 @@ impl<'a> Render<'a> {
 
                     if let Some(r) = self.recurse {
                         if file.is_directory() && r.tree && !r.is_too_deep(depth.0) {
-                            match file.to_dir(DirOptions { git: None }) {
+                            match file.to_dir() {
                                 Ok(d)  => { dir = Some(d); },
                                 Err(e) => { errors.push((e, None)) },
                             }

+ 12 - 11
src/output/grid_details.rs

@@ -6,6 +6,7 @@ use ansi_term::ANSIStrings;
 use term_grid as grid;
 
 use fs::{Dir, File};
+use fs::feature::git::GitCache;
 use fs::feature::xattr::FileAttributes;
 use fs::filter::FileFilter;
 
@@ -110,21 +111,21 @@ impl<'a> Render<'a> {
         }
     }
 
-    pub fn render<W: Write>(self, w: &mut W) -> IOResult<()> {
-        if let Some((grid, width)) = self.find_fitting_grid() {
+    pub fn render<W: Write>(self, git: Option<&GitCache>, w: &mut W) -> IOResult<()> {
+        if let Some((grid, width)) = self.find_fitting_grid(git) {
             write!(w, "{}", grid.fit_into_columns(width))
         }
         else {
-            self.give_up().render(w)
+            self.give_up().render(git, w)
         }
     }
 
-    pub fn find_fitting_grid(&self) -> Option<(grid::Grid, grid::Width)> {
+    pub fn find_fitting_grid(&self, git: Option<&GitCache>) -> Option<(grid::Grid, grid::Width)> {
         let options = self.details.table.as_ref().expect("Details table options not given!");
 
         let drender = self.details();
 
-        let (first_table, _) = self.make_table(options, &drender);
+        let (first_table, _) = self.make_table(options, git, &drender);
 
         let rows = self.files.iter()
                        .map(|file| first_table.row_for_file(file, file_has_xattrs(file)))
@@ -134,12 +135,12 @@ impl<'a> Render<'a> {
                              .map(|file| self.style.for_file(file, self.colours).paint().promote())
                              .collect::<Vec<TextCell>>();
 
-        let mut last_working_table = self.make_grid(1, options, &file_names, rows.clone(), &drender);
+        let mut last_working_table = self.make_grid(1, options, git, &file_names, rows.clone(), &drender);
 
         // If we can’t fit everything in a grid 100 columns wide, then
         // something has gone seriously awry
         for column_count in 2..100 {
-            let grid = self.make_grid(column_count, options, &file_names, rows.clone(), &drender);
+            let grid = self.make_grid(column_count, options, git, &file_names, rows.clone(), &drender);
 
             let the_grid_fits = {
                 let d = grid.fit_into_columns(column_count);
@@ -166,8 +167,8 @@ impl<'a> Render<'a> {
         None
     }
 
-    fn make_table<'t>(&'a self, options: &'a TableOptions, drender: &DetailsRender) -> (Table<'a>, Vec<DetailsRow>) {
-        let mut table = Table::new(options, self.dir, self.colours);
+    fn make_table<'t>(&'a self, options: &'a TableOptions, git: Option<&'a GitCache>, drender: &DetailsRender) -> (Table<'a>, Vec<DetailsRow>) {
+        let mut table = Table::new(options, self.dir, git, self.colours);
         let mut rows = Vec::new();
 
         if self.details.header {
@@ -179,11 +180,11 @@ impl<'a> Render<'a> {
         (table, rows)
     }
 
-    fn make_grid(&'a self, column_count: usize, options: &'a TableOptions, file_names: &[TextCell], rows: Vec<TableRow>, drender: &DetailsRender) -> grid::Grid {
+    fn make_grid(&'a self, column_count: usize, options: &'a TableOptions, git: Option<&GitCache>, file_names: &[TextCell], rows: Vec<TableRow>, drender: &DetailsRender) -> grid::Grid {
 
         let mut tables = Vec::new();
         for _ in 0 .. column_count {
-            tables.push(self.make_table(options, drender));
+            tables.push(self.make_table(options, git, drender));
         }
 
         let mut num_cells = rows.len();

+ 30 - 30
src/output/table.rs

@@ -14,6 +14,7 @@ use style::Colours;
 use output::cell::TextCell;
 use output::time::TimeFormat;
 use fs::{File, Dir, fields as f};
+use fs::feature::git::GitCache;
 
 
 /// Options for displaying a table.
@@ -24,6 +25,14 @@ pub struct Options {
     pub extra_columns: Columns,
 }
 
+// I had to make other types derive Debug,
+// and Mutex<UsersCache> is not that!
+impl fmt::Debug for Options {
+    fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
+        write!(f, "<table options>")
+    }
+}
+
 /// Extra columns to display in the table.
 #[derive(PartialEq, Debug)]
 pub struct Columns {
@@ -36,24 +45,12 @@ pub struct Columns {
     pub links: bool,
     pub blocks: bool,
     pub group: bool,
-    pub git: bool
-}
-
-impl fmt::Debug for Options {
-    fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
-        // I had to make other types derive Debug,
-        // and Mutex<UsersCache> is not that!
-        writeln!(f, "<table options>")
-    }
+    pub git: bool,
 }
 
 impl Columns {
-    pub fn should_scan_for_git(&self) -> bool {
-        self.git
-    }
-
-    pub fn for_dir(&self, dir: Option<&Dir>) -> Vec<Column> {
-        let mut columns = vec![];
+    pub fn collect(&self, actually_enable_git: bool) -> Vec<Column> {
+        let mut columns = Vec::with_capacity(4);
 
         if self.inode {
             columns.push(Column::Inode);
@@ -89,12 +86,8 @@ impl Columns {
             columns.push(Column::Timestamp(TimeType::Accessed));
         }
 
-        if cfg!(feature="git") {
-            if let Some(d) = dir {
-                if self.should_scan_for_git() && d.has_git_repo() {
-                    columns.push(Column::GitStatus);
-                }
-            }
+        if cfg!(feature="git") && self.git && actually_enable_git {
+            columns.push(Column::GitStatus);
         }
 
         columns
@@ -275,9 +268,6 @@ fn determine_time_zone() -> TZResult<TimeZone> {
 }
 
 
-
-
-
 pub struct Table<'a> {
     columns: Vec<Column>,
     colours: &'a Colours,
@@ -285,6 +275,7 @@ pub struct Table<'a> {
     widths: TableWidths,
     time_format: &'a TimeFormat,
     size_format: SizeFormat,
+    git: Option<&'a GitCache>,
 }
 
 #[derive(Clone)]
@@ -293,11 +284,13 @@ pub struct Row {
 }
 
 impl<'a, 'f> Table<'a> {
-    pub fn new(options: &'a Options, dir: Option<&'a Dir>, colours: &'a Colours) -> Table<'a> {
-        let colz = options.extra_columns.for_dir(dir);
-        let widths = TableWidths::zero(colz.len());
-        Table { colours, widths,
-            columns: colz,
+    pub fn new(options: &'a Options, dir: Option<&'a Dir>, git: Option<&'a GitCache>, colours: &'a Colours) -> Table<'a> {
+        let has_git = if let (Some(g), Some(d)) = (git, dir) { g.has_anything_for(&d.path) } else { false };
+        let columns = options.extra_columns.collect(has_git);
+        let widths = TableWidths::zero(columns.len());
+
+        Table {
+            colours, widths, columns, git,
             env:         &options.env,
             time_format: &options.time_format,
             size_format:  options.size_format,
@@ -347,7 +340,7 @@ impl<'a, 'f> Table<'a> {
             Column::Blocks         => file.blocks().render(self.colours),
             Column::User           => file.user().render(self.colours, &*self.env.lock_users()),
             Column::Group          => file.group().render(self.colours, &*self.env.lock_users()),
-            Column::GitStatus      => file.git_status().render(self.colours),
+            Column::GitStatus      => self.git_status(file).render(self.colours),
 
             Column::Timestamp(Modified)  => file.modified_time().render(self.colours.date, &self.env.tz, &self.time_format),
             Column::Timestamp(Created)   => file.created_time() .render(self.colours.date, &self.env.tz, &self.time_format),
@@ -355,6 +348,13 @@ impl<'a, 'f> Table<'a> {
         }
     }
 
+    fn git_status(&self, file: &File) -> f::Git {
+        debug!("Getting Git status for file {:?}", file.path);
+        self.git
+            .map(|g| g.get(&file.path, file.is_directory()))
+            .unwrap_or_default()
+    }
+
     pub fn render(&self, row: Row) -> TextCell {
         let mut cell = TextCell::default();
 

+ 7 - 7
xtests/git_1_recurse

@@ -3,14 +3,14 @@
 drwxrwxr-x - cassowary  1 Jan 12:34 N- moves
 
 /testcases/git/additions:
-.rw-rw-r-- 20 cassowary  1 Jan 12:34 edited
-.rw-rw-r--  0 cassowary  1 Jan 12:34 staged
-.rw-rw-r--  0 cassowary  1 Jan 12:34 unstaged
+.rw-rw-r-- 20 cassowary  1 Jan 12:34 NM edited
+.rw-rw-r--  0 cassowary  1 Jan 12:34 N- staged
+.rw-rw-r--  0 cassowary  1 Jan 12:34 -N unstaged
 
 /testcases/git/edits:
-.rw-rw-r-- 20 cassowary  1 Jan 12:34 both
-.rw-rw-r-- 15 cassowary  1 Jan 12:34 staged
-.rw-rw-r-- 20 cassowary  1 Jan 12:34 unstaged
+.rw-rw-r-- 20 cassowary  1 Jan 12:34 MM both
+.rw-rw-r-- 15 cassowary  1 Jan 12:34 M- staged
+.rw-rw-r-- 20 cassowary  1 Jan 12:34 -M unstaged
 
 /testcases/git/moves:
-.rw-rw-r-- 21 cassowary  1 Jan 12:34 thither
+.rw-rw-r-- 21 cassowary  1 Jan 12:34 N- thither

+ 9 - 9
xtests/git_2_recurse

@@ -3,22 +3,22 @@
 drwxrwxr-x - cassowary  1 Jan 12:34 -- target
 
 /testcases/git2/deeply:
-drwxrwxr-x - cassowary  1 Jan 12:34 nested
+drwxrwxr-x - cassowary  1 Jan 12:34 -N nested
 
 /testcases/git2/deeply/nested:
-drwxrwxr-x - cassowary  1 Jan 12:34 directory
-drwxrwxr-x - cassowary  1 Jan 12:34 repository
+drwxrwxr-x - cassowary  1 Jan 12:34 -N directory
+drwxrwxr-x - cassowary  1 Jan 12:34 -N repository
 
 /testcases/git2/deeply/nested/directory:
-.rw-rw-r--  0 cassowary  1 Jan 12:34 l8st
-.rw-rw-r-- 18 cassowary  1 Jan 12:34 upd8d
+.rw-rw-r--  0 cassowary  1 Jan 12:34 -N l8st
+.rw-rw-r-- 18 cassowary  1 Jan 12:34 -M upd8d
 
 /testcases/git2/deeply/nested/repository:
-.rw-rw-r-- 0 cassowary  1 Jan 12:34 subfile
+.rw-rw-r-- 0 cassowary  1 Jan 12:34 -- subfile
 
 /testcases/git2/ignoreds:
-.rw-rw-r-- 0 cassowary  1 Jan 12:34 music.m4a
-.rw-rw-r-- 0 cassowary  1 Jan 12:34 music.mp3
+.rw-rw-r-- 0 cassowary  1 Jan 12:34 -N music.m4a
+.rw-rw-r-- 0 cassowary  1 Jan 12:34 -- music.mp3
 
 /testcases/git2/target:
-.rw-rw-r-- 0 cassowary  1 Jan 12:34 another ignored file
+.rw-rw-r-- 0 cassowary  1 Jan 12:34 -- another ignored file