GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||
GeezerVison is showing github.com — Fix -NoEnumerate behaviour in Write-Output by vexx32 · Pull Request #9069 · PowerShell/PowerShell Conversation
|
|
Reverted the commit. I'll ignore the CodeFactor items for MshCommandRuntime.cs -- most of them look to be the usual Hungarian notation ones that we plan to ignore going forward anyway. 😄 Moved that change to #9074. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@PowerShell/powershell-committee reviewed this. We agree that we should fix this to get back to the Windows PowerShell behavior which is the correct behavior. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Rain Sallow (/u/ta11ow) (@vexx32) Please open new issue in Docs repo if needed. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Michael Klement (@mklement0) Could you please look the PR before we merge? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
The parameter type of and it behaves as expected: So I wonder what is the actual change that breaks it on PowerShell Core ... |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Perhaps #2035 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Dongbo Wang (@daxian-dbw) Yeah, I can see where that's confusing... And knowing what PSObject is for more than I used to, I think it makes more sense behaving as it currently does... but I am very sure that sentiment wouldn't necessarily be shared by most PowerShell users. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
Yes, the regression was caused by #2038. See another related issue: #5122 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
Dongbo Wang (@daxian-dbw) I shall do some digging to see where that occurs and what can be done about it. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
Rain Sallow (/u/ta11ow) (@vexx32) After #2038, the remaining argument are handled in a similar way as |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Hmm. I guess that makes some sense. It does seem rather unlikely that folks would deliberately use -NoEnumerate and then not expect a collection to be the result. 🤔 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
Make alternative internal attribute? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Maybe instead of fixing Write-Output -NoEnumerate (which would be nice of course) behavior for [OutputType()] could be changed to keep output type untouched if it's defined. Right now even without [OutputType()] defined you still get unwrap for a single object. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
Not sure I follow what you mean there. That attribute generally is metadata and more of a documentation attribute that anything else. As far as I know, it has no effect during execution. Nor do I really see how it would help with this current situation, where -NoEnumerate is simply broken? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I see. Well, I thought that it's more than metadata. And the only reason I would use NoEnumerate is to preserve Array on function return. If it has other uses, that's cool too. What I mean is, if that change wouldn't go thru because of other reasons I would still want some other way to have that feature. If it goes thru, that's great. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Gotcha. Yeah, that might work on a surface level. Unfortunately it would probably mean enumerating the collection twice (once in the parameter binder -> PSObject[], and once before output -> OutputType) which is a bit much imo. It would also not be retaining the original collection, so if it originally had NoteProperty or other ETS members, they would be stripped. In the few cases where -NoEnumerate are needed, this might be one motivation for using it. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Fix is to preserve input collection type in output. The regression was caused by #2038
Fix is to preserve input collection type in output. The regression was caused by PowerShell#2038
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
Write-Output -NoEnumerate outputs PSObject[] rather than Object[] and generally doesn't respect the input collection type
There was an error while loading. Please reload this page.