On 15.02.2012 23:22, Alexander Yakushev wrote:
> OK, then here's the "first round". I've put the whole extension into the
> separate lib/menubar directory and modified existing lib/awful/prompt.lua.
> Things I changed inside prompt.lua:
> - introduced two more arguments to run() function - change_callback and
> keypressed_callback
> * keypressed_callback is a way to catch key combinations before
> awful.prompt processes them. It is necessary because I want to catch
> certain combinations (like Left/Right arrows o) and do different actions
> for them.
> Keypressed callback also acts as a way to tell awful.prompt how
> to change the current command and prompt.
> * change_callback should be called any time when the key combination
> was pressed, no matter if it was caught by keypressed_callback or not.
> It is vital because I want to filter the entries list based on user
> input each time user types in a new symbol.
> - moved the update() function (inside keygrabber.run) higher in the
> code. Nothing important.
I think these changes to awful.prompt should be split out into their own
commits. If you want to get fancy, it could be done as three separate commits
(move update, add the two new arguments).
>From a quick first look, these changes look okish.
> +-- Options section
> +cache_entries = true
> +show_categories = true
> +g = { width = nil,
> + height = 20,
> + x = nil,
> + y = nil }
More documentation! (Also: Is 'g' really a good name?)
> +-- Private section
> +current_item = 1
> +previous_item = nil
> +current_category = nil
> +shownitems = nil
> +
> +instance = { prompt = nil,
> + widget = nil,
> + wibox = nil }
> +
> +common_args = { w = wibox.layout.fixed.horizontal(),
> + data = setmetatable({}, { __mode = 'kv' }) }
Make these local
> +-- Get how the menu item should be displayed.
> +-- @param o The menu item.
> +-- @return item name, item background color, nil, item icon.
> +local function label(o)
Instead of "nil", I'd prefer if this said "background image". The fact that it
always returns nil doesn't belong into the documentation.
> +local function label(o)
> + if o.focused then
> + return
> + colortext(o.name, awful.util.color_strip_alpha(theme.fg_focus)) or
> o.name,
> + theme.bg_focus, nil, o.icon
> + else
That long line looks fishy. What is the "or o.name" supposed to do? colortext()
will never return false nor nil, instead it causes lua errors if it gets some
"evil" arguments.
Also, I think this should be split up into multiple lines:
if o.focused then
local color = awful.util.color_strip_alpha(theme.fg_focus)
local name = colortext(o,name, color) or o.name
else
> +--- Awful.prompt keypressed callback to be used when the user presses a key.
> +-- @param mod Table of key combination modifiers (Control, Shift).
> +-- @param key The key that was pressed.
> +-- @param comm The current command in the prompt.
> +-- @return if the function processed the callback, new awful.prompt command,
> new awful.prompt prompt text.
> +function prompt_keypressed_callback(mod, key, comm)
I think this should be local. Can anyone use this function for his own stuff?
> +function show(scr)
> + if not instance.wibox then
> + initialize(scr)
That function takes no arguments.
> +-- Generate a pattern matching expression that ignores case.
> +-- @param s Original pattern matching expresion.
> +local function nocase (s)
> + s = string.gsub(s, "%a",
> + function (c)
> + return string.format("[%s%s]", string.lower(c),
> + string.upper(c))
> + end)
> + return s
> +end
I can stand colortext(), but this really does not belong into this module. Also,
I'm worried about the amount of dark magic in this function.
And since nocase() is only used on a user-specified pattern, I think this can be
removed completely.
> +--- Update the menubar according to the command inputed by user.
> +-- @param query The text to filter entries by.
> +function menulist_update(query)
I would replace "inputed" by "entered". Also, this feels like it is another
candidate for "local".
> + prompt.run({ prompt = "Run app: ", bg_cursor = "#222222" },
> instance.prompt.widget, function(s) end,
> + nil, awful.util.getdir("cache") .. "/history_menu", nil, hide,
> + menulist_update,
> + prompt_keypressed_callback)
Why the 'function(s) end'? This is the completion_callback argument which can be
nil.
> +--- Create a new menubar object.
> +-- @return menubar object.
> +function new()
> + if app_folders then
> + menu_gen.all_menu_dirs = app_folders
> + end
> + refresh()
> + -- Load categories icons and add IDs to them
> + for i, v in ipairs(menu_gen.all_categories) do
> + v.cat_id = i
> + end
> + return common_args.w
> +end
This doesn't create any new object at all. Instead it does various
initializations and then returns a pre-existing widget. Shouldn't this better be
called 'get()'?
> --- /dev/null
> +++ b/lib/menubar/menu_gen.lua.in
> @@ -0,0 +1,113 @@
> +-- Originally written by Antonio Terceiro
> +-- https://github.com/terceiro/awesome-freedesktop
> +-- Hacked by Alex Y. <[email protected]>
At first I wanted to complain about the non-standard header. Then I checked the
URL. No way. The code there doesn't have any copyright headers, there are no
licenses attached. You are submitting someone else's code and modify the license
in the process (all of awesome is GPLv2). This is a copyright violation.
[Didn't look at this file any further]
> --- /dev/null
> +++ b/lib/menubar/utils.lua.in
> @@ -0,0 +1,154 @@
> +---------------------------------------------------------------------------
> +-- @author Antonio Terceiro
> +-- @copyright 2009 Antonio Terceiro
> +-- @release @AWESOME_VERSION@
> +---------------------------------------------------------------------------
Not your name either, so same problem. This time it doesn't even say were this
file is from.
[Didn't look at this file either]
So could someone make Antonio add proper license information to his stuff? If
you really want to make me really happy, make him submit his own code himself.
That way git will track him as the author, too.
Please note: I haven't actually tried running any of this. Instead, I just
looked at the diff. I'm also not yet completely sure what this actually does.
How about some more words for the commit message? Perhaps even add some luadoc
in front of the module("menubar") line.
Cheers,
Uli
--
my $key = "\x49\x03\x93\x08\x19\x94\x96\x94\x28\x93\x83\x04\x68\x28\xa8\xf5".
"\x0a\xb9\x94\x02\x45\x81\x93\x1f\xbc\xd7\xf3\xad\x93\xf5\x32\x93";
my $cipher = Crypt::Rijndael->new( $key, Crypt::Rijndael::MODE_ECB() );
my $plain = $ciper->decrypt($daten);
--
To unsubscribe, send mail to [email protected].
