On Wed, Feb 29, 2012 at 3:59 PM, Uli Schlachter <psyc...@znc.in> wrote:
> On 26.02.2012 05:56, Anurag Priyam wrote:
[...]
>> Here is the correct patch, tested on both values (keygrabber =
>> true/false) this time. But before this patch is applied, we should
>> perhaps consider removing keygrabber support from menu instead.
>
> So should I merge this or not? Sending a patch with the words "but before this
> patch is applied" feels wrong (but has it's uses...).
Right. I am sorry. I wanted to know your take on it first :).
>> Optionally allowing keygrabbers adds to the complexity of the code.
>> And for what gains? Without keygrabber enabled, you can't even close
>> the menu by pressing 'Escape' key (this is what prompted me to change
>> the default to true).
>
> I guess I wouldn't mind. If it simplifies the code a lot, I'm all settled for
> this. However, I don't really have the time to look closely into this.
Patch attached. If you like it, let's scrap the previous one
(keygrabber = true by default).
> Anyone out there who opens up a popup menu, then clicks somewhere else and
> expects the menu to stay open? If yes, speak up now or stay quiet forever!
Yeah, this is another thing that annoys me. Though I am not sure how
I can fix it. I need to determine that mouse was clicked outside
menu. So maybe I can attach a handler to 'press' signal on mouse, but
if it was not clicked on any visible menu (not sure how to determine
that), then close it?
--
Anurag Priyam
From 4986efeb759903637c5760ac8e69f6fa2c53d491 Mon Sep 17 00:00:00 2001
From: Anurag Priyam <anurag08pri...@gmail.com>
Date: Thu, 1 Mar 2012 14:12:56 -0500
Subject: [PATCH] awful.menu: always enable keyboard navigation
I don't see why people would not want keyboard-enabled-menu by default.
Without it, you can't even use 'Escape' to quit the menu or press 'Enter' to
execute an entry. But instead of just enabling keyboard support by default, we
remove the option of disabling keyboard support altogether, which also
simplifies the implementation a bit.
Signed-off-by: Anurag Priyam <anurag08pri...@gmail.com>
---
awesomerc.lua.in | 2 +-
lib/awful/menu.lua.in | 24 ++++--------------------
2 files changed, 5 insertions(+), 21 deletions(-)
diff --git a/awesomerc.lua.in b/awesomerc.lua.in
index 3929d9f..ceac430 100644
--- a/awesomerc.lua.in
+++ b/awesomerc.lua.in
@@ -210,7 +210,7 @@ globalkeys = awful.util.table.join(
awful.client.focus.byidx(-1)
if client.focus then client.focus:raise() end
end),
- awful.key({ modkey, }, "w", function () mymainmenu:show({keygrabber=true}) end),
+ awful.key({ modkey, }, "w", function () mymainmenu:show() end),
-- Layout manipulation
awful.key({ modkey, "Shift" }, "j", function () awful.client.swap.byidx( 1) end),
diff --git a/lib/awful/menu.lua.in b/lib/awful/menu.lua.in
index 3e8566f..3f3fea0 100644
--- a/lib/awful/menu.lua.in
+++ b/lib/awful/menu.lua.in
@@ -316,26 +316,16 @@ end
--- Show a menu.
-- @param menu The menu to show.
--- @param args.keygrabber A boolean enabling or not the keyboard navigation.
-- @param args.coords Menu position defaulting to mouse.coords()
function show(menu, args)
args = args or {}
local coords = args.coords or nil
local screen_index = capi.mouse.screen
- local keygrabber = args.keygrabber or false
if not set_size(menu) then return end
set_coords(menu, screen_index, coords)
- if menu.parent then
- menu.keygrabber = menu.parent.keygrabber
- elseif keygrabber ~= nil then
- menu.keygrabber = keygrabber
- else
- menu.keygrabber = false
- end
-
- if not cur_menu and menu.keygrabber then
+ if not cur_menu then
capi.keygrabber.run(grabber)
end
cur_menu = menu
@@ -358,7 +348,7 @@ function hide(menu)
if cur_menu == menu then
cur_menu = cur_menu.parent
end
- if not cur_menu and menu.keygrabber then
+ if not cur_menu then
capi.keygrabber.stop()
end
menu.wibox.visible = false
@@ -366,7 +356,6 @@ end
--- Toggle menu visibility.
-- @param menu The menu to show if it's hidden, or to hide if it's shown.
--- @param args.keygrabber A boolean enabling or not the keyboard navigation.
-- @param args.coords Menu position {x,y}
function toggle(menu, args)
if menu.wibox.visible then
@@ -380,9 +369,7 @@ end
-- @param menu The mnenu to update.
function update(menu)
if menu.wibox.visible then
- menu:show({
- keygrabber = menu.keygrabber,
- coords = { x = menu.x, y = menu.y } })
+ menu:show({ coords = { x = menu.x, y = menu.y } })
end
end
@@ -488,7 +475,6 @@ end
--- Build a popup menu with running clients and shows it.
-- @param menu Menu table, see new() function for more informations
--- @param args.keygrabber A boolean enabling or not the keyboard navigation.
-- @return The menu.
function clients(menu, args) -- FIXME crude api
menu = menu or {}
@@ -644,9 +630,7 @@ end
-- } <br/>
-- end <br/>
-- end <br/>
--- m = awful.menu(terms) <br/>
--- m:show({keygrabber=true}) <br/>
--- return m <br/>
+-- awful.menu(terms):show() <br/>
-- end <br/>
--</code></p>
function new(args, parent)
--
1.7.5.4
