-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Make the Verbosity enum normal #50684
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.
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
base: release/10.0.2xx
Are you sure you want to change the base?
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
|
Does this fix the following error with 10.0.100-rc.1.25451.107: |
a6d4a0c to
1742e91
Compare
We fixed this separately by making that component more tolerant, but yes this does unify the representation of verbosity across the entire codebase. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the verbosity option handling in the .NET CLI by replacing the VerbosityOptions enum with a new Verbosity enum and implementing custom parsing logic. The main changes include:
- Replacing
VerbosityOptionsenum (with aliases likeq,m,n,d,diag) with a cleanerVerbosityenum containing only the full names - Adding custom parsing logic to support shorthand values like "q", "m", "n", "d", "diag"
- Improving error messages to be more descriptive
- Updating all completion scripts and localization files
Reviewed Changes
Copilot reviewed 93 out of 93 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Cli/Microsoft.DotNet.Cli.Utils/Verbosity.cs | New enum and data class replacing VerbosityOptions |
| src/Cli/Microsoft.DotNet.Cli.Utils/VerbosityOptions.cs | Deleted old enum |
| src/Cli/dotnet/CommonOptions.cs | Added custom parser for verbosity with better error handling |
| src/Cli/dotnet/Extensions/CommonOptionsExtensions.cs | Simplified extension methods using new enum |
| src/Cli/dotnet/NugetPackageDownloader/NuGetPackageDownloader.cs | Refactored verbosity comparison logic |
| test/dotnet.Tests/CliSchemaTests.cs | Updated test snapshots for schema changes |
| test/dotnet.Tests/CompletionTests/snapshots/* | Updated completion script snapshots |
| src/Cli/dotnet/xlf/*.xlf | Updated localization files with new error messages |
src/Cli/dotnet/NugetPackageDownloader/NuGetPackageDownloader.cs
Outdated
Show resolved
Hide resolved
src/Cli/dotnet/Commands/Workload/Config/WorkloadConfigCommand.cs
Outdated
Show resolved
Hide resolved
|
|
||
|
|
||
| complete -F _testhost testhost | ||
| complete -F _testhost testhost |
Copilot
AI
Nov 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The final line of the bash completion script has an unnecessary change from having a preceding blank line marker to not having one. This appears to be a formatting inconsistency that doesn't affect functionality but creates noise in the diff.
9215bdf to
16fa39a
Compare
|
Re-targeted this to release/10.0.2xx to get the unification benefits sooner and reduce churn between |
822fd03 to
ba60b0a
Compare
| quiet, | ||
| minimal, | ||
| normal, | ||
| detailed, | ||
| diagnostic |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know I asked this in the past, but I forgot the answer. Is it necessary for these to be lowercase? It is just anti-C# style to have public things be entirely lowercase.
| public static readonly Option<string[]?> TargetOption = CommonOptions.MSBuildTargetOption(); | ||
|
|
||
| public static readonly Option<Utils.VerbosityOptions?> VerbosityOption = CommonOptions.VerbosityOption(); | ||
| public static readonly Option<Utils.Verbosity?> VerbosityOption = CommonOptions.VerbosityOption(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was probably a name-clash reason that it was prefixed with Utils.. Does that clash still exist with Verbosity?
No description provided.