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

Add arguments as explicit key-value pairs with limited escaping #263

Closed
3 of 4 tasks
agc93 opened this issue Oct 28, 2024 · 5 comments · Fixed by #265
Closed
3 of 4 tasks

Add arguments as explicit key-value pairs with limited escaping #263

agc93 opened this issue Oct 28, 2024 · 5 comments · Fixed by #265

Comments

@agc93
Copy link

agc93 commented Oct 28, 2024

Details

This may be a bit too niche to include, but I'll ask anyway.

Context

Some applications have unintuitive command-line behaviour, where executable --option param argument and executable --option=param argument have different behaviours. For applications like this, using CliWrap can be a bit trickier since it (as far as I can tell) requires adding options of the format --option=param as a single argument and handling escaping yourself.

Request

I'd like an addition to the arguments API to add a key-value pair where CliWrap still does its normal intelligent escaping behaviour for the "value" of the pair but does not escape the whole argument.

Scenario

I want to end up with a command like the following: google-chrome --param-name="Parameter Value" "Argument Value" while retaining as much of CliWrap's (incredibly useful!) automatic escaping behaviour as possible.

Current behaviour:

Adding Name and Value separately

Adding the parameter name and value as separate values:

var command = Cli.Wrap("google-chrome")
        .WithArguments(args =>
        {
            args.Add(["--param-name", "Parameter Value"]).Add("Argument Value");
        });

results in the incorrect result of google-chrome --param-name "Parameter Value" "Argument Value" (no = between the parameter name and value will upset Chrome).

Adding the name and value as a single argument
var command = Cli.Wrap("google-chrome")
        .WithArguments(args =>
        {
            args.Add("--param-name=\"Parameter Value\"").Add("Argument Value");
        });

results in the incorrect result of google-chrome "--param-name=\"Parameter Value\"" "Argument Value", since CliWrap also escapes the parameter name.

Adding the name and value as a single argument, with escaping disabled
var command = Cli.Wrap("google-chrome")
        .WithArguments(args =>
        {
            args.Add("--param-name=\"Parameter Value\"", false).Add("Argument Value");
        });

results in the correct command, but I now have to handle correctly quoting Parameter Value myself rather than CliWrap handling it.

Example of behaviour

Invocation Result Notes
Desired: google-chrome --profile-directory="Display Name" "https://domain.tld?key=\"value\""
args.Add(["--profile-directory", "Display Name"]).Add("https://domain.tld?key=\"value\"") google-chrome --profile-directory "Parameter Value" "https://domain.tld?key=\"value\"" Missing =
args.Add("--profile-directory=\"Display Name\"").Add("https://domain.tld?key=\"value\"") google-chrome "--profile-directory=\"Display Name\"" "https://domain.tld?key=\"value\"" Option name (--profile-directory) also quoted, option value double quoted
args.Add("--profile-directory=\"Display Name\"", false).Add("https://domain.tld?key=\"value\"") google-chrome --profile-directory="Display Name" "https://domain.tld?key=\"value\"" Bypasses CliWrap escaping, must be DIY'ed

Ideal Behaviour

What would be nice is an API (maybe a new overload) to add values to the argument list as specifically key-value pairs where CliWrap will still intelligently escape the value.

For example, something like:

// it might make more sense for the default separator to be '=', but this is closer to existing behaviour
public ArgumentsBuilder Add(string key, string value, char separator = ' ') { 
        if (_buffer.Length > 0)
            _buffer.Append(' ');

        _buffer.Append($"{key}{separator}{Escape(value)}");

        return this;
}

would allow me to simply use:

var command = Cli.Wrap("google-chrome")
        .WithArguments(args =>
        {
            args.Add("--param-name", "Parameter Value", '=').Add("Argument Value");
        });

to get the desired behaviour without having to re-implement CliWrap's Escape functionality.

Alternatives

Alternatively, to let consumers recreate behaviour like this on their own, could it be worth opening up CliWrap's ArgumentsBuilder.Escape method to be public so that consumers can add arguments using Escape() without having to copy-paste it (my current solution)

Checklist

  • I have looked through existing issues to make sure that this feature has not been requested before
  • I have provided a descriptive title for this issue
  • I am aware that even valid feature requests may be rejected if they do not align with the project's goals
  • I have sponsored this project
@Tyrrrz
Copy link
Owner

Tyrrrz commented Nov 3, 2024

Hi.

I had been thinking before about adding a high-level API for setting options, but I ultimately never committed to it because there's too many different styles and approaches for receiving command-line arguments. Even if the parsing between two programs looks similar, there are still often nuanced differences (e.g. dotnet and git both use dash/space syntax, but they handle positional arguments differently, among other things).

I think in this scenario, I prefer the approach you described as the alternative (exposing Escape(...)), as it would allow you to solve the problem without making the library impose too many opinions. With that, you should then be able to make an extension method on the ArgumentsBuilder to provide a more tailored but reusable functionality specific to your own command-line syntax (essentially what you describe in the ideal solution).

@Tyrrrz
Copy link
Owner

Tyrrrz commented Nov 7, 2024

@agc93

It looks like you also didn't include this approach in the list, I'm curious if this works:

var command = Cli.Wrap("google-chrome")
        .WithArguments(args =>
        {
            args.Add("--param-name=Parameter Value").Add("Argument Value");
        });

Should result in:

google-chrome "--param-name=Parameter Value" "Argument Value"

...which, might work.

@agc93
Copy link
Author

agc93 commented Nov 8, 2024

@agc93

It looks like you also didn't include this approach in the list, I'm curious if this works:
...
...which, might work.

That's a good point, actually, I don't think I've tried that approach. I'll try that and see if the tools I'm using handle the "quoted name and value" scenario.

hawkeye116477 pushed a commit to hawkeye116477/CliWrap that referenced this issue Nov 22, 2024
@Tyrrrz
Copy link
Owner

Tyrrrz commented Dec 2, 2024

@agc93 have you had the chance to try it?

@agc93
Copy link
Author

agc93 commented Dec 3, 2024

I have, and it did work with Chrome, but did not work with one of the other tools I was trying to launch (which gets launched through flatpak).

Personally, I think exposing ArgumentsBuilder.Escape is probably the best middle ground, especially with the xmldoc warning that's added as part of #265 since I believe the quote-the-whole-pair method you suggested would cover 99% of scenarios, but that 1% can be handled using ArgumentsBuilder.Escape manually to keep the escaping behaviour consistent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants