Too eager quote escaping for Tool arguments

[microdee]

Usage Information

NUKE 8.0.0 (Windows,.NETCoreApp,Version=v8.0)

Description

In recent versions of Nuke the Tool delegates escapes " characters in arguments with \", so a C# string literal "myThing=\"stuff\"" actually produces myThing=\"stuff\" instead of what I expect myThing="stuff" which broke some of my scripts using >= Nuke 7. Is there a way to turn this off?

Reproduction Steps

One of the code snippet producing the wrong behavior for me, but it can be seen in every Tool usage:

Tool cmake = ToolResolver.GetPathTool("cmake");
CMake("-G \"Visual Studio 16 2019\"");

Same behavior is observed when using @ literals

CMake(@"-G ""Visual Studio 16 2019""");

Expected Behavior

CMake should be executed with arguments -G "Visual Studio 16 2019"

If I do that manually it works fine.

Actual Behavior

CMake is actually executed with arguments -G \"Visual Studio 16 2019\" (note the \ before quotes)

This breaks argument parsing and obviously results in an error from the Tool

Regression?

This worked in Nuke 7 and below

Known Workarounds

If I wrap my entire arguments into a string variable then use MyTool($"{myArgs:nq}") the quotes are not get prepended by \, but good luck explaining this to your colleagues at a PR review. Also it adds arbitrary boilerplate to get back the consistent behavior, to an API which was meticulously designed to take away as much boilerplate as possible.

Could you help with a pull-request?

Probably, I don't want to spend time on this tbh.

[microdee]

seems to be a duplicate of https://github.com/nuke-build/nuke/issues/1260 not really, only one person mentioned the same issue I have actually.

[microdee]

ok I also discovered this is actually several problems

So the example I provided would probably work fine because apparently spaces in interpolated string arguments are handled automagically in ArgumentStringHandler, I would prefer if it wouldn't do automagical things I don't expect C# to do either, but that's just my opinion.

(since I replaced that example with the CMake one from below to better illustrate the problem)

BUT: when I provide a bog standard string literal, like for example I want to tell CMake which generator it should use CMake("-G \"Visual Studio 16 2019\" ...") it will erroneously escape the quotes with a backslash during process invocation as well so it'll do -G \"Visual Studio 16 2019\" instead of -G "Visual Studio 16 2019". I couldn't find where this might happen in Nuke source.

BUT x 2: So a single interpolated string with the custom ArgumentStringHandler and with my "quoted partitions" of my arguments sitting in a variable "works", in the style I was declaring my arguments it didn't anymore. Let's say I need to pass 10 options for CMake (-DMY_OPTION=stuff), each can have some long name and to avoid being an unreadable mess I might do the following:

CMake(
    // here not \"quoting\" `generator` is the expected input for the current version of ArgumentStringHandler
    $"-G {generator} -A {architecture}"
    + $" -DCMAKE_BUILD_TYPE:STRING={config}"
    + $" -DCMAKE_INSTALL_PREFIX:PATH={cmakeOutput}"
    +  " -DMY_RUNTIME_LINKAGE:STRING=/MD"
    + $" -DMY_LIBRARY_MODE:STRING={LibraryMode}"
    // imagine plenty more of this
);

This would be disgusting to look at on a single line. Because I use a composition of multiple string literals here (mixed regular and interpolated) I assume the interpolation happens before the ArgumentStringHandler is invoked, therefore it can no longer double-quote necessary interpolation-arguments (like the {generator}) and it will erroneously append \ to " like on the regular string literal example, making it impossible to use quotes correctly in this case.

When I put this on a single line ArgumentStringHandler could do its magic, however imo the lack of readibility you get from the single line argument declaration completely overweight the miniscule inconvenience of putting the \" into your string literals yourself.

(my problem has nothing to do with CMake actually, I'm just using CMake as a potential example where this \" behavior makes it impossible to work with it)

I assume the same unwanted escaping also happens when arguments are composed in a more dynamic manner and passed in from a regular string variable. In such case handling quotes might already happen and doing any other transformation to that string breaks its function.

[IT-VBFK]

Another workaround I discovered is:

Tool cmake = ToolResolver.GetPathTool("cmake");
CMake($"-G {"Visual Studio 16 2019".DoubleQuote()}");

But this also seems to be a hack...

[matkoch]

I'm aware that ArgumentStringHandler can cause some friction, but the decision to keep it was deliberate.

You touched the point of boilerplate. For that reason alone, it should be preferred to use a typed-fluent API. They can be local to a build project, internal library, or even contributed to the project.

As for your examples, the following should work as expected:

1. CMake($"-G {"Visual Studio 16 2019"}") ... @IT-VBFK there's no DoubleQuote call needed.

2. In the following you had one regular string, which avoids using the string handler. Using all interpolated strings will work as expected. Tools like ReSharper and Rider correctly handle this when entering in one of the strings:

   CMake(
     $"-G {generator} -A {architecture}"
     + $" -DCMAKE_BUILD_TYPE:STRING={config}"
     + $" -DCMAKE_INSTALL_PREFIX:PATH={cmakeOutput}"
     + $" -DMY_RUNTIME_LINKAGE:STRING=/MD"
     + $" -DMY_LIBRARY_MODE:STRING={LibraryMode}"
   );

If you don't agree with my take on that, you can introduce your own Tool type pretty easily. Take a look at ToolExecutor. All involved types should be publicly available. I'd be open for a pull-request that allows easy switching between the current Tool delegate, and another type using regular strings.

[ITaluone]

I discovered this also: In my case I run some git action (which there is only Git(), and nothing else). In the most cases there is no problem, but e.g. git commit -m [msg] where msg is not just one word, this behavior happens.