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.

Sign up for GitHub

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

Jump to bottom

Issue 2279 #2282

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Issue 2279 #2282

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e023a78
Fix issue 2279
Oct 26, 2023
c3c1333
#2279 - add test
Oct 26, 2023
7a87dc9
2279 - remove unused declarations
Oct 26, 2023
43d2c18
#2279 - fix test
Oct 29, 2023
1847bcc
2279 - fix completioncontext issue 2279
Oct 31, 2023
4d8d7e8
add some tests - 2279
Oct 31, 2023
abe31f9
2279 - fix bug where = sign could not be used in a CLi option value
Nov 1, 2023
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
125 changes: 125 additions & 0 deletions src/System.CommandLine.Tests/CompletionContextTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System.CommandLine.Completions;
using ApprovalTests.Namers;
using FluentAssertions;
using Xunit;
using System.CommandLine;
using System.Threading;

namespace System.CommandLine.Tests
{
Expand Down Expand Up @@ -33,6 +36,128 @@ public void CommandLineText_preserves_command_line_prior_to_splitting_when_compl
.Be(commandLine);
}

[Fact]
public void CommandLineText_is_parsed_when_option_other_than_last_is_in_name_equals_sign_value_format()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test name is a little hard to understand, and CommandLinetext_is_parsed isn't very specific about the expectation. I think the expectation is that the tokens don't include the delimiters? (Also, that it doesn't throw, but that typically goes without saying.)

A more sentence-style test name is encouraged, e.g. something like When_equals_sign_is_used_as_delimiter_then_it_is_not_included_in_tokens.

Some of these tests seem to differ only by the arguments being parsed, so combining them into a [Theory] might give you fewer tests to name.

{

CliRootCommand command = new CliRootCommand
{
new CliCommand("inner")
{
new CliOption<string>("--optionOne"),
new CliOption<string>("--optionTwo")
}
};

var commandLine = "inner --optionOne=argument1 --optionTwo argument2";

var parseResult = command.Parse(commandLine);
parseResult.GetCompletions();

Assert.True(parseResult.Tokens[1].Value == "--optionOne");
Assert.True(parseResult.Tokens[2].Value == "argument1");
Assert.True(parseResult.Tokens[3].Value == "--optionTwo");
Assert.True(parseResult.Tokens[4].Value == "argument2");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We generally avoid the use of Assert.IsTrue in this codebase as it provides very little useful feedback when a test fails. Also, when there are multiple assertions, only the first failure is seen in the output, without feedback about whether the other assertions pass or fail.

I'd suggest changing the assertions to the following, which addresses both concerns:

parseResult.Tokens.Should().BeEquivalentSequenceTo("--optionOne", "argument1", "--optionTwo", "argument2");

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried

parseResult.Tokens.Should().BeEquivalentSequenceTo("--optionOne", "argument1", "--optionTwo", "argument2");

and i get this error

Expected item[0] to be System.String, but found System.CommandLine.Parsing.CliToken

I also tried this

parseResult.Tokens.Should().BeEquivalentSequenceTo( new CommandLine.Parsing.CliToken("inner", CommandLine.Parsing.CliTokenType.Command, null), new CommandLine.Parsing.CliToken("--optionOne", CommandLine.Parsing.CliTokenType.Command, null), new CommandLine.Parsing.CliToken("argument1", CommandLine.Parsing.CliTokenType.Command, null), new CommandLine.Parsing.CliToken("--optionTwo", CommandLine.Parsing.CliTokenType.Command, null), new CommandLine.Parsing.CliToken("argument2", CommandLine.Parsing.CliTokenType.Command, null) );

and got the following error

Expected object to be inner, but found inner.
Expected object to be --optionOne, but found --optionOne.
Expected object to be argument1, but found argument1.
Expected object to be --optionTwo, but found --optionTwo.
Expected object to be argument2, but found argument2.

Any ideas how to make this work?


}

[Fact]
public void CommandLineText_is_parsed_when_last_option_is_in_name_equals_sign_value_format()
{

CliRootCommand command = new CliRootCommand
{
new CliCommand("inner")
{
new CliOption<string>("--optionOne"),
new CliOption<string>("--optionTwo")
}
};

var commandLine = "inner --optionOne argument1 --optionTwo=argument2";

var parseResult = command.Parse(commandLine);
parseResult.GetCompletions();

Assert.True(parseResult.Tokens[1].Value == "--optionOne");
Assert.True(parseResult.Tokens[2].Value == "argument1");
Assert.True(parseResult.Tokens[3].Value == "--optionTwo");
Assert.True(parseResult.Tokens[4].Value == "argument2");

}

[Fact]
public void CommandLineText_is_parsed_when_equal_sign_used_in_multiple_option_params()
{

CliRootCommand command = new CliRootCommand
{
new CliCommand("inner")
{
new CliOption<string>("--optionOne"),
new CliOption<string>("--optionTwo")
}
};

var commandLine = "inner --optionOne=argument1 --optionTwo=argument2";

var parseResult = command.Parse(commandLine);
parseResult.GetCompletions();

Assert.True(parseResult.Tokens[1].Value == "--optionOne");
Assert.True(parseResult.Tokens[2].Value == "argument1");
Assert.True(parseResult.Tokens[3].Value == "--optionTwo");
Assert.True(parseResult.Tokens[4].Value == "argument2");

}

[Fact]
public void CommandLineText_is_parsed_when_equal_sign_used_in_option_value()
{

CliRootCommand command = new CliRootCommand
{
new CliCommand("inner")
{
new CliOption<string>("--optionOne"),
new CliOption<string>("--optionTwo")
}
};

var commandLine = "inner --optionOne -=Yay=-";

var parseResult = command.Parse(commandLine);
parseResult.GetCompletions();

Assert.True(parseResult.Tokens[0].Value == "inner");
Assert.True(parseResult.Tokens[1].Value == "--optionOne");
Assert.True(parseResult.Tokens[2].Value == "-=Yay=-");

}

[Fact]
public void CommandLineText_is_parsed_when_equal_sign_used_in_option_value_and_as_option_value_spacer()
{

CliRootCommand command = new CliRootCommand
{
new CliCommand("inner")
{
new CliOption<string>("--optionOne")
}
};

var commandLine = "inner --optionOne=-=Yay=-";

var parseResult = command.Parse(commandLine);
parseResult.GetCompletions();

Assert.True(parseResult.Tokens[0].Value == "inner");
Assert.True(parseResult.Tokens[1].Value == "--optionOne");
Assert.True(parseResult.Tokens[2].Value == "-=Yay=-");

}

[Fact]
public void CommandLineText_is_preserved_when_adjusting_position()
{
Expand Down
5 changes: 4 additions & 1 deletion src/System.CommandLine/Completions/CompletionContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,11 @@ internal CompletionContext(ParseResult parseResult, string wordToComplete)

var textAfterCursor = rawInput.Substring(position.Value);

return textBeforeCursor.Split(' ').LastOrDefault() +
var lastOrFirstWord = textBeforeCursor.Split(' ').LastOrDefault() +
textAfterCursor.Split(' ').FirstOrDefault();

return (lastOrFirstWord.StartsWith("--")) ? lastOrFirstWord.Split(new[] { '=' }, 2).Last() : lastOrFirstWord;

}

return "";
Expand Down