Skip to content
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

Error message enhancement for clear-content cmdlet when targeting a directory #8134

Merged
merged 9 commits into from Nov 1, 2018
Merged

Error message enhancement for clear-content cmdlet when targeting a directory #8134

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
80fa00b
Error message enhancement for clear-content cmdlet when targeting a d…
kvprasoon Oct 27, 2018
3503f1c
changing the implementation to FileSystemProvider.
kvprasoon Oct 28, 2018
253f2c2
Codefactor fix
kvprasoon Oct 28, 2018
1b6f0b5
Review comment changes
kvprasoon Oct 29, 2018
cb70bbb
Merge remote-tracking branch 'upstream/master'
kvprasoon Oct 29, 2018
05fca58
code review: correting intendation for opening data tag
kvprasoon Oct 30, 2018
36b6007
Merge remote-tracking branch 'upstream/master'
kvprasoon Oct 30, 2018
a1b02ae
[Feature] an empty commit for triggering Full test
kvprasoon Oct 31, 2018
5a8115a
[Feature] an empty commit for triggering Full test
kvprasoon Oct 31, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/System.Management.Automation/engine/SessionStateContent.cs
View file
Open in desktop
Expand Up @@ -3,6 +3,7 @@

using System.Collections.ObjectModel;
using System.Management.Automation.Provider;
using System.IO;
using Dbg = System.Management.Automation;

#pragma warning disable 1634, 1691 // Stops compiler from warning about unknown warnings
Expand Down Expand Up @@ -848,6 +849,10 @@ internal void ClearContent(string[] paths, bool force, bool literalPath)

try
{
if (Directory.Exists(path))
{
throw PSTraceSource.NewNotSupportedException(SessionStateStrings.ClearDirectoryContent, path);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd expect non-terminating error.

/cc @mklement0

Copy link
Member

Choose a reason for hiding this comment

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

Yes, this should be non-terminating error.

Copy link
Member

Choose a reason for hiding this comment

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

Also, the check should be in the FileSystemProvider in this function:

PowerShell/src/System.Management.Automation/namespaces/FileSystemProvider.cs

Line 6599 in b27380d

public void ClearContent(string path)
not in the engine.

providerInstance.ClearContent(path, context);
}
catch (NotSupportedException)
Expand Down Expand Up @@ -1025,4 +1030,3 @@ internal void ClearContent(string[] paths, bool force, bool literalPath)
}

#pragma warning restore 56500

5 changes: 4 additions & 1 deletion src/System.Management.Automation/resources/SessionStateStrings.resx
View file
Open in desktop
Expand Up @@ -284,7 +284,7 @@
</data>
<data name="WriteContainerContentException" xml:space="preserve">
<value>Unable to write content because it is a directory: '{0}'.</value>
</data>
</data>
<data name="LocationUndoStackIsEmpty" xml:space="preserve">
<value>There is no location history left to navigate backwards.</value>
</data>
Expand All @@ -296,6 +296,9 @@
</data>
<data name="ClearContentProviderException" xml:space="preserve">
<value>Attempting to perform the ClearContent operation on the '{0}' provider failed for path '{1}'. {2}</value>
</data>
<data name="ClearDirectoryContent" xml:space="preserve">
kvprasoon marked this conversation as resolved.
Show resolved Hide resolved
<value>Unable to clear content of '{0}' because it is a directory. Clear-Content cmdlet has to be used on a file.</value>
kvprasoon marked this conversation as resolved.
Show resolved Hide resolved
</data>
<data name="ClearContentDynamicParametersProviderException" xml:space="preserve">
<value>The dynamic parameters for the ClearContent operation cannot be retrieved from the '{0}' provider for path '{1}'. {2}</value>
Expand Down
4 changes: 4 additions & 0 deletions test/powershell/Modules/Microsoft.PowerShell.Management/Clear-Content.Tests.ps1
View file
Open in desktop
Expand Up @@ -105,6 +105,10 @@ Describe "Clear-Content cmdlet tests" -Tags "CI" {
}

Context "Proper errors should be delivered when bad locations are specified" {
It "should throw when targetting a directory." {
{ Clear-Content -Path . -ErrorAction Stop } | Should -Throw
kvprasoon marked this conversation as resolved.
Show resolved Hide resolved
}

It "should throw `"Cannot bind argument to parameter 'Path'`" when -Path is `$null" {
{ Clear-Content -Path $null -ErrorAction Stop } |
Should -Throw -ErrorId "ParameterArgumentValidationErrorNullNotAllowed,Microsoft.PowerShell.Commands.ClearContentCommand"
Expand Down