On Sun, Feb 15, 2009 at 12:14 PM, Viktor Kojouharov
<vkojouha...@gmail.com> wrote:
> Hi guys,
>
> Since my computer burned to the ground, I will be out of one for at
> least 2-3 weeks. But I have this nice looking patch, which adds edge and
> corner binding possibilities to E. Since I won't be able to commit it if
> raster approves it when he returns, I'd like to ask one of you to commit
> it in my stead.
>
> Now for the patch itself. The edge.5.diff contains all the needed stuff
> for this to work, besides the config dialog edj icon, which I hope toma
> will create (I just copied the one from conf_keybindings and renamed it
> so it would compile).
>
> It adds another dialog to the input section of the config panel. With
> this dialog, one can bind actions to any edge or corner, and specify a
> delay and modifiers for it. The default bindings (if you click restore
> to default), is to make the desks flip in the direction of the
> edge/corner with 0.3s delay. This is the same behaviour that was
> previously set by default through the virtual desks config, and the
> later was removed since it is not needed anymore.
>
> It also adds e_bindings_edge* set of functions to deal with the actual
> bindings, the special flip_in_direction action, config bindings struct
> and a major cleanup to the E_Zone struct and functions. A lot flags are
> removed since they are not needed anymore, as well as some functions
> that made use of these flags. The long-standing annoyance where dragging
> something to a hidden shelf does not show it is also addressed.
>
> The second file is a patch for the configs that ship with E. It's a
> separate file since I have NOT tested it (due to my laptop's unfortunate
> meltdown). It should work though.

Hi Viktor,

I did review your patch and it looks good, I'd just say you should use
EINA_LIST_FOREACH() whenever possible and that eina_stringshare is
safe to operate with NULL, so no need to check for null before calling
add or del.

One issue is:

+_modify_edge_binding_cb(void *data, void *data2)
...
+       int n;
+
+       sscanf(cfdata->locals.cur, "e%d", &n);

sscanf may fail, in this case it will return 0 and n will be
undefined. Either init n=0 or check return and abort, that seems more
reasonable. Also happens with _delete_edge_binding_cb and
_update_action_list... search for sscanf()

I have not studied the problem in depth, but I'd say that we should
try to avoid lots of list lookups whenever possible. As I said, I
don't really know if it is possible, but if we can add some extra
points to avoid walking lists so many times,  then let's do it.

Patch is really big so maybe I missed something, if others could
review it I'd appreciate.

Regards,

-- 
Gustavo Sverzut Barbieri
http://profusion.mobi embedded systems
--------------------------------------
MSN: barbi...@gmail.com
Skype: gsbarbieri
Mobile: +55 (19) 9225-2202

------------------------------------------------------------------------------
Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA
-OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise
-Strategies to boost innovation and cut costs with open source participation
-Receive a $600 discount off the registration fee with the source code: SFAD
http://p.sf.net/sfu/XcvMzF8H
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel

Reply via email to