On Sun, Apr 13, 2008 at 01:03:35PM -0300, Gustavo Sverzut Barbieri wrote:
> On Sun, Apr 13, 2008 at 7:17 AM, Lars Munch <[EMAIL PROTECTED]> wrote:
> > Hi
> >
> > Attached patch adds three new functions to edje/embryo, namely
> > part_raise, part_lower and get_mouse_buttons.
>
> I'd separate get_mouse_buttons() from the others. First of all, they
> have nothing to do
Your right, I will split the patch up in two.
> second part_*() might be dangerous to use in Edje
> parts, I can't see any problem, but I never thought about it too much,
> let's wait for raster or someone else to opine here. By having them
> separated, the mouse_buttons get in ASAP and the rest will wait for
> revision and might not be applied.
>
> As for the implementation, it's not good:
> 1) lacks get_mouse_buttons() in "docs"
Do you mean in the long list of exported functions at the top of
edje_embryo.c? In that case get_mouse_buttons() is already there. I
guess someone meant to implement it, but never got around to do it.
> 2) part_*() does not operate on the given part_id:
>
> +static Embryo_Cell
> +_edje_embryo_fn_part_raise(Embryo_Program *ep, Embryo_Cell *params)
> +{
> + int part_id = 0;
>
> no initialization should be done here.
>
> + ed = embryo_program_data_get(ep);
> +
> + evas_object_raise(ed->obj);
>
> where do you use part_id? no where. Maybe you tried to base your code
> on other calls that get part_id and they use just like that. But
> notice that they use "edje_object_*()" functions, that get an Edje
> object and a part name, so they use part_id to get a rp->part->name.
> Since the function you want have no edje_object_*() equivalent, then
> you must get the object itself. And not having the
> edje_object_part_raise() or edje_object_part_lower() is what make me
> wonder if this would break things or not. If they won't break things,
> then we should add these and use them from embryo.
Yes, it might break things. I just found this comment in the docs for
edje_object_part_object_get() "You should never modify the state of the
returned object (with evas_object_move() or evas_object_hide() for
example)". I will dig a little deaper and see what I can find.
Thanks for the review.
-- Lars Munch
-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference
Don't miss this year's exciting event. There's still time to save $100.
Use priority code J8TL2D2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
