Skip to content
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.

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

[PT Run] Open folder using shell instead of explorer.exe #7292

Merged
merged 2 commits into from Oct 16, 2020
Merged

[PT Run] Open folder using shell instead of explorer.exe #7292

merged 2 commits into from Oct 16, 2020

Conversation

davidegiacometti
Copy link
Collaborator

@davidegiacometti davidegiacometti commented Oct 14, 2020

Summary of the Pull Request

Open a folder from PT Run should not use explorer.exe but the shell instead.

PR Checklist

Info on Pull Request

PT Run is now opening folder using UseShellExecute instead of explorer.exe.

Validation Steps Performed

  • Check that PT Run is still opening folders as expected.
  • Replace explorer.exe with an alternative and check that PT run is using the new one.

@crutkas
Copy link
Member

crutkas commented Oct 15, 2020

This looks good to me but want to do a quick run here.

@htcfreek
Copy link
Collaborator

@crutkas
Do we have a similar command in other plugins, where we should respect other Explorer alternatives? (For example something like open program path in Explorer.)

@crutkas
Copy link
Member

crutkas commented Oct 15, 2020

good callout @htcfreek. We need to refactor the code base. I think what i would do here is lets get this in and i can do the refactor.

image

@htcfreek
Copy link
Collaborator

If possible here we should have a plugin framework with generalized functions/commands. But this is something for the future, I think.

@crutkas
Copy link
Member

crutkas commented Oct 16, 2020

@htcfreek yeah, that is part of the larger plugin effort but I am going to refactor into a common place.

Copy link
Contributor

@ryanbodrug-microsoft ryanbodrug-microsoft left a comment

Choose a reason for hiding this comment

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

LGTM

@ryanbodrug-microsoft
Copy link
Contributor

This is a nice fix! In order to fully fix the issue #4622 we should also update both:
Microsoft.Indexer.Plugin\ContextMenuLoader.cs and
Microsoft.Folder.Plugin\ContextMenuLoader.cs

To do something similar but to the directory path: eg.

 using (var process = new Process())
{
    var directory = Path.GetDirectoryName(record.FullPath);
    process.StartInfo.FileName = directory;
    process.StartInfo.UseShellExecute = true;
     process.Start();
}

@crutkas
Copy link
Member

crutkas commented Oct 16, 2020

I'm going to merge this in and then do work sometime on monday maybe on #7339 unless someone beats me to it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants