Enable --interactive by default for sessions that are local. (#47226)

Co-authored-by: Michael Yanni <[email protected]>

Author: baronfel

src/BuiltInTools/dotnet-watch/CommandLineOptions.cs

@@ -1,7 +1,6 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-
 using System.Collections.Immutable;
 using System.CommandLine;
 using System.CommandLine.Parsing;
@@ -32,15 +31,16 @@ internal sealed class CommandLineOptions
 
     public string Command => ExplicitCommand ?? DefaultCommand;
 
+    // this option is referenced from inner logic and so needs to be reference-able
+    public static CliOption<bool> NonInteractiveOption = new CliOption<bool>("--non-interactive") { Description = Resources.Help_NonInteractive, Arity = ArgumentArity.Zero };
+
     public static CommandLineOptions? Parse(IReadOnlyList<string> args, IReporter reporter, TextWriter output, out int errorCode)
     {
         // dotnet watch specific options:
-
-        var quietOption = new CliOption<bool>("--quiet", "-q") { Description = Resources.Help_Quiet };
-        var verboseOption = new CliOption<bool>("--verbose") { Description = Resources.Help_Verbose };
-        var listOption = new CliOption<bool>("--list") { Description = Resources.Help_List };
-        var noHotReloadOption = new CliOption<bool>("--no-hot-reload") { Description = Resources.Help_NoHotReload };
-        var nonInteractiveOption = new CliOption<bool>("--non-interactive") { Description = Resources.Help_NonInteractive };
+        var quietOption = new CliOption<bool>("--quiet", "-q") { Description = Resources.Help_Quiet, Arity = ArgumentArity.Zero };
+        var verboseOption = new CliOption<bool>("--verbose") { Description = Resources.Help_Verbose, Arity = ArgumentArity.Zero };
+        var listOption = new CliOption<bool>("--list") { Description = Resources.Help_List, Arity = ArgumentArity.Zero };
+        var noHotReloadOption = new CliOption<bool>("--no-hot-reload") { Description = Resources.Help_NoHotReload, Arity = ArgumentArity.Zero };
 
         verboseOption.Validators.Add(v =>
         {
@@ -54,17 +54,16 @@ internal sealed class CommandLineOptions
         [
             quietOption,
             verboseOption,
-            noHotReloadOption,
-            nonInteractiveOption,
             listOption,
+            noHotReloadOption,
+            NonInteractiveOption
         ];
 
         // Options we need to know about that are passed through to the subcommand:
-
         var shortProjectOption = new CliOption<string>("-p") { Hidden = true, Arity = ArgumentArity.ZeroOrOne, AllowMultipleArgumentsPerToken = false };
         var longProjectOption = new CliOption<string>("--project") { Hidden = true, Arity = ArgumentArity.ZeroOrOne, AllowMultipleArgumentsPerToken = false };
         var launchProfileOption = new CliOption<string>("--launch-profile", "-lp") { Hidden = true, Arity = ArgumentArity.ZeroOrOne, AllowMultipleArgumentsPerToken = false };
-        var noLaunchProfileOption = new CliOption<bool>("--no-launch-profile") { Hidden = true };
+        var noLaunchProfileOption = new CliOption<bool>("--no-launch-profile") { Hidden = true, Arity = ArgumentArity.Zero };
 
         var rootCommand = new CliRootCommand(Resources.Help)
         {
@@ -159,7 +158,7 @@ internal sealed class CommandLineOptions
             {
                 Quiet = parseResult.GetValue(quietOption),
                 NoHotReload = parseResult.GetValue(noHotReloadOption),
-                NonInteractive = parseResult.GetValue(nonInteractiveOption),
+                NonInteractive = parseResult.GetValue(NonInteractiveOption),
                 Verbose = parseResult.GetValue(verboseOption),
             },
 
@@ -188,11 +187,18 @@ private static IReadOnlyList<string> GetCommandArguments(
             // skip watch options:
             if (!watchOptions.Contains(optionResult.Option))
             {
-                Debug.Assert(optionResult.IdentifierToken != null);
+                if (optionResult.Option.Name.Equals("--interactive", StringComparison.Ordinal) && parseResult.GetValue(NonInteractiveOption))
+                {
+                    // skip forwarding the interactive token (which may be computed by default) when users pass --non-interactive to watch itself
+                    continue;
+                }
 
+                // Some options _may_ be computed or have defaults, so not all may have an IdentifierToken.
+                // For those that do not, use the Option's Name instead.
+                var optionNameToForward = optionResult.IdentifierToken?.Value ?? optionResult.Option.Name;
                 if (optionResult.Tokens.Count == 0)
                 {
-                    arguments.Add(optionResult.IdentifierToken.Value);
+                    arguments.Add(optionNameToForward);
                 }
                 else if (optionResult.Option.Name == "--property")
                 {
@@ -201,14 +207,14 @@ private static IReadOnlyList<string> GetCommandArguments(
                         // While dotnet-build allows "/p Name=Value", dotnet-msbuild does not.
                         // Any command that forwards args to dotnet-msbuild will fail if we don't use colon.
                         // See https://github.com/dotnet/sdk/issues/44655.
-                        arguments.Add($"{optionResult.IdentifierToken.Value}:{token.Value}");
+                        arguments.Add($"{optionNameToForward}:{token.Value}");
                     }
                 }
                 else
                 {
                     foreach (var token in optionResult.Tokens)
                     {
-                        arguments.Add(optionResult.IdentifierToken.Value);
+                        arguments.Add(optionNameToForward);
                         arguments.Add(token.Value);
                     }
                 }

src/Cli/dotnet/CommonOptions.cs

@@ -116,12 +116,28 @@ public static CliArgument<string> DefaultToCurrentDirectory(this CliArgument<str
             Arity = ArgumentArity.Zero
         }.ForwardAs("-restore:false");
 
-        public static CliOption<bool> InteractiveMsBuildForwardOption =
-            new ForwardedOption<bool>("--interactive")
-            {
-                Description = CommonLocalizableStrings.CommandInteractiveOptionDescription,
-                Arity = ArgumentArity.Zero
-            }.ForwardAs("-property:NuGetInteractive=true");
+        private static bool IsCIEnvironmentOrRedirected() =>
+            new Telemetry.CIEnvironmentDetectorForTelemetry().IsCIEnvironment() || Console.IsOutputRedirected;
+
+        /// <summary>
+        /// A 'template' for interactive usage across the whole dotnet CLI. Use this as a base and then specialize it for your use cases.
+        /// Despite being a 'forwarded option' there is no default forwarding configured, so if you want forwarding you can add it on a per-command basis.
+        /// </summary>
+        /// <param name="acceptArgument">Whether the option accepts an boolean argument. If false, the option will be a flag.</param>
+        /// <remarks>
+        // If not set by a user, this will default to true if the user is not in a CI environment as detected by <see cref="Telemetry.CIEnvironmentDetectorForTelemetry.IsCIEnvironment"/>.
+        // If this is set to function as a flag, then there is no simple user-provided way to circumvent the behavior.
+        // </remarks>
+        public static ForwardedOption<bool> InteractiveOption(bool acceptArgument = false) =>
+             new("--interactive")
+             {
+                 Description = CommonLocalizableStrings.CommandInteractiveOptionDescription,
+                 Arity = acceptArgument ? ArgumentArity.ZeroOrOne : ArgumentArity.Zero,
+                 // this default is called when no tokens/options are passed on the CLI args
+                 DefaultValueFactory = (ar) => IsCIEnvironmentOrRedirected()
+             };
+
+        public static CliOption<bool> InteractiveMsBuildForwardOption = InteractiveOption(acceptArgument: true).ForwardAsSingle(b => $"-property:NuGetInteractive={(b ? "true" : "false")}");
 
         public static CliOption<bool> DisableBuildServersOption =
             new ForwardedOption<bool>("--disable-build-servers")

src/Cli/dotnet/OptionForwardingExtensions.cs

@@ -5,15 +5,19 @@
 using System.CommandLine.Parsing;
 using System.CommandLine.StaticCompletions;
 
+#nullable enable
+
 namespace Microsoft.DotNet.Cli
 {
     public static class OptionForwardingExtensions
     {
-        public static ForwardedOption<T> Forward<T>(this ForwardedOption<T> option) => option.SetForwardingFunction((T o) => new string[] { option.Name });
+        public static ForwardedOption<T> Forward<T>(this ForwardedOption<T> option) => option.SetForwardingFunction((T? o) => [option.Name]);
+
+        public static ForwardedOption<T> ForwardAs<T>(this ForwardedOption<T> option, string value) => option.SetForwardingFunction((T? o) => [value]);
 
-        public static ForwardedOption<T> ForwardAs<T>(this ForwardedOption<T> option, string value) => option.SetForwardingFunction((T o) => new string[] { value });
+        public static ForwardedOption<T> ForwardAsSingle<T>(this ForwardedOption<T> option, Func<T?, string> format) => option.SetForwardingFunction(format);
 
-        public static ForwardedOption<T> ForwardAsSingle<T>(this ForwardedOption<T> option, Func<T, string> format) => option.SetForwardingFunction(format);
+        public static ForwardedOption<bool> ForwardIfEnabled(this ForwardedOption<bool> option, string value) => option.SetForwardingFunction((bool o) => o == true ? [value] : []);
 
         /// <summary>
         /// Set up an option to be forwaded as an output path to MSBuild
@@ -24,47 +28,53 @@ public static class OptionForwardingExtensions
         /// <returns>The option</returns>
         public static ForwardedOption<string> ForwardAsOutputPath(this ForwardedOption<string> option, string outputPropertyName, bool surroundWithDoubleQuotes = false)
         {
-            return option.SetForwardingFunction((string o) =>
+            return option.SetForwardingFunction((string? o) =>
             {
+                if (o is null)
+                {
+                    return [];
+                }
                 string argVal = CommandDirectoryContext.GetFullPath(o);
                 if (surroundWithDoubleQuotes)
                 {
                     //  Not sure if this is necessary, but this is what "dotnet test" previously did and so we are
                     //  preserving the behavior here after refactoring
                     argVal = TestCommandParser.SurroundWithDoubleQuotes(argVal);
                 }
-                return new string[]
-                {
+                return [
                     $"-property:{outputPropertyName}={argVal}",
                     "-property:_CommandLineDefinedOutputPath=true"
-                };
+                ];
             });
         }
 
         public static ForwardedOption<string[]> ForwardAsProperty(this ForwardedOption<string[]> option) => option
             .SetForwardingFunction((optionVals) =>
-                optionVals
+                (optionVals ?? [])
                     .SelectMany(Utils.MSBuildPropertyParser.ParseProperties)
                     .Select(keyValue => $"{option.Name}:{keyValue.key}={keyValue.value}")
                 );
 
-        public static CliOption<T> ForwardAsMany<T>(this ForwardedOption<T> option, Func<T, IEnumerable<string>> format) => option.SetForwardingFunction(format);
+        public static CliOption<T> ForwardAsMany<T>(this ForwardedOption<T> option, Func<T?, IEnumerable<string>> format) => option.SetForwardingFunction(format);
 
         public static CliOption<IEnumerable<string>> ForwardAsManyArgumentsEachPrefixedByOption(this ForwardedOption<IEnumerable<string>> option, string alias) => option.ForwardAsMany(o => ForwardedArguments(alias, o));
 
         public static IEnumerable<string> OptionValuesToBeForwarded(this ParseResult parseResult, CliCommand command) =>
             command.Options
                 .OfType<IForwardedOption>()
-                .SelectMany(o => o.GetForwardingFunction()(parseResult)) ?? Array.Empty<string>();
+                .Select(o => o.GetForwardingFunction())
+                .SelectMany(f => f is not null ? f(parseResult) : Array.Empty<string>());
 
 
-        public static IEnumerable<string> ForwardedOptionValues<T>(this ParseResult parseResult, CliCommand command, string alias) =>
-            command.Options?
+        public static IEnumerable<string> ForwardedOptionValues<T>(this ParseResult parseResult, CliCommand command, string alias)
+        {
+            var func = command.Options?
                 .Where(o => o.Name.Equals(alias) || o.Aliases.Contains(alias))?
                 .OfType<IForwardedOption>()?
                 .FirstOrDefault()?
-                .GetForwardingFunction()(parseResult)
-            ?? Array.Empty<string>();
+                .GetForwardingFunction();
+            return func?.Invoke(parseResult) ?? [];
+        }
 
         public static CliOption<T> AllowSingleArgPerToken<T>(this CliOption<T> option)
         {
@@ -94,9 +104,9 @@ public static CliOption<T> WithHelpDescription<T>(this CliOption<T> option, CliC
             return option;
         }
 
-        private static IEnumerable<string> ForwardedArguments(string alias, IEnumerable<string> arguments)
+        private static IEnumerable<string> ForwardedArguments(string alias, IEnumerable<string>? arguments)
         {
-            foreach (string arg in arguments)
+            foreach (string arg in arguments ?? [])
             {
                 yield return alias;
                 yield return arg;
@@ -113,34 +123,48 @@ public class ForwardedOption<T> : CliOption<T>, IForwardedOption
     {
         private Func<ParseResult, IEnumerable<string>> ForwardingFunction;
 
-        public ForwardedOption(string name, params string[] aliases) : base(name, aliases) { }
+        public ForwardedOption(string name, params string[] aliases) : base(name, aliases)
+        {
+            ForwardingFunction = _ => [];
+        }
 
-        public ForwardedOption(string name, Func<ArgumentResult, T> parseArgument, string description = null)
+        public ForwardedOption(string name, Func<ArgumentResult, T> parseArgument, string? description = null)
             : base(name)
         {
             CustomParser = parseArgument;
             Description = description;
+            ForwardingFunction = _ => [];
         }
 
-        public ForwardedOption<T> SetForwardingFunction(Func<T, IEnumerable<string>> func)
+        public ForwardedOption<T> SetForwardingFunction(Func<T?, IEnumerable<string>> func)
         {
             ForwardingFunction = GetForwardingFunction(func);
             return this;
         }
 
-        public ForwardedOption<T> SetForwardingFunction(Func<T, string> format)
+        public ForwardedOption<T> SetForwardingFunction(Func<T?, string> format)
         {
-            ForwardingFunction = GetForwardingFunction((o) => new string[] { format(o) });
+            ForwardingFunction = GetForwardingFunction((o) => [format(o)]);
             return this;
         }
 
-        public ForwardedOption<T> SetForwardingFunction(Func<T, ParseResult, IEnumerable<string>> func)
+        public ForwardedOption<T> SetForwardingFunction(Func<T?, ParseResult, IEnumerable<string>> func)
         {
-            ForwardingFunction = (ParseResult parseResult) => parseResult.GetResult(this) is not null ? func(parseResult.GetValue<T>(this), parseResult) : Array.Empty<string>();
+            ForwardingFunction = (ParseResult parseResult) =>
+            {
+                if (parseResult.GetResult(this) is OptionResult argresult && argresult.GetValue<T>(this) is T validValue)
+                {
+                    return func(validValue, parseResult) ?? [];
+                }
+                else
+                {
+                    return [];
+                }
+            };
             return this;
         }
 
-        public Func<ParseResult, IEnumerable<string>> GetForwardingFunction(Func<T, IEnumerable<string>> func)
+        public Func<ParseResult, IEnumerable<string>> GetForwardingFunction(Func<T?, IEnumerable<string>> func)
         {
             return (ParseResult parseResult) => parseResult.GetResult(this) is not null ? func(parseResult.GetValue<T>(this)) : Array.Empty<string>();
         }
@@ -153,7 +177,7 @@ public Func<ParseResult, IEnumerable<string>> GetForwardingFunction()
 
     public class DynamicForwardedOption<T> : ForwardedOption<T>, IDynamicOption
     {
-        public DynamicForwardedOption(string name, Func<ArgumentResult, T> parseArgument, string description = null)
+        public DynamicForwardedOption(string name, Func<ArgumentResult, T> parseArgument, string? description = null)
             : base(name, parseArgument, description)
         {
         }

src/Cli/dotnet/commands/dotnet-msbuild/MSBuildForwardingApp.cs

@@ -74,11 +74,9 @@ private void InitializeRequiredEnvironmentVariables()
         /// <summary>
         /// Test hook returning concatenated and escaped command line arguments that would be passed to MSBuild.
         /// </summary>
-        internal string GetArgumentsToMSBuild()
-        {
-            var argumentsUnescaped = _forwardingAppWithoutLogging.GetAllArguments();
-            return ArgumentEscaper.EscapeAndConcatenateArgArrayForProcessStart(argumentsUnescaped);
-        }
+        internal string GetArgumentsToMSBuild() => ArgumentEscaper.EscapeAndConcatenateArgArrayForProcessStart(GetArgumentTokensToMSBuild());
+
+        internal string[] GetArgumentTokensToMSBuild() => _forwardingAppWithoutLogging.GetAllArguments();
 
         public virtual int Execute()
         {

src/Cli/dotnet/commands/dotnet-nuget/NuGetCommandParser.cs

@@ -64,7 +64,7 @@ private static CliCommand GetDeleteCommand()
             {
                 Arity = ArgumentArity.Zero
             });
-            deleteCommand.Options.Add(new CliOption<bool>("--interactive"));
+            deleteCommand.Options.Add(CommonOptions.InteractiveOption());
 
             deleteCommand.SetAction(NuGetCommand.Run);
 
@@ -125,7 +125,7 @@ private static CliCommand GetPushCommand()
             {
                 Arity = ArgumentArity.Zero
             });
-            pushCommand.Options.Add(new CliOption<bool>("--interactive"));
+            pushCommand.Options.Add(CommonOptions.InteractiveOption());
             pushCommand.Options.Add(new CliOption<bool>("--skip-duplicate")
             {
                 Arity = ArgumentArity.Zero

src/Cli/dotnet/commands/dotnet-package/add/PackageAddCommandParser.cs

@@ -70,11 +70,7 @@ internal static class PackageAddCommandParser
             HelpName = LocalizableStrings.CmdPackageDirectory
         }.ForwardAsSingle(o => $"--package-directory {o}");
 
-        public static readonly CliOption<bool> InteractiveOption = new ForwardedOption<bool>("--interactive")
-        {
-            Description = CommonLocalizableStrings.CommandInteractiveOptionDescription,
-            Arity = ArgumentArity.Zero
-        }.ForwardAs("--interactive");
+        public static readonly CliOption<bool> InteractiveOption = CommonOptions.InteractiveOption().ForwardIfEnabled("--interactive");
 
         public static readonly CliOption<bool> PrereleaseOption = new ForwardedOption<bool>("--prerelease")
         {

src/Cli/dotnet/commands/dotnet-package/list/PackageListCommandParser.cs

@@ -72,11 +72,7 @@ internal static class PackageListCommandParser
         }.ForwardAsManyArgumentsEachPrefixedByOption("--source")
         .AllowSingleArgPerToken();
 
-        public static readonly CliOption InteractiveOption = new ForwardedOption<bool>("--interactive")
-        {
-            Description = CommonLocalizableStrings.CommandInteractiveOptionDescription,
-            Arity = ArgumentArity.Zero
-        }.ForwardAs("--interactive");
+        public static readonly CliOption InteractiveOption = CommonOptions.InteractiveOption().ForwardIfEnabled("--interactive");
 
         public static readonly CliOption VerbosityOption = new ForwardedOption<VerbosityOptions>("--verbosity", "-v")
         {