-
-
Notifications
You must be signed in to change notification settings - Fork 82
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
Add hotkey support #158
base: default
Are you sure you want to change the base?
Add hotkey support #158
Conversation
This rocks! That's a big PR so let me process it. I'm pretty busy at the moment but really want to integrate something like this. Thanks! |
Ok, clearly I was too slow to respond to this and I apologize. I've gone through most of the PR and what I found is that I really like your solution, but not really the way it's implemented. I don't like the hardcoded shortcut key or the fact that it only uses the default plugin. However, that's no reason to reject your PR. There's some extra work to adapt it to my liking but I could do the work myself. There are two possibilities:
Basically it depends on whether you want to help me more than you already have. But the extra work I could manage - it was the solution to find the path to the currently-selected file that I didn't have, and you've solved this! Let me know how you want to proceed. |
Is there an option 3 to merge it as is and we both work on it from here?
The hardcoding isn't nice, but I'd be interested in your thoughts on how to
extend it, I'm assuming we'd just add another export on the DLL to enable
this? We'd also need to export a second function to get the settings for
which plugin to use, and then we have a few options for managing the hotkey
(probably best to do this from inside the main app and get the hotkey app
to read these on startup?)
…On Wed, 9 Nov 2022 at 02:22, Charles Lechasseur ***@***.***> wrote:
Ok, clearly I was too slow to respond to this and I apologize.
I've gone through most of the PR and what I found is that I really like
your solution, but not really the way it's implemented. I don't like the
hardcoded shortcut key or the fact that it only uses the default plugin.
However, that's no reason to reject your PR. There's some extra work to
adapt it to my liking but I could do the work myself. There are two
possibilities:
1. I merge this PR as-is, then work on adapting it myself
2. I work with you to get this PR to the point I want, then I merge it
Basically it depends on whether you want to help me more than you already
have. But the extra work I could manage - it was the solution to find the
path to the currently-selected file that I didn't have, and you've solved
this!
Let me know how you want to proceed.
—
Reply to this email directly, view it on GitHub
<#158 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AALJUP5S3IRHI24P57YCC5TWHL4DVANCNFSM6AAAAAARJDCGGI>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, so if we go this way I'd like you to do one change before we merge it as-is.
I don't think we need an extra EXE project to support this. We could embed the code in the extension DLL and simply expose a Rundll32 entry point to fire it. This would have the advantage of being able to reuse all the code in the extension DLL, including Settings which allows you to fetch the list of pipeline plugins.
If you change your PR to implement it like this I'll merge and we can move from there.
How do you propose we handle the hotkey part of this? I thought we needed a
live window somewhere to receive the WM_HOTKEY message?
…On Sat, 12 Nov 2022, 22:54 Charles Lechasseur, ***@***.***> wrote:
***@***.**** requested changes on this pull request.
Ok, so if we go this way I'd like you to do one change before we merge it
as-is.
I don't think we need an extra EXE project to support this. We could embed
the code in the extension DLL and simply expose a Rundll32 entry point to
fire it. This would have the advantage of being able to reuse all the code
in the extension DLL, including Settings which allows you to fetch the list
of pipeline plugins.
If you change your PR to implement it like this I'll merge and we can move
from there.
—
Reply to this email directly, view it on GitHub
<#158 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AALJUPYBCL37PDT4XSQVY33WIAGZXANCNFSM6AAAAAARJDCGGI>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
There's nothing that I know of that prevents you from creating a window in a DLL called by rundll32. Is there? |
That's correct, but how do we handle the hotkey? Are you saying we
integrate it all into the DLL and have the listener startup with another
rundll call at windows start time?
…On Sun, 13 Nov 2022, 01:57 Charles Lechasseur, ***@***.***> wrote:
There's nothing that I know of that prevents you from creating a window in
a DLL called by rundll32. Is there?
—
Reply to this email directly, view it on GitHub
<#158 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AALJUP3UTIXIZNJ7MQYLGHTWIA4HNANCNFSM6AAAAAARJDCGGI>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Yes, that's what I'm suggesting. Export a new function that you can with rundll32 at startup. That function is like your previous application's WinMain. |
@clechasseur would you be willing to release a beta build of the app with this feature as is? |
That's doable. In the meantime you can use the installer generated by the PR: That one has the same version number as the latest official version however. It should still be possible to install it over an existing 20.0 installation. Note that I haven't tested it yet, so use at your own risk. 😉 |
Thanks, it seems to work great so far 👍 |
Seems like the installer has expired since then, any idea when a new build might come? Thank you for the work. |
That's unfortunate. Let me see what I can do to rebuild this. |
This adds the hotkey CTRL+ALT+C to load the path for a file currently selected in an explorer window.
Initial implementation of #3