Arcade: Darc Authenticate does not correctly handle spaces in editor path

[meganaquinn]

If there are spaces in the editor path (e.g. 'C:/Users/mquinn/AppData/Local/Programs/Microsoft VS Code/Code.exe' which is the default path for VSCode) the string is parsed into "File" and "Arguments" at the space break and the user gets a File Not Found exception when running 'darc authenticate'.

The logic for this lives in Darc.UxManager.GetParsedCommand. It's explicitly using a space to find the break between filename & arguments in the else statement.

Workaround for VS Code - Change the git default editor to 'Code --wait' in the git config.

The generic exception for the PopUp method is what shows up when this happens after calling 'darc authenticate' from the command line, with this call stack:

fail: Microsoft.DotNet.Darc.Operations.Operation[0]
There was an exception while trying to pop up an editor window.
System.ComponentModel.Win32Exception (2): The system cannot find the file specified
at System.Diagnostics.Process.StartWithShellExecuteEx(ProcessStartInfo startInfo)
at System.Diagnostics.Process.Start()
at Microsoft.DotNet.Darc.Helpers.UxManager.PopUp(EditorPopUp popUp) in /_/src/Microsoft.DotNet.Darc/src/Darc/Helpers/UxManager.cs:line 139

[riarenas]

Looks like an ideal low priority FR issue for someone onboarding ot the team. Will add it to the epic.

@MattGal feel free to punt it out if you have enough of such issues already.

[lukas-lansky]

Nice task. I wrote following tests:

        [Fact]
        public void PathCodeLocation()
        {
            var codeCommand = UxManager.GetParsedCommand("code --wait");

            Assert.Equal("code", codeCommand.FileName);
            Assert.Equal(" --wait", codeCommand.Arguments);
        }

        [Fact]
        public void EscapedCodeLocation()
        {
            var codeCommand = UxManager.GetParsedCommand(@"'C:\Users\lulansky\AppData\Local\Programs\Microsoft VS Code\Code.exe' --wait");

            Assert.Equal(@"C:\Users\lulansky\AppData\Local\Programs\Microsoft VS Code\Code.exe", codeCommand.FileName);
            Assert.Equal(" --wait", codeCommand.Arguments);
        }

... and they passed. The second one corresponds to doing git config --global core.editor "'C:\Users\lulansky\AppData\Local\Programs\Microsoft VS Code\Code.exe' --wait". That works for git too:

C:\Source\arcade-services [master ≡ +1 ~0 -0 ~]> git config --global core.editor "'C:\Users\lulansky\AppData\Local\Programs\Microsoft VS Code\Code.exe' --wait"
C:\Source\arcade-services [master ≡ +1 ~0 -0 ~]> git commit
hint: Waiting for your editor to close the file...
 [main 2019-10-23T11:18:16.242Z] update#setState idle
Aborting commit due to empty commit message.

The problem happens when one do git config --global core.editor "C:\Users\lulansky\AppData\Local\Programs\Microsoft VS Code\Code.exe --wait" (not escaping the path), but if I do that, git starts to complain:

C:\Source\arcade-services [master ≡ +1 ~0 -0 ~]> git config --global core.editor "C:\Users\lulansky\AppData\Local\Programs\Microsoft VS Code\Code.exe --wait"
C:\Source\arcade-services [master ≡ +1 ~0 -0 ~]> git commit
hint: Waiting for your editor to close the file... C:\Users\lulansky\AppData\Local\Programs\Microsoft VS Code\Code.exe --wait: C:UserslulanskyAppDataLocalProgramsMicrosoft: command not found

I wonder, what is a value of core.editor for which git works and darc not? @meganaquinn

[meganaquinn]

I used your test case and found a repro - if the string doesn't have the "--wait" (which at least on my machine the the default installation string does not) then it splits at the space. If it does have the "--wait" then it splits correctly.

[riarenas]

We don't consider darc changes to be rolled out until we make a production deployment of Arcade-services.

Although the darc version that contains this fix is already available in the feed: 1.1.0-beta.19531.1, the API that our darc update scripts use: https://maestro-prod.westus2.cloudapp.azure.com/api/assets/darc-version?api-version=2019-01-16 will still report 1.1.0-beta.19530.2 as the latest version until we deploy to prod.

Reopening and moving back to rollout column.

[lukas-lansky]

Thanks!

[MattGal]

@riarenas / @lukas-lansky is this rolled out yet?

[riarenas]

It is not. Still in the right place.

[MattGal]

how about today?

[riarenas]

Yeah!

[MattGal]

Yeah!