Skip to content

Conversation

@jeffbi
Copy link
Contributor

@jeffbi jeffbi commented May 5, 2017

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.

…item exists (#3700)

Also changed the ErrorId to "SymLinkExists".
<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

$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

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

Added newline to end of resource file.
</data>
</root>
<data name="ItemExists" xml:space="preserve">
<value>Item '{0}' already exists.</value>
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!

@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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants