8000 Change error message when using New-Item to create a symlink and the item exists by jeffbi · Pull Request #3703 · PowerShell/PowerShell · GitHub
[go: up one dir, main page]
Skip to content

Change error message when using New-Item to create a symlink and the item exists #3703

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 3 commits into from
May 5, 2017
Merged

Change error message when using New-Item to create a symlink and the item exists #3703

merged 3 commits into from
May 5, 2017

Conversation

jeffbi
Copy link
Contributor
@jeffbi jeffbi commented May 5, 2017 •
edited by daxian-dbw
Loading

Fix #3700
Improves the error displayed when attempting to create a SymbolicLink and the item named in -Path already exists.

Also changed the ErrorId to "SymLinkExists" to better identify the error.

Change error message when using New-Item to create a symlink and the …
115c664 
…item exists (#3700)

Also changed the ErrorId to "SymLinkExists".
iSazonov
iSazonov reviewed May 5, 2017
View reviewed changes
src/System.Management.Automation/resources/FileSystemProviderStrings.resx Outdated
@@ -336,4 +336,7 @@
<data name="DriveMaxSizeError" xml:space="preserve">
<value>Maximum size for drive has been exceeded: {0}.</value>
</data>
<data name="ItemExists" xml:space="preserve">
<value>Item '{0}' already exists.</value>
</data>
</root>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add Newline.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 Outdated
@@ -344,6 +344,9 @@ Describe "Hard link and symbolic link tests" -Tags "CI", "RequireAdminOnWindows"
$link.LinkType | Should BeExactly "SymbolicLink"
$link.Target | Should Be $nonFile
}
It "New-Item fails informatively when reversing Path and Target" {
Copy link
Collaborator
@iSazonov iSazonov May 5, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Please make the title more clear.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

Made test title more clear.
ad4b5f2 
Added newline to end of resource file.
iSazonov
iSazonov reviewed May 5, 2017
View reviewed changes
src/System.Management.Automation/resources/FileSystemProviderStrings.resx Outdated
<data name="ItemExists" xml:space="preserve">
<value>Item '{0}' already exists.</value>
8000
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe use the pattern from tests - "Path {0} to symbolic link already exists." ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about "The path {0} already exists."? That's similar to the error message when New-Item attempts to create a file that already exists, substituting "path" for "file".

I think that "Path {0} to symbolic link..." suggests that the existing item is itself a symbolic link, which is not necessarily the case.

Copy link
Collaborator

Choose a reason for hiding this comment

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

In this context we should mention "symbolic link".
Maybe "Cannot create symbolic link because the path {0} already exists."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like that one. Fixed.

I also changed the name of the new string, to be a bit more clear about its use.

@iSazonov
Copy link
Collaborator
iSazonov commented May 5, 2017

LGTM.

@daxian-dbw daxian-dbw changed the title Change error message when using New-Item to create a symlink and the item exists (#3700) Change error message when using New-Item to create a symlink and the item exists May 5, 2017
@daxian-dbw daxian-dbw merged commit 938e136 into PowerShell:master May 5, 2017
@jeffbi
Copy link
Contributor Author
jeffbi commented May 5, 2017

@iSazonov Thanks for the review!
< 8DA5 form class="js-comment-update" id="issuecomment-299523709-edit-form" data-turbo="false" action="/PowerShell/PowerShell/issue_comments/299523709" accept-charset="UTF-8" method="post">

@jeffbi jeffbi deleted the bad-error-3700 branch May 8, 2017 04:55
@jeffbi jeffbi restored the bad-error-3700 branch May 8, 2017 05:01
@jeffbi jeffbi deleted the bad-error-3700 branch May 8, 2017 05:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iSazonov iSazonov iSazonov left review comments

@daxian-dbw daxian-dbw daxian-dbw 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
@jeffbi @iSazonov @daxian-dbw @msftclas
0