Skip to content

Commit

Permalink
Merge #73
Browse files Browse the repository at this point in the history
73: Emit more informative error message when cwd does not exist r=matklad a=smoelius

Fixes #72 in the manner described in #72 (comment).

Nits are welcome.

Co-authored-by: Samuel Moelius <sam@moeli.us>
  • Loading branch information
bors[bot] and smoelius authored Jul 10, 2023
2 parents 9cf87f3 + e52c384 commit 0af5ceb
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 6 deletions.
12 changes: 8 additions & 4 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ pub struct Error {

/// Note: this is intentionally not public.
enum ErrorKind {
CurrentDir { err: io::Error },
CurrentDir { err: io::Error, path: Option<PathBuf> },
Var { err: env::VarError, var: OsString },
ReadFile { err: io::Error, path: PathBuf },
ReadDir { err: io::Error, path: PathBuf },
Expand All @@ -37,7 +37,11 @@ impl From<ErrorKind> for Error {
impl fmt::Display for Error {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match &*self.kind {
ErrorKind::CurrentDir { err } => write!(f, "failed to get current directory: {err}"),
ErrorKind::CurrentDir { err, path } => {
let suffix =
path.as_ref().map_or(String::new(), |path| format!(" `{}`", path.display()));
write!(f, "failed to get current directory{suffix}: {err}")
}
ErrorKind::Var { err, var } => {
let var = var.to_string_lossy();
write!(f, "failed to get environment variable `{var}`: {err}")
Expand Down Expand Up @@ -113,8 +117,8 @@ impl std::error::Error for Error {}

/// `pub(crate)` constructors, visible only in this crate.
impl Error {
pub(crate) fn new_current_dir(err: io::Error) -> Error {
ErrorKind::CurrentDir { err }.into()
pub(crate) fn new_current_dir(err: io::Error, path: Option<PathBuf>) -> Error {
ErrorKind::CurrentDir { err, path }.into()
}

pub(crate) fn new_var(err: env::VarError, var: OsString) -> Error {
Expand Down
15 changes: 13 additions & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ impl Shell {
///
/// Fails if [`std::env::current_dir`] returns an error.
pub fn new() -> Result<Shell> {
let cwd = current_dir().map_err(Error::new_current_dir)?;
let cwd = current_dir().map_err(|err| Error::new_current_dir(err, None))?;
let cwd = RefCell::new(cwd);
let env = RefCell::new(HashMap::new());
Ok(Shell { cwd, env })
Expand Down Expand Up @@ -1013,7 +1013,18 @@ impl<'a> Cmd<'a> {
None => Stdio::null(),
});

command.spawn().map_err(|err| Error::new_cmd_io(self, err))?
command.spawn().map_err(|err| {
// Try to determine whether the command failed because the current
// directory does not exist. Return an appropriate error in such a
// case.
if matches!(err.kind(), io::ErrorKind::NotFound) {
let cwd = self.shell.cwd.borrow();
if let Err(err) = cwd.metadata() {
return Error::new_current_dir(err, Some(cwd.clone()));
}
}
Error::new_cmd_io(self, err)
})?
};

let mut io_thread = None;
Expand Down
16 changes: 16 additions & 0 deletions tests/it/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -460,3 +460,19 @@ fn string_escapes() {
assert_eq!(cmd!(sh, "\"\"\"asdf\"\"\"").to_string(), r##""""asdf""""##);
assert_eq!(cmd!(sh, "\\\\").to_string(), r#"\\"#);
}

#[test]
fn nonexistent_current_directory() {
let sh = setup();
sh.change_dir("nonexistent");
let err = cmd!(sh, "ls").run().unwrap_err();
if cfg!(unix) {
assert!(err.to_string().starts_with("failed to get current directory"));
assert!(err.to_string().ends_with("No such file or directory (os error 2)"));
} else {
assert_eq!(
err.to_string(),
"io error when running command `ls`: The directory name is invalid. (os error 267)"
);
}
}

0 comments on commit 0af5ceb

Please sign in to comment.