Fix awful.keygrabber:add_keybinding function (#3568)

* Check the correct variable

* Fix table access

In every other use of _private.keybindings in this file, the key is a
string, not an awful.key

* Simplify code

key.key is always defined

* Add tests

* add(spec) add_keybinding unit test

* Revert "Add tests"

This reverts commit 808b17cd5c.

Co-authored-by: Aire-One <aireone@aireone.xyz>

lib/awful/keygrabber.lua

@@ -509,7 +509,7 @@ end
 -- @tparam string description.group The keybinding group
 
 function keygrabber:add_keybinding(key, _keycode, _callback, _description)
-    local mods = not akey._is_awful_key and akey or nil
+    local mods = not key._is_awful_key and key or nil
 
     if mods then
         gdebug.deprecate(":add_keybinding now takes `awful.key` objects instead"
@@ -523,12 +523,10 @@ function keygrabber:add_keybinding(key, _keycode, _callback, _description)
             description = _description,
             on_press    = _callback
         }
-    else
-        _keycode = key.key
     end
 
-    self._private.keybindings[key] = self._private.keybindings[key] or {}
-    table.insert(self._private.keybindings[_keycode], key)
+    self._private.keybindings[key.key] = self._private.keybindings[key.key] or {}
+    table.insert(self._private.keybindings[key.key], key)
 
     if self.export_keybindings then
         add_root_keybindings(self, {key})

spec/awful/keygrabber_spec.lua

@@ -0,0 +1,39 @@
+local gtable = require "gears.table"
+
+describe("awful.keygrabber", function()
+   package.loaded["gears.timer"] = {}
+   package.loaded["awful.keyboard"] = {
+      append_global_keybinding = function() end,
+   }
+   _G.awesome = gtable.join(_G.awesome, { connect_signal = function() end })
+   _G.key = {
+      set_index_miss_handler = function() end,
+      set_newindex_miss_handler = function() end,
+   }
+
+   local akeygrabber = require "awful.keygrabber"
+
+   local kg = nil
+   local fake_key = {
+      _is_awful_key = true,
+      key = {},
+   }
+
+   before_each(function()
+      kg = akeygrabber {
+         keybindings = {},
+         stop_key = "Escape",
+         stop_callback = function() end,
+         export_keybindings = true,
+      }
+   end)
+
+   -- issue #3567: add_keybinding fail when called with an `awful.key` instance
+   it("awful.keygrabber:add_keybinding() doesn't throw error", function()
+      kg:add_keybinding(fake_key)
+
+      -- dummy test that should be trusty if add_keybinding doesn't thow an exception
+      -- (if add_keybinding fails, we will not reach this line anyway...)
+      assert(kg, "kg is nil")
+   end)
+end)

spec/preload.lua

@@ -1,10 +1,13 @@
 -- This script is given to Busted via the --helper argument. Modules loaded here
 -- won't be cleared and reloaded by Busted. This is needed for lgi because lgi
 -- is not safe to reload and yet Busted manages to do this.
-require("lgi")
+require "lgi"
 
 -- Always show deprecated messages
-_G.awesome = {version = "v9999"}
+_G.awesome = {
+   version = "v9999",
+   api_level = 9999,
+}
 
 -- "fix" some intentional beautiful breakage done by .travis.yml
 require("beautiful").init { a_key = "a_value" }