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 — Make error message consistent when invalid script is passed to -File, better error when passed ambiguous arg by SteveL-MSFT · Pull Request #4573 · PowerShell/PowerShell

Conversation

@SteveL-MSFT

@SteveL-MSFT Steve Lee (SteveL-MSFT) commented Aug 14, 2017 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

Since -File is now the positional parameter for powershell.exe, made the error message consistent with
-Command when passed an invalid file.

If ambiguous arg is passed, give a better error message:

PS C:\users\poker\repos\PowerShell> powershell -no -file foo
Invalid argument '-no', did you mean:

        -nologo
        -noexit
        -noprofile
        -noninteractive
PowerShell v6.0.0-beta.5-42-g3ed9a816b2452551aaad1ac1245a1cbb35e87612-dirty
Copyright (C) Microsoft Corporation. All rights reserved.

Enable -WindowStyle to work

Fix #4351

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@SteveL-MSFT Steve Lee (SteveL-MSFT) assigned Ilya (iSazonov) Aug 14, 2017
@SteveL-MSFT
Steve Lee (SteveL-MSFT) requested a review from Jason Shirk (lzybkr) August 14, 2017 23:04
@msftclas Microsoft Contribution License Agreements (msftclas) added the cla-not-required label Aug 14, 2017
iSazonov
Ilya (iSazonov) reviewed Aug 15, 2017
View reviewed changes
Comment thread src/Microsoft.PowerShell.ConsoleHost/resources/CommandLineParameterParserStrings.resx Outdated

@iSazonov Ilya (iSazonov) Aug 15, 2017

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.

I believe argument is better term and we can be shorter:

The argument '{0}' is not recognized as the name of a script file.

Sorry, something went wrong.

Uh oh!

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

@SteveL-MSFT Steve Lee (SteveL-MSFT) Aug 15, 2017

Copy link
Copy Markdown
Member 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.

Will change

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@SteveL-MSFT
Steve Lee (SteveL-MSFT) force-pushed the console-no branch from e18b241 to d6d9505 Compare August 15, 2017 11:08
lzybkr
Jason Shirk (lzybkr) reviewed Aug 15, 2017
View reviewed changes
Comment thread src/Microsoft.PowerShell.ConsoleHost/host/msh/CommandLineParameterParser.cs Outdated

@lzybkr Jason Shirk (lzybkr) Aug 15, 2017

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.

Should this be validParameter.StartsWith(param, StringComparer.OrdinalIgnoreCase)?

Sorry, something went wrong.

Uh oh!

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

@SteveL-MSFT Steve Lee (SteveL-MSFT) Aug 16, 2017

Copy link
Copy Markdown
Member 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.

I was thinking to cover cases like: -format

Sorry, something went wrong.

Uh oh!

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

@SteveL-MSFT Steve Lee (SteveL-MSFT) Aug 16, 2017

Copy link
Copy Markdown
Member 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.

I'll add a test case for this

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
lzybkr
Jason Shirk (lzybkr) reviewed Aug 15, 2017
View reviewed changes
Comment thread src/Microsoft.PowerShell.ConsoleHost/host/msh/CommandLineParameterParser.cs Outdated

@lzybkr Jason Shirk (lzybkr) Aug 15, 2017 •
edited
Loading

Uh oh!

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

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.

I'd indent with spaces - we don't indent with tabs anywhere else that I know of.

Sorry, something went wrong.

Uh oh!

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

@SteveL-MSFT Steve Lee (SteveL-MSFT) Aug 16, 2017

Copy link
Copy Markdown
Member 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.

will change

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
lzybkr
Jason Shirk (lzybkr) reviewed Aug 15, 2017
View reviewed changes
Comment thread src/Microsoft.PowerShell.ConsoleHost/host/msh/CommandLineParameterParser.cs Outdated

@lzybkr Jason Shirk (lzybkr) Aug 15, 2017

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.

Showing the banner after the error is weird and not like the other errors I reviewed.

Sorry, something went wrong.

Uh oh!

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

@SteveL-MSFT Steve Lee (SteveL-MSFT) Aug 16, 2017 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member 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.

I thought that was weird in general, but it seems consistent with other uses of WriteCommandLineError. Unless you know the reason it was like this, I'll change all uses to not show the banner.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@SteveL-MSFT Steve Lee (SteveL-MSFT) changed the title Make error message consistent when invalid script is passed to -File Make error message consistent when invalid script is passed to -File, better error when passed ambiguous arg Aug 16, 2017
@SteveL-MSFT
Steve Lee (SteveL-MSFT) requested review from James Truher (JamesWTruher), Travis Plunk (TravisEz13), Aditya Patwardhan (adityapatwardhan), Andrew (anmenaga) and Dongbo Wang (daxian-dbw) as code owners August 16, 2017 09:29
@SteveL-MSFT
Steve Lee (SteveL-MSFT) force-pushed the console-no branch 2 times, most recently from a9d0bac to cb97564 Compare August 16, 2017 10:23
iSazonov
Ilya (iSazonov) reviewed Aug 16, 2017
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.

Leave a comment

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread src/Microsoft.PowerShell.ConsoleHost/host/msh/CommandLineParameterParser.cs Outdated

@iSazonov Ilya (iSazonov) Aug 16, 2017

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 !CORECLR is no longer relevant. Should we remove the parameters at all?

Sorry, something went wrong.

Uh oh!

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

@SteveL-MSFT Steve Lee (SteveL-MSFT) Aug 16, 2017

Copy link
Copy Markdown
Member 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.

I'm fixing that as part of enabling ApartmentState PR and not part of this PR

Sorry, something went wrong.

Uh oh!

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

@iSazonov Ilya (iSazonov) Aug 16, 2017

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.

Closed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleControl.cs Outdated

@iSazonov Ilya (iSazonov) Aug 16, 2017

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
#if !UNIX

Sorry, something went wrong.

Uh oh!

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

@SteveL-MSFT Steve Lee (SteveL-MSFT) Aug 16, 2017

Copy link
Copy Markdown
Member 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.

makes sense

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread src/Microsoft.PowerShell.ConsoleHost/resources/CommandLineParameterParserStrings.resx Outdated

@iSazonov Ilya (iSazonov) Aug 16, 2017

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.

"did you mean:" - Do you want add something else?

Sorry, something went wrong.

Uh oh!

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

@SteveL-MSFT Steve Lee (SteveL-MSFT) Aug 16, 2017

Copy link
Copy Markdown
Member 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 possible matches show up after this. See example above in the PR description.

Sorry, something went wrong.

Uh oh!

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

@iSazonov Ilya (iSazonov) Aug 16, 2017

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.

Closed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread test/powershell/Host/ConsoleHost.Tests.ps1 Outdated

@iSazonov Ilya (iSazonov) Aug 16, 2017

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.

We should exclude this on Unix.

Sorry, something went wrong.

Uh oh!

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

@SteveL-MSFT Steve Lee (SteveL-MSFT) Aug 16, 2017

Copy link
Copy Markdown
Member 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.

will do

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread test/powershell/Host/ConsoleHost.Tests.ps1 Outdated

@iSazonov Ilya (iSazonov) Aug 16, 2017

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.

All tests is excluded on Unix so we could use our pattern "$PSDefaultParameterValues["it:skip"] = ..."

Sorry, something went wrong.

Uh oh!

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

@SteveL-MSFT Steve Lee (SteveL-MSFT) Aug 16, 2017

Copy link
Copy Markdown
Member 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.

agree

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
iSazonov
Ilya (iSazonov) reviewed Aug 16, 2017
View reviewed changes
Comment thread test/powershell/Host/ConsoleHost.Tests.ps1 Outdated

@iSazonov Ilya (iSazonov) Aug 16, 2017

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.

Parentheses can be removed.

Sorry, something went wrong.

Uh oh!

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

@SteveL-MSFT Steve Lee (SteveL-MSFT) Aug 16, 2017

Copy link
Copy Markdown
Member 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.

Will remove

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@SteveL-MSFT
Steve Lee (SteveL-MSFT) force-pushed the console-no branch from eb660da to 951832a Compare August 16, 2017 11:43
@SteveL-MSFT

Steve Lee (SteveL-MSFT) commented Aug 16, 2017 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member Author

Not sure why some of the new tests fail in AppVeyor but not on my desktop. Figured it out, need to redirect stderr to stdout. However, I noticed the tests fail on Mac because the exit code isn't being set, but still shows as passed overall.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@SteveL-MSFT
Steve Lee (SteveL-MSFT) force-pushed the console-no branch 2 times, most recently from bb83bb5 to ab1862b Compare August 16, 2017 13:01
@SteveL-MSFT Steve Lee (SteveL-MSFT) added the Review - Committee The PR/Issue needs a review from the PowerShell Committee label Aug 16, 2017
@SteveL-MSFT

Steve Lee (SteveL-MSFT) commented Aug 16, 2017

Copy link
Copy Markdown
Member Author

Figured out why it's failing on non-Windows. It appears that exit codes from programs have to be in the range 0-255 and anything greater than 255 is mod 256. I verified this with a simple console c# app. In this case, the value for ExitCodeBadCommandLineParameter mod 256 == 0. Hmmm.

Although a breaking change, I think we should conform to libc standards which means this error code should be 64 "command line usage error". We should also update all the other error codes that powershell console returns.

In general, I don't think this should be a problem as typically 0 means success and non-0 means error so although a breaking change, I suspect it have limited real world impact.

cc @PowerShell/powershell-committee
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 Aug 16, 2017
@SteveL-MSFT
Steve Lee (SteveL-MSFT) force-pushed the console-no branch from be79184 to 5c6b694 Compare August 16, 2017 14:12
lzybkr
Jason Shirk (lzybkr) approved these changes Aug 16, 2017
View reviewed changes

@lzybkr Jason Shirk (lzybkr) 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.

I approve the changes, but there are 2 distinct fixes in the PR, so I think there should be 2 distinct commits.

Interactive add (git add -p) is an easy way to do that.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@SteveL-MSFT

Steve Lee (SteveL-MSFT) commented Aug 16, 2017

Copy link
Copy Markdown
Member Author

Jason Shirk (@lzybkr) are you referring to -WindowStyle and everything else as two commits?
All reactions

Sorry, something went wrong.

Uh oh!

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

Jason Shirk (lzybkr) commented Aug 16, 2017

Copy link
Copy Markdown
Contributor

Yeah, -WindowStyle is buried at the bottom of your PR description, so it felt like it's own thing.
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 Aug 16, 2017

Copy link
Copy Markdown
Member Author

Jason Shirk (@lzybkr) sure, I can make that two commits in the same PR.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Steve Lee (SteveL-MSFT) added 3 commits August 16, 2017 15:02
@SteveL-MSFT
Make invalid arg to -File consistent with -Command
d9c63b7 
Improve error message if ambiguous arg is passed to -File
@SteveL-MSFT
Update powershell console exit codes to match Unix standards
4c22b10
@SteveL-MSFT
Enable -WindowStyle to work
26f7db7
@SteveL-MSFT
Steve Lee (SteveL-MSFT) force-pushed the console-no branch from 5c6b694 to 26f7db7 Compare August 16, 2017 22:05
@SteveL-MSFT

Steve Lee (SteveL-MSFT) commented Aug 16, 2017

Copy link
Copy Markdown
Member Author

Split into 3 commits (Thanks Jason Shirk (@lzybkr) for the tip on git add -p!)
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 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 Aug 16, 2017
@SteveL-MSFT

Steve Lee (SteveL-MSFT) commented Aug 16, 2017

Copy link
Copy Markdown
Member Author

@PowerShell/powershell-committee reviewed this and is ok with changing the exit codes
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@TravisEz13
Travis Plunk (TravisEz13) removed their request for review August 16, 2017 23:09
iSazonov
Ilya (iSazonov) approved these changes Aug 17, 2017
View reviewed changes
@iSazonov
Ilya (iSazonov) merged commit 45d1d20 into PowerShell:master Aug 17, 2017
@TravisEz13

Travis Plunk (TravisEz13) commented Aug 17, 2017 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

Ilya (@iSazonov), Jason Shirk (@lzybkr) and Steve Lee (@SteveL-MSFT) thanks as always!
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.
@SteveL-MSFT
Steve Lee (SteveL-MSFT) deleted the console-no branch August 24, 2017 03:02
Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj-contribs/PowerShell that referenced this pull request Aug 6, 2026
@iSazonov
Merge pull request PowerShell#4573 from SteveL-MSFT/console-no
5efd965 
* Make invalid arg to -File consistent with -Command
Improve error message if ambiguous arg is passed to -File

* Update powershell console exit codes to match Unix standards

* Enable -WindowStyle to work
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iSazonov Ilya (iSazonov) iSazonov approved these changes
@adityapatwardhan Aditya Patwardhan (adityapatwardhan) Awaiting requested review from adityapatwardhan
@anmenaga Andrew (anmenaga) Awaiting requested review from anmenaga
@daxian-dbw Dongbo Wang (daxian-dbw) Awaiting requested review from daxian-dbw
@JamesWTruher James Truher (JamesWTruher) Awaiting requested review from JamesWTruher
+1 more reviewer
@lzybkr Jason Shirk (lzybkr) lzybkr approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

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

Assignees

@iSazonov Ilya (iSazonov)

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.

5 participants

@SteveL-MSFT @lzybkr @TravisEz13 @iSazonov @msftclas
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 01402520 — 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.