fix: Use logical path instead of physical path when available (#398)

* Get pathbuf from logical path. (fixes #204)

(also fixes #397)

* fix: Update directory module so that use_logical_path will work properly

* Remove test directory::use_logical_and_physical_paths

* Fix merge errors


Co-authored-by: Matan Kushner <hello@matchai.me>

src/context.rs

@@ -39,7 +39,12 @@ impl<'a> Context<'a> {
         let path = arguments
             .value_of("path")
             .map(From::from)
-            .unwrap_or_else(|| env::current_dir().expect("Unable to identify current directory."));
+            .unwrap_or_else(|| {
+                env::var("PWD").map(PathBuf::from).unwrap_or_else(|err| {
+                    log::debug!("Unable to get path from $PWD: {}", err);
+                    env::current_dir().expect("Unable to identify current directory.")
+                })
+            });
 
         Context::new_with_dir(arguments, path)
     }

src/modules/directory.rs

@@ -26,21 +26,22 @@ pub fn module<'a>(context: &'a Context) -> Option<Module<'a>> {
 
     // Using environment PWD is the standard approach for determining logical path
     // If this is None for any reason, we fall back to reading the os-provided path
-    let logical_current_dir = if config.use_logical_path {
+    let physical_current_dir = if config.use_logical_path {
-        match std::env::var("PWD") {
+        None
+    } else {
+        match std::env::current_dir() {
             Ok(x) => Some(x),
-            Err(_) => {
+            Err(e) => {
-                log::debug!("Asked for logical path, but PWD was invalid.");
+                log::debug!("Error getting physical current directory: {}", e);
                 None
             }
         }
-    } else {
-        None
     };
-    let current_dir = logical_current_dir
+    let current_dir = Path::new(
-        .as_ref()
+        physical_current_dir
-        .map(|d| Path::new(d))
+            .as_ref()
-        .unwrap_or_else(|| context.current_dir.as_ref());
+            .unwrap_or_else(|| &context.current_dir),
+    );
 
     let home_dir = dirs::home_dir().unwrap();
     log::debug!("Current directory: {:?}", current_dir);

tests/testsuite/directory.rs

@@ -451,66 +451,3 @@ fn git_repo_in_home_directory_truncate_to_repo_true() -> io::Result<()> {
     assert_eq!(expected, actual);
     Ok(())
 }
-
-#[test]
-#[ignore]
-fn use_logical_and_physical_paths() -> io::Result<()> {
-    /* This test is a bit of a smoke + mirrors trick because all it shows is that
-    the application is reading the PWD envar correctly (if the shell doesn't
-    correctly set PWD, we're still in trouble). */
-    let tmp_dir = Path::new("/tmp/starship/porthole/viewport");
-    let dir = tmp_dir.join("directory");
-    let sym = tmp_dir.join("symlink_to_directory");
-    fs::create_dir_all(&dir)?;
-    // Create a symlink on the appropriate system
-    #[cfg(target_family = "unix")]
-    std::os::unix::fs::symlink(&dir, &sym).unwrap();
-    #[cfg(target_family = "windows")]
-    std::os::windows::fs::symlink_file(&dir, &sym).unwrap();
-
-    // Test when using physical paths
-    let output = common::render_module("directory")
-        .use_config(toml::toml! {
-            [directory]
-            use_logical_path = false
-        })
-        .arg("--path")
-        .arg(&dir)
-        .env(
-            "PWD",
-            "/tmp/starship/porthole/viewport/symlink_to_directory",
-        )
-        .output()?;
-    let actual = String::from_utf8(output.stdout).unwrap();
-
-    let expected = format!(
-        "in {} ",
-        Color::Cyan.bold().paint("porthole/viewport/directory")
-    );
-    assert_eq!(expected, actual);
-
-    // Test when using logical paths
-    let output = common::render_module("directory")
-        .use_config(toml::toml! {
-            [directory]
-            use_logical_path = true
-        })
-        .arg("--path")
-        .arg(&sym)
-        .env(
-            "PWD",
-            "/tmp/starship/porthole/viewport/symlink_to_directory",
-        )
-        .output()?;
-    let actual = String::from_utf8(output.stdout).unwrap();
-
-    let expected = format!(
-        "in {} ",
-        Color::Cyan
-            .bold()
-            .paint("porthole/viewport/symlink_to_directory")
-    );
-    assert_eq!(expected, actual);
-
-    Ok(())
-}