As resolution to #1089, add a case so that plugin still works when num lock is on. You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany-plugins/pull/1093
-- Commit Summary --
* Work with numlock on
-- File Changes --
M geanynumberedbookmarks/src/geanynumberedbookmarks.c (2)
-- Patch Links --
https://github.com/geany/geany-plugins/pull/1093.patch https://github.com/geany/geany-plugins/pull/1093.diff
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 `std::string_view` from its Scintilla implementation, but thats ok with Bionic, and the versions used for nightlys, but not Trusty. Oh well C++17 was bound to leak at some point :-D
@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 them 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.
@Davidy22 pushed 1 commit.
000a4848b8b7ece72084ed2e7fdbe737f9766a35 Use GDK bitmasks
Now that you mention it, should have figured there would be mask docs when numlock code was exactly 16 away, added
@elextr commented on this pull request.
/* control+shift+number */
- if(ev->state==5 || ev->state==21) { + if(ev->state & (GDK_CONTROL_MASK | GDK_SHIFT_MASK)) {
This tests for control or shift, not control and shift
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 `std::string_view` from its Scintilla implementation, but thats ok with Bionic, and the versions used for nightlys, but not Trusty. Oh well C++17 was bound to leak at some point :-D
@frlan do you want to bring G-P CI distro up to the same standard as Geany?
/me is not @frlan (😄) but anyway: #1094
@Davidy22 pushed 1 commit.
5f59a76c7dfddb09eda39efbc62f603cbf2f1268 Fix shift/ctrl check
@Davidy22 commented on this pull request.
/* control+shift+number */
- if(ev->state==5 || ev->state==21) { + if(ev->state & (GDK_CONTROL_MASK | GDK_SHIFT_MASK)) {
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 `iShiftNumbers` array ie `)!@#$%^&*(`.
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 (`"` and £ not `@` and `#`). The code you pointed to is a good try and probably good enough for most cases, but it does assume that the digits are level 0 and shift is level 1, which may not be the case on all keyboards, but probably few enough it won't be a problem very often.
@Davidy22 I tried to rerun CI after merging #1094, but although it said the request was queued it doesn't seem to have happened, maybe you could push some inconsequential change (to a comment maybe) to re-trigger it.
@Davidy22 pushed 1 commit.
8a28a1354cf0d11a4ff31420dcbd0b2c12df5faa Fix comments to match flow
Ey, it passed. Noticed a meaningful comment correction to make a commit out of.
@Davidy22 pushed 1 commit.
a5e902906739c1eeebe191e252db9f21643f7a9c Merge branch 'geany:master' into master
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?
Does this patch need further testing before it's merged?
[Yes](https://github.com/geany/geany-plugins/pull/1093#issuecomment-894628779) 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.
github-comments@lists.geany.org