diff --git a/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt b/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt index d42c624ad9..b989eb855b 100644 --- a/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt +++ b/src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt @@ -148,7 +148,6 @@ System.CommandLine public interface IConsole : System.CommandLine.IO.IStandardError, System.CommandLine.IO.IStandardIn, System.CommandLine.IO.IStandardOut public abstract class IdentifierSymbol : Symbol public System.Collections.Generic.IReadOnlyCollection Aliases { get; } - public System.String Name { get; set; } public System.Void AddAlias(System.String alias) public System.Boolean HasAlias(System.String alias) public class LocalizationResources @@ -196,7 +195,6 @@ System.CommandLine public System.Boolean IsRequired { get; set; } public System.Type ValueType { get; } public System.Collections.Generic.IEnumerable GetCompletions(System.CommandLine.Completions.CompletionContext context) - public System.Boolean HasAliasIgnoringPrefix(System.String alias) public ParseResult Parse(System.String commandLine) public ParseResult Parse(System.String[] args) public class Option : Option, IValueDescriptor, System.CommandLine.Binding.IValueDescriptor diff --git a/src/System.CommandLine.DragonFruit/CommandLine.cs b/src/System.CommandLine.DragonFruit/CommandLine.cs index e9c407aa87..582ace0701 100644 --- a/src/System.CommandLine.DragonFruit/CommandLine.cs +++ b/src/System.CommandLine.DragonFruit/CommandLine.cs @@ -197,7 +197,7 @@ public static CommandLineBuilder ConfigureHelpFromXmlComments( { var kebabCasedParameterName = parameterDescription.Key.ToKebabCase(); - var option = builder.Command.Options.FirstOrDefault(o => o.HasAliasIgnoringPrefix(kebabCasedParameterName)); + var option = builder.Command.Options.FirstOrDefault(o => HasAliasIgnoringPrefix(o, kebabCasedParameterName)); if (option != null) { @@ -300,5 +300,39 @@ private static string GetDefaultXmlDocsFileLocation(Assembly assembly) return string.Empty; } + + /// + /// Indicates whether a given alias exists on the option, regardless of its prefix. + /// + /// The alias, which can include a prefix. + /// if the alias exists; otherwise, . + private static bool HasAliasIgnoringPrefix(Option option, string alias) + { + ReadOnlySpan rawAlias = alias.AsSpan(GetPrefixLength(alias)); + + foreach (string existingAlias in option.Aliases) + { + if (MemoryExtensions.Equals(existingAlias.AsSpan(GetPrefixLength(existingAlias)), rawAlias, StringComparison.CurrentCulture)) + { + return true; + } + } + + return false; + + static int GetPrefixLength(string alias) + { + if (alias[0] == '-') + { + return alias.Length > 1 && alias[1] == '-' ? 2 : 1; + } + else if (alias[0] == '/') + { + return 1; + } + + return 0; + } + } } } diff --git a/src/System.CommandLine.Tests/OptionTests.cs b/src/System.CommandLine.Tests/OptionTests.cs index bcc8bd5990..56b4a7752c 100644 --- a/src/System.CommandLine.Tests/OptionTests.cs +++ b/src/System.CommandLine.Tests/OptionTests.cs @@ -64,7 +64,6 @@ public void A_prefixed_alias_can_be_added_to_an_option() option.AddAlias("-a"); - option.HasAliasIgnoringPrefix("a").Should().BeTrue(); option.HasAlias("-a").Should().BeTrue(); } @@ -84,14 +83,6 @@ public void HasAlias_accepts_prefixed_short_value() option.HasAlias("-o").Should().BeTrue(); } - [Fact] - public void HasAliasIgnorePrefix_accepts_unprefixed_short_value() - { - var option = new Option(new[] { "-o", "--option" }); - - option.HasAliasIgnoringPrefix("o").Should().BeTrue(); - } - [Fact] public void HasAlias_accepts_prefixed_long_value() { @@ -100,14 +91,6 @@ public void HasAlias_accepts_prefixed_long_value() option.HasAlias("--option").Should().BeTrue(); } - [Fact] - public void HasAliasIgnorePrefix_accepts_unprefixed_long_value() - { - var option = new Option(new[] { "-o", "--option" }); - - option.HasAliasIgnoringPrefix("option").Should().BeTrue(); - } - [Fact] public void It_is_not_necessary_to_specify_a_prefix_when_adding_an_option() { diff --git a/src/System.CommandLine/Binding/ArgumentConversionResult.cs b/src/System.CommandLine/Binding/ArgumentConversionResult.cs index 9cc56e88f2..2e0ac7b25f 100644 --- a/src/System.CommandLine/Binding/ArgumentConversionResult.cs +++ b/src/System.CommandLine/Binding/ArgumentConversionResult.cs @@ -56,7 +56,7 @@ private static string FormatErrorMessage( if (argument.FirstParent?.Symbol is IdentifierSymbol identifierSymbol && argument.FirstParent.Next is null) { - var alias = identifierSymbol.Aliases.First(); + var alias = identifierSymbol.GetLongestAlias(removePrefix: false); switch (identifierSymbol) { diff --git a/src/System.CommandLine/IdentifierSymbol.cs b/src/System.CommandLine/IdentifierSymbol.cs index 95e16e92fe..d52b09d2d9 100644 --- a/src/System.CommandLine/IdentifierSymbol.cs +++ b/src/System.CommandLine/IdentifierSymbol.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System.Collections.Generic; +using System.CommandLine.Parsing; using System.Diagnostics; namespace System.CommandLine @@ -11,8 +12,7 @@ namespace System.CommandLine /// public abstract class IdentifierSymbol : Symbol { - private protected readonly HashSet _aliases = new(StringComparer.Ordinal); - private string? _specifiedName; + private readonly HashSet _aliases = new(StringComparer.Ordinal); /// /// Initializes a new instance of the class. @@ -30,7 +30,7 @@ protected IdentifierSymbol(string? description = null) /// The description of the symbol, which is displayed in command line help. protected IdentifierSymbol(string name, string? description = null) { - Name = name; + Name = name ?? throw new ArgumentNullException(nameof(name)); Description = description; } @@ -42,19 +42,18 @@ protected IdentifierSymbol(string name, string? description = null) /// public override string Name { - get => _specifiedName ??= DefaultName; set { - if (_specifiedName is null || !string.Equals(_specifiedName, value, StringComparison.Ordinal)) + if (_name is null || !string.Equals(_name, value, StringComparison.Ordinal)) { AddAlias(value); - if (_specifiedName is { }) + if (_name != null) { - RemoveAlias(_specifiedName); + RemoveAlias(_name); } - _specifiedName = value; + _name = value; } } } @@ -82,6 +81,19 @@ public void AddAlias(string alias) /// if the alias has already been defined; otherwise . public bool HasAlias(string alias) => _aliases.Contains(alias); + internal string GetLongestAlias(bool removePrefix) + { + string max = ""; + foreach (string alias in _aliases) + { + if (alias.Length > max.Length) + { + max = alias; + } + } + return removePrefix ? max.RemovePrefix() : max; + } + [DebuggerStepThrough] private void ThrowIfAliasIsInvalid(string alias) { diff --git a/src/System.CommandLine/LocalizationResources.cs b/src/System.CommandLine/LocalizationResources.cs index 0107ce06fe..45cc33b8de 100644 --- a/src/System.CommandLine/LocalizationResources.cs +++ b/src/System.CommandLine/LocalizationResources.cs @@ -102,7 +102,7 @@ public virtual string RequiredCommandWasNotProvided() => /// Interpolates values into a localized string similar to Option '{0}' is required. /// public virtual string RequiredOptionWasNotProvided(Option option) => - GetResourceString(Properties.Resources.RequiredOptionWasNotProvided, option.Aliases.OrderByDescending(x => x.Length).First()); + GetResourceString(Properties.Resources.RequiredOptionWasNotProvided, option.GetLongestAlias(removePrefix: false)); /// /// Interpolates values into a localized string similar to Argument '{0}' not recognized. Must be one of:{1}. diff --git a/src/System.CommandLine/Option.cs b/src/System.CommandLine/Option.cs index 50b667dfe0..1a9ad0f7c8 100644 --- a/src/System.CommandLine/Option.cs +++ b/src/System.CommandLine/Option.cs @@ -15,7 +15,6 @@ namespace System.CommandLine /// public abstract class Option : IdentifierSymbol, IValueDescriptor { - private string? _name; private List>? _validators; private protected Option(string name, string? description) : base(description) @@ -25,8 +24,6 @@ private protected Option(string name, string? description) : base(description) throw new ArgumentNullException(nameof(name)); } - _name = name.RemovePrefix(); - AddAlias(name); } @@ -82,45 +79,10 @@ public ArgumentArity Arity internal bool DisallowBinding { get; init; } - /// - public override string Name - { - set - { - if (!HasAlias(value)) - { - _name = null; - RemoveAlias(DefaultName); - } - - base.Name = value; - } - } - internal List> Validators => _validators ??= new(); internal bool HasValidators => _validators is not null && _validators.Count > 0; - /// - /// Indicates whether a given alias exists on the option, regardless of its prefix. - /// - /// The alias, which can include a prefix. - /// if the alias exists; otherwise, . - public bool HasAliasIgnoringPrefix(string alias) - { - ReadOnlySpan rawAlias = alias.AsSpan(alias.GetPrefixLength()); - - foreach (string existingAlias in _aliases) - { - if (MemoryExtensions.Equals(existingAlias.AsSpan(existingAlias.GetPrefixLength()), rawAlias, StringComparison.CurrentCulture)) - { - return true; - } - } - - return false; - } - /// /// Gets a value that indicates whether multiple argument tokens are allowed for each option identifier token. /// @@ -137,7 +99,7 @@ public bool HasAliasIgnoringPrefix(string alias) public bool AllowMultipleArgumentsPerToken { get; set; } internal virtual bool IsGreedy - => Argument is not null && Argument.Arity.MinimumNumberOfValues > 0 && Argument.ValueType != typeof(bool); + => Argument.Arity.MinimumNumberOfValues > 0 && Argument.ValueType != typeof(bool); /// /// Indicates whether the option is required when its parent command is invoked. @@ -156,20 +118,7 @@ internal virtual bool IsGreedy object? IValueDescriptor.GetDefaultValue() => Argument.GetDefaultValue(); - private protected override string DefaultName => _name ??= GetLongestAlias(); - - private string GetLongestAlias() - { - string max = ""; - foreach (string alias in _aliases) - { - if (alias.Length > max.Length) - { - max = alias; - } - } - return max.RemovePrefix(); - } + private protected override string DefaultName => GetLongestAlias(true); /// public override IEnumerable GetCompletions(CompletionContext context) diff --git a/src/System.CommandLine/Parsing/StringExtensions.cs b/src/System.CommandLine/Parsing/StringExtensions.cs index 5de4fbcbe3..6fcf60328a 100644 --- a/src/System.CommandLine/Parsing/StringExtensions.cs +++ b/src/System.CommandLine/Parsing/StringExtensions.cs @@ -32,7 +32,7 @@ internal static string RemovePrefix(this string alias) : alias; } - internal static int GetPrefixLength(this string alias) + private static int GetPrefixLength(this string alias) { if (alias[0] == '-') { diff --git a/src/System.CommandLine/Parsing/SymbolResultExtensions.cs b/src/System.CommandLine/Parsing/SymbolResultExtensions.cs index a5e76d7fdf..e447f37ada 100644 --- a/src/System.CommandLine/Parsing/SymbolResultExtensions.cs +++ b/src/System.CommandLine/Parsing/SymbolResultExtensions.cs @@ -2,7 +2,6 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System.Collections.Generic; -using System.Linq; namespace System.CommandLine.Parsing { @@ -31,11 +30,7 @@ internal static Token Token(this SymbolResult symbolResult) Token CreateImplicitToken(Option option) { - var optionName = option.Name; - - var defaultAlias = option.Aliases.First(alias => alias.RemovePrefix() == optionName); - - return new Token(defaultAlias, TokenType.Option, option, Parsing.Token.ImplicitPosition); + return new Token(option.GetLongestAlias(removePrefix: false), TokenType.Option, option, Parsing.Token.ImplicitPosition); } } } diff --git a/src/System.CommandLine/Symbol.cs b/src/System.CommandLine/Symbol.cs index df408ba2f3..0538fdfe07 100644 --- a/src/System.CommandLine/Symbol.cs +++ b/src/System.CommandLine/Symbol.cs @@ -11,7 +11,7 @@ namespace System.CommandLine /// public abstract class Symbol { - private string? _name; + private protected string? _name; private ParentNode? _firstParent; private protected Symbol()