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

refactor traversal #52

Merged
merged 1 commit into from
Jun 19, 2020
Merged

refactor traversal #52

merged 1 commit into from
Jun 19, 2020

Conversation

drahnr
Copy link
Owner

@drahnr drahnr commented Jun 19, 2020

What does this PR accomplish?

  • Bug Fix
  • Feature
  • Documentation

Changes proposed by this PR:

Fill in gaps in path traversal, required for #37 and #43

@drahnr drahnr force-pushed the bernhard-refactor-traversal branch from 4c136c4 to f389b2d Compare June 19, 2020 16:58
@drahnr drahnr requested a review from KuabeM June 19, 2020 16:58
@drahnr drahnr changed the title Bernhard refactor traversal refactor traversal Jun 19, 2020
@drahnr drahnr merged commit a26aee1 into master Jun 19, 2020
@drahnr drahnr deleted the bernhard-refactor-traversal branch June 19, 2020 20:11
P: AsRef<Path>,
{
let path = path.as_ref();
let path = path.canonicalize().unwrap();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is unwrap() safe here, why not ??

Copy link
Owner Author

Choose a reason for hiding this comment

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

Canonicalize should only fail in https://doc.rust-lang.org/src/std/fs.rs.html#1847-1848 cases, so yes, this should be fixed at some point, but it does not hurt right now, since we check the metadata before we ever feed the path to this function, which guarantees it's either a file or directory.

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