Quote SwiftFormat path to handle spaces

[vinocher-bc]

• Spaces in swiftformat.exe's path were not being handled, so formatting failed when the tool was installed in a path with spaces like C:\Program Files\nicklockwood\SwiftFormat\usr\bin\swiftformat.exe. Fixed by quoting path.

• Breakpoints could not be set because launch.json's outFiles was incorrect.

[tristanlabelle]

Looks good, thanks!

[tristanlabelle]

@vknabel for reviewing/merging

[tristanlabelle]

Hi @vknabel , how would it work to publish new versions of this extension? I see there is no GitHub actions workflow and we could help set that up, but how would the final push to the VSCode marketplace work?

[vknabel]

Hi @tristanlabelle! Totally forgot to automate the releases. The last months were tough. For now I released it manually and I will add a basic Github action for releasing. Later we can tune it and have a chat.

[furby-tm]

This change broke (for me on Fedora Linux), issue here #39.

I do not think all paths to swiftformat should be quoted across all operating systems, because as you can see, this revision broke the path to swiftformat on Linux and possibly macOS as well, one way to handle spaces in paths in a cross platform way, is by escaping them like so:

- my/very long path/to/swiftformat

# do this on macos and linux
+ my/very\ long\ path/to/swiftformat

# do this on windows
# (though I actually believe this only works on cmd prompt.)
+ my/very^ long^ path/to/swiftformat

Or possibly, keep the existing quote fix but only on Microsoft Windows, and keep the previous revision for only macOS and Linux, I believe you should be able to detect platforms in javascript with the following conditional statement checks:

if (process.platform === 'win32')
{
  // do something on windows.
  // (such as your current revision's changes.)
}
else
{
  // do something on macOS, linux, and even unicorns too.
  // (such as leaving the previous code, before your revision, in tact.)
}

above all this extension needs some unit testing, but I'm not familiar enough with vscode extensions to be able to provide you with any guidance there.

[tristanlabelle]

Rereading this change, the problem is that child_process.spawn is ill-behaved with spaces on Windows, so the workaround quoting logic should happen here:

vscode-swiftformat/src/execShell.ts

Line 14 in 027896d

const result = spawnSync(file, args ?? [], {

. This node API looks like a mess. See windowsVerbatimArguments, although I think we don't want to use that.

@vinocher-bc , can you follow-up since this is affecting other platforms?

[vinocher-bc]

Thanks for the report. Looking at this now.

…

On Thu, Jun 6, 2024 at 7:40 AM Tristan Labelle ***@***.***> wrote: Rereading this change, the problem is that child_process.spawn is ill-behaved with spaces on Windows, so the workaround quoting logic should happen here: https://github.com/vknabel/vscode-swiftformat/blob/027896d8212adbdf910b5aa6fd82c92e8fac69f2/src/execShell.ts#L14 . This node API looks like a mess. See windowsVerbatimArguments, although I think we don't want to use that. @vinocher-bc <https://github.com/vinocher-bc> , can you follow-up since this is affecting other platforms? — Reply to this email directly, view it on GitHub <#37 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BGLV3M2O74G5XPSD4YKACXTZGBYFHAVCNFSM6AAAAABICQMVGKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNJSG4YTAMJSGM> . You are receiving this because you were mentioned.Message ID: ***@***.***>