Skip to content

Conversation

@SteveL-MSFT
Copy link
Member

Previously powershell.exe treated unknown arguments as a command line to execute. To align with POSIX so that things like shebang scripts work correctly, we are changing powershell.exe so that it treats unknown arguments (aka positional argument) as a file. This means that powershell foo will now attempt to use foo as a PowerShell script whereas previously foo would be treated as a command to execute. This doesn't affect existing usage of either -File nor -Command. Fixed tests that didn't explicitly use -Command parameter.

Note that some of the changes below for trailing whitespace was due to VSCode setting.

Fix #1103
Fix #3959

@rkeithhill
Copy link
Collaborator

rkeithhill commented Jun 15, 2017

@SteveL-MSFT You might find limiting that settting to just PowerShell files to be handy - especially for this repo e.g.:

    "[powershell]": {
        "files.trimTrailingWhitespace": true
    },

Then VSCode will not mess with trailing whitespace on .cs and .resx files.

@lzybkr
Copy link
Contributor

lzybkr commented Jun 15, 2017

You probably should fix the resx file to only include your intended change - the other whitespace changes do have a visible effect in the binary, which is probably fine, but should be in a different commit.


In reply to: 308830296 [](ancestors = 308830296)

Copy link
Contributor

Choose a reason for hiding this comment

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

Does this set the exit code correctly? You don't have a test - you should add one as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

Will add test

Copy link
Contributor

Choose a reason for hiding this comment

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

C# now supports local functions - that would make this a bit simpler because you wouldn't the parameters.

Copy link
Member Author

Choose a reason for hiding this comment

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

Making ParseFile a local function would make it inconsistent with similar methods like ParseCommand(), ParseFormat(), ParseExecutionPolicy(), etc...

@alexandair
Copy link
Contributor

Isn't this a breaking change? It should be documented in the release notes.

@lzybkr lzybkr added the Breaking-Change breaking change that may affect users label Jun 16, 2017
… to execute. To align with POSIX so that things like shebang

scripts work correctly, we are changing powershell.exe so that it treats unknown arguments (aka positional argument) as a file.
This means that `powershell foo` will now attempt to use `foo` as a PowerShell script whereas previously `foo` would be treated
as a command to execute.  This doesn't affect existing usage of either `-File` nor `-Command`.  Fixed tests that didn't explicitly
use `-Command` parameter.
added test to validate exit code from script
@SteveL-MSFT SteveL-MSFT force-pushed the powershell-file-arg branch from 18436cb to a7a2497 Compare June 16, 2017 03:44
@SteveL-MSFT
Copy link
Member Author

@lzybkr any other feedback, would like to get this into beta.3 since it's a breaking change and get the feedback

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

Labels

Breaking-Change breaking change that may affect users

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants