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

don't mutate command on subsequent calls to Invoke extension #781

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 15 additions & 2 deletions src/System.CommandLine.Tests/CommandTests.cs
Expand Up @@ -336,18 +336,31 @@ public void When_multiple_options_are_configured_then_they_must_differ_by_name()
}

[Fact]
public void When_global_options_are_added_then_they_must_differ_from_local_options_by_name()
public void Global_options_may_be_added_with_aliases_that_conflict_with_local_options()
{
var command = new Command("the-command")
{
new Option("--same")
};

command
.Invoking(c => c.AddGlobalOption(new Option("--same")))
.Should()
.NotThrow<ArgumentException>();
}

[Fact]
public void Global_options_may_not_have_aliases_conflicting_with_other_global_option_aliases()
{
var command = new Command("the-command");

command.AddGlobalOption(new Option("--same"));

command
.Invoking(c => c.AddGlobalOption(new Option("--same")))
.Should()
.Throw<ArgumentException>()
.And
.Which
.Message
.Should()
.Be("Alias '--same' is already in use.");
Expand Down
10 changes: 9 additions & 1 deletion src/System.CommandLine/Builder/CommandLineBuilder.cs
Expand Up @@ -17,6 +17,10 @@ public class CommandLineBuilder : CommandBuilder
public CommandLineBuilder(Command rootCommand = null)
: base(rootCommand ?? new RootCommand())
{
if (rootCommand?.ImplicitParser != null)
{
throw new ArgumentException($"Command \"{rootCommand.Name}\" has already been configured.");
}
}

public bool EnableDirectives { get; set; } = true;
Expand All @@ -33,7 +37,7 @@ public Parser Build()
{
var rootCommand = Command;

return new Parser(
var parser = new Parser(
new CommandLineConfiguration(
new[] { rootCommand },
enablePosixBundling: EnablePosixBundling,
Expand All @@ -44,6 +48,10 @@ public Parser Build()
.Select(m => m.middleware)
.ToArray(),
helpBuilderFactory: HelpBuilderFactory));

Command.ImplicitParser = parser;

return parser;
}

internal void AddMiddleware(
Expand Down
Expand Up @@ -401,7 +401,6 @@ public static TBuilder UseHelpBuilder<TBuilder>(this TBuilder builder, Func<Bind
return builder;
}


public static CommandLineBuilder UseTypoCorrections(
this CommandLineBuilder builder, int maxLevenshteinDistance = 3)
{
Expand Down
4 changes: 3 additions & 1 deletion src/System.CommandLine/Command.cs
Expand Up @@ -3,6 +3,7 @@

using System.Collections;
using System.Collections.Generic;
using System.CommandLine.Builder;
using System.CommandLine.Collections;
using System.CommandLine.Invocation;
using System.CommandLine.Parsing;
Expand Down Expand Up @@ -32,7 +33,6 @@ public Command(string name, string description = null) : base(new[] { name }, de

public void AddGlobalOption(Option option)
{
Children.ThrowIfAnyAliasIsInUse(option);
_globalOptions.Add(option);
Children.AddWithoutAliasCollisionCheck(option);
}
Expand Down Expand Up @@ -68,5 +68,7 @@ private protected override void AddSymbol(Symbol symbol)
IEnumerable<IArgument> ICommand.Arguments => Arguments;

IEnumerable<IOption> ICommand.Options => Options;

internal Parser ImplicitParser { get; set; }
}
}
9 changes: 5 additions & 4 deletions src/System.CommandLine/CommandExtensions.cs
Expand Up @@ -42,11 +42,12 @@ public static class CommandExtensions

private static InvocationPipeline GetInvocationPipeline(Command command, string[] args)
{
var parser = new CommandLineBuilder(command)
.UseDefaults()
.Build();
var parser = command.ImplicitParser ??=
jonsequitur marked this conversation as resolved.
Show resolved Hide resolved
new CommandLineBuilder(command)
.UseDefaults()
.Build();

ParseResult parseResult = parser.Parse(args);
var parseResult = parser.Parse(args);

return new InvocationPipeline(parseResult);
}
Expand Down