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

Global Hotkeys support for gtk2 for Windows #94

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

domenn
Copy link
Contributor

@domenn domenn commented Sep 4, 2020

Global Hotkeys support for gtk2 for Windows.

I have further ideas for improvements, hence some commented out experimental code. This is "checkpoint" state that works.

I am willing to keep supporting code of this plugin in future, in case other people have (and report) issues with it regarding my changes.

@domenn domenn changed the title Mergable gtk2 win hotkey Global Hotkeys support for gtk2 for Windows Sep 4, 2020
@jlindgren90
Copy link
Member

This would be really useful. I'm afraid I don't personally have time to give the code a good review right now.

@domenn
Copy link
Contributor Author

domenn commented Sep 14, 2020

Thanks for initial positive response.
I am using audacious with those changes daily and it works without problems. I don't observe any regressions. Previously, there were issues with state not saving on close and things like that. But recently I put much higher effort into this to make it work without issues.

I also tested on Linux to make sure it works there as before.

About code review, is there anyone else? Otherwise I hope that you can manage some moment in future. Hopefully before next release.

If you need some more information from me about why I did the things the way I did them, feel free to ask. And note that this is not the first PR from my side into Audacious.

@jlindgren90
Copy link
Member

Hi, I'm belatedly getting to look at this.
I ran clang-format on the master branch so that we can separate out the formatting changes from the function changes.
Can you please rebase on top of master like so:

git checkout master
git pull
git checkout mergableGtk2WinHotkey 
git rebase 7dbcee3faf027db690e3f3ea6afbeb7f2bb24187
clang-format -i src/hotkey/*.{cc,h}
git commit -a -m "Run clang-format on src/hotkey"
git branch rebaseGtk2WinHotkey 0a35159f8bfe8fe1d9a264fd3fb2ca21fdb18dbf
git checkout rebaseGtk2WinHotkey 
git checkout mergableGtk2WinHotkey -- .
clang-format -i src/hotkey/*.{cc,h}
git commit -a -m "Rebase Global Hotkeys support for gtk2 for Windows"

@jlindgren90
Copy link
Member

Also please remove commented-out code and the #ifdef BUILT_FROM_CMAKE sections.

@domenn
Copy link
Contributor Author

domenn commented Dec 13, 2020

Done as you said.
I tried locally by my side, it compiles. I didn't test working, but it should work, because I tested last time when submitting PR. You test everything before you merge anyway, don't you?

@jlindgren90
Copy link
Member

Is there a way to handle global hotkeys without referencing the main Audacious window?

@domenn
Copy link
Contributor Author

domenn commented Dec 30, 2020

As far as my knowledge goes, no. Windows API function RegisterHotKey needs a window that is receiving messages. There may be a way to use some other window that is not "main Audacious window".

The way I did it needed significant research, and this is the method that I managed to come up with.

@jlindgren90
Copy link
Member

The documentation for RegisterHotKey says, "If this parameter [hWnd] is NULL, WM_HOTKEY messages are posted to the message queue of the calling thread and must be processed in the message loop." It looks like you would then retrieve those messages by passing hWnd = NULL again to GetMessage.

Please look into that method. The code in its current state is too tightly coupled to a specific UI plugin. Searching for the main window by title is particularly bad.

@domenn
Copy link
Contributor Author

domenn commented Dec 30, 2020

I'm pretty busy these days. So, I cannot promise anything.

@C-Duv
Copy link

C-Duv commented Apr 7, 2023

Is there a way to handle global hotkeys without referencing the main Audacious window?

Just to be sure, the related code is the following?:

bool WindowsWindow::is_main_window(bool allow_hidden) const
{
    if (main_window_hidden_ && !allow_hidden)
        return false;
    bool returning =
        (class_name_ == "gdkWindowToplevel" && [](const std::string & title) {
            return title == N_("Audacious") ||
                   title.find(translated_title()) != std::string::npos ||
                   title == N_("Buffering ..."); // msgid "%s - Audacious"
        }(win_header_));
    if (returning)
    {
        kind_ = AudaciousWindowKind::MAIN_WINDOW;
    }
    return returning;
}

(See in context)

@jlindgren90
Copy link
Member

That's part of it. The whole WindowsWindow class should go away, I think.

@Sokoloft
Copy link

Sokoloft commented Aug 9, 2023

Hello,

Sorry to necro. However I'd love this functionality too. Ideally using the media keys. With that, hopefully it'd be detected by KDE connect on my phone.

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