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

Enhancement: Add support for filtering files in Globbing alias #1384

Closed
kcamp opened this issue Nov 27, 2016 · 2 comments
Closed

Enhancement: Add support for filtering files in Globbing alias #1384

kcamp opened this issue Nov 27, 2016 · 2 comments
Milestone

Comments

@kcamp
Copy link
Contributor

kcamp commented Nov 27, 2016

GlobbingAliases.cs has support for filtering the globbing traversal at the directory level, but no support for filtering files. I have found that support for logic that is more complex than the globbing expression allows has been useful, and have seen gitter traffic for the same issue.

This would be a trivial addition to make at the method alias level. I would propose that two overloads be created --

GetFiles(this ICakeContext context, string pattern, Func<IFile, bool> filePredicate)
 ...

GetFiles(this ICakeContext context, string pattern, Func<IDirectory, bool> predicate, Func<IFile, bool> filePredicate)
...

The former would satisfy the direct need of the feature request, the latter would provide a graceful pairing of current + extended logic.

Assuming that this is something that seems useful and aligned with the intent of the project, the question I would raise is whether it's more appropriate to implement the logic directly in the alias method, similar to ..

// glob the files, and then filter down based on the filePredicate
var files = context.Globber.Match(pattern).OfType<FilePath>()
                            .Where(file => filePredicate(context.FileSystem.GetFile(file)));

or whether it would be more appropriately implemented in the globbing internals -- GlobVisitor, GlobVisitorContext, and so on.

Constraining it to the alias would certainly be less involved, but the latter seems like it would probably be a cleaner (and faster) implementation, particularly in eliminating the extra calls to FileSystem.GetFile(...) - it seems quite apt that it would amount to refactoring the GlobVisitor and GlobVisitorContext so that the logic in FindCandidates would be extended to encompass something like ..

if (includeFiles)
{
    foreach (var file in current.GetFiles("*", option))
    {
        var lastPath = file.Path.Segments.Last();
        if (node.IsMatch(lastPath) && context.ShouldInclude(file))
        ....

Thoughts on whether or not this would be worth pursuing? Any other feedback?

@kcamp
Copy link
Contributor Author

kcamp commented Dec 24, 2016

For what it's worth & work doesn't get duplicated, I've made good progress in extending the globbing to support this and expect to have a PR for it in the next couple of days.

kcamp added a commit to kcamp/cake that referenced this issue Jan 20, 2019
…liases

This refactors/revisits a previous attempt to achieve the same.
Earlier work was based on methods/implementations that would have
come to rely on method overloads (before the GlobberSettings
implementation was added).  Now that the settings type exists,
this represents a much stronger feature/change without any potential
risk for backward compatibility.

i.e., an implicitly typed predicate specified as IFileSystemInfo
would have created ambiguity between the desired overloads -
GetFiles(string pattern, Func<IFile, bool> predicate)
and
GetFiles(string pattern, Func<IDirectory,bool> predicate)
resulting in unpredictable behavior.

This adds the member to the GlobberSettings type and wires in
all the necessary logic, providing a clean implementation
that relies on default values (null) and a single point of entry
for invocation to provide a flexible solution without the syntactic
overhead.
kcamp added a commit to kcamp/cake that referenced this issue Feb 15, 2019
…liases

This refactors/revisits a previous attempt to achieve the same.
Earlier work was based on methods/implementations that would have
come to rely on method overloads (before the GlobberSettings
implementation was added).  Now that the settings type exists,
this represents a much stronger feature/change without any potential
risk for backward compatibility.

i.e., an implicitly typed predicate specified as IFileSystemInfo
would have created ambiguity between the desired overloads -
GetFiles(string pattern, Func<IFile, bool> predicate)
and
GetFiles(string pattern, Func<IDirectory,bool> predicate)
resulting in unpredictable behavior.

This adds the member to the GlobberSettings type and wires in
all the necessary logic, providing a clean implementation
that relies on default values (null) and a single point of entry
for invocation to provide a flexible solution without the syntactic
overhead.
kcamp added a commit to kcamp/cake that referenced this issue Feb 20, 2019
…liases

This refactors/revisits a previous attempt to achieve the same.
Earlier work was based on methods/implementations that would have
come to rely on method overloads (before the GlobberSettings
implementation was added).  Now that the settings type exists,
this represents a much stronger feature/change without any potential
risk for backward compatibility.

i.e., an implicitly typed predicate specified as IFileSystemInfo
would have created ambiguity between the desired overloads -
GetFiles(string pattern, Func<IFile, bool> predicate)
and
GetFiles(string pattern, Func<IDirectory,bool> predicate)
resulting in unpredictable behavior.

This adds the member to the GlobberSettings type and wires in
all the necessary logic, providing a clean implementation
that relies on default values (null) and a single point of entry
for invocation to provide a flexible solution without the syntactic
overhead.
kcamp added a commit to kcamp/cake that referenced this issue Feb 21, 2019
…liases

This refactors/revisits a previous attempt to achieve the same.
Earlier work was based on methods/implementations that would have
come to rely on method overloads (before the GlobberSettings
implementation was added).  Now that the settings type exists,
this represents a much stronger feature/change without any potential
risk for backward compatibility.

i.e., an implicitly typed predicate specified as IFileSystemInfo
would have created ambiguity between the desired overloads -
GetFiles(string pattern, Func<IFile, bool> predicate)
and
GetFiles(string pattern, Func<IDirectory,bool> predicate)
resulting in unpredictable behavior.

This adds the member to the GlobberSettings type and wires in
all the necessary logic, providing a clean implementation
that relies on default values (null) and a single point of entry
for invocation to provide a flexible solution without the syntactic
overhead.
@devlead devlead added this to the v0.33.0 milestone Feb 26, 2019
devlead pushed a commit to kcamp/cake that referenced this issue Feb 27, 2019
…liases

This refactors/revisits a previous attempt to achieve the same.
Earlier work was based on methods/implementations that would have
come to rely on method overloads (before the GlobberSettings
implementation was added).  Now that the settings type exists,
this represents a much stronger feature/change without any potential
risk for backward compatibility.

i.e., an implicitly typed predicate specified as IFileSystemInfo
would have created ambiguity between the desired overloads -
GetFiles(string pattern, Func<IFile, bool> predicate)
and
GetFiles(string pattern, Func<IDirectory,bool> predicate)
resulting in unpredictable behavior.

This adds the member to the GlobberSettings type and wires in
all the necessary logic, providing a clean implementation
that relies on default values (null) and a single point of entry
for invocation to provide a flexible solution without the syntactic
overhead.
devlead added a commit that referenced this issue Feb 27, 2019
* kcamp-GH-1384:
  GH-1384: Add support to pass Func<IFile,bool> to globbing aliases
@devlead
Copy link
Member

devlead commented Feb 27, 2019

Fixed by #2458

@devlead devlead closed this as completed Feb 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants