Merge pull request #1928 from tmccombs/full-path-fixes

Full path fixes

Author: tavianator

src/config.rs

@@ -17,7 +17,7 @@ pub struct Config {
 
     /// Cached current working directory for absolute path construction.
     /// Populated when `--full-path` is set; `None` means search by filename only.
-    pub cwd: Option<PathBuf>,
+    pub full_path_base: Option<PathBuf>,
 
     /// Whether to ignore hidden files and directories (or not).
     pub ignore_hidden: bool,

src/filesystem.rs

@@ -20,17 +20,6 @@ pub fn path_absolute_form(path: &Path) -> io::Result<PathBuf> {
     env::current_dir().map(|path_buf| path_buf.join(path))
 }
 
-/// Construct an absolute path from a potentially relative path and a
-/// pre-resolved working directory. Unlike `path_absolute_form`, this
-/// does not call `env::current_dir()` and cannot fail.
-pub fn make_absolute(path: &Path, cwd: &Path) -> PathBuf {
-    if path.is_absolute() {
-        return path.to_path_buf();
-    }
-    let path = path.strip_prefix(".").unwrap_or(path);
-    cwd.join(path)
-}
-
 pub fn absolute_path(path: &Path) -> io::Result<PathBuf> {
     let path_buf = path_absolute_form(path)?;
 
@@ -164,40 +153,4 @@ mod tests {
             Path::new("foo/bar/baz")
         );
     }
-
-    #[test]
-    fn make_absolute_with_relative_path() {
-        use super::make_absolute;
-        use std::path::PathBuf;
-
-        let cwd = Path::new("/home/user");
-        assert_eq!(
-            make_absolute(Path::new("foo/bar"), cwd),
-            PathBuf::from("/home/user/foo/bar")
-        );
-    }
-
-    #[test]
-    fn make_absolute_strips_dot_prefix() {
-        use super::make_absolute;
-        use std::path::PathBuf;
-
-        let cwd = Path::new("/home/user");
-        assert_eq!(
-            make_absolute(Path::new("./foo/bar"), cwd),
-            PathBuf::from("/home/user/foo/bar")
-        );
-    }
-
-    #[test]
-    fn make_absolute_with_absolute_path() {
-        use super::make_absolute;
-        use std::path::PathBuf;
-
-        let cwd = Path::new("/home/user");
-        assert_eq!(
-            make_absolute(Path::new("/absolute/path"), cwd),
-            PathBuf::from("/absolute/path")
-        );
-    }
 }

src/main.rs

@@ -245,7 +245,7 @@ fn construct_config(mut opts: Opts, pattern_regexps: &[String]) -> Result<Config
     let command = extract_command(&mut opts, colored_output)?;
     let has_command = command.is_some();
 
-    let cwd = if opts.full_path {
+    let full_path_base = if opts.full_path {
         Some(env::current_dir().context(
             "Could not determine current directory. \
              This is required for --full-path.",
@@ -256,7 +256,7 @@ fn construct_config(mut opts: Opts, pattern_regexps: &[String]) -> Result<Config
 
     Ok(Config {
         case_sensitive,
-        cwd,
+        full_path_base,
         ignore_hidden: !(opts.hidden || opts.rg_alias_ignore()),
         read_fdignore: !(opts.no_ignore || opts.rg_alias_ignore()),
         read_vcsignore: !(opts.no_ignore || opts.rg_alias_ignore() || opts.no_ignore_vcs),

src/walk.rs

@@ -524,7 +524,7 @@ impl WorkerState {
                 // Check the name first, since it doesn't require metadata
                 let entry_path = entry.path();
 
-                let search_str = search_str_for_entry(entry_path, config.cwd.as_deref());
+                let search_str = search_str_for_entry(entry_path, config.full_path_base.as_deref());
 
                 if !patterns
                     .iter()
@@ -665,11 +665,16 @@ impl WorkerState {
 
 fn search_str_for_entry<'a>(
     entry_path: &'a std::path::Path,
-    cwd: Option<&std::path::Path>,
+    full_path_base: Option<&std::path::Path>,
 ) -> Cow<'a, OsStr> {
-    if let Some(cwd) = cwd {
-        let abs_path = filesystem::make_absolute(entry_path, cwd);
-        Cow::Owned(abs_path.into_os_string())
+    if let Some(cwd) = full_path_base {
+        // If full_path_base is some, that means that we need to return
+        // the absolute path
+        if entry_path.is_absolute() {
+            return Cow::Borrowed(entry_path.as_os_str());
+        }
+        let path = entry_path.strip_prefix(".").unwrap_or(entry_path);
+        Cow::Owned(cwd.join(path).into())
     } else {
         match entry_path.file_name() {
             Some(filename) => Cow::Borrowed(filename),
@@ -690,3 +695,44 @@ fn search_str_for_entry<'a>(
 pub fn scan(paths: &[PathBuf], patterns: Vec<Regex>, config: Config) -> Result<ExitCode> {
     WorkerState::new(patterns, config).scan(paths)
 }
+
+#[cfg(test)]
+mod tests {
+    use super::search_str_for_entry;
+    use std::path::{Path, PathBuf};
+
+    #[test]
+    fn search_str_for_entry_with_relative_path() {
+        let full_path_base = Some(Path::new("/home/user"));
+        assert_eq!(
+            search_str_for_entry(Path::new("foo/bar"), full_path_base),
+            PathBuf::from("/home/user/foo/bar")
+        );
+    }
+
+    #[test]
+    fn search_str_for_entry_strips_dot_prefix() {
+        let full_path_base = Some(Path::new("/home/user"));
+        assert_eq!(
+            search_str_for_entry(Path::new("./foo/bar"), full_path_base),
+            PathBuf::from("/home/user/foo/bar")
+        );
+    }
+
+    #[test]
+    fn search_str_for_entry_with_absolute_path() {
+        let full_path_base = Some(Path::new("/home/user"));
+        assert_eq!(
+            search_str_for_entry(Path::new("/absolute/path"), full_path_base),
+            PathBuf::from("/absolute/path")
+        );
+    }
+
+    #[test]
+    fn search_str_no_base_dir() {
+        assert_eq!(
+            search_str_for_entry(Path::new("./foo/bar"), None),
+            PathBuf::from("bar")
+        );
+    }
+}