Make Get-ChildItem honor Depth parameter with Include/Exclude #4985
Conversation
Honors capped recursion when using Include or Exclude filters with Get-ChildItem
Windos
requested review from
BrucePay,
daxian-dbw,
lzybkr and
vors
as code owners
|
|
||
| int unUsedChildrenNotMatchingFilterCriteria = 0; | ||
| ProcessPathItems(providerInstance, providerPath, recurse, context, out unUsedChildrenNotMatchingFilterCriteria, ProcessMode.Enumerate); | ||
| ProcessPathItems(providerInstance, providerPath, recurse, context, out unUsedChildrenNotMatchingFilterCriteria, ProcessMode.Enumerate, depth: depth); |
There was a problem hiding this comment.
I'd prefer to exclude parameters with defaults and use overloads.
There was a problem hiding this comment.
Just pushed new commit swapping optional parameter for overload. Is it along the right track?
|
@daxian-dbw Can we ask @Windos to fix (replace with overloads) skipIsItemContainerCheck defaults in the PR? It seems adding new parameter (depth) complicate the Api and it would be good to eliminate that now. |
|
Well, the optional parameter was introduced to be an alternative of overloads, to reduce the number of overloads of a method. The optional parameter should never be used for public APIs or even internal APIs that may be called from other friend assemblies because the default value is baked into the caller, and that means if we change the default value, the caller assembly needs to be re-compile to get the new default value. |
|
@daxian-dbw I asked because adding second optional parameter reduced readability. Do we have to create overloads in this case or can we just replace its with one method with all the parameters? |
|
I'm happy to put the work in regardless of which direction is decided on. |
|
@iSazonov I think you made the right call. Adding the second optional parameter would worsen the readability. I think the current two overloads look good. |
|
@iSazonov @daxian-dbw can you update your review |
|
@daxian-dbw Is |
|
@daxian-dbw Can you update your review? |
|
@Windos Thanks for your contribution! |
4 participants
Addresses issue #3726 by adding decrementing depth to ProcessPathItems.
Includes tests specifically for the examples listed in the issue and also testing that recursion still works when excluding or including a string.