Add a CR to CommandNotFoundException error string by jpsnover · Pull Request #4934 · PowerShell/PowerShell · GitHub
Skip to content

Add a CR to CommandNotFoundException error string#4934

Merged
daxian-dbw merged 1 commit intomasterfrom
jpsnover-patch-1
Sep 29, 2017
Merged

Add a CR to CommandNotFoundException error string#4934
daxian-dbw merged 1 commit intomasterfrom
jpsnover-patch-1

Conversation

@jpsnover
Copy link
Copy Markdown
Contributor

@jpsnover jpsnover commented Sep 27, 2017

We're here at PowerShell Unplugged at Ignite, and this error message could really use a carriage return!

@jpsnover
Add a CR to CommandNotFoundException error string
90d4921 
We're here at PowerShell Unplugged at Ignite, and this error message could *really* use a carriage return!
SteveL-MSFT
SteveL-MSFT requested changes Sep 27, 2017
View reviewed changes
Copy link
Copy Markdown
Member

@SteveL-MSFT SteveL-MSFT left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution, but string resources should not contain newlines as it may produce unexpected results when it goes through FormatAndOutput

Comment thread src/System.Management.Automation/resources/DiscoveryExceptions.resx
Copy link
Copy Markdown
Member

@SteveL-MSFT SteveL-MSFT Sep 27, 2017

Choose a reason for hiding this comment

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

Formatting should be handled by FormatAndOutput. String messages should not contain newlines.

Copy link
Copy Markdown
Member

@SteveL-MSFT SteveL-MSFT Sep 29, 2017

Choose a reason for hiding this comment

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

Perhaps adding new newlines is something we should consider for other strings as it can improve readability.

@daxian-dbw daxian-dbw merged commit b9845d5 into master Sep 29, 2017
@daxian-dbw daxian-dbw deleted the jpsnover-patch-1 branch September 29, 2017 19:16
@iSazonov
Copy link
Copy Markdown
Collaborator

iSazonov commented Oct 2, 2017 •
edited
Loading

Please clarify what was idea here.
Before the change:

> qwerty
qwerty : The term 'qwerty' is not recognized as the name of a cmdlet, function, script file, or operable program. Check the spelling of the name, or if a path
was included, verify that the path is correct and try again.
At line:1 char:1
+ qwerty
+ ~~~~~~
    + CategoryInfo          : ObjectNotFound: (qwerty:String) [], CommandNotFoundException
    + FullyQualifiedErrorId : CommandNotFoundException

After the change:

PS C:\Users\sie\Documents\GitHub\iSazonov\PowerShell> qwerty
qwerty : The term 'qwerty' is not recognized as the name of a cmdlet, function, script file, or operable program.
      Check the spelling of the name, or if a path was included, verify that the path is correct and try again.
At line:1 char:1
+ qwerty
+ ~~~~~~
    + CategoryInfo          : ObjectNotFound: (qwerty:String) [], CommandNotFoundException
    + FullyQualifiedErrorId : CommandNotFoundException

Or idea was to change command line error formatting?
From:

> .\debug\powershell.exe -sma
The argument '-sma' is not recognized as the name of a script file. Check the spelling of the name, or if a path was included, verify that the path is correct and try again.

To:

> .\debug\powershell.exe -sma
The argument '-sma' is not recognized as the name of a script file. 
Check the spelling of the name, or if a path was included, verify that the path is correct and try again.

@iSazonov
Copy link
Copy Markdown
Collaborator

iSazonov commented Oct 3, 2017

@SteveL-MSFT Could you please clarify?

@SteveL-MSFT
Copy link
Copy Markdown
Member

SteveL-MSFT commented Oct 3, 2017

@SteveL-MSFT SteveL-MSFT mentioned this pull request Oct 3, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SteveL-MSFT SteveL-MSFT SteveL-MSFT approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jpsnover @iSazonov @SteveL-MSFT @daxian-dbw