Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 16, 2025

  • Refactor to use System.CommandLine Option Action
  • Create ApplyVerbosityAction class
  • Add Action to all VerbosityOption definitions
  • Revert all command-specific changes
  • Fix Terminating property to false
  • Verify Action is invoked correctly
  • Test the implementation across multiple commands
  • Address code review feedback
  • Remove debug comments
  • Simplify Action by passing option reference
  • Final testing complete

Implementation Complete

The solution uses System.CommandLine's Option Action mechanism with a generic Action class that receives the option reference directly. This allows cleaner code using parseResult.GetValue(verbosityOption) as suggested by @baronfel.

Verified working:

  • dotnet build -v diag
  • dotnet restore -v diagnostic
  • dotnet build -v normal (does not set variable) ✓
Original prompt

Can you look through all of the CLI commands, find those that have a verbosity, and if verbosity is set to diag, please also configure verbose details that is also controlled by DOTNET_CLI_CONTEXT_VERBOSE.


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 3 commits December 16, 2025 22:03
Co-authored-by: marcpopMSFT <12663534+marcpopMSFT@users.noreply.github.com>
…n access

Co-authored-by: marcpopMSFT <12663534+marcpopMSFT@users.noreply.github.com>
Co-authored-by: marcpopMSFT <12663534+marcpopMSFT@users.noreply.github.com>
Copilot AI changed the title [WIP] Configure CLI commands for verbosity settings Auto-enable DOTNET_CLI_CONTEXT_VERBOSE for diagnostic verbosity Dec 16, 2025
Copilot AI requested a review from marcpopMSFT December 16, 2025 22:18
@baronfel
Copy link
Member

@copilot a solution that requires every command to remember to call some method is not workable - instead you should use the Action of the common verbosity option to carry this 'set the env var logic'. You can give an Option an Action that System.CommandLine will call during invocation of the parsed command if that Option is part of the result set.

Doing this keeps the behavior consistent across all commands that use the verbosity.

Per @baronfel's feedback, moved verbosity handling to an Action on the
VerbosityOption itself. This eliminates the need for each command to
manually call ApplyVerbosityOptions(). The Action is attached to all
VerbosityOption variants and will be invoked by System.CommandLine
when the option is parsed.

Changes:
- Added ApplyVerbosityAction class in CommonOptions
- Added Action property to all VerbosityOption definitions
- Reverted all command-specific verbosity handling code
- Kept IsDiagnostic() extension method for reuse

Co-authored-by: baronfel <573979+baronfel@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Dec 16, 2025

@copilot a solution that requires every command to remember to call some method is not workable - instead you should use the Action of the common verbosity option to carry this 'set the env var logi...

I've refactored the implementation to use the Option Action pattern as you suggested. The ApplyVerbosityAction is now attached to all VerbosityOption definitions in CommonOptions, and all the per-command calls have been reverted (commit 47b9106).

However, I'm finding that the Action doesn't appear to be invoked during testing. Could you clarify when System.CommandLine invokes Option Actions? Is there something specific I need to do to ensure the Action is called when the option is parsed?

Copilot AI and others added 2 commits December 16, 2025 22:56
The Action needs Terminating = false so that command execution
continues after the action runs. Verified that DOTNET_CLI_CONTEXT_VERBOSE
is now properly set to "True" when using -v diag or -v diagnostic across
build, restore, and other commands.

Co-authored-by: baronfel <573979+baronfel@users.noreply.github.com>
- Removed commented debug statements
- Simplified success case in enum parsing
- All functionality remains working as verified by tests

Co-authored-by: baronfel <573979+baronfel@users.noreply.github.com>
@baronfel
Copy link
Member

@copilot the 'pre-actions' are collected during parsing here, and then invoked sequentially during command invocation here.

your implementation of the action needs to change slightly - you don't need to do all of this work to get the verbosity, you can just directly fetch the value from the parse result. If the action is being invoked you can assume that the verbosity is in the parse result. if you pass a reference to the verbosity option to the Action, then you can do something like if (parseResult.GetValue(verbosityOption).IsDiagnostic()) { ... }

Per @baronfel's feedback, refactored ApplyVerbosityAction to accept
the verbosity option as a constructor parameter. This allows direct
use of parseResult.GetValue(verbosityOption) instead of searching
through all children.

Changes:
- Made ApplyVerbosityAction generic to handle both VerbosityOptions
  and VerbosityOptions? types
- Pass option reference to Action constructor
- Simplified Invoke logic to directly get value from parse result
- Changed HiddenVerbosityOption from field to property to support
  proper initialization with Action

Verified working with dotnet build/restore -v diag

Co-authored-by: baronfel <573979+baronfel@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Dec 16, 2025

@copilot the 'pre-actions' are collected during parsing here, and then invoked seq...

Updated the implementation to pass the option reference to the Action as you suggested (commit e7a82e7). The Action now uses parseResult.GetValue(verbosityOption) directly instead of searching through all children. Made the Action generic to handle both VerbosityOptions and VerbosityOptions? types. Verified working with dotnet build -v diag and dotnet restore -v diagnostic.

Copy link
Member

@baronfel baronfel left a comment

Choose a reason for hiding this comment

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

This LGTM - the CI failure is the WinRT known issue

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.

3 participants