-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
[main] Update dependencies from dotnet/command-line-api #29131
Changes from 12 commits
5a564e2
33ae728
5981597
68dda8a
fa49eae
bb036dd
76d51bb
c12b639
358676c
4c8a5f4
dd5e233
03c78e4
0be3134
397544b
8a8e7d8
7268413
91f6fab
83f227b
3f1c10c
8c12a78
fe6e02b
4e99936
46d264c
6bec365
ecea358
7deda76
0f80fca
37a7d3c
113fe8f
1af00d5
c45ae0e
c145b9e
83e6b29
e7d3f2d
5e200a7
c91fc88
213425d
5188b5d
7ba6711
a0e0cbc
82496ce
167fccf
d1f4cd3
0669ec2
32538df
70330ff
d4f3b5f
799d1d6
9143238
585e936
c03f5a5
7ddfa93
8b78b37
ed5f5c5
504c3d8
b58ff13
9da3bc4
81bb95b
cb2a35f
422c6cf
20ec900
87bfeb8
0b34d1d
1e1e775
a36ba0b
97b5d95
1616c72
4f6c29b
00c79b2
af5f0fb
69f8f21
4a1ba6f
a850d5e
c4ab1cf
2125ad6
62135f0
db8f5b9
27a8a30
85fecce
fb73afe
fa67214
69ce4c8
08e0ea4
7ae744f
e3195bc
cb165bf
954b461
4ba2339
18dff4b
2c6f23b
4daa474
b21ccca
6bc18d3
ceb0610
d1d3d4e
c5b54eb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ | |
using System.CommandLine; | ||
using System.CommandLine.Help; | ||
using System.CommandLine.Parsing; | ||
using System.Diagnostics; | ||
using System.Globalization; | ||
using System.Text; | ||
using Microsoft.TemplateEngine.Abstractions; | ||
|
@@ -150,7 +151,24 @@ internal Option GetOption(IReadOnlySet<string> aliases) | |
if (!string.IsNullOrWhiteSpace(DefaultValue) | ||
|| (Type == ParameterType.String || Type == ParameterType.Choice) && DefaultValue != null) | ||
{ | ||
option.SetDefaultValue(DefaultValue); | ||
switch (option) | ||
{ | ||
case Option<string> stringOption: | ||
stringOption.SetDefaultValue(DefaultValue); | ||
break; | ||
case Option<bool> booleanOption: | ||
booleanOption.SetDefaultValue(bool.Parse(DefaultValue)); | ||
break; | ||
case Option<long> integerOption: | ||
integerOption.SetDefaultValue(long.Parse(DefaultValue)); | ||
break; | ||
case Option<float> floatOption: | ||
floatOption.SetDefaultValue(float.Parse(DefaultValue)); | ||
break; | ||
default: | ||
Debug.Fail($"Unexpected Option type: {option.GetType()}"); | ||
break; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. imo we should move this code to |
||
} | ||
} | ||
option.Description = GetOptionDescription(); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ | |
|
||
using System.CommandLine; | ||
using System.CommandLine.Parsing; | ||
using System.Diagnostics.CodeAnalysis; | ||
using Microsoft.TemplateEngine.Abstractions; | ||
using Microsoft.TemplateEngine.Abstractions.Constraints; | ||
using Microsoft.TemplateEngine.Edge; | ||
|
@@ -25,24 +26,15 @@ internal static class Extensions | |
/// <summary> | ||
/// Checks if <paramref name="parseResult"/> contains an error for <paramref name="option"/>. | ||
/// </summary> | ||
internal static bool HasErrorFor(this ParseResult parseResult, Option option) | ||
internal static bool HasErrorFor(this ParseResult parseResult, Option option, [NotNullWhen(true)] out ParseError? error) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is also a perf optimization: previously we were iterating the errors up to three times, now it's only once and the caller does not need to search for the error again |
||
{ | ||
if (!parseResult.Errors.Any()) | ||
{ | ||
return false; | ||
} | ||
error = parseResult.Errors.FirstOrDefault(e => IsOptionResult(e.SymbolResult, option) | ||
|| IsOptionResult(e.SymbolResult?.Parent, option)); | ||
|
||
if (parseResult.Errors.Any(e => e.SymbolResult?.Symbol == option)) | ||
{ | ||
return true; | ||
} | ||
|
||
if (parseResult.Errors.Any(e => e.SymbolResult?.Parent?.Symbol == option)) | ||
{ | ||
return true; | ||
} | ||
return error is not null; | ||
|
||
return false; | ||
static bool IsOptionResult(SymbolResult? symbolResult, Option option) | ||
=> symbolResult is OptionResult optionResult && optionResult.Option == option; | ||
} | ||
|
||
/// <summary> | ||
|
@@ -51,8 +43,8 @@ internal static bool HasErrorFor(this ParseResult parseResult, Option option) | |
internal static void FromAmongCaseInsensitive(this Option<string> option, string[]? allowedValues = null, string? allowedHiddenValue = null) | ||
{ | ||
allowedValues ??= Array.Empty<string>(); | ||
option.AddValidator(optionResult => ValidateAllowedValues(optionResult, allowedValues, allowedHiddenValue)); | ||
option.AddCompletions(allowedValues); | ||
option.Validators.Add(optionResult => ValidateAllowedValues(optionResult, allowedValues, allowedHiddenValue)); | ||
option.CompletionSources.Add(allowedValues); | ||
} | ||
|
||
/// <summary> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -54,15 +54,15 @@ internal void AddNoLegacyUsageValidators(Command command, params Symbol[] except | |
{ | ||
if (!except.Contains(option)) | ||
{ | ||
command.AddValidator(symbolResult => ValidateOptionUsage(symbolResult, option)); | ||
command.Validators.Add(symbolResult => ValidateOptionUsage(symbolResult, option)); | ||
} | ||
} | ||
|
||
foreach (Argument argument in argumentsToVerify) | ||
{ | ||
if (!except.Contains(argument)) | ||
{ | ||
command.AddValidator(symbolResult => ValidateArgumentUsage(symbolResult, argument)); | ||
command.Validators.Add(symbolResult => ValidateArgumentUsage(symbolResult, argument)); | ||
} | ||
} | ||
} | ||
|
@@ -79,7 +79,12 @@ internal void ValidateArgumentsAreNotUsed(CommandResult commandResult) | |
|
||
internal void ValidateOptionUsage(CommandResult commandResult, Option option) | ||
{ | ||
OptionResult? optionResult = commandResult.Parent?.Children.FirstOrDefault(symbol => symbol.Symbol == option) as OptionResult; | ||
if (commandResult.Parent is not CommandResult parentResult) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A parent of |
||
{ | ||
return; | ||
} | ||
|
||
OptionResult? optionResult = parentResult.Children.OfType<OptionResult>().FirstOrDefault(result => result.Option == option); | ||
if (optionResult != null) | ||
{ | ||
List<string> wrongTokens = new List<string>(); | ||
|
@@ -101,10 +106,15 @@ internal void ValidateOptionUsage(CommandResult commandResult, Option option) | |
|
||
private static void ValidateArgumentUsage(CommandResult commandResult, params Argument[] arguments) | ||
{ | ||
if (commandResult.Parent is not CommandResult parentResult) | ||
{ | ||
return; | ||
} | ||
|
||
List<string> wrongTokens = new List<string>(); | ||
foreach (Argument argument in arguments) | ||
{ | ||
var newCommandArgument = commandResult.Parent?.Children.FirstOrDefault(symbol => symbol.Symbol == argument) as ArgumentResult; | ||
var newCommandArgument = parentResult.Children.OfType<ArgumentResult>().FirstOrDefault(result => result.Argument == argument); | ||
if (newCommandArgument == null) | ||
{ | ||
continue; | ||
|
@@ -126,22 +136,22 @@ private static void ValidateArgumentUsage(CommandResult commandResult, params Ar | |
|
||
private void BuildLegacySymbols(Func<ParseResult, ITemplateEngineHost> hostBuilder) | ||
{ | ||
this.AddArgument(ShortNameArgument); | ||
this.AddArgument(RemainingArguments); | ||
this.Arguments.Add(ShortNameArgument); | ||
this.Arguments.Add(RemainingArguments); | ||
|
||
//legacy options | ||
Dictionary<FilterOptionDefinition, Option> options = new Dictionary<FilterOptionDefinition, Option>(); | ||
foreach (var filterDef in LegacyFilterDefinitions) | ||
{ | ||
options[filterDef] = filterDef.OptionFactory().AsHidden(); | ||
this.AddOption(options[filterDef]); | ||
this.Options.Add(options[filterDef]); | ||
} | ||
LegacyFilters = options; | ||
|
||
this.AddOption(InteractiveOption); | ||
this.AddOption(AddSourceOption); | ||
this.AddOption(ColumnsAllOption); | ||
this.AddOption(ColumnsOption); | ||
this.Options.Add(InteractiveOption); | ||
this.Options.Add(AddSourceOption); | ||
this.Options.Add(ColumnsAllOption); | ||
this.Options.Add(ColumnsOption); | ||
|
||
this.TreatUnmatchedTokensAsErrors = true; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -83,13 +83,8 @@ public bool Equals(InvalidTemplateOptionResult? other) | |
return this.Equals(other as object); | ||
} | ||
|
||
internal static new InvalidTemplateOptionResult FromParseResult(TemplateOption option, ParseResult parseResult) | ||
internal static InvalidTemplateOptionResult FromParseError(TemplateOption option, ParseResult parseResult, ParseError error) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. perf: we don't need to search for the error if the caller can provide it |
||
{ | ||
if (!parseResult.HasErrorFor(option.Option)) | ||
{ | ||
throw new ArgumentException($"{nameof(option)} does not have an error in {nameof(parseResult)}"); | ||
} | ||
|
||
OptionResult? optionResult = parseResult.FindResultFor(option.Option); | ||
if (optionResult == null) | ||
{ | ||
|
@@ -103,23 +98,7 @@ public bool Equals(InvalidTemplateOptionResult? other) | |
optionValue = string.Join(", ", optionResult.Tokens.Select(t => t.Value)); | ||
} | ||
|
||
string? errorMessage = null; | ||
if (!string.IsNullOrWhiteSpace(optionResult.ErrorMessage)) | ||
{ | ||
errorMessage = optionResult.ErrorMessage; | ||
} | ||
else | ||
{ | ||
foreach (var result in optionResult.Children) | ||
{ | ||
if (string.IsNullOrWhiteSpace(result.ErrorMessage)) | ||
{ | ||
continue; | ||
} | ||
errorMessage = result.ErrorMessage; | ||
break; | ||
} | ||
} | ||
string errorMessage = !string.IsNullOrWhiteSpace(optionResult.ErrorMessage) ? optionResult.ErrorMessage : error.Message; | ||
|
||
return new InvalidTemplateOptionResult( | ||
option, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the non-generic overload for setting default value was removed, as it was allowing for introducing silent bugs. Example: defining an
CliOption<int>
and passingbool
as default value.I know that this switch is far from perfect, I am open to suggestions about how it can be improved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is probably ok for the template engine - it has a relatively constrained set of types that are allowable as part of the grammar for template options (choice, string, bool, int, etc), and this just becomes an explicit mapping that needs to occur.
@dotnet/templating-engine-maintainers can you take a look and verify that all of our data types are manually mapped here now?