Draft: Generalize dirEntries to take a DirEntry predicate as parameter

[nordlow]

The current state of dirEntries is not generic enough because it cannot traverse any directory tree X (via SpanMode.depth) without having to traverse all sub-directories of X such as .git directory trees.

Adding a template parameter

alias pred  = (scope DirEntry entry) => true

to dirEntries alleviates this problem.

No tests yet and no template restriction enabled yet because I'm uncertain if we should require the pred to take the DirEntry by ref or not given that DirEntry.sizeof is 156.

Moreover, it's unfortunate that the template parameter useDIP1000 was recently added to

dirEntries(bool useDIP1000 = dip1000Enabled, alias pred = (scope DirEntry entry) => true)

as I presume that pred is gonna be a much more common non-default template parameter than then useDIP1000 is gonna be.

Shall we add a new function say treeEntries, dirTreeEntries, dirEntriesTree or dirEntriesFiltered or whatever instead so we can make pred the first template parameter?

Note that DirEntry member

@property bool isDir() scope

is non-const on posix so that pred cannot be

(const scope DirEntry entry) => true

in the general case. Will add documentation and tests when we have settled on solutions to these issues.

For the corresponding filtering in Rust see https://crates.io/crates/walkdir section "Example: skip hidden files and directories efficiently on unix".

[thewilsonator]

needs documentation

[dukc]

Moreover, it's unfortunate that the template parameter useDIP1000 was recently added

I don't think you need to worry about breakage when moving that to last position of the parameter list. The documentation says "set automatically - do not touch", it asserts null if set against what the compiler switch says, and it has existed for only a short while.

[nordlow]

I don't think you need to worry about breakage when moving that to last position of the parameter list. The documentation says "set automatically - do not touch", it asserts null if set against what the compiler switch says, and it has existed for only a short while.

Great. Will adjust then.

[atilaneves]

Needs tests.

[dlang-bot]

Thanks for your pull request and interest in making D better, @nordlow! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

• My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
• My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
• I have provided a detailed rationale explaining my changes
• New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

---

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + phobos#8586"

[nordlow]

Lots of seemingly unrelated CI errors...

[rikkimax]

The predicate should not be a template parameter.

It increases symbol count, and prevents configurability at runtime.

[thewilsonator]

@nordlow if you could add some documentation and a chengelog entry to this we can definitely merge this

[nordlow]

Great. What shall the default value of the predicate be? The one that I provided with DirEntry passed as ref or simply pred = void?

[rikkimax]

If null, old behavior. If non-null call it.

[nordlow]

If null, old behavior. If non-null call it.

I believe it's more common practise to default predicate lambda parameters to void instead of null. With defaulting to void

we can use static if (is(pred ==)).

Alternatively

alias pred = (scope ref DirEntry entry) => true

could be used.

[rikkimax]

If null, old behavior. If non-null call it.

I believe it's more common practise to default predicate lambda parameters to void instead of null. With defaulting to void

we can use static if (is(pred ==)).

Alternatively

alias pred = (scope ref DirEntry entry) => true

could be used.

All of which are related to template parameters. My comment was about converting it to a function parameter.

[nordlow]

If null, old behavior. If non-null call it.
All of which are related to template parameters. My comment was about converting it to a function parameter.

Ahh. I see.

[nordlow]

Are we ok with using

bool delegate(scope ref DirEntry entry) pred;

as a run-time parameter and store this in DirIteratorImpl as

private struct DirIteratorImpl
{
    bool delegate(scope ref DirEntry entry) pred;

    this(bool delegate(scope ref DirEntry entry) pred)
    {
        this.pred = pred;
    }
    ....

be used?

[rikkimax]

It would need to be @safe (none others are needed here I think).