From aa260899d452bde337e5e1a42e92ea6331ec9fc7 Mon Sep 17 00:00:00 2001 From: Zhenhui Xie Date: Thu, 24 Oct 2019 18:37:44 +0800 Subject: [PATCH] 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 --- src/context.rs | 7 +++- src/modules/directory.rs | 21 ++++++------ tests/testsuite/directory.rs | 63 ------------------------------------ 3 files changed, 17 insertions(+), 74 deletions(-) diff --git a/src/context.rs b/src/context.rs index 936dce9d..f1ab2997 100644 --- a/src/context.rs +++ b/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) } diff --git a/src/modules/directory.rs b/src/modules/directory.rs index 4436eebf..a3f73af5 100644 --- a/src/modules/directory.rs +++ b/src/modules/directory.rs @@ -26,21 +26,22 @@ pub fn module<'a>(context: &'a Context) -> Option> { // 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 { - match std::env::var("PWD") { + let physical_current_dir = if config.use_logical_path { + None + } else { + match std::env::current_dir() { Ok(x) => Some(x), - Err(_) => { - log::debug!("Asked for logical path, but PWD was invalid."); + Err(e) => { + log::debug!("Error getting physical current directory: {}", e); None } } - } else { - None }; - let current_dir = logical_current_dir - .as_ref() - .map(|d| Path::new(d)) - .unwrap_or_else(|| context.current_dir.as_ref()); + let current_dir = Path::new( + physical_current_dir + .as_ref() + .unwrap_or_else(|| &context.current_dir), + ); let home_dir = dirs::home_dir().unwrap(); log::debug!("Current directory: {:?}", current_dir); diff --git a/tests/testsuite/directory.rs b/tests/testsuite/directory.rs index 5a8af6bc..006764ae 100644 --- a/tests/testsuite/directory.rs +++ b/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(()) -}