On Fri, Mar 2, 2012 at 1:20 PM, Uli Schlachter <[email protected]> wrote: > However, I successfully started a program with the prompt (via pressing enter > and all that) and still had this problem. This is the part which I still don't > understand.
I can't reproduce it. >> awful.prompt is quite messy that way: a giant (>200 lines) of >> callback to `keygrabber.run` with too many return true and false. > > The "return true" would just be replaced by "return" and I only count a single > "return false". I don't say this code is great, but the patch wouldn't be all > that bad. Hmm. I gave it a shot but it there still remains a glitch (see 2nd patch): When I close the prompt, either by executing a command or by pressing 'Escape, the keygrabber stops but the prompt with its text remains stuck up there :-/. >> That's why I have been postponing a patch to replace all `return >> false` with `keygrabber.stop`. I wonder if a better approach would be >> to do something like: >> >> capi.keygrabber.run(function () >> if not grabber() then capi.keygrabber.stop() >> end >> >> Where `grabber` is the old callback. But if I have to define >> `grabber` before `run`, the diff will probably be very ugly: a giant >> code block moved from here to there :-|. >> >> What do you think? > > If it's messy, clean it up. ;-) I will see if I can :). > "The diff would be big" is a bad excuse for not doing something IMHO. However, > this "if" feels like a big hack saying "I changed the API, but on second > thought > I'd rather have the old one again". Big diff wasn't an excuse. Just that I try to avoid doing anything that makes it difficult for me to get some info out of git later. For example, I can't determine properly (or efficiently) which commit changed a given code block if it has been moved around or changed indentation a couple of times. Perhaps I just don't know better :-|. I would see that if hack as, "I changed the API, but I don't want to use it in my existing code yet for practical reasons (messy code, it may break, I don't have time), so I will temporarily emulate the old behavior with a simple wrapper" :). > I'm playing my usual "sorry, no time to code this myself"-card and return to > doing non-awesome stuff. (Yes, I do mean both meanings of this sentence) What else takes up your time? Just curious. College? Job? Apart from my undegrad studies, I work on some Comp. Bio. stuff when I get time. -- Anurag Priyam
From b30a297bf7459cc2e7b042e9071484abe4c024c1 Mon Sep 17 00:00:00 2001 From: Anurag Priyam <[email protected]> Date: Thu, 1 Mar 2012 15:43:29 -0500 Subject: [PATCH 1/2] awful.menu: remove superfluous return true from keygrabber's callback Returning true from the callback to `keygrabber` is no longer necessary to continue grabbing the keyboard. Signed-off-by: Anurag Priyam <[email protected]> --- lib/awful/menu.lua.in | 6 +----- 1 files changed, 1 insertions(+), 5 deletions(-) diff --git a/lib/awful/menu.lua.in b/lib/awful/menu.lua.in index 3f3fea0..8dd1763 100644 --- a/lib/awful/menu.lua.in +++ b/lib/awful/menu.lua.in @@ -194,9 +194,7 @@ end local function grabber(mod, key, event) - if event == "release" then - return true - end + if event ~= "press" then return end local sel = cur_menu.sel or 0 if util.table.hasitem(menu_keys.up, key) then @@ -216,8 +214,6 @@ local function grabber(mod, key, event) else check_access_key(cur_menu, key) end - - return true end -- 1.7.5.4
From 92d8d4ac6e3431107e35f746a15f9dbea5938aab Mon Sep 17 00:00:00 2001 From: Anurag Priyam <[email protected]> Date: Sun, 4 Mar 2012 14:30:23 -0500 Subject: [PATCH 2/2] awful.prompt: do not return a boolean to talk to keygrabber Keygrabber no longer entertains boolean value from its callback to continue or stop grabbing. It continues grabbing till `keygrabber.stop()` is explicitly called, so we remove superfluous `return true` and replace `return false` with `keygrabber.stop()` Signed-off-by: Anurag Priyam <[email protected]> --- lib/awful/prompt.lua.in | 13 +++---------- 1 files changed, 3 insertions(+), 10 deletions(-) diff --git a/lib/awful/prompt.lua.in b/lib/awful/prompt.lua.in index c398fc9..e29061e 100644 --- a/lib/awful/prompt.lua.in +++ b/lib/awful/prompt.lua.in @@ -204,7 +204,7 @@ function run(args, textbox, exe_callback, completion_callback, history_path, his capi.keygrabber.run( function (modifiers, key, event) - if event ~= "press" then return true end + if event ~= "press" then return end -- Convert index array to hash table local mod = {} for k, v in ipairs(modifiers) do mod[v] = true end @@ -213,14 +213,11 @@ function run(args, textbox, exe_callback, completion_callback, history_path, his or (not mod.Control and key == "Escape") then textbox:set_markup("") if done_callback then done_callback() end - return false + capi.keygrabber.stop() elseif (mod.Control and (key == "j" or key == "m")) or (not mod.Control and key == "Return") or (not mod.Control and key == "KP_Enter") then exec() - -- We already unregistered ourselves so we don't want to return - -- true, otherwise we may unregister someone else. - return true end -- Control cases @@ -312,7 +309,7 @@ function run(args, textbox, exe_callback, completion_callback, history_path, his if completion_callback then if key == "Tab" or key == "ISO_Left_Tab" then if key == "ISO_Left_Tab" then - if ncomp == 1 then return true end + if ncomp == 1 then return end if ncomp == 2 then command = command_before_comp textbox:set_font(font) @@ -320,7 +317,6 @@ function run(args, textbox, exe_callback, completion_callback, history_path, his text = command_before_comp, text_color = inv_col, cursor_color = cur_col, cursor_pos = cur_pos, cursor_ul = cur_ul, selectall = selectall, prompt = prettyprompt }) - return true end ncomp = ncomp - 2 @@ -335,7 +331,6 @@ function run(args, textbox, exe_callback, completion_callback, history_path, his -- execute if only one match found and autoexec flag set if matches and #matches == 1 and args.autoexec then exec() - return true end else ncomp = 1 @@ -429,8 +424,6 @@ function run(args, textbox, exe_callback, completion_callback, history_path, his cur_pos = cur_pos - 1 success = pcall(update) end - - return true end) end -- 1.7.5.4
