-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
Use Path::is_dir() in fs::read_dir()'s example. #33958
Conversation
Basically reverts rust-lang#25508. The `is_dir()` function has been stable since 1.5.0.
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
It's now useless to import Once fixed, it can be merged. |
@GuillaumeGomez It is still needed since there is a |
Oh right, sorry didn't see that one. Then it's all good. Thanks! @bors: r+ rollup |
📌 Commit 048f372 has been approved by |
Use Path::is_dir() in fs::read_dir()'s example. Basically reverts #25508. The `is_dir()` function has been stable since 1.5.0.
Use Path::is_dir() in fs::read_dir()'s example. Basically reverts rust-lang#25508. The `is_dir()` function has been stable since 1.5.0.
⛄ The build was interrupted to prioritize another pull request. |
/// for entry in try!(fs::read_dir(dir)) { | ||
/// let entry = try!(entry); | ||
/// if try!(fs::metadata(entry.path())).is_dir() { | ||
/// if try!(entry.file_type()).is_dir() { |
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 is actually changing the meaning of this example because file_type()
doesn't follow symlinks, perhaps this could change to calling is_dir
on the return value of path
to preserve the same behavior?
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.
Arf, I misread the doc then. :-/
My bad.
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.
@alexcrichton Oops sorry. Since this has been merged, do I submit another PR for this?
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.
@kennytm: Yes please.
…st-lang#33958. DirEntry.file_type().is_dir() will not follow symlinks, but the original example (fs::metadata(&path).is_dir()) does. Therefore the change in rust-lang#33958 introduced a subtle difference that now it won't enter linked folders. To preserve the same behavior, we use Path::is_dir() instead, which does follow symlink.
Restore original meaning of std::fs::read_dir's example changed in rust-lang#33958 `DirEntry.file_type().is_dir()` will not follow symlinks, but the original example (`fs::metadata(&path).is_dir()`) does. Therefore the change in rust-lang#33958 introduced a subtle difference that now it won't enter linked folders. To preserve the same behavior, we use `Path::is_dir()` instead, which does follow symlink. (See discussion in the previous PR for detail.)
Basically reverts #25508. The
is_dir()
function has been stable since 1.5.0.