From 93cbad6ed5f6a40bdd1e8cee6a9b1a39f17ab166 Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Thu, 11 Jun 2020 17:44:33 +0200 Subject: [PATCH 1/5] Add documentation to point to `!is_dir` instead of `is_file` --- src/libstd/fs.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/libstd/fs.rs b/src/libstd/fs.rs index f4c164a324e32..c2bea8d9d638f 100644 --- a/src/libstd/fs.rs +++ b/src/libstd/fs.rs @@ -1307,6 +1307,12 @@ impl FileType { /// [`is_dir`] and [`is_symlink`]; only zero or one of these /// tests may pass. /// + /// This property means it is often more useful to use `!file_type.is_dir()` + /// than `file_type.is_file()` when your goal is to read bytes from a + /// source: the former includes symlink and pipes when the latter does not, + /// meaning you will break workflows like `diff <( prog_a ) <( prog_b )` on + /// a Unix-like system for example. + /// /// [`is_dir`]: struct.FileType.html#method.is_dir /// [`is_symlink`]: struct.FileType.html#method.is_symlink /// From ec63f9d99b4faec04db0f924c24be9529f4febed Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Thu, 11 Jun 2020 18:15:57 +0200 Subject: [PATCH 2/5] Added the note to Metadata too --- src/libstd/fs.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/libstd/fs.rs b/src/libstd/fs.rs index c2bea8d9d638f..a9c481dfb809f 100644 --- a/src/libstd/fs.rs +++ b/src/libstd/fs.rs @@ -1033,6 +1033,12 @@ impl Metadata { /// [`is_dir`], and will be false for symlink metadata /// obtained from [`symlink_metadata`]. /// + /// This property means it is often more useful to use `!file_type.is_dir()` + /// than `file_type.is_file()` when your goal is to read bytes from a + /// source: the former includes symlink and pipes when the latter does not, + /// meaning you will break workflows like `diff <( prog_a ) <( prog_b )` on + /// a Unix-like system for example. + /// /// [`is_dir`]: struct.Metadata.html#method.is_dir /// [`symlink_metadata`]: fn.symlink_metadata.html /// From c1243dbcd96f43d013e38f01efe91eb35b81fa18 Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Thu, 11 Jun 2020 18:17:00 +0200 Subject: [PATCH 3/5] Make a note about is_dir vs is_file in Path too --- src/libstd/path.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/libstd/path.rs b/src/libstd/path.rs index 8ff7508ba6457..35f6b5838a92a 100644 --- a/src/libstd/path.rs +++ b/src/libstd/path.rs @@ -2503,11 +2503,15 @@ impl Path { /// # See Also /// /// This is a convenience function that coerces errors to false. If you want to - /// check errors, call [fs::metadata] and handle its Result. Then call - /// [fs::Metadata::is_file] if it was Ok. + /// check errors, call [`fs::metadata`] and handle its Result. Then call + /// [`fs::Metadata::is_file`] if it was Ok. /// - /// [fs::metadata]: ../../std/fs/fn.metadata.html - /// [fs::Metadata::is_file]: ../../std/fs/struct.Metadata.html#method.is_file + /// Note that the explanation about using `!is_dir` instead of `is_file` + /// that is present in the [`fs::Metadata`] documentation also applies here. + /// + /// [`fs::metadata`]: ../../std/fs/fn.metadata.html + /// [`fs::Metadata`]: ../../std/fs/struct.Metadata.html + /// [`fs::Metadata::is_file`]: ../../std/fs/struct.Metadata.html#method.is_file #[stable(feature = "path_ext", since = "1.5.0")] pub fn is_file(&self) -> bool { fs::metadata(self).map(|m| m.is_file()).unwrap_or(false) From d25d6c5bd8c211a5e606b2ac37bbecb9be1ac12b Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Sat, 27 Jun 2020 18:10:58 +0200 Subject: [PATCH 4/5] Update the documentation to point to open instead of is_file and is_dir --- src/libstd/fs.rs | 24 ++++++++++++++---------- src/libstd/path.rs | 2 +- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/libstd/fs.rs b/src/libstd/fs.rs index a9c481dfb809f..23bd8f6498b4a 100644 --- a/src/libstd/fs.rs +++ b/src/libstd/fs.rs @@ -1033,14 +1033,16 @@ impl Metadata { /// [`is_dir`], and will be false for symlink metadata /// obtained from [`symlink_metadata`]. /// - /// This property means it is often more useful to use `!file_type.is_dir()` - /// than `file_type.is_file()` when your goal is to read bytes from a - /// source: the former includes symlink and pipes when the latter does not, - /// meaning you will break workflows like `diff <( prog_a ) <( prog_b )` on - /// a Unix-like system for example. + /// When the goal is simply to read from (or write to) the source, the most + /// reliable way to test the source can be read (or written to) is to open + /// it. Only using `is_file` can break workflows like `diff <( prog_a )` on + /// a Unix-like system for example. See [`File::open`] or + /// [`OpenOptions::open`] for more information. /// /// [`is_dir`]: struct.Metadata.html#method.is_dir /// [`symlink_metadata`]: fn.symlink_metadata.html + /// [`File::open`]: struct.File.html#method.open + /// [`OpenOptions::open`]: struct.OpenOptions.html#method.open /// /// # Examples /// @@ -1313,14 +1315,16 @@ impl FileType { /// [`is_dir`] and [`is_symlink`]; only zero or one of these /// tests may pass. /// - /// This property means it is often more useful to use `!file_type.is_dir()` - /// than `file_type.is_file()` when your goal is to read bytes from a - /// source: the former includes symlink and pipes when the latter does not, - /// meaning you will break workflows like `diff <( prog_a ) <( prog_b )` on - /// a Unix-like system for example. + /// When the goal is simply to read from (or write to) the source, the most + /// reliable way to test the source can be read (or written to) is to open + /// it. Only using `is_file` can break workflows like `diff <( prog_a )` on + /// a Unix-like system for example. See [`File::open`] or + /// [`OpenOptions::open`] for more information. /// /// [`is_dir`]: struct.FileType.html#method.is_dir /// [`is_symlink`]: struct.FileType.html#method.is_symlink + /// [`File::open`]: struct.File.html#method.open + /// [`OpenOptions::open`]: struct.OpenOptions.html#method.open /// /// # Examples /// diff --git a/src/libstd/path.rs b/src/libstd/path.rs index 35f6b5838a92a..fa54b2063b424 100644 --- a/src/libstd/path.rs +++ b/src/libstd/path.rs @@ -2506,7 +2506,7 @@ impl Path { /// check errors, call [`fs::metadata`] and handle its Result. Then call /// [`fs::Metadata::is_file`] if it was Ok. /// - /// Note that the explanation about using `!is_dir` instead of `is_file` + /// Note that the explanation about using `open` instead of `is_file` /// that is present in the [`fs::Metadata`] documentation also applies here. /// /// [`fs::metadata`]: ../../std/fs/fn.metadata.html From 8e8c54aa3a8d92d8443ec4596754d14b2d196899 Mon Sep 17 00:00:00 2001 From: Alexis Bourget Date: Sat, 27 Jun 2020 22:59:47 +0200 Subject: [PATCH 5/5] Added the parapgrah to path::Path::is_file too --- src/libstd/path.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/libstd/path.rs b/src/libstd/path.rs index fa54b2063b424..f14a9ff72f62f 100644 --- a/src/libstd/path.rs +++ b/src/libstd/path.rs @@ -2506,12 +2506,17 @@ impl Path { /// check errors, call [`fs::metadata`] and handle its Result. Then call /// [`fs::Metadata::is_file`] if it was Ok. /// - /// Note that the explanation about using `open` instead of `is_file` - /// that is present in the [`fs::Metadata`] documentation also applies here. + /// When the goal is simply to read from (or write to) the source, the most + /// reliable way to test the source can be read (or written to) is to open + /// it. Only using `is_file` can break workflows like `diff <( prog_a )` on + /// a Unix-like system for example. See [`File::open`] or + /// [`OpenOptions::open`] for more information. /// /// [`fs::metadata`]: ../../std/fs/fn.metadata.html /// [`fs::Metadata`]: ../../std/fs/struct.Metadata.html /// [`fs::Metadata::is_file`]: ../../std/fs/struct.Metadata.html#method.is_file + /// [`File::open`]: ../../std/fs/struct.File.html#method.open + /// [`OpenOptions::open`]: ../../std/fs/struct.OpenOptions.html#method.open #[stable(feature = "path_ext", since = "1.5.0")] pub fn is_file(&self) -> bool { fs::metadata(self).map(|m| m.is_file()).unwrap_or(false)