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

Allow passing $true/$false as parameter to script using -File by SteveL-MSFT · Pull Request #4178 · PowerShell/PowerShell · GitHub

Allow passing $true/$false as parameter to script using -File - #4178

Merged
Dongbo Wang (daxian-dbw) merged 2 commits into
PowerShell:masterfrom
SteveL-MSFT:file-bool-arg
Aug 1, 2017
Merged

Allow passing $true/$false as parameter to script using -File#4178
Dongbo Wang (daxian-dbw) merged 2 commits into
PowerShell:masterfrom
SteveL-MSFT:file-bool-arg

Conversation

Steve Lee (SteveL-MSFT) commented Jul 3, 2017
edited
Loading

Copy link
Copy Markdown
Member

Using powershell.exe to execute a PowerShell script using -File currently provides no way to pass $true/$false as parameter values. Current behavior is that -File accumulates passed parameters as strings only.

Fix is to special case this based on discussion with PS-Committee to support $true/$false as parsed values to parameters. Switch values is also supported as currently documented syntax doesn't work.

Fix #4036

Doc change is MicrosoftDocs/PowerShell-Docs#1430

Copy link
Copy Markdown
Member

The commits has changes in src/Modules/Shared/Pester which should not be included. I think you need to run git submodule update to resolve it.

Steve Lee (SteveL-MSFT) force-pushed the file-bool-arg branch 2 times, most recently from a26bda3 to c62eccd Compare July 17, 2017 21:55

Copy link
Copy Markdown
Member Author

Dongbo Wang (@daxian-dbw) fixed

Copy link
Copy Markdown
Member Author

Jason Shirk (@lzybkr) if you get a chance, can you review this? thanks

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Instead of walking the string twice, move the call to IndexOf to before the if and test offset >= 0 instead.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

good point, will fix

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

In the common case, you are creating the same string twice with arg.Substring(offiset + 1), once here, and once below.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

So this can be made much simpler, something like:

string argValue = arg.Substring(offset + 1);
bool? boolValue = GetBoolValue(argValue);
_collectedArgs.Add(new CommandParameter(arg.Substring(0, offset), boolValue ?? argValue);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

will fix

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

null-coalescing operator requires same type on both sides, I can use ternary operator to simplify it instead

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Nevermind, even ternary expects same type

Ilya (iSazonov) Jul 25, 2017
edited
Loading

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

We can use out parameter.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Again, you can simply a bunch like above with ??.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

will fix

Steve Lee (SteveL-MSFT) Jul 25, 2017
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

same as above, has to be same types so can't simplify with null-coalescing or ternary operator

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

You can still simplify though, something like:

object argValue = arg.Substring(offset + 1);
 _collectedArgs.Add(new CommandParameter(arg.Substring(0, offset), GetArgPossiblyAsBool(argValue)));

And GetArgPossiblyAsBool returns object.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Will change

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Doesn't work, can't implicitly convert object to bool/string

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I don't think these tests reflect the real user scenario.

Passing $BoolString is completely different than passing $true.

$BoolString is getting replaced with $true, but $true gets replaced with True.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

See comment above regarding usage from different shells.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Updated tests for both $true and true

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

We should have a test that accepts a string parameter and you pass true and false - we want to make sure that when we convert to bool, the bool gets properly converted back to the correct string.

And come to think about it, there is a problem - we'll lose the case that the user specified.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Yeah, that's a problem. I've changed it so it only works with switches and added tests for other cases

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think this needs to be True, not $true.

Or maybe it needs to be both $true and True - because it depends on the shell how $true is expanded.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Isn't the use case:

powershell -file foo.ps1 -myswitch:$true

Not clear to me when we need True as I don't think we want to support:

powershell -file foo.ps1 -myswitch:true

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Ok, I think I understand what you're getting it by your comment below. From a non-powershell shell like bash (which I thought would be the primary use case), I would think we want to support PowerShell syntax for passing $true and $false. From within powershell, I suppose we shouldn't expect the user to escape $true to be just a string. I can make this change.

…lse cannot be passed

as a parameter/switch value.
Fix is to special case this based on discussion with PS-Committee.

Copy link
Copy Markdown
Member Author

Jason Shirk (@lzybkr) feedback addressed

_collectedArgs.Add(new CommandParameter(arg.Substring(0, offset), arg.Substring(offset + 1)));
string argValue = arg.Substring(offset + 1);
bool? boolValue = GetBoolValue(argValue);
if (boolValue != null)

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Sorry again -
Can we use LanguagePrimitives.TryConvertTo or LanguagePrimitives.TryConvertArg?
Can we use TryGetBoolValue pattern?

                               string argValue1 = arg.Substring(offset + 1); 
                               string argValue0 = arg.Substring(0, offset);
                               if (TryGetBoolValue(argValue1, out bool boolValue))  
                               {  
                                       _collectedArgs.Add(new CommandParameter(argValue0, boolValue));  
                              }  
                               else  
                               {  
                                       _collectedArgs.Add(new CommandParameter(argValue0, argValue1));  
                               }  

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Sure

Ilya (iSazonov) Jul 28, 2017
edited
Loading

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Please add a protection comment like "Don't change case!". And below too.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

sure

Copy link
Copy Markdown
Member Author

Jason Shirk (@lzybkr) any other concerns?

Copy link
Copy Markdown

C:\Temp>type test.ps1
Param( [bool] $test )
Echo $test
C:\Temp>pwsh.exe -file test.ps1 -test $false
test.ps1: Cannot process argument transformation on parameter 'test'. Cannot convert value "System.String" to type "System.Boolean". Boolean parameters accept only Boolean values and numbers, such as $True, $False, 1 or 0.

C:\Temp>pwsh.exe -command $PSVersionTable

Name Value


PSVersion 7.0.0
PSEdition Core
GitCommitId 7.0.0
OS Microsoft Windows 10.0.18363
Platform Win32NT
PSCompatibleVersions {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion 2.3
SerializationVersion 1.1.0.1
WSManStackVersion 3.0

Copy link
Copy Markdown
Collaborator

Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj/PowerShell that referenced this pull request Aug 6, 2026
…hell#4178)

Using powershell.exe to execute a PowerShell script using -File currently provides no way to pass $true/$false as parameter values. Current behavior is that -File accumulates passed parameters as strings only.

Fix is to special case this based on discussion with PS-Committee to support $true/$false as parsed values to parameters. Switch values is also supported as currently documented syntax doesn't work.
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.

Executing powershell script with bool parameter doesnt work

6 participants


Back | FazBrowse Home | New Git URL