Skip to content

Commit

Permalink
Fix read_dir logic error, improve error messages
Browse files Browse the repository at this point in the history
* `fs::read_dir` skips entries for the current and parent directories, so
  we want it to be completely empty to delete the temporary directory.

* Add a local `fs::read_dir` wrapper for `fs_err::read_dir` to match the
  others in that module.

* Show a warning, not an info message, if the tempdir isn't empty.

* Include the paths in the tempdir in the warning message.
  • Loading branch information
9999years committed Nov 18, 2024
1 parent 35b6cc7 commit e76e393
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 12 deletions.
8 changes: 8 additions & 0 deletions clippy.toml
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,14 @@ reason = "Use git_prole::fs::read_to_string"
path = "std::fs::read_to_string"
reason = "Use git_prole::fs::read_to_string"

[[disallowed-methods]]
path = "fs_err::read_dir"
reason = "Use git_prole::fs::read_dir"

[[disallowed-methods]]
path = "std::fs::read_dir"
reason = "Use git_prole::fs::read_dir"

[[disallowed-methods]]
path = "fs_err::copy"
reason = "Use git_prole::fs::copy"
Expand Down
39 changes: 29 additions & 10 deletions src/convert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -558,16 +558,7 @@ where
);
tracing::info!("You may need to `cd .` to refresh your shell");

// Make sure to delete the tempdir if it's empty
match fs_err::read_dir(&self.tempdir) {
Ok(rd) => {
if rd.count() == 1 {
tracing::info!("Temporary directory isn't empty: {0}", self.tempdir)
}
}
Err(err) => miette::bail!(err),
};
fs::remove_dir(&self.tempdir)?;
remove_tempdir_if_empty(&self.tempdir)?;

Ok(())
}
Expand Down Expand Up @@ -682,3 +673,31 @@ impl MainWorktreePlan {
self.inner.destination(convert_plan).join(".git")
}
}

fn remove_tempdir_if_empty(tempdir: &Utf8Path) -> miette::Result<()> {
let contents = fs::read_dir(tempdir)?.collect::<Vec<_>>();
// From `std::fs::read_dir` documentation:
// > Entries for the current and parent directories (typically . and ..) are skipped.
if !contents.is_empty() {
tracing::warn!(
"Temporary directory isn't empty: {}\n{}",
tempdir.display_path_cwd(),
display_dir_contents(&contents)
);
} else {
fs::remove_dir(tempdir)?;
}

Ok(())
}

fn display_dir_contents(contents: &[std::io::Result<fs_err::DirEntry>]) -> String {
format_bulleted_list(contents.iter().map(|item| {
match item {
Ok(entry) => entry.file_name().display_path_cwd(),
Err(error) => error
.if_supports_color(Stream::Stdout, |text| text.red())
.to_string(),
}
}))
}
10 changes: 10 additions & 0 deletions src/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
use std::fmt::Debug;
use std::path::Path;
use std::path::PathBuf;

use miette::IntoDiagnostic;
use tracing::instrument;
Expand Down Expand Up @@ -72,3 +73,12 @@ where
#[expect(clippy::disallowed_methods)]
fs_err::write(path, contents).into_diagnostic()
}

#[instrument(level = "trace")]
pub fn read_dir<P>(path: P) -> miette::Result<fs_err::ReadDir>
where
P: Into<PathBuf> + Debug,
{
#[expect(clippy::disallowed_methods)]
fs_err::read_dir(path).into_diagnostic()
}
7 changes: 5 additions & 2 deletions src/path_display.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,13 @@ pub trait PathDisplay: Debug + AsRef<Path> {

impl<P> PathDisplay for P
where
P: AsRef<Utf8Path> + AsRef<Path> + Debug,
P: AsRef<Path> + Debug,
{
fn display_path_from(&self, base: impl AsRef<Utf8Path>) -> String {
try_display(self, base).unwrap_or_else(|| display_backup(self))
let path = self.as_ref();
Utf8Path::from_path(path)
.and_then(|utf8path| try_display(utf8path, base))
.unwrap_or_else(|| display_backup(self))
}
}

Expand Down

0 comments on commit e76e393

Please sign in to comment.