On Wed, Mar 26, 2008 at 12:37 AM, <[EMAIL PROTECTED]> wrote: > > > but for instance the code you did there can be avoided > > by use of parts of "type: GROUP" using "source: "button_bar"" > > > aha! That wasn't on the edjebook :). It was strange that something so > obvious should be missing!
Yes, this is kinda new, at least newer than old edje book ;-) > > + ed = embryo_program_data_get(ep); > > + > > + rp = ed->table_parts[part_id % ed->table_parts_size]; > > + if (!rp || rp->part->type != EDJE_PART_TYPE_SWALLOW) return 0; > > > > no, see below. > > > > > + evas_object_smart_member_del(new_obj); > > + return 0; > > + } > > + > > + _edje_real_part_swallow(rp, new_obj); > > > > This is not ok. The first statement should be a simple > > evas_object_del(new_obj), > > > Ok, I took the first line from unswallow but it was possibly not what I > needed. You shouldn't look at edje internals in order to implement a "wrapper". Here you're doing a simple wrapper to expose edje functionality to embryo scripting. The call "evas_object_smart_member_del(new_obj);" will just remove the association of new_obj with its parent... which parent? Oh yeah, which parent? None! We have not set one, so this function is dummy. And since you just return you "leak" the object. Actually it will not be visible since objects have 0x0 size and are hidden by default. This will not leak memory, since Evas keeps a reference to all objects and will collect them when canvas is deleted, but still, it will not do what you want. > > the second should be > > evas_object_part_swallow(), this also remove the need of > > embryo_program_data_get() and working with table_parts and like... > > They're not really wrong, as it will work, but it's not good as it's > > too intrusive, let's use the public api and have just one place to > > work on the internals. > > I went that way at first, but I liked the compile time validation that > PART:"foo" did by checking if "foo" was a part... and since it does the > validation while looking up the part I ended with a part index... getting > the name and then using the public API didn't sound very good either :S. PART:"foo" will do a check at compile time in order to discover the integer that is used to represent "foo". During runtime, evas_object_part_swallow() will also check if swallow part exists and if it's a swallow. And yes, using the public API is what is good. You should use it whenever possible. Like I said, you're just doing a wrapper for this function, so this alone is one key factor. But another one is avoiding code duplication and thus error duplication. If you look at edje_object_part_swallow() you'll see that it uses _edje_real_part_recursive_get(), which I have no idea of the real purpose of having that, but even without knowing I know that it and your code might get out of sync and do different things, what is not expected. -- Gustavo Sverzut Barbieri http://profusion.mobi Embedded Systems -------------------------------------- MSN: [EMAIL PROTECTED] Skype: gsbarbieri Mobile: +55 (81) 9927 0010 ------------------------------------------------------------------------- Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace _______________________________________________ enlightenment-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
