Skip to content

Support Invoke-Item -Path <folder> #4262

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
daxian-dbw merged 9 commits into from
Jul 28, 2017
Merged

Support Invoke-Item -Path <folder> #4262

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
65513b8
Support Invoke-Item -Path <folder>
SteveL-MSFT Jul 15, 2017
706a7b2
added validation that shell window opened on Windows
SteveL-MSFT Jul 15, 2017
1a3134c
special case directories to work on Unix
SteveL-MSFT Jul 15, 2017
08f6a43
reorganized code to make it more readable
SteveL-MSFT Jul 15, 2017
1aa5e77
changed fixed sleep to polling to make test more reliable
SteveL-MSFT Jul 15, 2017
c04d1be
added validation for Linux
SteveL-MSFT Jul 15, 2017
186f8ff
reorganized code based on feedback
SteveL-MSFT Jul 18, 2017
46442da
added MacOS test
SteveL-MSFT Jul 25, 2017
4a4cd71
increased timeout for waiting for window to open
SteveL-MSFT Jul 27, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
special case directories to work on Unix
  • Loading branch information
@SteveL-MSFT
SteveL-MSFT committed Jul 26, 2017
commit 1a3134c4f199adb02fb3f0a6b264e82e4beaa642
36 changes: 27 additions & 9 deletions src/System.Management.Automation/namespaces/FileSystemProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1323,12 +1323,38 @@ protected override void InvokeDefaultAction(string path)

string resource = StringUtil.Format(FileSystemProviderStrings.InvokeItemResourceFileTemplate, path);

#if UNIX
void StartProcessWithOpen(string filename)
{
System.Diagnostics.Process process = new System.Diagnostics.Process();
// The file is possibly not an executable, so we try invoking the default program that handles this file.
const string quoteFormat = "\"{0}\"";
process.StartInfo.FileName = Platform.IsLinux ? "xdg-open" : /* OS X */ "open";
if (NativeCommandParameterBinder.NeedQuotes(filename))
{
path = string.Format(CultureInfo.InvariantCulture, quoteFormat, filename);

@iSazonov iSazonov Jul 16, 2017

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we check NeedQuotes? Maybe do that for every filename?

@daxian-dbw daxian-dbw Jul 17, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NeedQuotes check is needed only if you are specifying arguments for ProcessInfo, for example, the argument string may have space in it.

@iSazonov iSazonov Jul 17, 2017

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought we could remove the 'if' and each name quoted - "filename.txt", "file name.txt".

}
process.StartInfo.Arguments = filename;
// xdg-open on Ubuntu outputs debug info to stderr, suppress it
process.StartInfo.RedirectStandardError = true;

@daxian-dbw daxian-dbw Jul 17, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if xdg-open or open fails to open a file? In that case, user won't see anything after running Invoke-Item and will suspect that Invoke-Item doesn't work.

@SteveL-MSFT SteveL-MSFT Jul 17, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's probably true. I guess we'll just have to have the debug spew coming out.

@iSazonov iSazonov Jul 17, 2017

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What error an user see if run the xdg-open or open from command line?

@SteveL-MSFT SteveL-MSFT Jul 18, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's the same stderr if run directly

process.Start();
}
#endif

if (ShouldProcess(resource, action))
{
System.Diagnostics.Process invokeProcess = new System.Diagnostics.Process();

try
{
// special case directories on UNIX so that we can open them in the window-manager
// corefx doesn't return error if you try to start a directory
#if UNIX
if (Directory.Exists(path))
{
StartProcessWithOpen(path);
}
#endif
// Try Process.Start first.
// - In FullCLR, this is all we need to do.
// - In CoreCLR, this works for executables on Win/Unix platforms
Expand All @@ -1339,15 +1365,7 @@ protected override void InvokeDefaultAction(string path)
catch (Win32Exception ex) when (ex.NativeErrorCode == 13)
{
// Error code 13 -- Permission denied.
// The file is possibly not an executable, so we try invoking the default program that handles this file.
const string quoteFormat = "\"{0}\"";
invokeProcess.StartInfo.FileName = Platform.IsLinux ? "xdg-open" : /* OS X */ "open";
if (NativeCommandParameterBinder.NeedQuotes(path))
{
path = string.Format(CultureInfo.InvariantCulture, quoteFormat, path);
}
invokeProcess.StartInfo.Arguments = path;
invokeProcess.Start();
StartProcessWithOpen(path);
}
#elif CORECLR
catch (Win32Exception ex) when (ex.NativeErrorCode == 193 || ex.NativeErrorCode == 5)
Expand Down