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

Single-instance mode #10

Closed
cryzed opened this issue May 9, 2016 · 4 comments
Closed

Single-instance mode #10

cryzed opened this issue May 9, 2016 · 4 comments

Comments

@cryzed
Copy link
Contributor

cryzed commented May 9, 2016

It would be great if ANGRYsearch could check if another instance is running when started and bring that window up instead. Currently I'm using KDE's krunner and it keeps spawning new instances which minimize to the tray when I close them.

I am using Ctrl+Q to close the window, but I rebound this shortcut in KDE's KWin to always close the active window, which triggers the close button behavior of ANGRYsearch, i.e. it just minimizes it to the systemtray.

So either: close the ANGRYsearch instance when clicking the close button manually instead of using Esc or Ctrl+Q (when not manually rebound) or check for another running instance when starting and bring that one to the foreground if found. I think making the close button behavior the same as the one triggered by the Esc and Ctrl+Q shortcuts would make more sense.

The issue is that self.tray_icon keeps the application running, because it isn't closed or still visible after the main window has been closed -- calling hide() on it should cause the main window to automatically get rid of it when it closes. Also ANGRYsearch currently uses three or four different methods to close the application in various ways:

  • sys.exit (triggered by the tray icon's Quit action)
  • sys.exit(app.exec_()) (triggered by main window's keyPressEvent)
    • this doesn't make sense and also causes exit code 255, the application is already running, you don't need to pass in app.exec_() again
  • sys.exit(app.exec_()) (triggered by if-main)
    • this seems correct but returns exit code 134, related?
  • Qc.QCoreApplication.instance().quit() (triggered by middle-click on the tray icon)
    • I'm not sure why you are using this way here

IMHO these should all cause all of the application's windows to close (i.e. use closeEvent properly). The app.exec_() call in if-main should then return properly, and the return code fed into sys.exit(). I'm not sure why Qt's exit code is 134 currently. Is there maybe a reason for all these different ways or is it simply old code? Especially considering that your Esc and Ctrl+Q currently don't even call the logic in your closeEvent method -- i.e. the settings don't get saved. I suggest you change sys.exit(app.exec_()) in keyPressEvent to self.close().

EDIT: I added a pull request with these changes implemented. The return code seems to be 0 most of the time now too.

DoTheEvo added a commit that referenced this issue May 9, 2016
Improve closing behavior (See #10)
@DoTheEvo
Copy link
Owner

DoTheEvo commented May 9, 2016

I was thinking about single instance in the beginning, since thats how everything works.

That on run the application would check if another instance is running, if it would find one it would send to it a signal to maximize and focus and then close itself. But it turned out complicated and my tests did not work so I let it go since even cold startup is rather fast...

for closing... it seems I used first thing I googled out at the time of writing various parts... embarrassing...

Thanks for the pull request!

@cryzed
Copy link
Contributor Author

cryzed commented May 9, 2016

I think my main issue was with the confusing closing behavior, single instance is now not needed anymore, at least for me. Do you think I could get you to make a new release on the AUR?

@DoTheEvo
Copy link
Owner

DoTheEvo commented May 9, 2016

Do you think I could get you to make a new release on the AUR?

done

@cryzed
Copy link
Contributor Author

cryzed commented May 9, 2016

Neat, thank you!

@cryzed cryzed closed this as completed May 9, 2016
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

No branches or pull requests

2 participants