Skip to content
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

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

Merged
38 changes: 22 additions & 16 deletions src/BuiltInTools/dotnet-watch/CommandLineOptions.cs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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 =>
{
Expand All @@ -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)
{
Expand Down Expand Up @@ -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),
},

Expand Down Expand Up @@ -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")
{
Expand All @@ -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);
}
}
Expand Down
28 changes: 22 additions & 6 deletions src/Cli/dotnet/CommonOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
81 changes: 56 additions & 25 deletions src/Cli/dotnet/OptionForwardingExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -24,47 +28,60 @@ 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();
if (func is not null)
{
return func(parseResult) ?? [];
}
else
{
return [];
}
}

public static CliOption<T> AllowSingleArgPerToken<T>(this CliOption<T> option)
{
Expand Down Expand Up @@ -94,9 +111,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;
Expand All @@ -113,34 +130,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>();
}
Expand All @@ -153,7 +184,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)
{
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down
4 changes: 2 additions & 2 deletions src/Cli/dotnet/commands/dotnet-nuget/NuGetCommandParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
{
Expand Down
Loading