FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Fix powershell to not crash on converting recursive array to bool by PetSerAl · Pull Request #3208 · PowerShell/PowerShell · GitHub

Fix powershell to not crash on converting recursive array to bool - #3208

Merged
Dongbo Wang (daxian-dbw) merged 2 commits into
PowerShell:masterfrom
PetSerAl:recursive-array-fix
Feb 27, 2017
Merged

Fix powershell to not crash on converting recursive array to bool#3208
Dongbo Wang (daxian-dbw) merged 2 commits into
PowerShell:masterfrom
PetSerAl:recursive-array-fix

Conversation

PetSerAl commented Feb 26, 2017
edited by daxian-dbw
Loading

Copy link
Copy Markdown
Contributor

Fixing #3207

@PetSerAl,
Thanks for your contribution.
To ensure that the project team has proper rights to use your work, please complete the Contribution License Agreement at https://cla.microsoft.com.

It will cover your contributions to all Microsoft-managed open source projects.
Thanks,
Microsoft Pull Request Bot

Copy link
Copy Markdown
Contributor

Code changes look good - we do need the cla to be signed before it can be merged.

Dongbo Wang (daxian-dbw) changed the title Fixes issue #3207 Fix powershell to not crash on converting recursive array to bool Feb 27, 2017

Copy link
Copy Markdown
Member

PetSerAl Thanks for your contribution!
I updated the title to reflect the change. It's recommended to have a meaningful PR title describing what changes you made (see the guideline in CONTRIBUTING.md).

Copy link
Copy Markdown

Hi, I am closing and re-opening this PR to bump the CLA bot. Sorry for the inconvenience!

PetSerAl,
Thanks for having already signed the Contribution License Agreement. Your agreement has not been validated yet. We will now review your pull request.
Thanks,
Microsoft Pull Request Bot

PetSerAl commented Feb 27, 2017
edited
Loading

Copy link
Copy Markdown
Contributor Author

One possible breaking change came to my mind:

$a = ,,$false
$b = ,[PSObject](,$false)
!!$a # $true
!!$b # was $false, now is $true

So, this PR make PowerShell behavior more consistent in regard to wrapping array into [PSObject]. But, is this change in behavior acceptable, Dongbo Wang (@daxian-dbw)?

Dongbo Wang (daxian-dbw) commented Feb 27, 2017
edited
Loading

Copy link
Copy Markdown
Member

I think the behavior with the fix reflects the original intention of the code. Here is the comment in "case 1" before returning true:

// the first element is an array with more than zero elements
return true;

And as you said, now the conversion is more consistent:
Before Fix

$true -eq $a
True
$true -eq $b
False

After Fix

$true -eq $a
True
$true -eq $b
True

Using a nested array in if condition should be rare. I think this behavior change is acceptable. Jason Shirk (@lzybkr) any thoughts?

Copy link
Copy Markdown
Contributor

It's definitely obscure, I'm not worried about the change.

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants


Back | FazBrowse Home | New Git URL