Skip to content

The handler should always return an integer #2092

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

Merged
merged 4 commits into from
Mar 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ System.CommandLine
public System.Collections.Generic.IEnumerator<Symbol> GetEnumerator()
public ParseResult Parse(System.Collections.Generic.IReadOnlyList<System.String> args, CommandLineConfiguration configuration = null)
public ParseResult Parse(System.String commandLine, CommandLineConfiguration configuration = null)
public System.Void SetHandler(System.Action<System.CommandLine.Invocation.InvocationContext> handle)
public System.Void SetHandler(System.Func<System.CommandLine.Invocation.InvocationContext,System.Threading.CancellationToken,System.Threading.Tasks.Task> handle)
public System.Void SetHandler(System.Func<System.CommandLine.Invocation.InvocationContext,System.Int32> handler)
public System.Void SetHandler(System.Func<System.CommandLine.Invocation.InvocationContext,System.Threading.CancellationToken,System.Threading.Tasks.Task<System.Int32>> handler)
public static class CommandExtensions
public static System.Int32 Invoke(this Command command, System.String[] args, IConsole console = null)
public static System.Int32 Invoke(this Command command, System.String commandLine, IConsole console = null)
Expand All @@ -69,7 +69,7 @@ System.CommandLine
public CommandLineBuilder RegisterWithDotnetSuggest()
public CommandLineBuilder UseDefaults()
public CommandLineBuilder UseEnvironmentVariableDirective()
public CommandLineBuilder UseExceptionHandler(System.Action<System.Exception,System.CommandLine.Invocation.InvocationContext> onException = null, System.Nullable<System.Int32> errorExitCode = null)
public CommandLineBuilder UseExceptionHandler(System.Func<System.Exception,System.CommandLine.Invocation.InvocationContext,System.Int32> onException = null, System.Int32 errorExitCode = 1)
public CommandLineBuilder UseHelp(System.Nullable<System.Int32> maxWidth = null)
public CommandLineBuilder UseHelp(System.String name, System.String[] helpAliases)
public CommandLineBuilder UseHelp(System.Action<System.CommandLine.Help.HelpContext> customize, System.Nullable<System.Int32> maxWidth = null)
Expand Down Expand Up @@ -104,10 +104,10 @@ System.CommandLine
public static System.Void Write(this IConsole console, System.String value)
public static System.Void WriteLine(this IConsole console, System.String value)
public class Directive : Symbol
.ctor(System.String name, System.Action<System.CommandLine.Invocation.InvocationContext> syncHandler = null, System.Func<System.CommandLine.Invocation.InvocationContext,System.Threading.CancellationToken,System.Threading.Tasks.Task> asyncHandler = null)
.ctor(System.String name, System.Func<System.CommandLine.Invocation.InvocationContext,System.Int32> syncHandler = null, System.Func<System.CommandLine.Invocation.InvocationContext,System.Threading.CancellationToken,System.Threading.Tasks.Task<System.Int32>> asyncHandler = null)
public System.Collections.Generic.IEnumerable<System.CommandLine.Completions.CompletionItem> GetCompletions(System.CommandLine.Completions.CompletionContext context)
public System.Void SetAsynchronousHandler(System.Func<System.CommandLine.Invocation.InvocationContext,System.Threading.CancellationToken,System.Threading.Tasks.Task> handler)
public System.Void SetSynchronousHandler(System.Action<System.CommandLine.Invocation.InvocationContext> handler)
public System.Void SetAsynchronousHandler(System.Func<System.CommandLine.Invocation.InvocationContext,System.Threading.CancellationToken,System.Threading.Tasks.Task<System.Int32>> handler)
public System.Void SetSynchronousHandler(System.Func<System.CommandLine.Invocation.InvocationContext,System.Int32> handler)
public class EnvironmentVariablesDirective : Directive
.ctor()
public interface ICommandHandler
Expand Down Expand Up @@ -255,9 +255,7 @@ System.CommandLine.Invocation
.ctor(System.CommandLine.ParseResult parseResult, System.CommandLine.IConsole console = null)
public System.CommandLine.Binding.BindingContext BindingContext { get; }
public System.CommandLine.IConsole Console { get; set; }
public System.Int32 ExitCode { get; set; }
public System.CommandLine.Help.HelpBuilder HelpBuilder { get; }
public System.Action<InvocationContext> InvocationResult { get; set; }
public System.CommandLine.ParseResult ParseResult { get; set; }
public T GetValue<T>(Option<T> option)
public T GetValue<T>(Argument<T> argument)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ private static RootCommand BuildCommand()
{
bool boolean = ctx.ParseResult.GetValue(boolOption);
string text = ctx.ParseResult.GetValue(stringOption);

return 0;
});

return command;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,17 +48,17 @@ public override string InvokeContents()
{
case ReturnPattern.InvocationContextExitCode:
builder.Append(@"
return await global::System.Threading.Tasks.Task.FromResult(context.ExitCode);");
return 0;");
break;
case ReturnPattern.FunctionReturnValue:
builder.Append(@"
return await global::System.Threading.Tasks.Task.FromResult(rv);");
return rv;");
break;
case ReturnPattern.AwaitFunction:
builder.Append(@"
await rv;");
builder.Append(@"
return context.ExitCode;");
return 0;");
break;
case ReturnPattern.AwaitFunctionReturnValue:
builder.Append(@"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,17 +50,17 @@ public virtual string InvokeContents()
{
case ReturnPattern.InvocationContextExitCode:
builder.Append(@"
return await Task.FromResult(context.ExitCode);");
return 0;");
break;
case ReturnPattern.FunctionReturnValue:
builder.Append(@"
return await Task.FromResult(rv);");
return rv;");
break;
case ReturnPattern.AwaitFunction:
builder.Append(@"
await rv;");
builder.Append(@"
return context.ExitCode;");
return 0;");
break;
case ReturnPattern.AwaitFunctionReturnValue:
builder.Append(@"
Expand Down
1 change: 0 additions & 1 deletion src/System.CommandLine.Hosting/HostingExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ public static CommandLineBuilder UseHost(this CommandLineBuilder builder,
services.AddSingleton(invocation);
services.AddSingleton(invocation.BindingContext);
services.AddSingleton(invocation.Console);
services.AddTransient(_ => invocation.InvocationResult);
services.AddTransient(_ => invocation.ParseResult);
});
hostBuilder.UseInvocationLifetime(invocation);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -321,16 +321,12 @@ public async Task Unexpected_return_types_result_in_exit_code_0_if_no_exception_
private static void CaptureMethod<T>(T value, InvocationContext invocationContext)
{
BoundValueCapturer.Capture(value, invocationContext);

invocationContext.InvocationResult = ctx => BoundValueCapturer.Apply(ctx);
}

private static Action<T, InvocationContext> CaptureDelegate<T>()
=> (value, invocationContext) =>
{
BoundValueCapturer.Capture(value, invocationContext);

invocationContext.InvocationResult = ctx => BoundValueCapturer.Apply(ctx);
};

private static class BoundValueCapturer
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -611,13 +611,10 @@ internal static async Task<int> GetExitCodeAsync(object? returnValue, Invocation
{
case Task<int> exitCodeTask:
return await exitCodeTask;
case Task task:
await task;
return context.ExitCode;
case int exitCode:
return exitCode;
default:
return context.ExitCode;
return 0;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ static async Task Main(string[] args)
string cherry = ctx.ParseResult.GetValue(cherryOption);
string durian = ctx.ParseResult.GetValue(durianOption);

return Task.CompletedTask;
return Task.FromResult(0);
});

var commandLine = new CommandLineBuilder(rootCommand)
Expand Down
6 changes: 4 additions & 2 deletions src/System.CommandLine.Suggest/SuggestionDispatcher.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.CommandLine.Parsing;
using System.IO;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;

namespace System.CommandLine.Suggest
Expand All @@ -31,6 +32,7 @@ public SuggestionDispatcher(ISuggestionRegistration suggestionRegistration, ISug
CompleteScriptCommand.SetHandler(context =>
{
SuggestionShellScriptHandler.Handle(context.Console, context.ParseResult.GetValue(shellTypeArgument));
return 0;
});

ListCommand = new Command("list")
Expand All @@ -48,7 +50,7 @@ public SuggestionDispatcher(ISuggestionRegistration suggestionRegistration, ISug
ExecutableOption,
PositionOption
};
GetCommand.SetHandler(context => Get(context));
GetCommand.SetHandler(Get);

var commandPathOption = new Option<string>("--command-path") { Description = "The path to the command for which to register suggestions" };

Expand Down Expand Up @@ -132,7 +134,7 @@ private void Register(
}
}

private Task<int> Get(InvocationContext context)
private Task<int> Get(InvocationContext context, CancellationToken cancellationToken)
{
var parseResult = context.ParseResult;
var commandPath = parseResult.GetValue(ExecutableOption);
Expand Down
2 changes: 1 addition & 1 deletion src/System.CommandLine.Tests/ArgumentTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ public async Task Custom_argument_parser_is_only_called_once()
};

var command = new RootCommand();
command.SetHandler((ctx) => handlerWasCalled = true);
command.SetHandler((ctx) => { handlerWasCalled = true; return 0; });
command.Options.Add(option);

await command.InvokeAsync("--value 42");
Expand Down
24 changes: 2 additions & 22 deletions src/System.CommandLine.Tests/Binding/SetHandlerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,26 +16,6 @@ namespace System.CommandLine.Tests.Binding
{
public class SetHandlerTests
{
[Fact]
public async Task Unexpected_return_types_result_in_exit_code_0_if_no_exception_was_thrown()
{
var wasCalled = false;

var command = new Command("wat");

var handle = (InvocationContext ctx, CancellationToken cancellationToken) =>
{
wasCalled = true;
return Task.FromResult(new { NovelType = true });
};

command.SetHandler(handle);

var exitCode = await command.InvokeAsync("");
wasCalled.Should().BeTrue();
exitCode.Should().Be(0);
}

[Fact]
public async Task When_User_Requests_Cancellation_Its_Reflected_By_The_Token_Passed_To_Handler()
{
Expand All @@ -47,11 +27,11 @@ public async Task When_User_Requests_Cancellation_Its_Reflected_By_The_Token_Pas
try
{
await Task.Delay(Timeout.InfiniteTimeSpan, cancellationToken);
context.ExitCode = ExpectedExitCode * -1;
return ExpectedExitCode * -1;
}
catch (OperationCanceledException)
{
context.ExitCode = ExpectedExitCode;
return ExpectedExitCode;
}
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ public async Task Sets_environment_variable_to_value()
{
asserted = true;
Environment.GetEnvironmentVariable(variable).Should().Be(value);
return 0;
});

var config = new CommandLineBuilder(rootCommand)
Expand All @@ -45,6 +46,7 @@ public async Task Trims_environment_variable_name()
{
asserted = true;
Environment.GetEnvironmentVariable(variable).Should().Be(value);
return 0;
});

var config = new CommandLineBuilder(rootCommand)
Expand All @@ -67,6 +69,7 @@ public async Task Trims_environment_variable_value()
{
asserted = true;
Environment.GetEnvironmentVariable(variable).Should().Be(value);
return 0;
});

var config = new CommandLineBuilder(rootCommand)
Expand All @@ -89,6 +92,7 @@ public async Task Sets_environment_variable_value_containing_equals_sign()
{
asserted = true;
Environment.GetEnvironmentVariable(variable).Should().Be(value);
return 0;
});

var config = new CommandLineBuilder(rootCommand)
Expand All @@ -110,6 +114,7 @@ public async Task Ignores_environment_directive_without_equals_sign()
{
asserted = true;
Environment.GetEnvironmentVariable(variable).Should().BeNull();
return 0;
});

var config = new CommandLineBuilder(rootCommand)
Expand All @@ -132,6 +137,7 @@ public static async Task Ignores_environment_directive_with_empty_variable_name(
asserted = true;
var env = Environment.GetEnvironmentVariables();
env.Values.Cast<string>().Should().NotContain(value);
return 0;
});

var config = new CommandLineBuilder(rootCommand)
Expand All @@ -154,6 +160,7 @@ public static async Task Ignores_environment_directive_with_whitespace_variable_
asserted = true;
var env = Environment.GetEnvironmentVariables();
env.Values.Cast<string>().Should().NotContain(value);
return 0;
});

var config = new CommandLineBuilder(rootCommand)
Expand Down
4 changes: 2 additions & 2 deletions src/System.CommandLine.Tests/GlobalOptionTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public void When_a_required_global_option_is_omitted_it_results_in_an_error()
{
var command = new Command("child");
var rootCommand = new RootCommand { command };
command.SetHandler((_) => { });
command.SetHandler((_) => 0);
var requiredOption = new Option<bool>("--i-must-be-set")
{
IsRequired = true,
Expand Down Expand Up @@ -69,7 +69,7 @@ public void When_a_required_global_option_is_present_on_child_of_command_it_was_
{
var command = new Command("child");
var rootCommand = new RootCommand { command };
command.SetHandler((_) => { });
command.SetHandler((_) => 0);
var requiredOption = new Option<bool>("--i-must-be-set")
{
IsRequired = true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,12 @@ private static Task<int> Program(string[] args)
// and reason why we need a timeout on termination processing.
CancellationToken token = infiniteDelay ? CancellationToken.None : cancellationToken;
await Task.Delay(Timeout.InfiniteTimeSpan, token);

return 0;
}
catch (OperationCanceledException)
{
context.ExitCode = GracefulExitCode;
return GracefulExitCode;
}
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public async Task RootCommand_InvokeAsync_returns_0_when_handler_is_successful()
var wasCalled = false;
var rootCommand = new RootCommand();

rootCommand.SetHandler((_) => wasCalled = true);
rootCommand.SetHandler((_) => { wasCalled = true; return 0; });

var result = await rootCommand.InvokeAsync("");

Expand All @@ -66,7 +66,7 @@ public void RootCommand_Invoke_returns_0_when_handler_is_successful()
var wasCalled = false;
var rootCommand = new RootCommand();

rootCommand.SetHandler((_) => wasCalled = true);
rootCommand.SetHandler((_) => { wasCalled = true; return 0; });

int result = rootCommand.Invoke("");

Expand Down Expand Up @@ -127,8 +127,7 @@ public async Task RootCommand_InvokeAsync_can_set_custom_result_code()

rootCommand.SetHandler((context, cancellationToken) =>
{
context.ExitCode = 123;
return Task.CompletedTask;
return Task.FromResult(123);
});

var resultCode = await rootCommand.InvokeAsync("");
Expand All @@ -143,8 +142,7 @@ public void RootCommand_Invoke_can_set_custom_result_code()

rootCommand.SetHandler((context, cancellationToken) =>
{
context.ExitCode = 123;
return Task.CompletedTask;
return Task.FromResult(123);
});

int resultCode = rootCommand.Invoke("");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,10 @@ public async Task InvokeAsync_chooses_the_appropriate_command()
var secondWasCalled = false;

var first = new Command("first");
first.SetHandler((_) => firstWasCalled = true);
first.SetHandler((_) => { firstWasCalled = true; return 0; });

var second = new Command("second");
second.SetHandler((_) => secondWasCalled = true);
second.SetHandler((_) => { secondWasCalled = true; return 0; });

var config = new CommandLineBuilder(new RootCommand
{
Expand All @@ -64,10 +64,10 @@ public void Invoke_chooses_the_appropriate_command()
var secondWasCalled = false;

var first = new Command("first");
first.SetHandler((_) => firstWasCalled = true);
first.SetHandler((_) => { firstWasCalled = true; return 0; });

var second = new Command("second");
second.SetHandler((_) => secondWasCalled = true);
second.SetHandler((_) => { secondWasCalled = true; return 0; });

var config = new CommandLineBuilder(new RootCommand
{
Expand Down Expand Up @@ -350,7 +350,7 @@ public async Task Middleware_can_throw_OperationCancelledException()
{
throw new OperationCanceledException(cancellationToken);
})
.UseExceptionHandler((ex, ctx) => ctx.ExitCode = ex is OperationCanceledException ? 123 : 456)
.UseExceptionHandler((ex, ctx) => ex is OperationCanceledException ? 123 : 456)
.Build();

int result = await config.InvokeAsync("the-command");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ public void A_command_can_be_specified_in_more_than_one_position(
string expectedParent)
{
var reusedCommand = new Command("reused");
reusedCommand.SetHandler((_) => { });
reusedCommand.SetHandler((_) => 0);
reusedCommand.Add(new Option<string>("--the-option"));

var outer = new Command("outer")
Expand Down
Loading