Implement Emacs key bindings#6037
Conversation
Emacs style key bindings are re-added to JabRef through the preferences menu. The supported key bindings have feature parity with the previous implementation in JabRef v<4, and additionally support any class that extends TextInputControl. In practice, this means that the new implementation supports both TextFields and TextAreas by default. Some functionality may still be missing Co-authored-by: Felix Luthman <34520175+felixlut@users.noreply.github.com> Co-authored-by: Tommy Samuelsson <Zodbigt@users.noreply.github.com> Co-authored-by: muachilin <32566798+muachilin@users.noreply.github.com> Co-authored-by: Kristoffer Gunnarsson <kristoffergunnarsson47@gmail.com>
|
I see we have some localization strings to fix, will do that soon. |
Siedlerchr
left a comment
There was a problem hiding this comment.
Thanks for your contribution! I hope you enjoyed it so far!
In general looks good to me, I have just a few codestyle remarks.
Let's see what the others have to say :)
koppor
left a comment
There was a problem hiding this comment.
Thank you for the work on that and to dive into the depth of JavaFX and keyboard handling.
- I am sorry that I have you to point to
org.jabref.logic.bst.BibtexCaseChanger#changeCase. That functionality really has to be used (instead of rewriting this upper/lower case thing). - I think, we did not have code for delete characters until beginning/end of the word. Thus, this really has to be reimplemented. I hope, you checked Google Guava and Apache Commons that there is not such a functionality. - Please add a comment on the implementation that you checked and since it did not exist, you implemented it.
- I check in the entry editor. Ctrl+A works fine. Alt+B for going backword one word and Alt+F for going forward one word (same algorithm then with alt+d) did not work. Can you add them, too? (Semantics of emacs: Ctrl: One thing, Alt: the next semantic level... Thus, if Ctrl+B is one letter back, Alt+B is one word back).
- Ctrl/kbd>+Z for "Undo" does not work anymore. Do you have an idea?
tobiasdiez
left a comment
There was a problem hiding this comment.
I really like that you added the corresponding key bindings as normal keybindings in the dialog. This gives quite some flexibility and unified user experience. I would actually push this flexibility a bit further and remove the "Emacs" mode completely and simply let users manage the corresponding keybindings on their own. That is, doesn't limit the new keybindings to be used by emacs users. The bindings can be initialized with standard bindings if there are any (like Ctrl + right/left to jump one word) or left empty if they are normally not used but are somewhat emacs specific.
Reasoning: I guess not many people will use the emacs mode, but more users will be happy about the customization of additional keybindings and will adapt them to their liking.
@Siedlerchr @koppor @calixtus what you think?
|
I agree, since the special implementation of emacs bindings besides the normal KeyBindings introduces a lot of unneccessary complexity. If it's possible to implement the same functionality in the current KeyBindings framework - go for it! |
|
In case we go that way, we need to have a button in the "customize key bindings" dialog to "Set Emacs keybindings", where a dialog pops up asking whether to rebind See #6037 (comment) for a screenshot for JabRef 4.3.1 See https://docs.jabref.org/setup/customkeybindings for the reset capability of the dialog. |
|
@koppor why do we need this? Ok, it's a bit more work for someone using emacs bindings to initially configure the bindings by hand for each shortcut. But I would prefer to have less visual distractions for 99.9% of all users instead of a button for the rest. We are not an code editor that we need to support fancy shortcut systems. How many normal user applications actually let you configure keybindings in such a detail? |
b8ef7b7 to
21c6e5e
Compare
|
I would ❤️ to see the button "bash keybings". (I replaced "Emacs" by "bash", maybe this suits better?). See https://gist.github.com/tuxfight3r/60051ac67c5f0445efee for the list of supported key bindings. "By accident" they also work on Emacs. I personally use the bash keybindings daily in my life. Either in git bash and in cmd.exe (by using clink). Thus, I am really used to them and would like to use them in my favorite literature management software. Since we dropped the plugin system (#152) users demanding other keyboard shortcuts (Sublime?) can add a similar functionality. |
|
@muachilin @stevensdavid It would be nice if you could implement the suggested changes so that we can get this feature into JabRef. Thanks! Thus it would be nice if you could remove most of the emac's specific code and let the user decide about their preferred keybord shortcut for this. If you then still have energy left, a button in the keybinding dialog setting these keybindings to their default emacs value would be nice (and would make @koppor very happy). |
|
@muachilin We would really like to see that this features comes into JabRef. May I ask whether you can find some time to finish this PR? It seems to be close to get it done. We have resources to provide timely feedback. |
|
Except KILL_WORD_BACKWARD everything seems to work now. |
|
Please also check if they work on the code area tab, I noticed for the mac thing that it has some quirks |
|
Worked a little bit on the key bindings dialog too.
|
|
Thanks a lot for finishing this PR @calixtus! The code looks very good to me, and I have only a few comments - oh wait, I actually don't have any comments... One suggestion for the UI: What do you think about replacing the preset Label + Dropdown + Load button combination by a MenuButton with label "Presets" and if you click on it you get the list of possible presets; once a preset is selected there it is automatically loaded. You could also try to put this button next to the "Reset Bindings"; or maybe even remove the "Reset bindings" and add a "Default" preset. |
|
I guess a MenuButton should do the thing. |
|
The clear bindings button to the bottom left is no standard button, but a JavaFX dialog ButtonType. So I cannot put there a custom ui element. Sorry for that. |
|
Thank you so much for finishing this @calixtus |
* upstream/master: (164 commits) Bump lucene-queryparser from 8.6.3 to 8.7.0 (JabRef#7089) Fix endnote importer when keyword style is null (JabRef#7084) Rename Firefox extension file in Windows (JabRef#7092) Add support for Microsoft Edge browser in Windows and Linux builds (JabRef#7056) Bump unirest-java from 3.11.03 to 3.11.05 (JabRef#7087) Bump com.github.ben-manes.versions from 0.33.0 to 0.36.0 (JabRef#7088) Bump gittools/actions from v0.9.4 to v0.9.5 (JabRef#7091) Feature/add abstract field to complex search (JabRef#7079) Add missing authors Implement Emacs key bindings (JabRef#6037) Special field code maintenance (JabRef#7078) Follow up fix for 7077 Reset preview layouts on clear Fix preview settings not saved due to l10n (JabRef#7077) Add link to existing documentation of filed types Remove unused supportsPaging Squashed 'src/main/resources/csl-styles/' changes from 5c376b8..f4399aa Fix more 404 links Fix custom theme styles not applied to the entry preview (JabRef#7071) Fix Shared Database Tests (JabRef#7040) Bump byte-buddy-parent from 1.10.17 to 1.10.18 (JabRef#7059) ... # Conflicts: # src/main/java/org/jabref/gui/DefaultInjector.java
Implement Emacs key bindings (fixes #6017)
Emacs style key bindings are re-added to JabRef through the preferences
menu. The supported key bindings have feature parity with the previous
implementation in JabRef v<4, and additionally support any class that
extends TextInputControl. In practice, this means that the new
implementation supports both TextFields and TextAreas by default. Some
functionality may still be missing.
Co-authored-by: Felix Luthman 34520175+felixlut@users.noreply.github.com
Co-authored-by: Tommy Samuelsson Zodbigt@users.noreply.github.com
Co-authored-by: muachilin 32566798+muachilin@users.noreply.github.com
Co-authored-by: Kristoffer Gunnarsson kristoffergunnarsson47@gmail.com