Skip to content

Conversation

@jonsequitur
Copy link
Contributor

@jonsequitur jonsequitur commented Apr 20, 2022

This removes the distinction between line-delimited and space-delimited response files, fixing #1665.

It does this by generalizing the mechanism where tokens prefixed with @ can be replaced with other tokens. Response files are one example of this but now alternative mechanisms can be introduced.

var parser = new CommandLineBuilder(command)
             .UseTokenReplacer((string tokenToReplace, out IReadOnlyList<string> tokens, out string message) =>
             {
                 tokens = new[] { "123" };
                 message = null;
                 return true;
             })
             .Build();

var result = parser.Parse("@interpolate-me");

result.GetValueForArgument(argument).Should().Be(123);

@jonsequitur jonsequitur marked this pull request as ready for review April 20, 2022 15:50
var command = new RootCommand { argument };

var parser = new CommandLineBuilder(command)
.UseTokenReplacer((string tokenToReplace, out IReadOnlyList<string> tokens, out string message) =>
Copy link
Member

Choose a reason for hiding this comment

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

Should this test also assert on the value of tokenToReplace? I assume in this situation it would be "@interpolate-me" or would it be "interpolate-me"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add a test for this. The received tokenToReplace will not include the @. Open to changing that. My assumption was that this should be transparently removed, sort of like quotation marks.

Copy link
Member

Choose a reason for hiding this comment

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

That sounds good to me

@jonsequitur jonsequitur merged commit dd44dd0 into dotnet:main Apr 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants