Skip to content

Conversation

@baronfel
Copy link
Member

@baronfel baronfel commented Sep 8, 2025

No description provided.

@github-actions

This comment was marked as outdated.

@KalleOlaviNiemitalo
Copy link
Contributor

Does this fix the following error with 10.0.100-rc.1.25451.107:

C:\>dotnet package search --verbosity=normal ValueTuple
System.InvalidCastException: Specified cast is not valid.
   at System.CommandLine.Binding.ArgumentConverter.ConvertObject(ArgumentResult argumentResult, Type type, Object value)
   at System.CommandLine.Binding.ArgumentConverter.ConvertIfNeeded(ArgumentConversionResult conversionResult, Type toType)
   at System.CommandLine.Parsing.OptionResult.GetValueOrDefault[T]()
   at System.CommandLine.Parsing.SymbolResult.GetValue[T](String name)
   at Microsoft.DotNet.Cli.Extensions.ParseResultExtensions.SafelyGetValueForOption[T](ParseResult parseResult, String name)
   at Microsoft.DotNet.Cli.Telemetry.TelemetryFilter.LogVerbosityForAllTopLevelCommand(ICollection`1 result, ParseResult parseResult, String topLevelCommandName, Dictionary`2 measurements)
   at Microsoft.DotNet.Cli.Telemetry.TelemetryFilter.Filter(Object objectToFilter)
   at Microsoft.DotNet.Cli.Utils.TelemetryEventEntry.SendFiltered(Object o)
   at Microsoft.DotNet.Cli.Program.ProcessArgs(String[] args, TimeSpan startupTime)
   at Microsoft.DotNet.Cli.Program.Main(String[] args)

@baronfel baronfel force-pushed the redo-verbosity-enum branch from a6d4a0c to 1742e91 Compare November 5, 2025 17:05
@baronfel
Copy link
Member Author

baronfel commented Nov 5, 2025

Does this fix the following error with 10.0.100-rc.1.25451.107:

C:\>dotnet package search --verbosity=normal ValueTuple
System.InvalidCastException: Specified cast is not valid.
   at System.CommandLine.Binding.ArgumentConverter.ConvertObject(ArgumentResult argumentResult, Type type, Object value)
   at System.CommandLine.Binding.ArgumentConverter.ConvertIfNeeded(ArgumentConversionResult conversionResult, Type toType)
   at System.CommandLine.Parsing.OptionResult.GetValueOrDefault[T]()
   at System.CommandLine.Parsing.SymbolResult.GetValue[T](String name)
   at Microsoft.DotNet.Cli.Extensions.ParseResultExtensions.SafelyGetValueForOption[T](ParseResult parseResult, String name)
   at Microsoft.DotNet.Cli.Telemetry.TelemetryFilter.LogVerbosityForAllTopLevelCommand(ICollection`1 result, ParseResult parseResult, String topLevelCommandName, Dictionary`2 measurements)
   at Microsoft.DotNet.Cli.Telemetry.TelemetryFilter.Filter(Object objectToFilter)
   at Microsoft.DotNet.Cli.Utils.TelemetryEventEntry.SendFiltered(Object o)
   at Microsoft.DotNet.Cli.Program.ProcessArgs(String[] args, TimeSpan startupTime)
   at Microsoft.DotNet.Cli.Program.Main(String[] args)

We fixed this separately by making that component more tolerant, but yes this does unify the representation of verbosity across the entire codebase.

@baronfel baronfel marked this pull request as ready for review November 5, 2025 17:46
@baronfel baronfel requested a review from a team as a code owner November 5, 2025 17:46
Copilot AI review requested due to automatic review settings November 5, 2025 17:46
Copy link
Contributor

Copilot AI left a 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 VerbosityOptions enum (with aliases like q, m, n, d, diag) with a cleaner Verbosity enum 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



complete -F _testhost testhost
complete -F _testhost testhost
Copy link

Copilot AI Nov 5, 2025

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.

Copilot uses AI. Check for mistakes.
@baronfel baronfel force-pushed the redo-verbosity-enum branch from 9215bdf to 16fa39a Compare November 8, 2025 04:21
@baronfel baronfel requested review from a team, MiYanni and tmat as code owners November 8, 2025 04:21
@baronfel baronfel changed the base branch from main to release/10.0.2xx November 8, 2025 04:21
@baronfel
Copy link
Member Author

baronfel commented Nov 8, 2025

Re-targeted this to release/10.0.2xx to get the unification benefits sooner and reduce churn between main and the 10.x series in the future.

@baronfel baronfel force-pushed the redo-verbosity-enum branch from 822fd03 to ba60b0a Compare November 10, 2025 00:55
Comment on lines +9 to +13
quiet,
minimal,
normal,
detailed,
diagnostic
Copy link
Member

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();
Copy link
Member

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants