Fix "Invoke-Item" to accept a file path with spaces on Unix platforms#3850
Fix "Invoke-Item" to accept a file path with spaces on Unix platforms#3850
Conversation
adityapatwardhan
left a comment
There was a problem hiding this comment.
Looks good, except one small comment.
| #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?
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
|
The cmdlet seems to have the most unpredictable behavior😕 |
|
@iSazonov I think we are focusing on 2 scenarios for this cmdlet:
|
|
@daxian-dbw Thanks for clarify. Could you move the comment to the PR description for future docs? |
Address #2900
Root Cause
On Unix platforms, we are currently depending on
xdg-openon Linux andopenon OSX to open the default program for a registered file type, so the file path is served as an argument toxdg-openandopen. 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-Itemon Linux and OSX -- it cannot invoke an executable properly. The fix is to first runProcess.Startdirectly with the file path, and if it fails, then tryxdg-openoropen.Additional Info
We are focusing on 2 scenarios for "Invoke-Item":