Skip to content

Add any/all lambda containment operators - #11068

Closed
Mathias R. Jessen (IISResetMe) wants to merge 6 commits into
PowerShell:masterfrom
IISResetMe:feature/any-all-lambda7
Closed

Mathias R. Jessen (IISResetMe) wants to merge 6 commits into
PowerShell:masterfrom
IISResetMe:feature/any-all-lambda7

Conversation

@IISResetMe

@IISResetMe Mathias R. Jessen (IISResetMe) commented Nov 14, 2019 •

Copy link
Copy Markdown
Collaborator

PR Summary

This pull request adds 2 new containment operators:

  • -any (ex. 1..5 -any { $_ -eq 2 })
    • Takes a collection as its lhs operand and a predicate as its rhs operand. Returns true if the predicate is true for any lhs value, returns false otherwise
  • -all (ex. 1..5 -all { $_ -gt 0 })
    • Takes a collection as its lhs operand and a predicate as its rhs operand. Returns true if the predicate is true for all lhs value, returns false otherwise

If a non-delegate-like object is passed as the rhs operand, we perform a scalar equality comparison against each item in lhs.

A related update to the EditorSyntax definition has been prepared here: PowerShell/EditorSyntax#189

PR Context

This PR is partly motivated by the discussion in #7925, and my own frustration with having to cast nested loop structures to perform similar containment testing.

For example, to find a directory in which all files a marked hidden:

# with the current binary operators
Get-ChildItem -Directory -Recurse |Where {
  $files = @($_ |Get-ChildItem -File -Force) 
  $files.Count -gt 0 -and ($files |Where Attributes -notlike "*hidden*").Count -eq 0
}

# with `-all`
Get-ChildItem -Directory -Recurse |Where {
  @($_ |Get-ChildItem -File -Force) -all {$_.Attributes -like "*hidden*"}
}

(I'm aware that dynamic parameter aliases exist specifically for attribute filtering in the FileSystem provider, but that's sort of missing the point, -any/-all would work in any context)

TODO:

  • Should we include a negation? (-notany/-none and -notall)
  • Should we include case-sensitive variants? (only applies to the -contains fallback edge case)
  • Should we include more/better tests? (for containment operators in general)

PR Checklist

Comment thread src/System.Management.Automation/engine/lang/parserutils.cs Outdated
Comment thread src/System.Management.Automation/engine/lang/parserutils.cs Outdated
Comment thread src/System.Management.Automation/engine/lang/parserutils.cs Outdated
Mathias R. Jessen (IISResetMe) added a commit to IISResetMe/EditorSyntax that referenced this pull request Nov 14, 2019
To reflect changes to binary comparison operators in PowerShell/PowerShell#11068
@IISResetMe
Mathias R. Jessen (IISResetMe) marked this pull request as ready for review November 14, 2019 15:43
Comment thread src/System.Management.Automation/engine/lang/parserutils.cs Outdated
Mathias R. Jessen (IISResetMe) added a commit to IISResetMe/EditorSyntax that referenced this pull request Nov 16, 2019
To reflect changes to binary comparison operators in PowerShell/PowerShell#11068
@vexx32 Rain Sallow (/u/ta11ow) (vexx32) added the CL-Engine Indicates that a PR should be marked as an engine change in the Change Log label Nov 18, 2019
@vexx32

Copy link
Copy Markdown
Collaborator

RE: The additional questions -- yes, I'd say it's worth including the negative versions, but I don't see a lot of value in a case-sensitive version, since folx can just use case-sensitive operators inside the scriptblock expression to get the same effect.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks lovely overall!

Couple minor thoughts in a few parts, but other than that this is good to go imo! 💖

Comment thread src/System.Management.Automation/engine/lang/parserutils.cs Outdated
Comment thread src/System.Management.Automation/engine/lang/parserutils.cs Outdated
Comment thread src/System.Management.Automation/engine/lang/parserutils.cs Outdated
Comment thread src/System.Management.Automation/engine/lang/parserutils.cs Outdated
Comment thread src/System.Management.Automation/engine/lang/parserutils.cs Outdated
Comment thread src/System.Management.Automation/engine/parser/Compiler.cs Outdated
Comment thread src/System.Management.Automation/engine/parser/Compiler.cs Outdated
Comment thread src/System.Management.Automation/engine/parser/Compiler.cs Outdated
@ghost ghost added Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept and removed Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept labels Nov 18, 2019
@ghost ghost added the Review - Needed The PR is being reviewed label May 27, 2020
@ghost

Copy link
Copy Markdown

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Mainainer, Please provide feedback and/or mark it as Waiting on Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CL-Engine Indicates that a PR should be marked as an engine change in the Change Log Review - Needed The PR is being reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants