File explorer support #27

stefnotch · 2022-07-14T20:56:43Z

I already implemented this, so I figured I'd carefully open a pull request with the feature. I'd love some feedback :)

And upgrade to .net 5

stefnotch · 2022-07-15T10:06:24Z

Okay, the failing pipeline is absolutely my fault, as I attempted to upgrade to .NET 5 without changing the pipeline scripts.

So either I downgrade back, or a lovely maintainer takes a look at how to upgrade the pipeline.

jjw24 · 2022-07-15T10:21:39Z

I am currently afk on my mobile, will take a look later :)

stefnotch

To make reviewing slightly easier, I'll point out some important changes

stefnotch · 2022-07-15T10:23:55Z

Wox.Plugin.Runner/Runner.cs

@@ -148,11 +151,18 @@ private ProcessArguments GetProcessArguments(Command c, IEnumerable<string> term
            }

            var workingDir = c.WorkingDirectory;
+            if (workingDir == "{explorer}")


Working directory can be set to {explorer}, which will trigger the new behavior where it tries to get the path of the topmost file explorer window.

jjw24 · 2022-07-15T10:26:18Z

Wox.Plugin.Runner/Services/ExplorerPathsService.cs

+        public static List<string> GetOpenExplorerPaths()
+        {
+            var results = new List<string>();
+            if (OperatingSystem.IsWindows())


Just saw this, is this referring to OS type? Flow is windows only so not sure if this check is necessary.

stefnotch · 2022-07-15T10:27:08Z

Wox.Plugin.Runner/Services/ExplorerPathsService.cs

+{
+    class ExplorerPathsService
+    {
+        public static List<string> GetOpenExplorerPaths()


This uses the "Shell" to iterate over all file explorer windows and read the file paths and HWNDs from them.
Then it uses "Win32" stuff to iterate over all open windows (sorted by how close to the user they are), and read the HWNDs from there.

Finally, it combines that info to return a sorted list of open file explorer windows. This means that GetOpenExplorerPaths().First() will always return the topmost open file explorer.

stefnotch · 2022-07-15T10:28:46Z

Wox.Plugin.Runner/Wox.Plugin.Runner.csproj

    <OutputType>Library</OutputType>
    <SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\</SolutionDir>
    <RestorePackages>true</RestorePackages>
    <GenerateAssemblyInfo>false</GenerateAssemblyInfo>
+    <Nullable>enable</Nullable>


I also enabled the nullable references stuff, which makes it a lot easier to spot potential null pointer exceptions at compile time.

taooceros · 2022-07-15T10:31:05Z

Lgtm. Shall we also implement similar thing in flow? I remember @Garulf has shared a similar idea about pasting the first explorer path to flow.

jjw24 · 2022-07-15T10:33:37Z

Lgtm. Shall we also implement similar thing in flow? I remember @Garulf has shared a similar idea about pasting the first explorer path to flow.

Was about to ask you same thing if we want to expose this via the flow API instead?

stefnotch · 2022-07-15T10:36:34Z

Lgtm. Shall we also implement similar thing in flow? I remember @Garulf has shared a similar idea about pasting the first explorer path to flow.

Oh, that'd be a lovely API to have in Flow. I personally use this for

Starting VSCode with the open directory
Starting a HTTP file server with the open directory

and have thought about things like

Find a file in the current directory. Heck, if combined with ripgrep, it would put the Windows file searching to shame if used on code-bases.
git init with the current directory or git status

taooceros

Also could you resolve the null safety check since we have enabled it? To mark all property that allows null as nullable and create a constructor for all other properties.

taooceros · 2022-07-15T13:32:48Z

Wox.Plugin.Runner/Services/ExplorerPathsService.cs

+                        var result = fullResults.Find(v => v.HWND == hwnd);
+                        if(result != null)
+                        {
+                            result.ZIndex = zIndex;


why do we set the result.ZIndex to zIndex? To put the window up to front?

Yeah, that's used for sorting the windows correctly.

I wonder why do we want to sort out the windows?
Shouldn't we pick the window with the largest zindex?

My thought process was that it might make sense to be able to get other open file explorer paths. But honestly, I'm not sure if that would ever be useful.

(See https://github.com/Flow-Launcher/Flow.Launcher/pull/1018/files#r922377206 )

Oh I messed up with the logic. So the EnumWindow Call will enumerate with the ZIndex order?

Do you plan to make usage of the ZIndex? But I think it would be a bit hard to do so lol. Maybe adding multiple results instead of only the first one, but order them based on the zindex? BTW, you can order the results with the Score property for Result class.

I slightly tweaked the code, let me know if it's easier to understand now

Wox.Plugin.Runner/Wox.Plugin.Runner.csproj

taooceros · 2022-07-15T13:36:08Z

Flow-Launcher/Flow.Launcher#907
Maybe we can add a similar feature for explorer plugin as well.

taooceros · 2022-07-15T13:38:01Z

Flow-Launcher/Flow.Launcher#1018

And for this one, do you have any clues about a better implementation?

…-file-explorer

stefnotch · 2022-07-15T17:11:42Z

@taooceros I did my best to fix the null warnings, however there were quite a few places where I ended up placing a !, since I wouldn't know how to do it without big refactors.

stefnotch · 2022-07-15T17:17:01Z

Flow-Launcher/Flow.Launcher#1018

And for this one, do you have any clues about a better implementation?

Alrighty, I took a look at it and left some comments there.

Wox.Plugin.Runner/Runner.cs

taooceros · 2022-07-15T22:37:18Z

Wox.Plugin.Runner/Settings/RunnerSettingsViewModel.cs

@@ -21,9 +21,9 @@ public void LoadCommands()
                RunnerConfiguration.Commands.Select( c => new CommandViewModel( c ) ) );
        }

-        public ObservableCollection<CommandViewModel> Commands { get; set; }
+        public ObservableCollection<CommandViewModel>? Commands { get; set; }


Maybe refactor a bit to make this not nullable. The viewmodel is not intended to have null Commands.

taooceros · 2022-07-15T22:38:34Z

@taooceros I did my best to fix the null warnings, however there were quite a few places where I ended up placing a !, since I wouldn't know how to do it without big refactors.

Thanks it looks great. I think when you ends up placing a !, maybe just suppress the nullity check with null!, or otherwise use a ? instead.

taooceros · 2022-07-15T22:53:20Z

btw do you mind fixing the UI overflow? Thanks

taooceros · 2022-07-15T23:00:39Z

btw maybe it would be even useful if we can put this as part of the parameter? I think it would be fairly easy to just use a replace (though you may need to consider a bit about the escaping). I wonder whether it is possible to use the ArgumentList api instead of the current one so that we don't need to worry about the escaping.

stefnotch · 2022-07-16T08:16:47Z

@taooceros Okay, I modified the null checks. I didn't fix the XAML stuff, since that's way out of my depth. (I don't understand how it works.)

stefnotch · 2022-07-16T10:02:20Z

btw maybe it would be even useful if we can put this as part of the parameter? I think it would be fairly easy to just use a replace (though you may need to consider a bit about the escaping). I wonder whether it is possible to use the ArgumentList api instead of the current one so that we don't need to worry about the escaping.

Yeah, probably would be useful. I think that could be done in a future pull request?

taooceros · 2022-07-16T12:47:35Z

@taooceros Okay, I modified the null checks. I didn't fix the XAML stuff, since that's way out of my depth. (I don't understand how it works.)

No problem, I will fix that part.

taooceros · 2022-07-16T21:57:22Z

Done, do you want to include the multiple explorer path or leave it in the future?

stefnotch · 2022-07-17T06:29:45Z

@taooceros Let's leave it for the future

jjw24 · 2022-07-23T10:53:58Z

This is ready to roll?

stefnotch · 2022-07-23T11:00:07Z

From my side: Yes, I've been using it for a week now without issues and it's pretty sweet.

There is Flow-Launcher/Flow.Launcher#1275 for getting a part of the functionality into Flow itself, but that's a separate story.

taooceros · 2022-07-23T15:19:30Z

Ah sorry I was supposed to approve it earlier but I forget

taooceros · 2022-07-25T15:10:47Z

Oh there's one more thing that needs to be done (I always forget). Update the version in plugin.json.

File explorer support

6a67fd1

And upgrade to .net 5

jjw24 added 2 commits July 15, 2022 20:17

Update to .Net 5 build

3cc47fe

Update to .Net 5 publish

95f0e1c

jjw24 assigned stefnotch Jul 15, 2022

jjw24 requested a review from taooceros July 15, 2022 10:21

jjw24 added the enhancement New feature or request label Jul 15, 2022

stefnotch commented Jul 15, 2022

View reviewed changes

jjw24 reviewed Jul 15, 2022

View reviewed changes

stefnotch commented Jul 15, 2022

View reviewed changes

taooceros requested changes Jul 15, 2022

View reviewed changes

stefnotch added 3 commits July 15, 2022 19:10

Remove useless lines from .csproj

360aa1e

Fix null warnings

c6c05a6

Merge remote-tracking branch 'stefnotch/feat-file-explorer' into feat…

6bd718d

…-file-explorer

taooceros reviewed Jul 15, 2022

View reviewed changes

Wox.Plugin.Runner/Runner.cs Outdated Show resolved Hide resolved

taooceros reviewed Jul 15, 2022

View reviewed changes

Tweak null checks

1fcc0e7

stefnotch added 2 commits July 16, 2022 10:27

Slightly tweak the explorer paths getting

858002c

Add a comment

3fd1fc3

stefnotch mentioned this pull request Jul 16, 2022

Insert active explorer path Flow-Launcher/Flow.Launcher#1275

Merged

stefnotch mentioned this pull request Jul 16, 2022

Use open file explorer directory #26

Closed

Adjust UI For button to display

c48ed99

taooceros previously approved these changes Jul 23, 2022

View reviewed changes

Update Version

a0f66e8

taooceros dismissed their stale review via a0f66e8 July 25, 2022 15:11

taooceros previously approved these changes Jul 25, 2022

View reviewed changes

update readme

ccbbe45

jjw24 dismissed taooceros’s stale review via ccbbe45 July 28, 2022 10:34

jjw24 approved these changes Jul 28, 2022

View reviewed changes

jjw24 merged commit 953b73b into jjw24:master Jul 28, 2022

File explorer support #27

File explorer support #27

Uh oh!

Conversation

stefnotch commented Jul 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stefnotch commented Jul 15, 2022

Uh oh!

jjw24 commented Jul 15, 2022

Uh oh!

stefnotch left a comment

Choose a reason for hiding this comment

Uh oh!

stefnotch Jul 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

taooceros commented Jul 15, 2022

Uh oh!

jjw24 commented Jul 15, 2022

Uh oh!

stefnotch commented Jul 15, 2022

Uh oh!

taooceros left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stefnotch Jul 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

taooceros commented Jul 15, 2022

Uh oh!

taooceros commented Jul 15, 2022

Uh oh!

stefnotch commented Jul 15, 2022

Uh oh!

stefnotch commented Jul 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

taooceros commented Jul 15, 2022

Uh oh!

taooceros commented Jul 15, 2022

Uh oh!

taooceros commented Jul 15, 2022

Uh oh!

stefnotch commented Jul 16, 2022

Uh oh!

stefnotch commented Jul 16, 2022

Uh oh!

taooceros commented Jul 16, 2022

Uh oh!

taooceros commented Jul 16, 2022

Uh oh!

stefnotch commented Jul 17, 2022

Uh oh!

jjw24 commented Jul 23, 2022

Uh oh!

stefnotch commented Jul 14, 2022 •

edited

Loading

stefnotch Jul 15, 2022 •

edited

Loading

stefnotch Jul 15, 2022 •

edited

Loading

stefnotch commented Jul 15, 2022 •

edited

Loading

taooceros commented Jul 23, 2022 •

edited

Loading