Skip to content

Commit 1bc03fe

Browse files
adamsitnikadamsitnik
authored
Move most of the built in middlewares out of middleware (#2043)
* move HelpOption out of middleware * move environment variable directive out of middleware * move parse directive out of middleware * add a tests for combinding [perse] with --help and --version * move suggest directive out of middleware * move parse error reporting out of middleware * report parsing errors as soon as unmatched token is found * move typo corrections out of middleware * make TypoCorrection a static class as all it needs is inside InvocationContext * move exception handling out of middleware * make internal properties fields to avoid method JITting * reduce the number of fields * provide default exit codes in explicit way
1 parent 7030070 commit 1bc03fe

18 files changed

+312
-277
lines changed

‎src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,8 @@ System.CommandLine
8383
public static CommandLineBuilder UseHelp(this CommandLineBuilder builder, System.Action<System.CommandLine.Help.HelpContext> customize, System.Nullable<System.Int32> maxWidth = null)
8484
public static TBuilder UseHelpBuilder<TBuilder>(this TBuilder builder, System.Func<System.CommandLine.Binding.BindingContext,System.CommandLine.Help.HelpBuilder> getHelpBuilder)
8585
public static CommandLineBuilder UseLocalizationResources(this CommandLineBuilder builder, LocalizationResources validationMessages)
86-
public static CommandLineBuilder UseParseDirective(this CommandLineBuilder builder, System.Nullable<System.Int32> errorExitCode = null)
87-
public static CommandLineBuilder UseParseErrorReporting(this CommandLineBuilder builder, System.Nullable<System.Int32> errorExitCode = null)
86+
public static CommandLineBuilder UseParseDirective(this CommandLineBuilder builder, System.Int32 errorExitCode = 1)
87+
public static CommandLineBuilder UseParseErrorReporting(this CommandLineBuilder builder, System.Int32 errorExitCode = 1)
8888
public static CommandLineBuilder UseSuggestDirective(this CommandLineBuilder builder)
8989
public static CommandLineBuilder UseTokenReplacer(this CommandLineBuilder builder, System.CommandLine.Parsing.TryReplaceToken replaceToken)
9090
public static CommandLineBuilder UseTypoCorrections(this CommandLineBuilder builder, System.Int32 maxLevenshteinDistance = 3)

‎src/System.CommandLine.Hosting.Tests/HostingHandlerTest.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ public static async Task Parameter_is_available_in_property()
5555
}
5656

5757
[Fact]
58-
public static async Task Can_have_diferent_handlers_based_on_command()
58+
public static async Task Can_have_different_handlers_based_on_command()
5959
{
6060
var root = new RootCommand();
6161

‎src/System.CommandLine.Tests/ParseDirectiveTests.cs‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,56 @@ public async Task Parse_directive_writes_parse_diagram()
4646
.Be($"![ {RootCommand.ExecutableName} [ subcommand [ -c <34> ] ] ] ???--> --nonexistent wat" + Environment.NewLine);
4747
}
4848

49+
[Fact]
50+
public async Task When_parse_directive_is_used_the_help_is_not_displayed()
51+
{
52+
var rootCommand = new RootCommand();
53+
54+
var parser = new CommandLineBuilder(rootCommand)
55+
.UseParseDirective()
56+
.UseVersionOption()
57+
.UseHelp()
58+
.Build();
59+
60+
var result = parser.Parse("[parse] --help");
61+
62+
output.WriteLine(result.Diagram());
63+
64+
var console = new TestConsole();
65+
66+
await result.InvokeAsync(console);
67+
68+
console.Out
69+
.ToString()
70+
.Should()
71+
.Be($"[ {RootCommand.ExecutableName} [ --help ] ]" + Environment.NewLine);
72+
}
73+
74+
[Fact]
75+
public async Task When_parse_directive_is_used_the_version_is_not_displayed()
76+
{
77+
var rootCommand = new RootCommand();
78+
79+
var parser = new CommandLineBuilder(rootCommand)
80+
.UseParseDirective()
81+
.UseVersionOption()
82+
.UseHelp()
83+
.Build();
84+
85+
var result = parser.Parse("[parse] --version");
86+
87+
output.WriteLine(result.Diagram());
88+
89+
var console = new TestConsole();
90+
91+
await result.InvokeAsync(console);
92+
93+
console.Out
94+
.ToString()
95+
.Should()
96+
.Be($"[ {RootCommand.ExecutableName} [ --version ] ]" + Environment.NewLine);
97+
}
98+
4999
[Fact]
50100
public async Task When_there_are_no_errors_then_parse_directive_sets_exit_code_0()
51101
{

‎src/System.CommandLine/Builder/CommandLineBuilder.cs‎

Lines changed: 37 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,33 @@ namespace System.CommandLine
1414
/// </summary>
1515
public class CommandLineBuilder
1616
{
17+
/// <inheritdoc cref="CommandLineConfiguration.EnablePosixBundling"/>
18+
internal bool EnablePosixBundling = true;
19+
20+
/// <inheritdoc cref="CommandLineConfiguration.EnableTokenReplacement"/>
21+
internal bool EnableTokenReplacement = true;
22+
23+
/// <inheritdoc cref="CommandLineConfiguration.ParseErrorReportingExitCode"/>
24+
internal int? ParseErrorReportingExitCode;
25+
26+
/// <inheritdoc cref="CommandLineConfiguration.EnableDirectives"/>
27+
internal bool EnableDirectives = true;
28+
29+
/// <inheritdoc cref="CommandLineConfiguration.EnableEnvironmentVariableDirective"/>
30+
internal bool EnableEnvironmentVariableDirective;
31+
32+
/// <inheritdoc cref="CommandLineConfiguration.ParseDirectiveExitCode"/>
33+
internal int? ParseDirectiveExitCode;
34+
35+
/// <inheritdoc cref="CommandLineConfiguration.EnableSuggestDirective"/>
36+
internal bool EnableSuggestDirective;
37+
38+
/// <inheritdoc cref="CommandLineConfiguration.MaxLevenshteinDistance"/>
39+
internal int MaxLevenshteinDistance;
40+
41+
/// <inheritdoc cref="CommandLineConfiguration.ExceptionHandler"/>
42+
internal Action<Exception, InvocationContext>? ExceptionHandler;
43+
1744
// for every generic type with type argument being struct JIT needs to compile a dedicated version
1845
// (because each struct is of a different size)
1946
// that is why we don't use List<ValueTuple> for middleware
@@ -33,19 +60,6 @@ public CommandLineBuilder(Command? rootCommand = null)
3360
/// </summary>
3461
public Command Command { get; }
3562

36-
/// <summary>
37-
/// Determines whether the parser recognizes command line directives.
38-
/// </summary>
39-
/// <seealso cref="DirectiveCollection"/>
40-
internal bool EnableDirectives { get; set; } = true;
41-
42-
/// <summary>
43-
/// Determines whether the parser recognize and expands POSIX-style bundled options.
44-
/// </summary>
45-
internal bool EnablePosixBundling { get; set; } = true;
46-
47-
internal bool EnableTokenReplacement { get; set; } = true;
48-
4963
internal void CustomizeHelpLayout(Action<HelpContext> customize) =>
5064
_customizeHelpBuilder = customize;
5165

@@ -68,19 +82,19 @@ HelpBuilder CreateHelpBuilder(BindingContext bindingContext)
6882
}
6983
}
7084

71-
internal HelpOption? HelpOption { get; set; }
85+
internal HelpOption? HelpOption;
7286

73-
internal VersionOption? VersionOption { get; set; }
87+
internal VersionOption? VersionOption;
7488

75-
internal int? MaxHelpWidth { get; set; }
89+
internal int? MaxHelpWidth;
7690

7791
internal LocalizationResources LocalizationResources
7892
{
7993
get => _localizationResources ??= LocalizationResources.Instance;
8094
set => _localizationResources = value;
8195
}
8296

83-
internal TryReplaceToken? TokenReplacer { get; set; }
97+
internal TryReplaceToken? TokenReplacer;
8498

8599
/// <summary>
86100
/// Creates a parser based on the configuration of the command line builder.
@@ -92,6 +106,12 @@ public Parser Build() =>
92106
enablePosixBundling: EnablePosixBundling,
93107
enableDirectives: EnableDirectives,
94108
enableTokenReplacement: EnableTokenReplacement,
109+
enableEnvironmentVariableDirective: EnableEnvironmentVariableDirective,
110+
parseDirectiveExitCode: ParseDirectiveExitCode,
111+
enableSuggestDirective: EnableSuggestDirective,
112+
parseErrorReportingExitCode: ParseErrorReportingExitCode,
113+
maxLevenshteinDistance: MaxLevenshteinDistance,
114+
exceptionHandler: ExceptionHandler,
95115
resources: LocalizationResources,
96116
middlewarePipeline: _middlewareList is null
97117
? Array.Empty<InvocationMiddleware>()

‎src/System.CommandLine/Builder/CommandLineBuilderExtensions.cs‎

Lines changed: 15 additions & 119 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
using System.CommandLine.IO;
99
using System.CommandLine.Parsing;
1010
using System.IO;
11-
using System.Linq;
1211
using System.Threading;
1312
using System.Threading.Tasks;
1413
using static System.Environment;
@@ -230,27 +229,7 @@ await feature.EnsureRegistered(async () =>
230229
public static CommandLineBuilder UseEnvironmentVariableDirective(
231230
this CommandLineBuilder builder)
232231
{
233-
builder.AddMiddleware((context, next) =>
234-
{
235-
if (context.ParseResult.Directives.TryGetValue("env", out var keyValuePairs))
236-
{
237-
for (var i = 0; i < keyValuePairs.Count; i++)
238-
{
239-
var envDirective = keyValuePairs[i];
240-
var components = envDirective.Split(new[] { '=' }, count: 2);
241-
var variable = components.Length > 0 ? components[0].Trim() : string.Empty;
242-
if (string.IsNullOrEmpty(variable) || components.Length < 2)
243-
{
244-
continue;
245-
}
246-
247-
var value = components[1].Trim();
248-
SetEnvironmentVariable(variable, value);
249-
}
250-
}
251-
252-
return next(context);
253-
}, MiddlewareOrderInternal.EnvironmentVariableDirective);
232+
builder.EnableEnvironmentVariableDirective = true;
254233

255234
return builder;
256235
}
@@ -302,17 +281,7 @@ public static CommandLineBuilder UseExceptionHandler(
302281
Action<Exception, InvocationContext>? onException = null,
303282
int? errorExitCode = null)
304283
{
305-
builder.AddMiddleware(async (context, next) =>
306-
{
307-
try
308-
{
309-
await next(context);
310-
}
311-
catch (Exception exception)
312-
{
313-
(onException ?? Default)(exception, context);
314-
}
315-
}, MiddlewareOrderInternal.ExceptionHandler);
284+
builder.ExceptionHandler = onException ?? Default;
316285

317286
return builder;
318287

@@ -397,14 +366,6 @@ internal static CommandLineBuilder UseHelp(
397366
builder.HelpOption = helpOption;
398367
builder.Command.AddGlobalOption(helpOption);
399368
builder.MaxHelpWidth = maxWidth;
400-
401-
builder.AddMiddleware(async (context, next) =>
402-
{
403-
if (!ShowHelp(context, builder.HelpOption))
404-
{
405-
await next(context);
406-
}
407-
}, MiddlewareOrderInternal.HelpOption);
408369
}
409370
return builder;
410371
}
@@ -475,19 +436,9 @@ public static CommandLineBuilder AddMiddleware(
475436
/// <returns>The same instance of <see cref="CommandLineBuilder"/>.</returns>
476437
public static CommandLineBuilder UseParseDirective(
477438
this CommandLineBuilder builder,
478-
int? errorExitCode = null)
439+
int errorExitCode = 1)
479440
{
480-
builder.AddMiddleware(async (context, next) =>
481-
{
482-
if (context.ParseResult.Directives.ContainsKey("parse"))
483-
{
484-
context.InvocationResult = ctx => ParseDirectiveResult.Apply(ctx, errorExitCode);
485-
}
486-
else
487-
{
488-
await next(context);
489-
}
490-
}, MiddlewareOrderInternal.ParseDirective);
441+
builder.ParseDirectiveExitCode = errorExitCode;
491442

492443
return builder;
493444
}
@@ -500,19 +451,9 @@ public static CommandLineBuilder UseParseDirective(
500451
/// <returns>The same instance of <see cref="CommandLineBuilder"/>.</returns>
501452
public static CommandLineBuilder UseParseErrorReporting(
502453
this CommandLineBuilder builder,
503-
int? errorExitCode = null)
454+
int errorExitCode = 1)
504455
{
505-
builder.AddMiddleware(async (context, next) =>
506-
{
507-
if (context.ParseResult.Errors.Count > 0)
508-
{
509-
context.InvocationResult = ctx => ParseErrorResult.Apply(ctx, errorExitCode);
510-
}
511-
else
512-
{
513-
await next(context);
514-
}
515-
}, MiddlewareOrderInternal.ParseErrorReporting);
456+
builder.ParseErrorReportingExitCode = errorExitCode;
516457

517458
return builder;
518459
}
@@ -526,28 +467,7 @@ public static CommandLineBuilder UseParseErrorReporting(
526467
public static CommandLineBuilder UseSuggestDirective(
527468
this CommandLineBuilder builder)
528469
{
529-
builder.AddMiddleware(async (context, next) =>
530-
{
531-
if (context.ParseResult.Directives.TryGetValue("suggest", out var values))
532-
{
533-
int position;
534-
535-
if (values.FirstOrDefault() is { } positionString)
536-
{
537-
position = int.Parse(positionString);
538-
}
539-
else
540-
{
541-
position = context.ParseResult.CommandLineText?.Length ?? 0;
542-
}
543-
544-
context.InvocationResult = ctx => SuggestDirectiveResult.Apply(ctx, position);
545-
}
546-
else
547-
{
548-
await next(context);
549-
}
550-
}, MiddlewareOrderInternal.SuggestDirective);
470+
builder.EnableSuggestDirective = true;
551471

552472
return builder;
553473
}
@@ -562,17 +482,12 @@ public static CommandLineBuilder UseTypoCorrections(
562482
this CommandLineBuilder builder,
563483
int maxLevenshteinDistance = 3)
564484
{
565-
builder.AddMiddleware(async (context, next) =>
485+
if (maxLevenshteinDistance <= 0)
566486
{
567-
if (context.ParseResult.CommandResult.Command.TreatUnmatchedTokensAsErrors &&
568-
context.ParseResult.UnmatchedTokens.Count > 0)
569-
{
570-
var typoCorrection = new TypoCorrection(maxLevenshteinDistance);
487+
throw new ArgumentOutOfRangeException(nameof(maxLevenshteinDistance));
488+
}
571489

572-
typoCorrection.ProvideSuggestions(context.ParseResult, context.Console);
573-
}
574-
await next(context);
575-
}, MiddlewareOrderInternal.TypoCorrection);
490+
builder.MaxLevenshteinDistance = maxLevenshteinDistance;
576491

577492
return builder;
578493
}
@@ -619,10 +534,8 @@ public static CommandLineBuilder UseVersionOption(
619534
return builder;
620535
}
621536

622-
var versionOption = new VersionOption(builder);
623-
624-
builder.VersionOption = versionOption;
625-
builder.Command.Options.Add(versionOption);
537+
builder.VersionOption = new (builder);
538+
builder.Command.Options.Add(builder.VersionOption);
626539

627540
return builder;
628541
}
@@ -634,32 +547,15 @@ public static CommandLineBuilder UseVersionOption(
634547
this CommandLineBuilder builder,
635548
params string[] aliases)
636549
{
637-
var command = builder.Command;
638-
639550
if (builder.VersionOption is not null)
640551
{
641552
return builder;
642553
}
643554

644-
var versionOption = new VersionOption(aliases, builder);
645-
646-
builder.VersionOption = versionOption;
647-
command.Options.Add(versionOption);
555+
builder.VersionOption = new (aliases, builder);
556+
builder.Command.Options.Add(builder.VersionOption);
648557

649558
return builder;
650559
}
651-
652-
private static bool ShowHelp(
653-
InvocationContext context,
654-
Option helpOption)
655-
{
656-
if (context.ParseResult.FindResultFor(helpOption) is { })
657-
{
658-
context.InvocationResult = HelpResult.Apply;
659-
return true;
660-
}
661-
662-
return false;
663-
}
664560
}
665561
}

0 commit comments

Comments
 (0)