Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add exists method on Dir #33

Merged
merged 1 commit into from
Sep 20, 2024
Merged

Add exists method on Dir #33

merged 1 commit into from
Sep 20, 2024

Conversation

aurelj
Copy link
Contributor

@aurelj aurelj commented Jul 26, 2024

This is pretty useful to be able to check for the existence of a file without having to actually open it.

@@ -194,6 +194,25 @@ impl<'a, IO: ReadWriteSeek, TP: TimeProvider, OCC: OemCpConverter> Dir<'a, IO, T
}
}

/// Check to see if a file or directory with the given name exists
pub async fn exists(&self, mut path: &str, is_dir: Option<bool>) -> Result<bool, Error<IO::Error>> {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is_dir: Option<bool> in the API feels a bit weird, I don't think it's needed.

IIRC, if is_dir is None, find_entry will return the entry whether it's a file or a dir. Could you check this, and also add a test case?

@aurelj
Copy link
Contributor Author

aurelj commented Sep 12, 2024

Yes, indeed, if is_dir is None, find_entry will return the entry whether it's a file or a dir.

I understand why is_dir: Option<bool> feels weird in the API.
It may be clearer to have instead 3 separate functions in the API:

pub async fn exists(&self, mut path: &str) -> Result<bool, Error<IO::Error>>;
pub async fn file_exists(&self, mut path: &str) -> Result<bool, Error<IO::Error>>;
pub async fn dir_exists(&self, mut path: &str) -> Result<bool, Error<IO::Error>>;

(each one would only call a non-public function with a is_dir parameter)

Does it feel like a better API to you ?

Oh, and yes, I will add some tests.

Copy link
Owner

@MabezDev MabezDev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@MabezDev MabezDev merged commit 4d2a2db into MabezDev:master Sep 20, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants