PowerShell-Docs icon indicating copy to clipboard operation
PowerShell-Docs copied to clipboard

Does this advice regarding ShouldProcess and ShouldContinue in "How to Request Confirmations - PowerShell" make sense to anyone?

Open Dave-Lowndes opened this issue 5 years ago • 3 comments

The recommendation here to first call ShouldProcess and then (conditionally) call ShouldContinue seems wrong to me - leaving me totally confused about what ought to be straightforward.

Given the code snippet example and the default situation where ShouldProcess won't actually prompt because the ConfirmImpact level isn't hit, only ShouldContinue will output something - in the example it shows: "Confirm Continue with this operation?"

Which isn't very informative as to what the consequences are. Should the message output there have more information or not?

However, if the ConfirmImpact level is hit, both ShowProcess and ShouldContinue will prompt in order to continue - guaranteeing irritating the user.

If this really is the best practice usage of these methods, this topic could do with more explanation as to why this is the case and the example expanding to fully illustrate it. If it's not actually best practice - please tell us what is.


Document Details

Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

Dave-Lowndes avatar Feb 28 '20 10:02 Dave-Lowndes

ShouldContinue and ShouldProcess should probably not be used together. ShouldContinue is going to prompt unless you take extra measures, but ShouldProcess may not prompt, depending on preference and commandline options:

PS> cat t2.ps1
[CmdletBinding(ConfirmImpact="High",SupportsShouldProcess)]
param ( )
END {
    if ( $PSCmdlet.ShouldContinue("are you sure you want to do it?","THIS IS DANGEROUS") ) {
        "continue"
    }
    if ( $PSCmdlet.ShouldProcess("again????") ) {
        "process"
    }
}
PS> ./t2

THIS IS DANGEROUS
are you sure you want to do it?
[Y] Yes  [N] No  [S] Suspend  [?] Help (default is "Y"): 
continue

Confirm
Are you sure you want to perform this action?
Performing the operation "t2.ps1" on target "again????".
[Y] Yes  [A] Yes to All  [N] No  [L] No to All  [S] Suspend  [?] Help (default is "Y"): 
process
PS> ./t2 -confirm:$false

THIS IS DANGEROUS
are you sure you want to do it?
[Y] Yes  [N] No  [S] Suspend  [?] Help (default is "Y"): 
continue
process
PS> cat ./t4.ps1
[CmdletBinding(ConfirmImpact="High",SupportsShouldProcess)]
param ( [switch]$force )
END {
    if ( $force -or $PSCmdlet.ShouldContinue("are you sure you want to do it?","THIS IS DANGEROUS") ) {
        "continue"
    }
    if ( $PSCmdlet.ShouldProcess("again????") ) {
        "process"
    }
}
PS> ./t4

THIS IS DANGEROUS
are you sure you want to do it?
[Y] Yes  [N] No  [S] Suspend  [?] Help (default is "Y"): 
continue

Confirm
Are you sure you want to perform this action?
Performing the operation "t4.ps1" on target "again????".
[Y] Yes  [A] Yes to All  [N] No  [L] No to All  [S] Suspend  [?] Help (default is "Y"): 
process
PS> ./t4 -force    
continue

Confirm
Are you sure you want to perform this action?
Performing the operation "t4.ps1" on target "again????".
[Y] Yes  [A] Yes to All  [N] No  [L] No to All  [S] Suspend  [?] Help (default is "Y"): 
process
PS> ./t4 -force -confirm:$false
continue
process

there's not really a good reason to use both ShouldProcess and ShouldContinue for the same operation as they serve essentially the same purpose - ShouldContinue provides the added behavior that it will prompt for confirmation unless you code around it.

JamesWTruher avatar Feb 28 '20 19:02 JamesWTruher

OK, thanks James, that's what I thought. Given that we're in agreement then it'd be good to get that misleading documentation sorted out 👍

Dave-Lowndes avatar Feb 28 '20 20:02 Dave-Lowndes

(General style comment) Issue heading is too generic. It needs a reference to the specific details, e.g.

Does this advice regarding ShouldProcess and ShouldContinue in "How to Request Confirmations - PowerShell" make sense to anyone?

This would allow anybody raising a new (unrelated) issue to skip this one when checking for existing issues similar to theirs.

UberKluger avatar Jun 08 '21 07:06 UberKluger