Skip to content

Commit e76e393

Browse files
committed
Fix read_dir logic error, improve error messages
* `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.
1 parent 35b6cc7 commit e76e393

File tree

4 files changed

+52
-12
lines changed

4 files changed

+52
-12
lines changed

clippy.toml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,14 @@ reason = "Use git_prole::fs::read_to_string"
6262
path = "std::fs::read_to_string"
6363
reason = "Use git_prole::fs::read_to_string"
6464

65+
[[disallowed-methods]]
66+
path = "fs_err::read_dir"
67+
reason = "Use git_prole::fs::read_dir"
68+
69+
[[disallowed-methods]]
70+
path = "std::fs::read_dir"
71+
reason = "Use git_prole::fs::read_dir"
72+
6573
[[disallowed-methods]]
6674
path = "fs_err::copy"
6775
reason = "Use git_prole::fs::copy"

src/convert.rs

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -558,16 +558,7 @@ where
558558
);
559559
tracing::info!("You may need to `cd .` to refresh your shell");
560560

561-
// Make sure to delete the tempdir if it's empty
562-
match fs_err::read_dir(&self.tempdir) {
563-
Ok(rd) => {
564-
if rd.count() == 1 {
565-
tracing::info!("Temporary directory isn't empty: {0}", self.tempdir)
566-
}
567-
}
568-
Err(err) => miette::bail!(err),
569-
};
570-
fs::remove_dir(&self.tempdir)?;
561+
remove_tempdir_if_empty(&self.tempdir)?;
571562

572563
Ok(())
573564
}
@@ -682,3 +673,31 @@ impl MainWorktreePlan {
682673
self.inner.destination(convert_plan).join(".git")
683674
}
684675
}
676+
677+
fn remove_tempdir_if_empty(tempdir: &Utf8Path) -> miette::Result<()> {
678+
let contents = fs::read_dir(tempdir)?.collect::<Vec<_>>();
679+
// From `std::fs::read_dir` documentation:
680+
// > Entries for the current and parent directories (typically . and ..) are skipped.
681+
if !contents.is_empty() {
682+
tracing::warn!(
683+
"Temporary directory isn't empty: {}\n{}",
684+
tempdir.display_path_cwd(),
685+
display_dir_contents(&contents)
686+
);
687+
} else {
688+
fs::remove_dir(tempdir)?;
689+
}
690+
691+
Ok(())
692+
}
693+
694+
fn display_dir_contents(contents: &[std::io::Result<fs_err::DirEntry>]) -> String {
695+
format_bulleted_list(contents.iter().map(|item| {
696+
match item {
697+
Ok(entry) => entry.file_name().display_path_cwd(),
698+
Err(error) => error
699+
.if_supports_color(Stream::Stdout, |text| text.red())
700+
.to_string(),
701+
}
702+
}))
703+
}

src/fs.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
44
use std::fmt::Debug;
55
use std::path::Path;
6+
use std::path::PathBuf;
67

78
use miette::IntoDiagnostic;
89
use tracing::instrument;
@@ -72,3 +73,12 @@ where
7273
#[expect(clippy::disallowed_methods)]
7374
fs_err::write(path, contents).into_diagnostic()
7475
}
76+
77+
#[instrument(level = "trace")]
78+
pub fn read_dir<P>(path: P) -> miette::Result<fs_err::ReadDir>
79+
where
80+
P: Into<PathBuf> + Debug,
81+
{
82+
#[expect(clippy::disallowed_methods)]
83+
fs_err::read_dir(path).into_diagnostic()
84+
}

src/path_display.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,13 @@ pub trait PathDisplay: Debug + AsRef<Path> {
3030

3131
impl<P> PathDisplay for P
3232
where
33-
P: AsRef<Utf8Path> + AsRef<Path> + Debug,
33+
P: AsRef<Path> + Debug,
3434
{
3535
fn display_path_from(&self, base: impl AsRef<Utf8Path>) -> String {
36-
try_display(self, base).unwrap_or_else(|| display_backup(self))
36+
let path = self.as_ref();
37+
Utf8Path::from_path(path)
38+
.and_then(|utf8path| try_display(utf8path, base))
39+
.unwrap_or_else(|| display_backup(self))
3740
}
3841
}
3942

0 commit comments

Comments
 (0)