On Sun, Jul 8, 2012 at 4:31 AM, Michael Stapelberg <[email protected]> wrote:
> Hi DR, > > Quoting DR (2012-07-07 17:45:52) > > Since I failed to convince Michael to accept my newer patch, I decide to > drop > > those more "advanced" patch, and try to push this simple patch into i3. > Thanks for updating your patch. Here are my comments: > > 1) You are not using FREE(last_urgent_ws);, but free(last_urgent_ws) in > line 1489, which is unsafe (it is not guaranteed that last_urgent_ws > is != NULL). > > 2) You are using strdup() instead of sstrdup() in line 1490, so you > don’t handle -EMEMORY. > > 3) Instead of using if (last_urgent_ws) free(last_urgent_ws); you should > use FREE(last_urgent_ws) in line 1526. > > 4) Your patch doesn’t actually work when the user is not navigating away > from the workspace using the keyboard, but when he is focusing the > urgent window with the mouse. i3bar will properly update the urgency > indicator for that workspace, but it won’t hide. > This behavior is intended, so user can easily (using only his mouse) navigate back to previous workspace before the urgency happen. > > I’d suggest to save a flag which is set to true when i3bar calls > unhide_bars() due to an urgent workspace. Then, check if that flag is > set, but no more urgent workspaces are found (!has_urgent && > !mod_pressed && hidden_due_to_urgency) and call hide_bars(). > > Best regards, > Michael >
