Dear Cedric

Hello^^

>> In fact, you change to much thing that shouldn't in your patch. As an
>> example you are modifying
>> st_collections_group_parts_part_repeat_events when it's not related at
>> all to the current patch. Please only change the function that should.
>
> Indeed, It is necessary in order to change the part I want.
> In st_cellection_group_inherit, it copy all property of the group. And if I
> wirte the specific part or description in child group, its property is 
> overwritten.
> To overwrite, the pointer of current part or current description is necessary 
> and
> the properties is written in the current point. So I needed chage the code 
> like below.
>
> static void
> st_collections_group_parts_part_repeat_events(void)
>    {
>       Edje_Part_Collection *pc;
>       Edje_Part *ep;
>
>       check_arg_count(1);
>
>       pc = eina_list_data_get(eina_list_last(edje_collections));
>       ep = current_part;
>
>       ep->repeat_events = parse_bool(0);
>    }
>
> But I think the below code is more simple.
>
> static void
> st_collections_group_parts_part_repeat_events(void)
>    {
>       check_arg_count(1);
>
>       current_part->repeat_events = parse_bool(0);
>    }
>
> All function have to change like this in order to change the property.

> Hum, I will have to read your patch more carefully then, but if you
> can, could you then split it in two. One for the previous change and
> another for the inherit infrastructure.

The working to split it in two takes long time because I have to change
all function again. The change is simple and easy to understand. If you
cannot understand my code, I'll give explanation to you.

>> 7. Do not increase the size of runtime structure, especially Edje_Part
>> and also Edje_program as much as possible. The reorder is maybe doable
>> when you find the keyword, that would avoid completly the reorder
>> structure inside Edje_Part.
>
> If I increase the size of runtime structure, previously built edj files may 
> have
> a problem. You concerned about it, right? I didn't think about that. then how
> can I change the code? Do you have any idea? I saw the code you change
> in 62086. Should I change like that?

> The issue is that with your change, we now have pointer that are not
> used at runtime, but are allocated. This is a waste of memory in my
> opinion. It will not break previous edje file, thanks to eet, but
> still that's not a good thing in my opinion. Maybe instead of adding
> this structure, you could try to put the part/program at the right
> place when you find the keyword that tell you where to put it.
>    As for can_override, I think it should be part of a more generic
> infrastructure to detect when setting a value override an existing
> property. So lets forget about that one for the moment. You don't need
> to change the way you handle it in your patch. When we improve edje
> error, we could improve that at that time.

ok, I understand your opinion. I'll try to change the reorder structure.

>> 8. Right now, your implementation is a bit like virtual in C++, the
>> group that inherit can change any part the way it want. Maybe, their
>> is two other cases that should be considered :
>>   - program and part could be locked and impossible to change at all,
>> throwing a warning if the group try to change the content of this
>> items.
>>   - pure virtual part/program, that would prevent the compilation of
>> a group if they are not implemented (like virtual = 0 in C++).
>
> I think there is not locked or virtual part in current edc. Is there anything 
> I don't know?
> Or you just mean the C++ concept? I don't know C++ in detail. If I have to 
> change,
> please give a explanations to me.

> Yes, they don't exist yet. But it would have been a good addition or a
> futur direction for this work. It really help to provide clean code.
> There is no need to know C++ in detail. The idea is to provide part
> that can't be changed and part that absolutly need to be implemented.
> It's not as easy as it sounds and it's clearly a possible addition to
> your work. So no need to provide to provide it yet. It was just a
> question of what could be done in the futur once we have inheritance.

Thanks for explanation. I'll consider it later.

>>>     Future work
>>>
>>>     1. Delete - The part, description, program can be deleted.
>
>> I don't know if this is really usefull as all the object model I know
>> don't provide this kinde of feature. Do you have a use case for that ?
>> Or maybe you know a language that provide this kind of feature, so I
>> could read about it.
>
> In our theme, genlist has a lot of style. Specific part can belong to some
> group or not. Let's assume I make new genlist style. I want to inherit a group
> and change some property and delete one part. If deletion is not supported,
> I cannot inherit a group. Let's think about this. ^^

> Hum, in my opinion it may become a little bit hugly if we do follow
> such path. In the exemple your are describing, I would have prefered
> that a common genlist group was created and that both style inherit
> from it. It would keep thing clearer, and if someone modify the first
> style without looking at the second style, it will not break. If he
> rename a part, add some that does have some unintended effect on the
> second style.
>     In general, a clean hierarchie will prevent some breakage and
> delete clearly provide a way to do things that will be difficult to
> maintain. So I would prefer that's this feature doesn't go in.

Then I'll let it be. And if there is nessarity later, let think about it at 
that time.

I'll change the part of reorder structure and upload again.
Thank you.

Jaehwan Kim.
------------------------------------------------------------------------------
BlackBerry® DevCon Americas, Oct. 18-20, San Francisco, CA
http://p.sf.net/sfu/rim-devcon-copy2
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel

Reply via email to