-
Notifications
You must be signed in to change notification settings - Fork 270
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
Make numbered bookmarks work with numlock on #1093
base: master
Are you sure you want to change the base?
Conversation
What's this about a julia lexxer failing the build? The build worked on my machine and I made the most innocent looking change, were there catastrophic side effects to a || I didn't know about? |
The Geany-plugins CI is still using an olde distro to build on, Trusty, Geany itself has been updated to use a supported distro because of problems with Trusty, and it was upgraded to Bionic. (Xenial having reached end of standard life the month before the upgrade). What has happened is that the new Julia filetype has leaked a C++17 object @frlan do you want to bring G-P CI distro up to the same standard as Geany? |
@Davidy22 I would suggest that you use the official mask https://developer.gnome.org/gdk3/unstable/gdk3-Windows.html#GdkModifierType to remove all but the wanted modifiers, any of the others can change at any time in any version of GTK, or with any new keyboard with extra keys, so the current implementation is very fragile. |
Now that you mention it, should have figured there would be mask docs when numlock state was exactly 16 away, added |
/* control+shift+number */ | ||
if(ev->state==5 || ev->state==21) { | ||
if(ev->state & (GDK_CONTROL_MASK | GDK_SHIFT_MASK)) { |
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.
This tests for control or shift, not control and shift
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.
Oh word, that wasn't the way I thought it'd work. Thought it was checking right too when I was checking ctrl+number.
|
Just a note that this plugin will only work for keyboards that are similar to US keyboards where the shift characters of the numbers match the majik numbers in the Since this is a plugin such limitations can be accepted but perhaps the initialisation of that array could be changed to use character literals to be a bit more obvious. |
I actually thought the magic numbers in iShiftNumbers were the punctuation keys too when I first looked at it, but when I was writing the fix I noticed the chunk of code starting line 1503 that seems to be finding out on load what the numbers turn into when you hit shift, the Fraser fellow seems to have thought of that. Still a slightly obtuse magic number list though |
Ok, the magic number list is only the fallback default and is British keyboard ( |
Ey, it passed. Noticed a meaningful comment correction to make a commit out of. |
Looks good to me, tested and works (on my German keyboard) with and without Numlock turned on. |
Can anybody with a French keyboard (at least I think its a French keyboard I'm remembering) with numbers on the top and symbols below check if it works, perhaps even though they are on top the numbers will still be level 0? |
I just tried this plugin and I have the same problem. I found the open issue. When I disable numlock the numbered bookmarks work. I have a Logitech K120 USB wired keyboard. Thank you for working on this @Davidy22 . Does this patch need further testing before it's merged? |
Yes with keyboards with non-US intl layout, or those could be ignored since nobody with one has bothered to test in two and a half years. |
As resolution to #1089, add a case so that plugin still works when num lock is on.