kvakulo / Switcheroo

The humble incremental-search task switcher for Windows
www.switcheroo.io
GNU General Public License v3.0
789 stars 122 forks source link

* Added CloseProcesses, a multiple window closer. #48

Closed cutecycle closed 6 years ago

cutecycle commented 9 years ago

Tried my hand at adding a "Close all processes appearing in the filter" function [Ctrl+Q]. New to the code, so:

HellBrick commented 9 years ago

I like the feature, seems useful for closing those infinitely multiplying Explorer windows ;)

I have one concern about the implementation though: it seems if you're trying to close a bunch of windows and one of them prevents itself from being closed (imagine the classic 'wanna save changes?' dialog), the rest of them won't even attempt to be closed. I would try something like this:

https://github.com/HellBrick/Switcheroo/commit/7c4b757f7d2025a8b833ecc42962f3d163a1ca7d https://github.com/HellBrick/Switcheroo/commit/17cb4525701ba7cc9501e8d56f5ebe5060593c13

(I'm not sure it actually works, just bouncing an idea around =))

kvakulo commented 9 years ago

Very cool! Thanks for your contribution @cutecycle!

I'll take a closer look at it shortly - just some quick thoughts:

  1. I'm thinking that the shortcut could be CTRL + SHIFT + W to make it similar to the current close a single window shortcut
  2. The feature can be potential destructive, if one accidentally presses the combination when for instance the filter is empty. Maybe a confirmation box should be shown when using this shortcut? Something like: "Are you sure you want to close all 30 windows? [ ] Don't show this message again"

/ Regin

cutecycle commented 9 years ago

@HellBrick i was definitely worried about those! I'll merge to check it out later today. Thanks @kvakulo! Maybe a warning if the filter contains more than one process, or more than ten windows of the same process? And an underlined-W for a "Quit Anyway"?

HellBrick commented 9 years ago

I definitely like the idea of a warning if the filtered windows belong to more than 1 process. And this also automatically handles the empty filter case. Not sure whether there's a need for a warning if too many windows of the same process are closed, but if the warning comes with a 'Don't ask again' checkbox, then I see no harm in having it.

kvakulo commented 9 years ago

+1 to showing the warning only if there are windows from more than one process.

Let's skip the warning if there are only windows from one process no matter how many.

And let's skip the 'Don't ask again' checkbox too.

/ Regin

cutecycle commented 9 years ago

Admittedly, I'm out of my element in VS/C#! I'm not quite sure how to do a proper stack trace but with the RemoveWindow I get an InvalidOperationException with CloseProcesses even with the new changes; I'm guessing it's related to modifying the container of window references? Like, RemoveWindow is trying to remove from an empty _unfilteredWindowList?

HellBrick commented 9 years ago

There's a problem with modifying _filteredWindowList when iterating through it, it should be copied before closing stuff: https://github.com/HellBrick/Switcheroo/commit/19c3b542c29e9aee81a400f08e8cba1a3463190b

cutecycle commented 9 years ago

I tried to add help text for the close all windows command; however it expands the width of the window but leaves the width of the window list the same.

kvakulo commented 9 years ago

@cutecycle Just skip the help text for now, I'll try to figure out how to add more space, or maybe an additional window with documentation for the advanced stuff :)

Do you want to work further on this feature, or should I merge it? Then I'll prepare to include it with the next release.

/ Regin

cutecycle commented 9 years ago

I wanted to make one more commit with a warning MessageBox for closing over ten windows, though I'm not at my machine at the moment. The process name check would be a bit more complicated so I'll leave it for now.

HellBrick commented 9 years ago

It's not that complicated =) https://github.com/HellBrick/Switcheroo/commit/682422ef5b717812d06addc35acb94441fd3ce44. You may also want to revert d6c4ca2ad4fd4b51422ee3f066fafe1e5c249479, since it intentionally disabled things.

P.S. The code formatting became really horrible now that there were 3 different instances of Visual Studio used to edit the file ;) Do any of you guys know a way to distribute formatting settings with the repository to avoid this problem with external contributions?

cutecycle commented 9 years ago

Awesome! Yeah, at one point I was using 2015 and gvim as well as VsVim which uses a different indentation format within VS, and eventually turned on invisibles and was just like... whoops, this is a mess.

The convention project wide is 8 spaces for one tab, then?

HellBrick commented 9 years ago

As far as I can see, @kvakulo's original code uses 4 spaces as indent.

kvakulo commented 6 years ago

I'll try to finish this in #90