-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
Andrei Thorp wrote:
> I wanna get my patches reviewed dammit ;)
And I want to get done reviweing them, dammit.
> Subject: [PATCH 4/4] basic_mpd: Imported
> diff --git a/basic_mpd/init.lua b/basic_mpd/init.lua
> new file mode 100644
> index 0000000..6df6727
> --- /dev/null
> +++ b/basic_mpd/init.lua
[snip]
> +local defaults = {
> + format = "$title - $album - $artist",
> + length = 75,
> + unknown = "(unknown)",
> +}
> +local settings = {}
> +for key, value in pairs(defaults) do
> + settings[key] = value
> +end
Uhm... why don't you just put the defaults in settings and let the user override
them?
> +local widget = widget({ type = "textbox",
> + name = "mpd-playing",
> + align = "left" })
"align"? Wasn't that the funny thing which was teared, feathered, shot, buried
and exploded by farhaven? (and then he started to *really* hurt it)
"name"? If "align" is a zombie, "name" never was alive in the first place,
right?
> +-- Utility function to handle the text for MPD
> +-- @param songinfo: a table with fields "artist", "album", "title" in text
> +-- @return formatted (settings.format) string to display on the widget. This
> +-- respects settings.length and tries to make fields as close to the same
> +-- lenghths as possible if shortening is required.
^^^^^^^^
lengths
> +local function format_metadata(songinfo)
> + format = settings.format or defaults.format
> +
> + if (settings.length or defaults.length) <= 0 then
> + return ""
> + end
> +
> + used_keys = {}
> + start = 1
> + stop = 1
> + while start do
> + start, stop = string.find(format, "%$%w+", stop)
> + key = string.match(format, "%$(%w+)", stop)
> + if key then
> + if songinfo[key] then
> + used_keys[key] = songinfo[key]
> + else
> + used_keys[key] = settings.unknown or
> + defaults.unknown
> + end
> + end
> + end
Ah, the joy of doing string-processing in Lua...
I don't remember the details, but there was some lua function which let you do
for match in <here's the part that I forgot> do
I *think* it might have been :gmatch() (for match in format:gmatch("<dunno>")?)
> + retval = ""
> + while true do
> + retval = string.gsub(format, "%$(%w+)", used_keys)
> + if #retval > (settings.length or defaults.length) then
> + longest_key = nil
> + longest_value = ""
> + for key, value in pairs(used_keys) do
> + if #value > #longest_value then
> + longest_key = key
> + longest_value = value
> + end
> + end
> + if longest_key then
> + -- shorten the longest by 1
> + used_keys[longest_key] = string.sub(
> used_keys[longest_key],
> + 1,
> + #longest_value - 1)
> + else
> + -- Seems like the format itself's too long
> + err = "obvious.basic_mpd: impossible to fit
> " ..
> + "output into " .. (settings.length or
> + defaults.length) .. " characters.\n"
> ..
> + "Widget paused."
> + naughty.notify({ text = err, timeout = 0 })
> + lib.hooks.timer.stop(update)
> + return ""
> + end
> + else
> + -- All good!
> + break
> + end
> + end
> + return awful.util.escape(retval)
> +end
I haven't even tried to understand it, looks awful. :(
> +-- Updates the widget's display
> +function update()
> + local status = lib.mpd.send("status")
> + local now_playing, songstats
> +
> + if not status.state then
> + now_playing = "Music Off"
> + now_playing = lib.util.colour("yellow", now_playing)
You misspelled "color" *duck*
> + elseif status.state == "stop" then
> + now_playing = "Music Stopped"
> + else
> + songstats = lib.mpd.send("playlistid " .. status.songid)
Heh, my "mpd lib" can do that as-well... The do-it-yourself lib.
> + format = settings.format or defaults.format
> + if type(format) == "string" then
> + now_playing = format_metadata(songstats)
> + elseif type(format) == "function" then
> + now_playing = format(songstats)
> + else
> + naughty.notify({ text = "obvious.basic_mpd: Invalid
> " ..
> + "display format. Widget " ..
> + "paused." })
> + lib.hooks.timer.stop(update)
> + end
> + end
> +
> + widget.text = now_playing
> +end
> +update()
> +lib.hooks.timer.register(1, 30, update, "basic_mpd widget refresh rate")
> +
> +-- SETTINGS
[snip, lots of functions here]
Uhm, why? Just let the user modify settings directly. If he wants to shoot it
into his foot, let him.
> diff --git a/basic_mpd/readme b/basic_mpd/readme
> new file mode 100644
> index 0000000..20d2d29
> --- /dev/null
> +++ b/basic_mpd/readme
skipped this, I'm better at critizing code than free text. (Ask farhaven, he
gotta do sth too :P)
> diff --git a/lib/mpd/init.lua b/lib/mpd/init.lua
> new file mode 100644
> index 0000000..d1f0e35
> --- /dev/null
> +++ b/lib/mpd/init.lua
> @@ -0,0 +1,160 @@
> +-- Small interface to MusicPD
> +-- based on a netcat version from Steve Jothen <sjothen at gmail dot com>
> +-- (see http://github.com/otkrove/ion3-config/tree/master/mpd.lua)
> +--
> +-- Copyright (c) 2008-2009, Alexandre Perrin <[email protected]>
> +-- All rights reserved.
> +--
> +-- Redistribution and use in source and binary forms, with or without
> +-- modification, are permitted provided that the following conditions
> +-- are met:
> +--
> +-- 1. Redistributions of source code must retain the above copyright
> +-- notice, this list of conditions and the following disclaimer.
> +-- 2. Redistributions in binary form must reproduce the above copyright
> +-- notice, this list of conditions and the following disclaimer in the
> +-- documentation and/or other materials provided with the distribution.
> +-- 4. Neither the name of the author nor the names of its contributors
> +-- may be used to endorse or promote products derived from this software
> +-- without specific prior written permission.
> +--
> +-- THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
> +-- ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> +-- IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> +-- ARE DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
> +-- FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
> +-- DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
> +-- OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> +-- HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
> +-- LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
> +-- OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> +-- SUCH DAMAGE.
[snip, haven't looked closely at the stuff after this]
This screams "GPL INCOMPATIBLE". Ask jd if awesome modules have to be
GPL-compatible, I'm not sure what the GPL says about scripting languages like
lua.
Uli
- --
"Do you know that books smell like nutmeg or some spice from a foreign land?"
-- Faber in Fahrenheit 451
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
iQEcBAEBCAAGBQJKcaxgAAoJECLkKOvLj8sG3ycIAJ4+u30RFhbdyTzq/IjQQG7w
7L2DXPpIQJrEHPUT9VVZ7RNzI5uwKaDKlD14rli/Zd5hDR1DXWaLul4Xlzyy2iGm
sRokrZaRJPL7W0kU0MNDJUxQY3r5kwAtwOeVHgIzsSQvtP2VndkXqL8NtGob9EGv
GR5DJhY6PLgZCZCRzrNhUplEBWL4rsKEaKM08sI9IqwLRMdSxv69kemA3FG+qYmF
ukm/PUWqIre/Mh9+1EZlO3bGVTdhg25iHwQk491IQBKwDW401QAw1KfhSq0PcDys
GyD9fbOvPWFIDneT7ett1GVvnQWWtLpOQqJMfzQUo+PUYlDSHWgZ0VytZWjs1Cc=
=PbKr
-----END PGP SIGNATURE-----
--
To unsubscribe, send mail to [email protected].
