CommandLine

[matthid]

Did you notice https://github.com/fsharp/FAKE/blob/release/next/src/app/Fake.Core.Process/CmdLineParsing.fs?

are there any issues compared to https://github.com/vbfox/FoxSharp/blob/master/src/BlackFox.CommandLine/MsvcrCommandLine.fs

At least I tried to come up with a fully conforming implementation according to Microsoft documentation, but hey you never know they have really strange rules :)

[vbfox]

No I didn't know about it :)

The code I uploaded here is actually very old, started as some personnal project in F#, then cleaned up to be used in some modules at work including some FAKE ones (I was always annoyed that most FAKE modules uses incorrect escaping)

It even has a C# port that I made for the paket bootstrapper (That's were it gained the reverse capacity of parsing command line because I never used it elsewhere)

One thing our work codebase has that I don't have here is the possibility to use "" instead of \" to escape quotes, it's only supported since 2008 msvcrt but some apps like microsoft SQL tools actually don't like the backspace variant because they use non-msvcrt parsing code (One day I might clean our helpers up & contribute them but that would need legal approval)

The one missing thing in this code is to reverse mono & .Net core algorithms on linux/mac. On these platforms there is no string -> string[] parsing done by the target program and an array is transmited directly instead, but as .Net doesn't have an API that can express that in Process there result is that mscorlib does the encoding... And I would need to check that they are msvcrt compatible and if not fix the encoding.

(Microsoft has planned add APIs to .Net core to transmit arrays directly, but there was a lot of discussions about what to do on windows, do they implement reverse MSVCRT like here and if yes what version or just throw an exception)

[vbfox]

I tried to run my test suite on your code and here are the differences :

• My code handle removing quotes when unnecesssary that mostly a stylistic choice :)
• In windowsCommandLineToArgv the double-double quotes weirdness isn't implemented either as pre-2008 or post-2008 but with a 3rd variant (http://www.daviddeley.com/autohotkey/parameters/parameters.htm#WINCRULESDDQ 5.7)
• In windowsArgvToCommandLine escape handling seems wrong theses tests fail :

let escapeTests = [
    test "Odd_backslash_escaped" {
        verifyEscape @"""a\\\\b c"" d e" [@"a\\b c"; "d"; "e"]
    }

    test "Even_backslash_escaped" {
        verifyEscape @"""a\\\\\b c"" d e" [@"a\\\b c"; "d"; "e"]
    }
]

I just added a property-based test that at least prove that the lib is reversable one way : 7b6ea76#diff-0d92d8a37f6bc8f0633870d6d9c57b20R159 (The other way is more annoying to test as the information that an argument was quoted or not is lost in the transformation)

[matthid]

Funny IIRC my implementation is basically a strict port of https://github.com/dotnet/corefx/blob/ed4e3eafae1f454011732cadf734060c8e262c59/src/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs#L548-L640 which is exactly what .net core is doing (which should be relevant here)

Also

> Args.toWindowsCommandLine [@"a\\\b c"; "d"; "e"] |> printfn "CLI: %s";;
CLI: "a\\\b c" "d" "e"
val it : unit = ()

> Args.toWindowsCommandLine [@"a\\b c"; "d"; "e"] |> printfn "CLI: %s";;
CLI: "a\\b c" "d" "e"
val it : unit = ()

Seems to conform with https://docs.microsoft.com/de-de/cpp/cpp/parsing-cpp-command-line-arguments
with rule:

Backslashes are interpreted literally, unless they immediately precede a double quotation mark.

[matthid]

Hm I tried to reproduce the double quoting stuff:

C:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise>echo printfn^ ^"^%A^" ^(System.Environment.GetCommandLine
Args^(^)^) | fsi -- test ^"^"

Microsoft (R) F# Interactive version 10.2.3 for F# 4.5
Copyright (c) Microsoft Corporation. All Rights Reserved.

For help type #help;;

> [|"fsi"; "--"; "test"; ""|]
val it : unit = ()

So maybe they have changed that again in the mean time and this only hits "old" programs?

At least for FSI/.NET double quotes behave according to documentation (ie open and close an empty argument)

[vbfox]

Funny IIRC my implementation is basically a strict port of dotnet/corefx:src/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs@ed4e3ea#L548-L640 which is exactly what .net core is doing (which should be relevant here)

Ah yes thanks 👍 my lib is wrong as I applied all tests reversely now and theses fail. It's supposed to only do that before double quotes. I'll fix that code but need the C# version of the fix too for the paket bootstrapper

Hm I tried to reproduce the double quoting stuff:

The case is this one (There are more complex ones but they can't easily be expressed on the command line as cmd parser interferes, need to call CreateProcess/Process.Start directly):

g:\Code\_ext\FAKE>echo printfn^ ^"^%A^" ^(System.Environment.GetCommandLineArgs^(^)^) | fsi -- test ^"^"^"Call Me Ishmael^"^"^"

Microsoft (R) F# Interactive version 10.1.0 for F# 4.1
Copyright (c) Microsoft Corporation. All Rights Reserved.

For help type #help;;

> [|"fsi"; "--"; "test"; ""Call Me Ishmael""|]
val it : unit = ()

and:

let s = CmdLineParsing.windowsCommandLineToArgv @"""""""CallMeIshmael"""""""
printfn "%A" s

// [|"CallMeIshmael"|]

The result of parsing """Call Me Ishmael"""can be either one argument containing quotes for post 2008 ("Call Me Ishmael") or 3 with the first and last having quotes for the old crt ("Call, Me, Ishmael")

[matthid]

Ok I guess I'll have to think about that and test around. However, I also think this shouldn't have any impact for the use case of fake. Realistically we only use the array->string conversation and the string->array conversation only to revert the first. Both shouldn't be hit by either one of the special cases. Therefore we might consider leaving as is which conforms to official documentation.
In fact we might even error out if we encounter that specific scenario...

[vbfox]

Yes, the more logical escaping for quotes is \" it's a lot more portable and easy to use than double quotting so it's used a lot more. Also inner-quotes, the capability to use either "foo bar" or fo"o b"ar are essentially never used in the wild.

So whenever they are implemented or not isn't that important.

[vbfox]

@matthid BTW why are most FAKE module still using the StringBuilder module instead of something more specialized to build command lines if you have it in the codebase ?

[vbfox]

0.1.1 published with the fix for the \ bug

[matthid]

Well I tried to point people to it while migration but for at first the runtime and getting things to bootstrap was my priority later I wanted to get the damn thing released instead of reworking all modules again

This might be something for fake 6, for now the runtime and getting things modularized and manageable was more important in the end ;)

It boils down to too few time and people..

[matthid]

Anyway thanks for the insights I'd assume they will come in handy at one time ;)

[matthid]

In fact the property based test found an issue with empty arguments in the fake implementation:

the argument list [ "test", "" ] would yield the command line test "" but parsing that back would result in [ "test" ]. Thanks for making me do some tests ;)
I just test for """ now and throw an exception with an explanation to use a different syntax.

[vbfox]

👍

On the fact that FAKE don't use it I was mainly asking because I wondered when porting squirrel if I should propose the use of my lib but the API is young and might need work, but if I knew there was an equivalent in FAKE itself I would have done the porting.

(The first real usage of this lib btw was for an internal fork of the dotcover FAKE module as it embed other tools cmd lines in it's own escaping can become a problem pretty fast, never contributed it back because it was quite a change and our fork is also messed up by various investigation into the nunit error while tearing down appdomain shitshow)

[vbfox]

Funny on the empty arguments, it was broken in my first impl in the other direction and someone fixed it in paket : fsprojects/Paket#2551