Respect $VerbosePreference in ShouldProcess #26511

yotsuda · 2025-11-22T12:18:38Z

PR Summary

Fix ShouldProcess to respect $VerbosePreference variable in addition to -Verbose parameter.

PR Context

$PSCmdlet.ShouldProcess() was only emitting verbose output when the -Verbose parameter was explicitly specified, completely ignoring the $VerbosePreference variable setting. This is inconsistent with how other cmdlets handle verbose output.

The issue occurred because DoShouldProcess() and CalculatePossibleShouldProcessOptimization() were checking this.Verbose (which only tracks the -Verbose parameter flag) instead of calling IsWriteVerboseEnabled() (which properly checks both -Verbose parameter and $VerbosePreference variable).

Changes:

Modified MshCommandRuntime.cs to use IsWriteVerboseEnabled() instead of this.Verbose in two locations
Added comprehensive tests to verify the fix

Before:

function Test-SP {
    [CmdletBinding(SupportsShouldProcess)]
    param($Target = "TestTarget", $Action = "TestAction")
    if ($PSCmdlet.ShouldProcess($Target, $Action)) { "OK" }
}
$VerbosePreference = 'Continue'
Test-SP  # No verbose output (bug)

After:

function Test-SP {
    [CmdletBinding(SupportsShouldProcess)]
    param($Target = "TestTarget", $Action = "TestAction")
    if ($PSCmdlet.ShouldProcess($Target, $Action)) { "OK" }
}
$VerbosePreference = 'Continue'
Test-SP  # VERBOSE: Performing the operation "TestAction" on target "TestTarget". (fixed!)

PR Checklist

ShouldProcess was only emitting verbose output when the -Verbose parameter was explicitly specified, ignoring the $VerbosePreference variable setting. Changed to use IsWriteVerboseEnabled() instead of checking the Verbose flag directly in both DoShouldProcess() and CalculatePossibleShouldProcessOptimization(). This method properly respects both the -Verbose parameter and $VerbosePreference variable.

iSazonov · 2025-11-22T13:16:25Z

From #12148 (comment)

ShouldProgress needs to respect $ProgressPreference = 'Continue', but if -Verbose:$false, then that should take precedent.

@yotsuda Could you please add test for -Verbose:$false?

iSazonov · 2025-11-22T13:18:48Z

test/powershell/Language/Scripting/ShouldProcess.Tests.ps1

+
+    Context "VerbosePreference variable" {
+        It "Emits verbose output when VerbosePreference is Continue" {
+            $VerbosePreference = 'Continue'


Please save and then restore the value.

Thanks for the review! Fixed in 5d8a7d1. All tests now save and restore $VerbosePreference using try-finally blocks.

- Add test case for -Verbose:$false taking precedence over $VerbosePreference - Save and restore $VerbosePreference in all test cases using try-finally blocks - Clean up whitespace

Copilot

Pull request overview

This PR fixes a bug where ShouldProcess was only emitting verbose output when the -Verbose parameter was explicitly specified, completely ignoring the $VerbosePreference variable. This makes the behavior consistent with how other cmdlets handle verbose output by properly checking both the parameter and the preference variable.

Key Changes:

Changed two locations in DoShouldProcess() and CalculatePossibleShouldProcessOptimization() from checking this.Verbose to calling IsWriteVerboseEnabled()
Added comprehensive test coverage for both $VerbosePreference variable and -Verbose parameter scenarios

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
`src/System.Management.Automation/engine/MshCommandRuntime.cs`	Fixed two method calls to use `IsWriteVerboseEnabled()` instead of `this.Verbose` to properly respect both `-Verbose` parameter and `$VerbosePreference` variable
`test/powershell/Language/Scripting/ShouldProcess.Tests.ps1`	Added new test file with comprehensive coverage for `ShouldProcess` verbose output behavior with both preference variable and parameter scenarios

Comments suppressed due to low confidence (1)

src/System.Management.Automation/engine/MshCommandRuntime.cs:1649

Both branches of this 'if' statement return - consider using '?' to express intent better.

                if (IsWriteVerboseEnabled())
                {
                    return ShouldProcessPossibleOptimization.AutoYes_CanCallShouldProcessAsynchronously;
                }
                else
                {
                    return ShouldProcessPossibleOptimization.AutoYes_CanSkipShouldProcessCall;
                }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

iSazonov · 2025-11-24T10:20:03Z

@yotsuda Please look test errors.

- Add ShouldEmitVerboseFromShouldProcess() method that checks only -Verbose parameter and $VerbosePreference variable - Excludes -Debug parameter effect to avoid unintended side effects - Update History.Tests.ps1 expected count from 12 to 13 to reflect the correct behavior when $VerbosePreference is 'Continue'

yotsuda · 2025-11-26T04:31:39Z

@yotsuda Please look test errors.

Fixed in ce12d77. The original implementation using IsWriteVerboseEnabled() was affected by -Debug parameter, causing unintended behavior.

Created a dedicated method ShouldEmitVerboseFromShouldProcess() that checks only:

-Verbose parameter (if explicitly set)
$VerbosePreference variable

Also updated History.Tests.ps1 expected count (12 → 13) as it now correctly emits verbose output.
All CI checks passing.

iSazonov · 2025-11-26T05:18:02Z

The original implementation using IsWriteVerboseEnabled() was affected by -Debug parameter, causing unintended behavior.

Hmm, IsWriteVerboseEnabled is not directly check Debug parameter. So it is not right fix.

yotsuda · 2025-11-27T09:15:53Z

Hmm, IsWriteVerboseEnabled is not directly check Debug parameter. So it is not right fix.

@iSazonov You're right that IsWriteVerboseEnabled() doesn't directly check -Debug. However, it uses the VerbosePreference property getter, which returns Continue/Inquire when -Debug is set (line 3116-3128), before checking $VerbosePreference.

The WG decision specifies only -Verbose and $VerbosePreference - not -Debug. So I created ShouldEmitVerboseFromShouldProcess() to check these two factors directly, bypassing the -Debug logic in the property getter.

iSazonov · 2025-11-29T17:20:26Z

Thanks for clarify!

I don't see that WG discussed the influence of -Debug switch presence on the issue. I believe that this parameter is taken into account in all preferences for a reason. We should probably ask for confirmation from the WG in original issue.

SteveL-MSFT · 2025-12-17T17:05:18Z

Cmdlet-WG already reviewed and approved the issue, so this is good from a behavior perspective

iSazonov · 2025-12-17T18:13:28Z

@SteveL-MSFT It is not clear should we take into account the influence of -Debug switch presence or ignore it as implemented in the PR?

microsoft-github-policy-service · 2025-12-25T06:03:29Z

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

iSazonov added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Nov 22, 2025

iSazonov reviewed Nov 22, 2025

View reviewed changes

Add test for -Verbose:$false and fix VerbosePreference state management

5d8a7d1

- Add test case for -Verbose:$false taking precedence over $VerbosePreference - Save and restore $VerbosePreference in all test cases using try-finally blocks - Clean up whitespace

iSazonov requested a review from Copilot November 22, 2025 16:54

Copilot started reviewing on behalf of iSazonov November 22, 2025 16:54 View session

Copilot finished reviewing on behalf of iSazonov November 22, 2025 16:56

Copilot AI reviewed Nov 22, 2025

View reviewed changes

yotsuda marked this pull request as draft November 24, 2025 22:44

yotsuda marked this pull request as ready for review November 26, 2025 04:28

iSazonov added WG-Cmdlets general cmdlet issues WG-NeedsReview Needs a review by the labeled Working Group labels Dec 5, 2025

jdhitsolutions added this to Cmdlets Working Group Dec 10, 2025

microsoft-github-policy-service bot added the Review - Needed The PR is being reviewed label Dec 12, 2025

SteveL-MSFT moved this to In-Progress-PullRequests in Cmdlets Working Group Dec 17, 2025

SteveL-MSFT removed the WG-Cmdlets general cmdlet issues label Dec 17, 2025

microsoft-github-policy-service bot removed the Review - Needed The PR is being reviewed label Dec 17, 2025

SteveL-MSFT added Review - Needed The PR is being reviewed and removed WG-NeedsReview Needs a review by the labeled Working Group labels Dec 17, 2025

microsoft-github-policy-service bot removed the Review - Needed The PR is being reviewed label Dec 17, 2025

SteveL-MSFT added WG-Cmdlets general cmdlet issues Review - Needed The PR is being reviewed labels Dec 17, 2025

microsoft-github-policy-service bot removed the Review - Needed The PR is being reviewed label Dec 17, 2025

SteveL-MSFT added Review - Needed The PR is being reviewed WG-Reviewed A Working Group has reviewed this and made a recommendation labels Dec 17, 2025

SteveL-MSFT moved this from In-Progress-PullRequests to Reviewed in Cmdlets Working Group Dec 17, 2025

microsoft-github-policy-service bot removed the Review - Needed The PR is being reviewed label Dec 17, 2025

microsoft-github-policy-service bot added the Review - Needed The PR is being reviewed label Dec 25, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Respect $VerbosePreference in ShouldProcess #26511

Respect $VerbosePreference in ShouldProcess #26511

yotsuda commented Nov 22, 2025

Uh oh!

iSazonov commented Nov 22, 2025

Uh oh!

iSazonov Nov 22, 2025

Uh oh!

yotsuda Nov 22, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

iSazonov commented Nov 24, 2025

Uh oh!

yotsuda commented Nov 26, 2025

Uh oh!

iSazonov commented Nov 26, 2025

Uh oh!

yotsuda commented Nov 27, 2025

Uh oh!

iSazonov commented Nov 29, 2025

Uh oh!

SteveL-MSFT commented Dec 17, 2025 •

edited

Loading

Uh oh!

iSazonov commented Dec 17, 2025

Uh oh!

microsoft-github-policy-service bot commented Dec 25, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Respect $VerbosePreference in ShouldProcess #26511

Are you sure you want to change the base?

Respect $VerbosePreference in ShouldProcess #26511

Conversation

yotsuda commented Nov 22, 2025

PR Summary

PR Context

PR Checklist

Uh oh!

iSazonov commented Nov 22, 2025

Uh oh!

iSazonov Nov 22, 2025

Choose a reason for hiding this comment

Uh oh!

yotsuda Nov 22, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

iSazonov commented Nov 24, 2025

Uh oh!

yotsuda commented Nov 26, 2025

Uh oh!

iSazonov commented Nov 26, 2025

Uh oh!

yotsuda commented Nov 27, 2025

Uh oh!

iSazonov commented Nov 29, 2025

Uh oh!

SteveL-MSFT commented Dec 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

iSazonov commented Dec 17, 2025

Uh oh!

microsoft-github-policy-service bot commented Dec 25, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

SteveL-MSFT commented Dec 17, 2025 •

edited

Loading