Remove interactive special case for logger verbosity in dotnet run#47389
Remove interactive special case for logger verbosity in dotnet run#47389baronfel merged 8 commits intodotnet:mainfrom
dotnet run#47389Conversation
Part of dotnet/sdk#47602 ### Context With `NuGetInteractive=true` being passed in more scenarios as of dotnet/sdk#47226, the default verbosity for `dotnet run` is now `minimal` for user-present scenarios - that's gross. ### Changes Made Broadly what I'm trying to do here is not require passing `-v m` to loggers to get the authentication-related messages. Right now `dotnet run` does this and it's quite noisy compared to previous behavior. This changed because recently I made the SDK start passing `--interactive` when the user is at the keyboard (similar logic to Terminal Logger's own enablement), and `dotnet run` has logic to force verbosity to minimal when that happens so that the auth messages print where a user can see them. I kind of think of auth messages as messages that we should write regardless of verbosity (like errors are), so this is a step down that path for TL. This change ensures that auth messages are always written in the TL experience, as immediate messages. If this is accepted, then the SDK could remove the [special case it currently has](dotnet/sdk#47389). ### Testing Updated snapshot baselines, manual testing. ### Notes
b47469f to
301f9e9
Compare
dotnet run
There was a problem hiding this comment.
Pull Request Overview
This PR centralizes verbosity handling by moving VerbosityOptions into the Cli.Utils library, extending MSBuildArgs to carry verbosity state, and simplifying the dotnet run logger setup to use a single factory function.
- Add
Verbosityproperty and related clone methods toMSBuildArgsand updateAnalyzeMSBuildArgumentsto capture it. - Remove ad-hoc interactive verbosity logic in
RunCommand, replacing it withSetupSilentBuildArgsand a unifiedMakeTerminalLoggercall. - Update all command parsers and tests to use the new
Utils.VerbosityOptionsand propagate the verbosity option throughAnalyzeMSBuildArguments.
Reviewed Changes
Copilot reviewed 43 out of 43 changed files in this pull request and generated no comments.
Show a summary per file
| File or Group | Description |
|---|---|
| test/dotnet.Tests/CommandTests/Tool/Update/UpdateToolParserTests.cs (and similar *ParserTests.cs files) | Swapped result.GetValue<T> calls for GetRequiredValue where appropriate. |
| test/dotnet.Tests/CommandTests/MSBuild/GivenDotnetRunInvocation.cs | Added RunCommandParser.VerbosityOption to the AnalyzeMSBuildArguments call. |
| test/Microsoft.DotNet.Tools.Tests.ComponentMocks/ToolPackageDownloaderMock2.cs | Updated overrides to use fully-qualified Cli.Utils.VerbosityOptions. |
| test/Microsoft.DotNet.PackageInstall.Tests/…ToolPackageInstallerNugetCacheTests.cs | Added using Microsoft.DotNet.Cli.Utils; for new verbosity enum. |
| src/Cli/dotnet/CommonOptions.cs & CommonOptionsExtensions.cs | Removed inline VerbosityOptions enum and updated imports to Utils. |
| src/Cli/Microsoft.DotNet.Cli.Utils/VerbosityOptions.cs & MSBuildArgs.cs & MSBuildForwardingAppWithoutLogging.cs | Introduced VerbosityOptions enum, added verbosity support to MSBuildArgs, and emitted --verbosity in forwarding. |
| src/Cli/dotnet/Commands/** | Updated all parsers and commands to use Utils.VerbosityOptions and pass the verbosity option to AnalyzeMSBuildArguments. |
Comments suppressed due to low confidence (3)
src/Cli/dotnet/Commands/Run/RunCommand.cs:415
- [nitpick] The variable name
thingis unclear—consider renaming it to something more descriptive liketerminalLoggerorlogger.
var thing = TerminalLogger.CreateTerminalOrConsoleLogger([$"--verbosity:{msbuildVerbosity}", ..buildArgs.OtherMSBuildArgs]);
src/Cli/dotnet/Commands/Run/RunCommand.cs:316
- [nitpick] Consider adding unit tests to verify that
dotnet runcorrectly includes the--verbosityflag inMSBuildArgsfor both default and explicit verbosity levels, ensuringSetupSilentBuildArgsandMakeTerminalLoggerbehave as intended.
private MSBuildArgs SetupSilentBuildArgs(MSBuildArgs msbuildArgs)
test/Microsoft.DotNet.Tools.Tests.ComponentMocks/ToolPackageDownloaderMock2.cs:59
- The namespace qualifier
Cli.Utils.VerbosityOptionsmay not resolve correctly; ensure you either addusing Microsoft.DotNet.Cli.Utils;or fully qualify it asMicrosoft.DotNet.Cli.Utils.VerbosityOptions.
protected override void CreateAssetFile(PackageId packageId, NuGetVersion version, DirectoryPath packagesRootPath, string assetFilePath, string runtimeJsonGraph, Cli.Utils.VerbosityOptions verbosity, string? targetFramework = null)
| quiet, | ||
| q, | ||
| minimal, | ||
| m, | ||
| normal, | ||
| n, | ||
| detailed, | ||
| d, | ||
| diagnostic, | ||
| diag |
There was a problem hiding this comment.
Consider if we could make the equivalent enum items correspond to a single value like:
| quiet, | |
| q, | |
| minimal, | |
| m, | |
| normal, | |
| n, | |
| detailed, | |
| d, | |
| diagnostic, | |
| diag | |
| quiet, | |
| q = quiet, | |
| minimal, | |
| m = minimal, | |
| normal, | |
| n = normal, | |
| detailed, | |
| d = detailed, | |
| diagnostic, | |
| diag = diagnostic, |
Which would make tests like verbosity == VerbosityOptions.quiet simpler (no need to check also for VerbosityOptions.q.
There was a problem hiding this comment.
I do agree with you, but I actually want to rewrite this enum entirely (I have a WIP branch) and handle that as a separate change.
| // if the restoreArgs contain a `-bl` then let's probe it | ||
| List<ILogger> loggersForBuild = [ | ||
| MakeTerminalLogger(verbosity) | ||
| TerminalLogger.CreateTerminalOrConsoleLogger([$"--verbosity:{LoggerVerbosity.Quiet.ToString().ToLowerInvariant()}", ..buildArgs.OtherMSBuildArgs]) |
There was a problem hiding this comment.
It feels like there could be a shared utility function for this common pattern - creating TerminalLogger from LoggerVerbosity and OtherMSBuildArgs
There was a problem hiding this comment.
I think you're right - the Testing team especially may like that. @Evangelink?
There was a problem hiding this comment.
I actually meant only in this repo, to share the code since I saw it repeating several times.
There was a problem hiding this comment.
I know - the test team has a lot of similar stuff to run in the test command in this repo :)
611318e to
820b90f
Compare
… it in run command if necessary
…er to the last-provided one
3199459 to
9be9a4f
Compare
This lights up Terminal Logger support in
dotnet runagain, using the new factory function.This also changes the verbosity override in anticipation of MSBuild identifying and always printing auth messages.
My initial version of this applied the verbosity by splatting it as an argument along with the other loose MSBuildArgs. This was hard to track. So I've also added Verbosity tracking to MSBuildArgs so we can set it with less string-munging. Here's the before-and-after of that change:

The first
dotnet runI wasn't able to setquieteasily, sominimalkept getting applied, overwriting myquietsetting.The second
dotnet runis after I was able to track the Verbosity setting for MSBuild, and ensure we only sent one. TheBuilding...is because this project did opt into building messages in its launchsettings.json.