On Fri, May 21, 2010 at 5:54 AM, Julien Danjou <[email protected]> wrote:
> On Fri, May 21 2010, Daniel Graña wrote: > > > This is my first attempt to ideal patch, please review it at > > http://gist.github.com/408472 > > 1. You should fix the indentation to respect the one used in awesome; > Agree, but I tried hard to follow it, where I failed? > 2. In systray_init(): > screen.systray.registered = false; > is useless (it's false by default); > Done, I preferred to initialize just in case, but you are right here. > 3. You should not do: > screen.systray.registered = true; > in wibox_systray_refresh(), but rather in systray_register(). > Much clear. Done. > This is just my opinion, feel free to correct me if you think I'm wrong. > The patch would need some volunteer to test it, but it seems ok and > mergeable to me. > Cool, latest patch http://gist.github.com/409014 I did the changes you suggested and reused the "screent_t screen" variable where possible. > > Do you plan to make an extra patch to unregister the systray when the > widget disappear ? > Prefer to do it in one patch, but can't find what is the best place to unregister systray. What call systray widget destructor? May be in wibox_systray_refresh and unregister if no systray is found. Thanks, Daniel.
