Skip to content

Fix #1665 #1714

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

Merged
merged 5 commits into from
Apr 20, 2022
Merged

Fix #1665 #1714

merged 5 commits into from
Apr 20, 2022

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