-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
RFC: Expand the scope of std::fs
#1044
Changes from 1 commit
f3153e6
b3d1ad6
91c0df5
e6c4854
ebf5514
bc8759d
fedf7fd
11265b4
4edf628
6feacbb
646d41c
d8bc0fe
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -376,6 +376,43 @@ pub fn template<P: AsRef<Path>>(&mut self, path: P) -> &mut Self; | |
At this time, however, it it not proposed to add this method to | ||
`CreateDirOptions`. | ||
|
||
## Adding `FileType` | ||
|
||
Currently there is no enumeration or newtype representing a list of "file types" | ||
on the local filesystem. This is partly done because the need is not so high | ||
right now. Some situations, however, imply that it is more efficient to learn | ||
the file type at once instead of testing for each individual file type itself. | ||
|
||
For example some platforms' `DirEntry` type can know the `FileType` without an | ||
extra syscall. If code were to test a `DirEntry` separately for whether it's a | ||
file or a directory, it may issue more syscalls necessary than if it instead | ||
learned the type and then tested that if it was a file or directory. | ||
|
||
The full set of file types, however, is not always known nor portable across | ||
platforms, so this RFC proposes the following hierarchy: | ||
|
||
```rust | ||
#[derive(Copy, Clone, PartialEq, Eq, Hash)] | ||
pub struct FileType(..); | ||
|
||
impl FileType { | ||
pub fn is_dir(&self) -> bool; | ||
pub fn is_file(&self) -> bool; | ||
pub fn is_soft_link(&self) -> bool; | ||
} | ||
``` | ||
|
||
Extension traits can be added in the future for testing for other more flavorful | ||
kinds of files on various platforms (such as unix sockets on unix platforms). | ||
|
||
#### Dealing with `is_{file,dir}` and `file_type` methods | ||
|
||
Currently the `fs::Metadata` structure exposes stable `is_file` and `is_dir` | ||
accessors. The struct will also grow a `file_type` accessor for this newtype | ||
struct being added. It is proposed that `Metadata` will retain the | ||
`is_{file,dir}` convenience methods, but no other "file type testers" will be | ||
added. | ||
|
||
## Adding `fs::equivalent` | ||
|
||
A new function `equivalent` will be added to the `fs` module along the lines of | ||
|
@@ -406,11 +443,6 @@ The following APIs will be added to `std::fs`: | |
/// Returns the metadata of the file pointed to by `p`, and this function, | ||
/// unlike `metadata` will **not** follow soft links. | ||
pub fn soft_link_metadata<P: AsRef<Path>>(p: P) -> io::Result<Metadata>; | ||
|
||
impl Metadata { | ||
/// Tests whether this metadata is for a soft link or not. | ||
pub fn is_soft_link(&self) -> bool; | ||
} | ||
``` | ||
|
||
## Binding `realpath` | ||
|
@@ -460,7 +492,6 @@ pub trait PathExt { | |
fn exists(&self) -> bool; | ||
fn is_dir(&self) -> bool; | ||
fn is_file(&self) -> bool; | ||
fn is_soft_link(&self) -> bool; | ||
fn metadata(&self) -> io::Result<Metadata>; | ||
fn soft_link_metadata(&self) -> io::Result<Metadata>; | ||
fn canonicalize(&self) -> io::Result<PathBuf>; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we have a method and a free function? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you don't already have a fs::canonicalize("foo");
// vs
use std::path::Path;
Path::new("foo").canonicalize(); The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Couldn't that fall out of AsPath / UFCS, though? Path::canonicalize("foo") or similar? |
||
|
@@ -492,15 +523,12 @@ impl DirEntry { | |
/// return metadata. | ||
pub fn metadata(&self) -> io::Result<Metadata>; | ||
|
||
/// Accessors for testing what file type this `DirEntry` contains. | ||
/// Return what file type this `DirEntry` contains. | ||
/// | ||
/// On some platforms this may not require reading the metadata of the | ||
/// underlying file from the filesystem, but on other platforms it may be | ||
/// required to do so. | ||
pub fn is_dir(&self) -> bool; | ||
pub fn is_file(&self) -> bool; | ||
pub fn is_soft_link(&self) -> bool; | ||
// ... | ||
pub fn file_type(&self) -> FileType; | ||
|
||
/// Returns the file name for this directory entry. | ||
pub fn file_name(&self) -> OsString; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happen if this is a directory or something else ? Do we call this function ? |
||
|
@@ -542,6 +570,9 @@ mod os::unix::fs { | |
necessarily seem appropriate for all the methods and using inherent methods | ||
also seems more logical. | ||
|
||
* Instead of continuing to add `is_<file_type>` accessors to `Metadata` and | ||
`DirEntry`, there could | ||
|
||
# Unresolved questions | ||
|
||
* What is the ultimate role of crates like `liblibc`, and how do we draw the | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This duplication makes me somewhat sad (i.e.
.metadata().is_file()
would be equivalent I believe)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately you'd need to do
.metadata().map(|m| m.is_file()).unwrap_or(false)
instead of just.metdata().is_file()
. Are you thinking of adding an extension trait forio::Result<Metadata>
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Err, no way.
Ah, yeah. I’m still doubting the usefulness of this abstraction. Especially because not being able to retrieve metadata doesn’t imply it is not a file or a directory (same applies to
exists
too).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would be pretty worried about never providing conveniences like this in Rust, and I suppose the alternative would be to return a tri-state value (Yes, Maybe, No), but I feel like that's somewhat overkill for these convenience functions...