Allow Filesystem::walkToRoot to walk to a given root directory. See D13185#139299.
Details
Details
- Reviewers
epriestley - Group Reviewers
Blessed Reviewers - Commits
- rPHU75611019e189: Allow `Filesystem::walkToRoot` to walk to a given root directory
Added unit tests.
Diff Detail
Diff Detail
- Repository
- rPHU libphutil
- Branch
- master
- Lint
Lint Passed - Unit
Tests Passed - Build Status
Buildable 6665 Build 6687: [Placeholder Plan] Wait for 30 Seconds
Event Timeline
Comment Actions
Yeah, I wanted to add something like this:
if (!self::isDescendant($path, $root)) { return array(); }
But this was problematic for my testing purposes because Filesystem::isDescendant($path, $root) requires $root and $path to actually exist. Would you be open to changing this method to be more general?
Comment Actions
What do you think about creating some actual directories inside __tests__/data/ or similar and checking those?
Makes it slightly harder to get the expected values right for stuff above the library, but that seems surmountable to me.
Comment Actions
Personally, I find the behavior of Filesystem::isDescendant to be non-obvious and so I think fixing that is the most correct solution. Adding directories in the __tests__/data directory is less obtrussive however.
Comment Actions
@epriestley, I haven't had time to extend this further to check for "path is not a descendent of root". Is this fine to push as is?
Comment Actions
One possible issue, rest of it looks great to me.
| src/filesystem/Filesystem.php | ||
|---|---|---|
| 767 | This is probably wrong on Windows -- there won't be a leading directory separator on $root, right? Maybe cleaner would be: while ($parts) {
// compute the next thing
// add it to the list
// if we just added the root, break out of the loop
// array_pop parts
} | |