Fix "Invoke-Item" to accept a file path with spaces on Unix platforms - #3850
Conversation
Sorry, something went wrong.
Aditya Patwardhan (adityapatwardhan)
left a comment
There was a problem hiding this comment.
Looks good, except one small comment.
Sorry, something went wrong.
| #else | ||
| invokeProcess.StartInfo.FileName = path; | ||
| invokeProcess.Start(); | ||
| finally { /* Nothing to do in FullCLR */ } |
There was a problem hiding this comment.
Do we need a empty finally block?
Sorry, something went wrong.
There was a problem hiding this comment.
The finally block is kept there for FullCLR section only to match the try block above, so that we don't need to enclose try { and } in if/def. I will add one more comment to make the purpose more clear.
An empty finally block will be ignored during compilation (release version), so there won't be any performance concern. See the code below:
static void Main(string[] args)
{
Process p = null;
try { p = new Process(); } finally { }
}
Corresponding IL
.method private hidebysig static void Main(string[] args) cil managed
{
.entrypoint
// Code size 7 (0x7)
.maxstack 8
IL_0000: newobj instance void [System.Diagnostics.Process]System.Diagnostics.Process::.ctor()
IL_0005: pop
IL_0006: ret
} // end of method Program::Main
Sorry, something went wrong.
|
The cmdlet seems to have the most unpredictable behavior😕 |
Sorry, something went wrong.
|
Ilya (@iSazonov) I think we are focusing on 2 scenarios for this cmdlet:
|
Sorry, something went wrong.
|
Dongbo Wang (@daxian-dbw) Thanks for clarify. Could you move the comment to the PR description for future docs? |
Sorry, something went wrong.
…PowerShell#3850) Use the method NativeCommandParameterBinder.NeedQuotes, which is used by powershell native command processor, to check if quotes are needed. If yes, add quotes in the same way as our native command processor. Also, make 'Invoke-Item' on Linux and OSX able to invoke an executable properly.
Address #2900
Root Cause
On Unix platforms, we are currently depending on xdg-open on Linux and open on OSX to open the default program for a registered file type, so the file path is served as an argument to xdg-open and open. However, we didn't handle the case when path contains spaces.
Fix
Use the method NativeCommandParameterBinder.NeedQuotes, which is used by powershell native command processor, to check if quote is needed. If yes, add quotes in the same way as our native command processor (see the code here).
There is another problem with the current Invoke-Item on Linux and OSX -- it cannot invoke an executable properly. The fix is to first run Process.Start directly with the file path, and if it fails, then try xdg-open or open.
Additional Info
We are focusing on 2 scenarios for "Invoke-Item":