GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Fix for #6855 - Measure-Object should handle scriptblock properties. by BrucePay · Pull Request #6934 · PowerShell/PowerShell

Conversation

@BrucePay

@BrucePay Bruce Payette (BrucePay) commented May 25, 2018 •
edited by daxian-dbw
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Collaborator

PR Summary

Fixed issue #6855 by renaming the class that does all the work from MshExpression to PSPropertyExpression, making the class public and then in MeasureObjectCommand, lifting it up to the parameter level. Previously the implementation exposed the -Property as string then wrapped it internally as a PSPropertyExpression. Now the parameter type is PSPropertyExpression directly allowing for both wildcard strings and scriptblocks. Note that this is technically a breaking change (the public type of the parameter has changed) but this was reviewed by the committee and approved. (The vast majority of changes in this PR are due to the renaming. The code changes are relatively small.)

PSPropertyExpression now lives in a public namespace where it can be used by cmdlet and script authors to easily add the same type of functionality to their commands.

I also modified PSPropertyExpression to treat hashtables as objects so that

@{prop = 3} | measure-object prop

and

@{prop = 3} | measure-object {$_.prop}

will work the identically. (Previously the example using just the property name would just fail.)

Breaking Change Summary

This PR brings in a breaking change in an edge case:

This could be a breaking change when it comes to the ConstrainedLanguage mode.
@{prop = 3} | measure-object Count works in ConstrainedLanguage mode today, but with this change, the conversion from Hashtable to PSCustomObject will throw exception.

It has been reviewed by the committee and agreed to accept this breaking change.
Please see the discussion at #6934 (comment) for more information.

PR Checklist

  • PR has a meaningful title
    • Use the present tense and imperative mood when describing your changes
  • Summarized changes
  • Change is not breaking
  • Make sure all .h, .cpp, .cs, .ps1 and .psm1 files have the correct copyright header
  • This PR is ready to merge and is not Work in Progress.
    • If the PR is work in progress, please add the prefix WIP: to the beginning of the title and remove the prefix when the PR is ready.
  • User-facing changes
    • Not Applicable
    • OR
    • User-facing Documentation needed
      • Issue filed - Issue link:
  • Testing - New and feature
    • Not Applicable or can only be tested interactively
    • OR
    • Make sure you've added a new test if existing tests do not effectively test the code changed
      • Add [feature] if the change is significant or affects feature tests

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

powercode reacted with thumbs up emoji SeeminglyScience reacted with hooray emoji
All reactions
  • 👍 1 reaction
  • 🎉 1 reaction
@BrucePay
Bruce Payette (BrucePay) requested review from James Truher (JamesWTruher), Paul Higinbotham (PaulHigin), Dan Travison (dantraMSFT), Dongbo Wang (daxian-dbw) and sergei (vors) as code owners May 25, 2018 01:10
@BrucePay Bruce Payette (BrucePay) assigned Dongbo Wang (daxian-dbw) May 25, 2018
iSazonov
Ilya (iSazonov) reviewed May 25, 2018
View reviewed changes
Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/Measure-Object.Tests.ps1 Outdated

@iSazonov Ilya (iSazonov) May 25, 2018

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

If we use pipeline make sense use some values:

$result = "a,b,c", "d,e" | Measure-Object -Word  {$_ -split ','}
$result.Words | Should -Be 5

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@BrucePay Bruce Payette (BrucePay) Jun 1, 2018

Copy link
Copy Markdown
Collaborator Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Fixed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

iSazonov reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/Measure-Object.Tests.ps1 Outdated

@iSazonov Ilya (iSazonov) May 25, 2018

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Maybe move the function and tests in new Context block?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@BrucePay Bruce Payette (BrucePay) Jun 1, 2018

Copy link
Copy Markdown
Collaborator Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Moved into a separate Describe.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread test/powershell/Modules/Microsoft.PowerShell.Utility/Measure-Object.Tests.ps1 Outdated

@iSazonov Ilya (iSazonov) May 25, 2018

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Please add empty new line.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@BrucePay Bruce Payette (BrucePay) Jun 1, 2018

Copy link
Copy Markdown
Collaborator Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Added.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

iSazonov reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@iSazonov

Ilya (iSazonov) commented May 25, 2018

Copy link
Copy Markdown
Collaborator

We should get PowerShell Committee conclusion for new public API.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@BrucePay

Bruce Payette (BrucePay) commented May 29, 2018

Copy link
Copy Markdown
Collaborator Author

Ilya (@iSazonov) We reviewed this in the last committee meeting and approved the name. I thought Steve Lee (@SteveL-MSFT) had updated the bug to reflect this but I don't see it.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@iSazonov

Ilya (iSazonov) commented May 29, 2018

Copy link
Copy Markdown
Collaborator

Bruce Payette (@BrucePay) Thanks! I am extecting that the PR will marked by the label.
Seems we lost all MSFT team in recent weeks. I wonder.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@BrucePay

Bruce Payette (BrucePay) commented Jun 1, 2018

Copy link
Copy Markdown
Collaborator Author

Oops - I had the wrong issue number in the comment so you might have looked at the wrong issue. #6855 is, in fact, marked "Committee Reviewed".

iSazonov reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@BrucePay
Bruce Payette (BrucePay) requested review from Travis Plunk (TravisEz13) and Joey Aiello (joeyaiello) as code owners June 1, 2018 00:41
iSazonov
Ilya (iSazonov) reviewed Jun 1, 2018
View reviewed changes

@iSazonov Ilya (iSazonov) left a comment

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM with one minor comment.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread src/System.Management.Automation/FormatAndOutput/common/Utilities/Mshexpression.cs Outdated

@iSazonov Ilya (iSazonov) Jun 1, 2018

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Could you please use C# 7 syntax?

if (PSObject.Base(target) as Hashtable targetAsHash) 

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@iSazonov Ilya (iSazonov) added the Documentation Needed in this repo Documentation is needed in this repo label Jun 1, 2018
iSazonov
Ilya (iSazonov) approved these changes Jun 1, 2018
View reviewed changes
@BrucePay

Bruce Payette (BrucePay) commented Jun 1, 2018

Copy link
Copy Markdown
Collaborator Author

Steve Lee (@SteveL-MSFT) Says we're ignoring CodeFactor issues for now.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@SteveL-MSFT

Steve Lee (SteveL-MSFT) commented Jun 1, 2018

Copy link
Copy Markdown
Member

Ilya (@iSazonov) FYI - Most of my team has been involved with helping port existing Windows PowerShell modules owned by other teams to work with PSCore6 so you may find us not as active. Hopefully we'll be back to fully engaged by end of June (or mostly engaged).

iSazonov reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@iSazonov

Ilya (iSazonov) commented Jun 2, 2018

Copy link
Copy Markdown
Collaborator

I'm glad that this process began. I do not see this leading to corrections in PowerShell Core - it convinces me that PowerShell Core is very good! 😄

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

daxian-dbw
Dongbo Wang (daxian-dbw) requested changes Jun 5, 2018
View reviewed changes

@daxian-dbw Dongbo Wang (daxian-dbw) left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

/cc Paul Higinbotham (@PaulHigin) Now that PSPropertyExpression is publically exposed and it has the ability to run scripts, Paul you may want to check if this will introduce any security concerns to the ConstrainedLanguage mode.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread src/System.Management.Automation/FormatAndOutput/common/Utilities/Mshexpression.cs Outdated

@daxian-dbw Dongbo Wang (daxian-dbw) Jun 5, 2018

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

This could be a breaking change when it comes to the ConstrainedLanguage mode.
@{prop = 3} | measure-object Count works in ConstrainedLanguage mode today, but with this change, the conversion from Hashtable to PSCustomObject will throw exception.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@BrucePay Bruce Payette (BrucePay) Jun 8, 2018

Copy link
Copy Markdown
Collaborator Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Hmm - true - good catch. It's pretty obscure and currently the only useful scenario for piping hashtables into measure-object (other than instance-counting). Still, we should probably get a committee review on this.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread src/System.Management.Automation/FormatAndOutput/common/Utilities/Mshexpression.cs Outdated

@daxian-dbw Dongbo Wang (daxian-dbw) Jun 5, 2018

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Can you please change the name to WrapHashtableAsPSCustomObject? C# method naming conversion suggests using verbs or verb phrases to name methods.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@BrucePay Bruce Payette (BrucePay) Jun 8, 2018

Copy link
Copy Markdown
Collaborator Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

The point of the rule is that the names of methods should be imperative which this is. There are other examples of (type 1) conditional clauses in .NET such as ThrowIfCancellationRequested on System.Threading.CancellationToken.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@daxian-dbw Dongbo Wang (daxian-dbw) Jun 13, 2018

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

OK. #Closed

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread src/System.Management.Automation/FormatAndOutput/common/Utilities/Mshexpression.cs Outdated

@daxian-dbw Dongbo Wang (daxian-dbw) Jun 5, 2018

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

If we want to handle hashtable specially, shall we expand that to IDictionary? LanguagePrimitives.ConvertPSObjectToType is able to convert IDictionary to PSObject.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@BrucePay Bruce Payette (BrucePay) Jun 8, 2018

Copy link
Copy Markdown
Collaborator Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

The goal is to match the behaviour of [PSCustomObject] @{a=1; b=2}. Currently casting Dictionary<T,T> to PSCustomObject doesn't result in a custom object so I don't think we should introduce that behaviour here.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@daxian-dbw Dongbo Wang (daxian-dbw) Jun 13, 2018

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Understood. Good to know that we are matching the behavior of [PSCustomObject] @{a=1; b=2}.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@BrucePay Bruce Payette (BrucePay) added the Review - Committee The PR/Issue needs a review from the PowerShell Committee label Jun 8, 2018
daxian-dbw
Dongbo Wang (daxian-dbw) approved these changes Jun 13, 2018
View reviewed changes

@daxian-dbw Dongbo Wang (daxian-dbw) left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Waiting on the committee's review.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@PaulHigin

Paul Higinbotham (PaulHigin) commented Jun 14, 2018

Copy link
Copy Markdown
Contributor

Dongbo Wang (@daxian-dbw) Bruce Payette (@BrucePay) Sorry for the late review. To answer Dongbo's question, from a security point of view I think this is Ok. MshExpression (now PSPropertyExpression) takes a ScriptBlock in its constructor which it will invoke. But as long as the passed in ScriptBlock is created in a constrained environment it will be marked accordingly and the script block will be invoked with CL mode constraints.

A constructor also takes a string "expression", but as near as I can tell it is never evaluated as script and is used to look up properties. Is this correct?

However, we should include a CL mode test to ensure the expression scriptblock is run in CL mode.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@daxian-dbw

Dongbo Wang (daxian-dbw) commented Jun 14, 2018

Copy link
Copy Markdown
Member

A constructor also takes a string "expression", but as near as I can tell it is never evaluated as script and is used to look up properties. Is this correct?

Paul Higinbotham (@PaulHigin) Yes, that's correct. It's for property lookup.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

daxian-dbw
Dongbo Wang (daxian-dbw) reviewed Jun 14, 2018
View reviewed changes
Comment thread src/System.Management.Automation/engine/parser/TypeResolver.cs Outdated

@daxian-dbw Dongbo Wang (daxian-dbw) Jun 14, 2018

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Paul Higinbotham (@PaulHigin) I would like you to take another look at this change. PSPropertyExpression is added to CoreTypes, and that means users can call its methods in ConstrainedLanguage mode. Do you have any concerns about it?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@PaulHigin Paul Higinbotham (PaulHigin) Jun 14, 2018

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Thanks, I didn't see that. If this type is going to be white listed then it needs to go through a separate/full security review. I see that some internal methods are now public, and these along with constructors need to be reviewed for vulnerabilities. Please set up a meeting for this.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@BrucePay Bruce Payette (BrucePay) Jun 15, 2018

Copy link
Copy Markdown
Collaborator Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

No - it's only intended to be a type accelerator. I added it in the wrong place. I'll move it to the TypeAccelerators() method where it should be.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

daxian-dbw and PaulHigin reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

@daxian-dbw Dongbo Wang (daxian-dbw) Jun 15, 2018

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Yeah, I agree it should be moved :)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
PaulHigin
Paul Higinbotham (PaulHigin) suggested changes Jun 14, 2018
View reviewed changes
Comment thread src/System.Management.Automation/engine/parser/TypeResolver.cs Outdated

@PaulHigin Paul Higinbotham (PaulHigin) Jun 14, 2018

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Thanks, I didn't see that. If this type is going to be white listed then it needs to go through a separate/full security review. I see that some internal methods are now public, and these along with constructors need to be reviewed for vulnerabilities. Please set up a meeting for this.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@SteveL-MSFT Steve Lee (SteveL-MSFT) added Committee-Reviewed PS-Committee has reviewed this and made a decision and removed Review - Committee The PR/Issue needs a review from the PowerShell Committee labels Jun 14, 2018
@SteveL-MSFT

Steve Lee (SteveL-MSFT) commented Jun 14, 2018

Copy link
Copy Markdown
Member

@PowerShell/powershell-committee reviewed this and is fine with this breaking change which is likely only an edge case, but the positive impact for most users is beneficial

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@SteveL-MSFT Steve Lee (SteveL-MSFT) added the Breaking-Change breaking change that may affect users label Jun 14, 2018
@BrucePay
Bruce Payette (BrucePay) requested review from Francisco Gamino (Francisco-Gamino), Aditya Patwardhan (adityapatwardhan), Andrew (anmenaga) and Mike Richmond (mirichmo) as code owners June 15, 2018 22:55
Bruce Payette added 6 commits June 15, 2018 17:10
Bruce Payette
merge conflict
78e9618
Bruce Payette
Added missing type accelerator test, cleaned up tests
5c94dc3
Bruce Payette
Fix for PowerShell#6855 - Measure-Object should handle scriptblock pr…
0b0f423
…operties. Fixed by renaming MshExpression to PSPropertyExpression and making it

public. Then in MeasureObjectCommand, lifting it up to the parameter level. Previously the implementation exposed the Property as string and
wrapped it internally as a PSPropertyExpression. Now the parameter type is PSPropertyExpression directly allowing for both wildcard strings
and scriptblocks. PSPropertyExpression now lives in a public namespace where it can be used by cmdlet and script authors to easily add the
same type of functionality to their commands. I also modified PSPropertyExpression to handle hashtables properly as objects so
@{prop = 3} | measure-object prop
and
@{prop = 3} | measure-object {$_.prop}
will work the same. (Previously the example using just the property name would fail.)
Bruce Payette
modified IfHashtableWrapAsPSCustomObject function to use C# 7 pattern…
662406e
… matching
Bruce Payette
Moved PSPropertyExpression shortcut to type accelerator list.
0a9525c
Added missing type accelerator test, cleaned up tests.
Bruce Payette
merge conflict
a7386a9
@BrucePay
Bruce Payette (BrucePay) force-pushed the brucepay_MeasureObject branch from 6ba5c16 to a7386a9 Compare June 16, 2018 00:13
@daxian-dbw

Dongbo Wang (daxian-dbw) commented Jun 18, 2018

Copy link
Copy Markdown
Member

Paul Higinbotham (@PaulHigin) New commits have been pushed. Can you please update your review? Thanks

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

PaulHigin
Paul Higinbotham (PaulHigin) approved these changes Jun 18, 2018
View reviewed changes

@PaulHigin Paul Higinbotham (PaulHigin) left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@daxian-dbw

Dongbo Wang (daxian-dbw) commented Jun 19, 2018

Copy link
Copy Markdown
Member

Bruce Payette (@BrucePay) I just noticed that we didn't run feature tests for this PR CI. Can you please push another commit with [feature] in the commit message? That will trigger the feature test run. You can also update the current head commit by running git commit --amend in your local brucepay_MeasureObject branch, and then forcing push to the remote by git push --force-with-lease <your fork repo> brucepay_MeasureObject. Thanks!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@daxian-dbw
[Feature] Dummy commit to trigger daily test run
999a31c
@daxian-dbw

Dongbo Wang (daxian-dbw) commented Jun 19, 2018

Copy link
Copy Markdown
Member

Bruce Payette (@BrucePay) I have submitted a dummy commit to trigger the feature test run for this PR.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@daxian-dbw
Dongbo Wang (daxian-dbw) merged commit 3a4c410 into PowerShell:master Jun 20, 2018
@iSazonov Ilya (iSazonov) mentioned this pull request Aug 31, 2018
Join-String cmdlet for creating text from pipeline input #7660
Merged
6 of 11 tasks
@joeyaiello Joey Aiello (joeyaiello) mentioned this pull request Oct 15, 2018
Docs needed for 'Fix for #6855 - Measure-Object should handle scriptblock properties.' MicrosoftDocs/PowerShell-Docs#3034
Closed
@joeyaiello Joey Aiello (joeyaiello) removed Documentation Needed in this repo Documentation is needed in this repo labels Oct 15, 2018
@KyleMit Kyle Mitofsky (KyleMit) mentioned this pull request Oct 15, 2022
Document that Select-Object, Sort-Object and Group-Object handle HashTable Properties in PowerShell 6+ MicrosoftDocs/PowerShell-Docs#9311
Closed
3 tasks done
Mikey Lombardi (He/Him) (michaeltlombardi) added a commit to michaeltlombardi/PowerShellDocs that referenced this pull request Oct 17, 2022
@michaeltlombardi
(MicrosoftDocsGH-9311) Document *-Object hashtable handling
ba0ccd2
Prior to this change, the documentation for `Group-Object`,
`Select-Object`, and `Sort-Object` did not specifically address using
those cmdlets against **hashtable** input.

In PowerShell 6 an improvement (see PowerShell/PowerShell#6934) enabled
users to reference the values of **hashtable** keys in these commands
without having to specify calculated properties.

This change:

- Updates the documentation for `Group-Object`, `Select-Object`, and
  `Sort-Object` in versions 7.0 and above to show how users can group,
  select, and sort **hashtable** objects by their key values
- Updates the documentation for `Group-Object`, `Select-Object`, and
  `Sort-Object` in Windows PowerShell 5.1 to show how users can group,
  select, and sort **hashtable** objects with calculated properties
- Cleans up Vale linting warnings across those files
- Fixes AB#20552
- Resolves MicrosoftDocs#9311
@michaeltlombardi Mikey Lombardi (He/Him) (michaeltlombardi) mentioned this pull request Oct 17, 2022
(GH-9311) Document *-Object hashtable handling MicrosoftDocs/PowerShell-Docs#9318
Merged
4 tasks done
Sean Wheeler (sdwheeler) pushed a commit to michaeltlombardi/PowerShellDocs that referenced this pull request Oct 17, 2022
@michaeltlombardi @sdwheeler
(MicrosoftDocsGH-9311) Document *-Object hashtable handling
265e370
Prior to this change, the documentation for `Group-Object`,
`Select-Object`, and `Sort-Object` did not specifically address using
those cmdlets against **hashtable** input.

In PowerShell 6 an improvement (see PowerShell/PowerShell#6934) enabled
users to reference the values of **hashtable** keys in these commands
without having to specify calculated properties.

This change:

- Updates the documentation for `Group-Object`, `Select-Object`, and
  `Sort-Object` in versions 7.0 and above to show how users can group,
  select, and sort **hashtable** objects by their key values
- Updates the documentation for `Group-Object`, `Select-Object`, and
  `Sort-Object` in Windows PowerShell 5.1 to show how users can group,
  select, and sort **hashtable** objects with calculated properties
- Cleans up Vale linting warnings across those files
- Fixes AB#20552
- Resolves MicrosoftDocs#9311
Sean Wheeler (sdwheeler) pushed a commit to MicrosoftDocs/PowerShell-Docs that referenced this pull request Oct 17, 2022
@michaeltlombardi
(GH-9311) Document *-Object hashtable handling (#9318)
73a8430
Prior to this change, the documentation for `Group-Object`,
`Select-Object`, and `Sort-Object` did not specifically address using
those cmdlets against **hashtable** input.

In PowerShell 6 an improvement (see PowerShell/PowerShell#6934) enabled
users to reference the values of **hashtable** keys in these commands
without having to specify calculated properties.

This change:

- Updates the documentation for `Group-Object`, `Select-Object`, and
  `Sort-Object` in versions 7.0 and above to show how users can group,
  select, and sort **hashtable** objects by their key values
- Updates the documentation for `Group-Object`, `Select-Object`, and
  `Sort-Object` in Windows PowerShell 5.1 to show how users can group,
  select, and sort **hashtable** objects with calculated properties
- Cleans up Vale linting warnings across those files
- Fixes AB#20552
- Resolves #9311
Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj-contribs/PowerShell that referenced this pull request Aug 6, 2026
Bruce Payette @daxian-dbw
Fix for PowerShell#6855 - Measure-Object should handle scriptblock pr…
019f5e2
…operties. (PowerShell#6934)

Measure-Object should handle `ScriptBlock` properties. Fixed by renaming `MshExpression` to `PSPropertyExpression` and making it public. Then in `MeasureObjectCommand`, lifting it up to the parameter level. Previously the implementation exposed the Property as a string and
wrapped it internally as a `PSPropertyExpression`. Now the parameter type is `PSPropertyExpression` directly allowing for both wildcard strings and `ScriptBlock`.

`PSPropertyExpression` now lives in a public namespace where it can be used by cmdlet and script authors to easily add the same type of functionality to their commands. I also modified `PSPropertyExpression` to handle hashtables properly as objects so
         @{prop = 3} | measure-object prop
and
         @{prop = 3} | measure-object {$_.prop}
will work the same. (Previously the example using just the property name would fail.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@daxian-dbw Dongbo Wang (daxian-dbw) daxian-dbw approved these changes
@iSazonov Ilya (iSazonov) iSazonov approved these changes
@dantraMSFT Dan Travison (dantraMSFT) Awaiting requested review from dantraMSFT
@JamesWTruher James Truher (JamesWTruher) Awaiting requested review from JamesWTruher
@vors sergei (vors) Awaiting requested review from vors
@joeyaiello Joey Aiello (joeyaiello) Awaiting requested review from joeyaiello
@TravisEz13 Travis Plunk (TravisEz13) Awaiting requested review from TravisEz13
@adityapatwardhan Aditya Patwardhan (adityapatwardhan) Awaiting requested review from adityapatwardhan
@anmenaga Andrew (anmenaga) Awaiting requested review from anmenaga
@Francisco-Gamino Francisco Gamino (Francisco-Gamino) Awaiting requested review from Francisco-Gamino
@mirichmo Mike Richmond (mirichmo) Awaiting requested review from mirichmo
+1 more reviewer
@PaulHigin Paul Higinbotham (PaulHigin) PaulHigin approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@daxian-dbw Dongbo Wang (daxian-dbw)

Labels

Breaking-Change breaking change that may affect users Committee-Reviewed PS-Committee has reviewed this and made a decision

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@BrucePay @iSazonov @SteveL-MSFT @PaulHigin @daxian-dbw @joeyaiello
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.