keygrabber: Remove the hardcoded way to add keybindings.

There is now a real API for this.

lib/awful/keygrabber.lua

@@ -71,6 +71,7 @@ local unpack = unpack or table.unpack -- luacheck: globals unpack (compatibility
 local gtable = require("gears.table")
 local gobject = require("gears.object")
 local gtimer = require("gears.timer")
+local akeyboard = require("awful.keyboard")
 local glib = require("lgi").GLib
 local capi = { keygrabber = keygrabber, root = root, awesome = awesome }
 
@@ -90,9 +91,6 @@ local conversion = nil
 --BEGIN one day create a proper API to add and remove keybindings at runtime.
 -- Doing it this way is horrible.
 
--- This list of keybindings to add in the next event loop cycle.
-local delay_list = {}
-
 -- Read the modifiers name and map their keysyms to the modkeys
 local function generate_conversion_map()
     if conversion then return conversion end
@@ -119,60 +117,24 @@ local function add_root_keybindings(self, list)
         list, "`add_root_keybindings` needs to be called with a list of keybindings"
     )
 
-    local was_started = #delay_list > 0
+    for _, kb in ipairs(list) do
-
+        if kb.on_press then
-    -- When multiple `awful.keygrabber` objects are created in `rc.lua`, avoid
+            local old_press = kb.on_press
-    -- unpacking and repacking all keys for each instance and instead merge
+            kb.on_press = function(...)
-    -- everything into one operation. In not so extreme cases, not doing so
+                self:start()
-    -- would slow down `awesome.restart()` by a small, but noticeable amount
+                old_press(...)
-    -- of time.
-    table.insert(delay_list, {self, list})
-
-    -- As of Awesome v4.3, `root.keys()` is an all or nothing API and there
-    -- isn't a standard mechanism to add and remove keybindings at runtime
-    -- without replacing the full list. Given `rc.lua` sets this list, not
-    -- using a delayed call would cause all `awful.keygrabber` created above
-    -- `root.keys(globalkeys)` to be silently overwritten. --FIXME v5
-    if not was_started then
-        gtimer.delayed_call(function()
-            local ret = {}
-
-            for _, obj in ipairs(delay_list) do
-                local obj_self, obj_list = obj[1], obj[2]
-
-                for _, v in ipairs(obj_list) do
-                    local mods, key, press, release, data = unpack(v)
-
-                    if type(release) == 'table' then
-                        data = release
-                        release = nil
-                    end
-
-                    if press then
-                        local old_press = press
-                        press = function(...)
-                            obj_self:start()
-                            old_press(...)
-                        end
-                    end
-
-                    if release then
-                        local old_release = release
-                        release = function(...)
-                            obj_self:start()
-                            old_release(...)
-                        end
-                    end
-
-                    table.insert(ret, akey(mods, key, press, release, data))
-                end
             end
+        end
 
-            -- Wow...
+        if kb.on_release then
-            capi.root.keys(gtable.join( capi.root.keys(), unpack(ret) ))
+            local old_release = kb.on_release
+            kb.on_release = function(...)
+                self:start()
+                old_release(...)
+            end
+        end
 
-            delay_list = {}
+        akeyboard.append_global_keybinding(kb)
-        end)
     end
 end
 
@@ -249,13 +211,13 @@ local function runner(self, modifiers, key, event)
         end
 
         for _,v in ipairs(self._private.keybindings[key]) do
-            if #filtered_modifiers == #v[1] then
+            if #filtered_modifiers == #v.modifiers then
                 local match = true
-                for _,v2 in ipairs(v[1]) do
+                for _,v2 in ipairs(v.modifiers) do
                     match = match and mod[v2]
                 end
-                if match then
+                if match and v.on_press then
-                    v[3](self)
+                    v.on_press(self)
 
                     if self.mask_event_callback ~= false then
                         return
@@ -400,20 +362,13 @@ end
 
 --- The keybindings associated with this keygrabber.
 --
--- The keybindings syntax is the same as `awful.key` or `awful.prompt.hooks`. It
+-- This property contains a table of `awful.key` objects.
--- consists of a table with 4 entries.
---
--- * `mods` A table with modifier keys, such as `shift`, `mod4`, `mod1` (alt) or
---  `control`.
--- * `key` The key name, such as `left` or `f`
--- * `callback` A function that will be called when the key combination is
---  pressed.
--- * `description` A table various metadata to be used for `awful.hotkeys_popup`.
 --
 -- @property keybindings
 -- @param table
 -- @see export_keybindings
 -- @see root_keybindings
+-- @see awful.key
 
 --- If any key is pressed that is not in this list, the keygrabber is stopped.
 --
@@ -538,27 +493,62 @@ end
 -- Those keybindings will automatically start the keygrabbing when hit.
 --
 -- @method add_keybinding
--- @tparam table mods A table with modifier keys, such as `shift`, `mod4`, `mod1` (alt) or
+-- @tparam awful.key key The key.
---  `control`.
--- @tparam string key The key name, such as `left` or `f`
--- @tparam function callback A function that will be called when the key
--- combination is pressed.
--- @tparam[opt] table description A table various metadata to be used for `awful.hotkeys_popup`.
--- @tparam string description.description The keybinding description
 -- @tparam string description.group The keybinding group
-function keygrabber:add_keybinding(mods, key, callback, description)
+
+function keygrabber:add_keybinding(key, _keycode, _callback, _description)
+    local mods = not akey._is_awful_key and akey or nil
+
+    if mods then
+        gdebug.deprecate(":add_keybinding now takes `awful.key` objects instead"
+            .. " of multiple parameters",
+            {deprecated_in=5}
+        )
+
+        key = akey {
+            modifiers   = mods,
+            key         = _keycode,
+            description = _description,
+            on_press    = _callback
+        }
+    else
+        _keycode = key.key
+    end
+
     self._private.keybindings[key] = self._private.keybindings[key] or {}
-    table.insert(self._private.keybindings[key], {
+    table.insert(self._private.keybindings[_keycode], key)
-        mods, key, callback, description
-    })
 
     if self.export_keybindings then
-        add_root_keybindings(self, {{mods, key, callback, description}})
+        add_root_keybindings(self, {key})
     end
 end
 
 function keygrabber:set_root_keybindings(keys)
-    add_root_keybindings(self, keys)
+    local real_keys = {}
+
+    -- Handle the pre-object-oriented input structures.
+    for _, key in ipairs(keys) do
+        if key._is_awful_key then
+            table.insert(real_keys, key)
+        else
+            gdebug.deprecate(":set_root_keybindings now takes `awful.key` "
+                .. " objects instead of tables",
+                {deprecated_in=5}
+            )
+
+            local mods, keycode, press, release, data = unpack(key)
+
+            table.insert(real_keys, akey {
+                modifiers   = mods,
+                key         = keycode,
+                description = (data or {}).description,
+                on_press    = press,
+                on_release  = release,
+            })
+        end
+    end
+
+    add_root_keybindings(self, real_keys)
 end
 
 -- Turn into a set.
@@ -689,26 +679,41 @@ function keygrab.run_with_keybindings(args)
 
     -- Build the hook map
     for _,v in ipairs(args.keybindings or {}) do
-        if #v >= 3 and #v <= 4 then
+        if v._is_awful_key then
-            local _,key,callback = unpack(v)
+            ret._private.keybindings[v.key] = ret._private.keybindings[v.key] or {}
+            table.insert(ret._private.keybindings[v.key], v)
+        elseif #v >= 3 and #v <= 4 then
+            gdebug.deprecate("keybindings now contains `awful.key` objects"
+                .. "instead of multiple tables",
+                {deprecated_in=5}
+            )
+
+            local modifiers, key, callback = unpack(v)
             if type(callback) == "function" then
+
+                local k = akey {
+                    modifiers = modifiers,
+                    key       = key,
+                    on_press  = callback,
+                }
+
                 ret._private.keybindings[key] = ret._private.keybindings[key] or {}
-                table.insert(ret._private.keybindings[key], v)
+                table.insert(ret._private.keybindings[key], k)
             else
                 gdebug.print_warning(
                     "The hook's 3rd parameter has to be a function. " ..
-                        gdebug.dump(v)
+                        gdebug.dump(v or {})
                 )
             end
         else
             gdebug.print_warning(
-                "The hook has to have at least 3 parameters. ".. gdebug.dump(v)
+                "The keybindings should be awful.key objects".. gdebug.dump(v or {})
             )
         end
     end
 
     if args.export_keybindings then
-        add_root_keybindings(ret, args.keybindings)
+        ret:set_root_keybindings(args.keybindings)
     end
 
     local mt = getmetatable(ret)

lib/awful/root.lua

@@ -43,11 +43,11 @@ for _, type_name in ipairs { "button", "key" } do
                 if idx then
                     for i=1, #v do
                         assert(
-                            new_values[idx+i] == v[i],
+                            new_values[idx] == v[i],
                             "The root private "..type_name.." table is corrupted"
                         )
 
-                        table.remove(new_values, idx+i)
+                        table.remove(new_values, idx)
                     end
                 end
 

tests/examples/text/awful/keygrabber/alttab.lua

@@ -1,41 +1,50 @@
--- --DOC_HEADER --DOC_NO_USAGE
+--DOC_HEADER --DOC_NO_USAGE
---
+
--- local was_called = {} --DOC_HIDE
+local was_called = {} --DOC_HIDE
---
+
--- local awful = {keygrabber = require("awful.keygrabber"), --DOC_HIDE
+local awful = { keygrabber = require("awful.keygrabber"), --DOC_HIDE
---     client={focus={history={--DOC_HIDE
+    key = require("awful.key"), --DOC_HIDE
---         disable_tracking = function() was_called[1] = true end, --DOC_HIDE
+    client={focus={history={--DOC_HIDE
---         enable_tracking  = function() was_called[2] = true end, --DOC_HIDE
+        disable_tracking = function() was_called[1] = true end, --DOC_HIDE
---         select_next      = function() was_called[3] = true end, --DOC_HIDE
+        enable_tracking  = function() was_called[2] = true end, --DOC_HIDE
---         select_previous  = function() was_called[4] = true end, --DOC_HIDE
+        select_next      = function() was_called[3] = true end, --DOC_HIDE
--- }}}}--DOC_HIDE
+        select_previous  = function() was_called[4] = true end, --DOC_HIDE
---
+}}}}--DOC_HIDE
---     awful.keygrabber {
+
---         keybindings = {
+    awful.keygrabber {
---             {{"Mod1"         }, "Tab", awful.client.focus.history.select_previous},
+        keybindings = {
---             {{"Mod1", "Shift"}, "Tab", awful.client.focus.history.select_next    },
+            awful.key {
---         },
+                modifiers = {"Mod1"},
---         -- Note that it is using the key name and not the modifier name.
+                key       = "Tab",
---         stop_key           = "Mod1",
+                on_press  = awful.client.focus.history.select_previous
---         stop_event         = "release",
+            },
---         start_callback     = awful.client.focus.history.disable_tracking,
+            awful.key {
---         stop_callback      = awful.client.focus.history.enable_tracking,
+                modifiers = {"Mod1", "Shift"},
---         export_keybindings = true,
+                key       = "Tab",
---     }
+                on_press  = awful.client.focus.history.select_next
---
+            },
--- --DOC_HIDE Trigger the keybinging
+        },
--- require("gears.timer").run_delayed_calls_now() --DOC_HIDE `export_keybindings` is async
+        -- Note that it is using the key name and not the modifier name.
--- root.fake_input("key_press", "Alt_L")--DOC_HIDE
+        stop_key           = "Mod1",
--- root.fake_input("key_press", "Tab")--DOC_HIDE
+        stop_event         = "release",
--- root.fake_input("key_release", "Tab")--DOC_HIDE
+        start_callback     = awful.client.focus.history.disable_tracking,
--- root.fake_input("key_release", "Alt_L")--DOC_HIDE
+        stop_callback      = awful.client.focus.history.enable_tracking,
--- assert(was_called[1] and was_called[1] and was_called[2] and was_called[4])--DOC_HIDE
+        export_keybindings = true,
--- assert(not was_called[3]) --DOC_HIDE
+    }
---
+
--- --DOC_HIDE Now make sure it can be triggered again
+--DOC_HIDE Trigger the keybinging
--- root.fake_input("key_press", "Alt_L")--DOC_HIDE
+require("gears.timer").run_delayed_calls_now() --DOC_HIDE `export_keybindings` is async
--- root.fake_input("key_press", "Shift_L")--DOC_HIDE
+root.fake_input("key_press", "Alt_L")--DOC_HIDE
--- root.fake_input("key_press", "Tab")--DOC_HIDE
+root.fake_input("key_press", "Tab")--DOC_HIDE
--- root.fake_input("key_release", "Tab")--DOC_HIDE
+root.fake_input("key_release", "Tab")--DOC_HIDE
---
+root.fake_input("key_release", "Alt_L")--DOC_HIDE
--- assert(was_called[3]) --DOC_HIDE
+assert(was_called[1] and was_called[1] and was_called[2] and was_called[4])--DOC_HIDE
+assert(not was_called[3]) --DOC_HIDE
+
+--DOC_HIDE Now make sure it can be triggered again
+root.fake_input("key_press", "Alt_L")--DOC_HIDE
+root.fake_input("key_press", "Shift_L")--DOC_HIDE
+root.fake_input("key_press", "Tab")--DOC_HIDE
+root.fake_input("key_release", "Tab")--DOC_HIDE
+
+assert(was_called[3]) --DOC_HIDE

tests/examples/text/awful/keygrabber/autostart.lua

@@ -7,7 +7,7 @@ local autostart_works = false --DOC_HIDE
 
 awful.keygrabber {
     autostart      = true,
-    stop_key    = "Return",
+    stop_key       = "Return",
     stop_callback  = function(_, _, _, sequence)
         autostart_works = true --DOC_HIDE
         assert(sequence == "abc") --DOC_HIDE

tests/examples/text/awful/keygrabber/root_keybindings.lua

@@ -1,68 +1,72 @@
--- --DOC_GEN_OUTPUT --DOC_HIDE
+--DOC_GEN_OUTPUT --DOC_HIDE
--- local awful = { keygrabber = require("awful.keygrabber") } --DOC_HIDE
+local awful = { keygrabber = require("awful.keygrabber"), --DOC_HIDE
---
+    key = require("awful.key") } --DOC_HIDE
--- local keybinding_works = {} --DOC_HIDE
+
---
+local keybinding_works = {} --DOC_HIDE
--- local g = --DOC_HIDE
+
--- awful.keygrabber {
+local g = --DOC_HIDE
---     mask_modkeys = true,
+awful.keygrabber {
---     root_keybindings = {
+    mask_modkeys = true,
---         {{"Mod4"}, "i", function(self)
+    root_keybindings = {
---             print("Is now active!", self)
+        awful.key {
---             keybinding_works[1] = true --DOC_HIDE
+            modifiers = {"Mod4"},
---         end},
+            key       = "i",
---     },
+            on_press  = function(self)
---     keybindings = {
+                print("Is now active!", self)
---         {{"Mod4", "Shift"}, "i", function(self)
+                keybinding_works[1] = true --DOC_HIDE
---             print("Called again!")
+            end
---             keybinding_works[3] = true --DOC_HIDE
+        },
---             self:stop()
+    },
---         end},
+    keybindings = {
---     },
+        awful.key {
---     keypressed_callback  = function(_, modifiers, key)
+            modifiers = {"Mod4", "Shift"},
---         print("A key was pressed:", key, "with", #modifiers, "modifier!")
+            key       = "i",
---         keybinding_works[2] = keybinding_works[2] and keybinding_works[2] + 1 or 1 --DOC_HIDE
+            on_press  = function(self)
---     end,
+                print("Called again!")
--- }
+                keybinding_works[3] = true --DOC_HIDE
--- --DOC_NEWLINE
+                self:stop()
--- -- The following will **NOT** trigger the keygrabbing because it isn't exported
+            end
--- -- to the root (global) keys. Adding `export_keybindings` would solve that
+        },
--- require("gears.timer").run_delayed_calls_now() --DOC_HIDE `root_keybindings` is async
+    },
--- root._execute_keybinding({"Mod4", "Shift"}, "i")
+    keypressed_callback  = function(_, modifiers, key)
--- assert(#keybinding_works == 0)
+        print("A key was pressed:", key, "with", #modifiers, "modifier!")
---
+        keybinding_works[2] = keybinding_works[2] and keybinding_works[2] + 1 or 1 --DOC_HIDE
--- --DOC_NEWLINE
+    end,
--- -- But this will start the keygrabber because it is part of the root_keybindings
+}
--- root._execute_keybinding({"Mod4"}, "i")
+--DOC_NEWLINE
--- assert(keybinding_works[1]) --DOC_HIDE
+-- The following will **NOT** trigger the keygrabbing because it isn't exported
--- assert(not keybinding_works[2]) --DOC_HIDE
+-- to the root (global) keys. Adding `export_keybindings` would solve that
---
+require("gears.timer").run_delayed_calls_now() --DOC_HIDE `root_keybindings` is async
--- --DOC_NEWLINE
+root._execute_keybinding({"Mod4", "Shift"}, "i")
--- -- Note that that keygrabber is running, all callbacks should work:
+assert(#keybinding_works == 0)
--- root.fake_input("key_press"  , "a")
+
--- root.fake_input("key_release"  , "a")
+--DOC_NEWLINE
--- assert(keybinding_works[2] == 1) --DOC_HIDE
+-- But this will start the keygrabber because it is part of the root_keybindings
---
+root._execute_keybinding({"Mod4"}, "i")
--- --DOC_NEWLINE
+assert(keybinding_works[1]) --DOC_HIDE
--- -- Calling the root keybindings now wont work because they are not part of
+assert(not keybinding_works[2]) --DOC_HIDE
--- -- the keygrabber internal (own) keybindings, so `keypressed_callback` will
+
--- -- be called.
+--DOC_NEWLINE
--- root._execute_keybinding({"Mod4"}, "i")
+-- Note that that keygrabber is running, all callbacks should work:
--- assert(keybinding_works[2] == 2) --DOC_HIDE because mask_modkeys is set
+root.fake_input("key_press"  , "a")
--- assert(g == awful.keygrabber.current_instance) --DOC_HIDE
+root.fake_input("key_release"  , "a")
--- assert(not keybinding_works[3])--DOC_HIDE
+assert(keybinding_works[2] == 1) --DOC_HIDE
---
+
---
+--DOC_NEWLINE
--- --DOC_NEWLINE
+-- Calling the root keybindings now wont work because they are not part of
--- -- Now the keygrabber own keybindings will work
+-- the keygrabber internal (own) keybindings, so `keypressed_callback` will
--- root._execute_keybinding({"Mod4", "Shift"}, "i")
+-- be called.
--- assert(keybinding_works[3])--DOC_HIDE
+root._execute_keybinding({"Mod4"}, "i")
--- keybinding_works[2] = 0--DOC_HIDE
+assert(keybinding_works[2] == 2) --DOC_HIDE because mask_modkeys is set
--- assert(not awful.keygrabber.current_instance) --DOC_HIDE
+assert(g == awful.keygrabber.current_instance) --DOC_HIDE
--- root.fake_input("key_press"  , "a") --DOC_HIDE
+assert(not keybinding_works[3])--DOC_HIDE
--- root.fake_input("key_release"  , "a") --DOC_HIDE
+
--- assert(keybinding_works[2] == 0) --DOC_HIDE
+--DOC_NEWLINE
-print("Is now active!", "nil")
+-- Now the keygrabber own keybindings will work
-print("A key was pressed:", "a", "with", "0", "modifier!")
+root._execute_keybinding({"Mod4", "Shift"}, "i")
-print("A key was pressed:", "i", "with", "1", "modifier!")
+assert(keybinding_works[3])--DOC_HIDE
-print("Called again!")
+keybinding_works[2] = 0--DOC_HIDE
+assert(not awful.keygrabber.current_instance) --DOC_HIDE
+root.fake_input("key_press"  , "a") --DOC_HIDE
+root.fake_input("key_release"  , "a") --DOC_HIDE
+assert(keybinding_works[2] == 0) --DOC_HIDE

tests/examples/text/awful/keygrabber/vimode.lua

@@ -2,7 +2,8 @@
 
 local gears = {table = require("gears.table")} --DOC_HIDE
 
-local awful = { keygrabber = require("awful.keygrabber") } --DOC_HIDE
+local awful = { keygrabber = require("awful.keygrabber"), --DOC_HIDE
+    key = require("awful.key") } --DOC_HIDE
 
     local map, actions = {
         verbs = {
@@ -44,6 +45,6 @@ local awful = { keygrabber = require("awful.keygrabber") } --DOC_HIDE
         stop_callback = parse,
         stop_key   = gears.table.keys(map.verbs),
         root_keybindings = {
-            {{"Mod4"}, "v"}
+            awful.key({"Mod4"}, "v")
         },
     }

tests/test-keygrabber.lua

@@ -7,11 +7,20 @@ local awful = require("awful")
 local keygrabber_a_active = false
 local keygrabber_b_active = false
 
+-- Disable the deprecation to test both the current and legacy APIs.
+local gdebug = require("gears.debug")
+gdebug.deprecate = function() end
+
 local steps = {
     function()
+
         awful.keygrabber {
             keybindings = {
-                {{}, "a", function() keygrabber_a_active = true end},
+                awful.key {
+                    modifiers = {},
+                    key       = "a",
+                    on_press  = function() keygrabber_a_active = true end
+                }
             },
             stop_key = "Escape",
             stop_callback = function() keygrabber_a_active = false end,