-
Notifications
You must be signed in to change notification settings - Fork 260
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 support for a second keybind per action #870
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,19 +16,23 @@ | |
|
||
"""Terminator by Chris Jones <[email protected]> | ||
|
||
Validator and functions for dealing with Terminator's customisable | ||
Validator and functions for dealing with Terminator's customisable | ||
keyboard shortcuts. | ||
|
||
""" | ||
|
||
import re | ||
from gi.repository import Gtk, Gdk | ||
from gi.repository import Gdk | ||
from .util import err | ||
|
||
|
||
class KeymapError(Exception): | ||
"""Custom exception for errors in keybinding configurations""" | ||
|
||
|
||
MODIFIER = re.compile('<([^<]+)>') | ||
|
||
|
||
class Keybindings: | ||
"""Class to handle loading and lookup of Terminator keybindings""" | ||
|
||
|
@@ -37,7 +41,7 @@ class Keybindings: | |
'control': Gdk.ModifierType.CONTROL_MASK, | ||
'primary': Gdk.ModifierType.CONTROL_MASK, | ||
'shift': Gdk.ModifierType.SHIFT_MASK, | ||
'alt': Gdk.ModifierType.MOD1_MASK, | ||
'alt': Gdk.ModifierType.MOD1_MASK, # Gdk.ModifierType.ALT_MASK ? | ||
'super': Gdk.ModifierType.SUPER_MASK, | ||
'hyper': Gdk.ModifierType.HYPER_MASK, | ||
'mod2': Gdk.ModifierType.MOD2_MASK | ||
|
@@ -62,19 +66,19 @@ def reload(self): | |
self._lookup = {} | ||
self._masks = 0 | ||
for action, bindings in list(self.keys.items()): | ||
if not isinstance(bindings, tuple): | ||
bindings = (bindings,) | ||
if not isinstance(bindings, list): | ||
bindings = [bindings, ''] | ||
|
||
for binding in bindings: | ||
if not binding or binding == "None": | ||
continue | ||
|
||
try: | ||
keyval, mask = self._parsebinding(binding) | ||
keyval, mask = self.parsebinding(binding) | ||
# Does much the same, but with poorer error handling. | ||
#keyval, mask = Gtk.accelerator_parse(binding) | ||
except KeymapError as e: | ||
err ("keybindings.reload failed to parse binding '%s': %s" % (binding, e)) | ||
# keyval, mask = Gtk.accelerator_parse(binding) | ||
except KeymapError as exc: | ||
err(f"keybindings.reload failed to parse binding '{binding}': {exc}") | ||
else: | ||
if mask & Gdk.ModifierType.SHIFT_MASK: | ||
if keyval == Gdk.KEY_Tab: | ||
|
@@ -91,7 +95,7 @@ def reload(self): | |
self._lookup[mask][keyval] = action | ||
self._masks |= mask | ||
|
||
def _parsebinding(self, binding): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why change this method name? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The underscore kept it as an internal method. This affected the tests from being able to access the method. |
||
def parsebinding(self, binding): | ||
"""Parse an individual binding using gtk's binding function""" | ||
mask = 0 | ||
modifiers = re.findall(MODIFIER, binding) | ||
|
@@ -103,27 +107,25 @@ def _parsebinding(self, binding): | |
raise KeymapError('No key found') | ||
keyval = Gdk.keyval_from_name(key) | ||
if keyval == 0: | ||
raise KeymapError("Key '%s' is unrecognised" % key) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are you changing this string formatting? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. F-strings have been preferred since Python 3.6 went GA. |
||
raise KeymapError(f"Key '{key}' is unrecognised") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are you changing this string formatting? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. F-strings have been preferred since Python 3.6 went GA. |
||
return (keyval, mask) | ||
|
||
def _lookup_modifier(self, modifier): | ||
"""Map modifier names to gtk values""" | ||
try: | ||
return self.modifiers[modifier.lower()] | ||
except KeyError: | ||
raise KeymapError("Unhandled modifier '<%s>'" % modifier) | ||
except KeyError as exc: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are you changing this string formatting? |
||
raise KeymapError(f"Unhandled modifier '<{modifier}>'") from exc | ||
|
||
def lookup(self, event): | ||
"""Translate a keyboard event into a mapped key""" | ||
try: | ||
_found, keyval, _egp, _lvl, consumed = self.keymap.translate_keyboard_state( | ||
event.hardware_keycode, | ||
_, keyval, _, _, consumed = self.keymap.translate_keyboard_state( | ||
event.hardware_keycode, | ||
Gdk.ModifierType(event.get_state() & ~Gdk.ModifierType.LOCK_MASK), | ||
event.group) | ||
except TypeError: | ||
err ("keybindings.lookup failed to translate keyboard event: %s" % | ||
dir(event)) | ||
err(f"keybindings.lookup failed to translate keyboard event: {dir(event)}") | ||
return None | ||
mask = (event.get_state() & ~consumed) & self._masks | ||
return self._lookup.get(mask, self.empty).get(keyval, None) | ||
|
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.
Why remove GTK here? Is it necessary?
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.
It's an unused import here. There was one call that was commented out.
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.
Should we remove the comment too?
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.
I'd rather keep code cleanup changes separate from feature or bugfix changes. It means when I go back theough the history I can tell exactly what changes did what. This is the point I was trying to get across with my earlier comment. There's too much going on in this commit. The stuff may be good to include in another PR, but in a PR that is just supposed to implement a feature, the code should just reflect what is necessary to implement that feature. I definitely welcome bugfixes and code cleanup PRs, just not mixed in with feature implementation PRs