[+CC: Benjamin Stava <[email protected]>]
On Mon, Apr 2, 2012 at 1:30 AM, Uli Schlachter <[email protected]> wrote:
> On 31.03.2012 13:40, Anurag Priyam wrote:
>> Fixed a bug.
>
> I diff'd the two patches. I wouldn't quite call that "a bug", but ok. :-P
Left that way, it would have become a bug :P.
>> +-- <p>Switch to a client matching the given condition if running, else
>> spawn it.
>
> This tag is never closed?
>
>> +-- If multiple clients match the given condition then the next one is
>> focussed.
>> +--
>> +-- @param cmd the command to execute
>> +-- @param matcher a function that returns true to indicate a matching client
>> +-- @param merge if true then merge tags when clients are not visible
>> +--
>> +-- @usage run or raise urxvt (perhaps, with tabs) on modkey + semicolon
>> +-- <p><code>
>
> Neither does this '<p>' ever get the joy of being matched by a kind and
> friendly
> '</p>'.
That was intentional, since all browsers gracefully handle unclosed
paragraphs. I will change it, nevertheless.
>> +-- awful.key({ modkey, }, 'semicolon', function () <br/>
>> +-- local cmd = 'urxvt' <br/>
>
> I don't really care, but does this need a local var? Can't it just be given
> inline in the argument to run_or_raise()?
Yeah, of course. Don't know why I use another variable for it.
>> +-- local matcher = function (c) <br/>
>> +-- return awful.rules.match(c, {class = 'URxvt'}) <br/>
>> +-- end <br/>
>> +-- awful.client.run_or_raise(cmd, matcher)
>> +-- end);
>> +-- </code>
>> +function run_or_raise(cmd, matcher, merge)
>> + local clients = capi.client.get(s)
>
> This 's' is falling out of thin air? Didn't you mean to have that as an
> argument
> to this function? (Or just always as nil?)
Copy paste error from awful.client.cycle :P.
>> + local findex = util.table.hasitem(clients, capi.client.focus) or 1
>> + local start = util.cycle(#clients, findex + 1)
>> +
>> + for c in cycle(matcher, start) do
>
> If you use 's' above to calculate the index, you should pass in 's' here, too.
> Also, I didn't know we had such a function, nice find.
Huh? I added that function only few days back, specifically for this.
How can you forget it so soon? Btw, I just realized that I had made
a mistake in naming it cycle: there is another function with the same
name (around line 387) that swaps clients in a cyclic fashion. Why is
it needed? Maybe we should call my function 'iterate'?
>> + jumpto(c, merge)
>> + return
>> + end
>> +
>> + -- client not found, spawn it
>> + util.spawn(cmd)
>
> I wonder if this should be turned into a callback instead? Well, at first I
> was
> thinking about someone needing spawn_with_shell, then I thought about that.
I don't think we need the flexibility provided by a callback here.
> Hm, on second thought (actually, third) I don't think that 'spawn_with_shell'
> thingie is a valid use case and if someone asks for it, he will get told to
> use
> bash -c.
Actually, let's make it make it spawn_with_shell instead: better than
documenting or answering on the ML a thousand times that one can use
'bash -c'. And we just found a case in Benjamin's mail later in this
thread.
Gotta run for class now. Will send in the modified patch later.
--
Anurag Priyam
--
To unsubscribe, send mail to [email protected].
