Page MenuHomePhabricator

Add a efl_part information to .eo files
Closed, ResolvedPublic

Description

efl_part API seems to be a quite useful API, but it is not something usable by binding currently as the returned type of efl_part is a generic one. It would also be quite nice if bindings could do something like that :

obj.bg.color = 0xced;

Where bg is the bg part that provide a color interface.

Current idea is to add a section to all object definition in .eo to describe each part with name, type and documentation. Maybe something like :

parts {
  bg: Efl.Ui.Part.Background; [[Provide various property to alter visually the background of the widget.]]
}

To prevent problem with bindings, we will need to enforce that there should not be any naming collision with function and property name.

cedric created this task.Mar 29 2017, 6:25 PM
cedric triaged this task as TODO priority.Mar 29 2017, 6:51 PM
cedric added projects: efl, Restricted Project.

Sorry for jumping in here, but i think efl_part is more designed for parts that are just found at runtime, so having something like

parts { 
   bg : Efl.Ui.Part.Background; 
}

in a .eo file is not quite helpfull for that case, since the names of the parts are often just created in runtime, or do really depend on the runtime, (for example elm_layout)

For the case that we know the name of a part at the compilation time, why not just use a @property ? This would bring obj.bg.color for free. :)

The main difference with @property is that this generate a different code as it call efl_part for the user in bindings. I think you believe that we only want to return efl_part for top widget like Edje and Elm_Layout (In which case, they are generic Eo object more or less), but in fact we want that for leaf class in the hierarchy, like button where the part are defined by the selected style and theme. The idea of making it a part is to enforce the idea that this should be supported by a theme and that developers can rely on it.

(But it doesn't mean it could not have a different syntax like @part bg: Efl.Ui.Part.Background;)

q66 added a comment.Apr 6 2017, 5:40 AM

Yeah, I agree with @cedric here. This is something to look into. The parts syntax is probably good enough. I don't like the @part variant.

Like my comment T5307#85241 i think we're screwing up in order to cope with legacy. If we say a style is actually a subclass of our class, then it can define more properties, which are the parts you're mentioning. The class.constructor could then walk the properties and check their backing store (edje parts, eet fields, dbus interfaces...)

as @bu5hm4n said, there are some cases we want to address properties that only exist in runtime, like by string. If this remains true (ie: no way we can get rid of that), then a basic property-by-name would be handy. That would be the equivalent of Python's __getitem__(name) method that is called when you do obj.name. JavaScript also provides some of that... in C, however, we'd need efl_property_get(o, "name") which could return an Eina_Value, also with getters. If that's the case, it should exist in the basic Efl.Object and all @property should automatically be exposed in the base implementation, which the user can override and then call efl_super() or not.

I think you are missunderstanding what efl_part is. efl_part is an extention of the concept of edje part. As in every where we do have a "part" being specified in our API, what we are actually doing is exposing the API of another object through it. It has nothing to do with inheritance. I guess the cleanest example I can think of would be a combobox. combobox is basically a button, an entry, a genlist and maybe some times also a multibutton entry. Now how do you address all the various component of that object without actually having conflict (Both the genlist and the multibutton have conflicting item API). Clean solution : efl_part(combobox, "list") -> a list object, efl_part(combobox, "multi") -> a multi button entry. I hope this explain better the idea.

Added personnal reason, I dislike the property string, like we do for swallow and text in edje, as there is no way to know from looking at the object what you are allowed to do. You have a hidden API which is the style that you have to dig to see what you can do with. That is not making efl easy.

In your example it's just properties... seriously:

methods {
   @property list {
       value {
          list: Efl.Ui.List;
       }
   }
   @property multibutton {
       value {
         multibutton: Efl.Ui.Multibutton;
       }
   }
}

what's wrong with that? If that's to be provided by one "style" but not the other, that's what I meant with subclasses. One subclass will add these properties, the other won't. It's all in there, no new concept to learn...

As for property string, it's useful for dynamic systems where you know the properties in runtime... like you got a JSON and want to expose that. But eventually we can delegate that to Efl_Model and call it done.

Oh, so really multiplying class like crazy. Don't you think that this will increase complexity for developers ? I feel meh about the amount of class this would create.

Oh and fully yes to use Efl_Model for the dynamic properties. That is what it is really designed for.

well, calling a "single class" but then pointing users to N styles, all with their own API is the same... "N" api to look at the end. But at least one is Eo, the classes users know how to work with, they learn once and replicate. It's also saner if you think of external providers... you can download a "MyCompany.Combobox" that extends in a different way... I doubt "MyCompany" could extend the styles of a widget... but a subclass is always that, can come from external libraries and the application own code.

jpeg added a comment.Apr 10 2017, 9:26 PM

Clearly this is linked to T5307. Styles could be subclasses, and definitely those would be much easier to document, use, discover, and even implement IMHO. Yes, this will mean a lot of classes. Other frameworks take this approach, so it is not uncommon. In EFL we like to factorize code, which is fine, but may confuse users. Subclassing means most of the logic will remain common.

But efl_part() is not related to subclasses. efl_part() returns a pseudo-object that implements various interfaces partially, what @cedric is saying here is that it is not clear which APIs are implemented by which part. So C# and C++ lack auto-completion.

But both @barbieri and @cedric's ideas can be adopted at the same time:

In your example it's just properties... seriously:

methods {
    @property list {
        value {
           list: Efl.Ui.List;
        }
    }
    @property multibutton {
        value {
          multibutton: Efl.Ui.Multibutton;
        }
    }
 }

One catch here is that the part can not be a Efl.Ui.Multibutton as we do not want to expose the real internal object. Only a volatile Efl.Part object. This allows us to prevent API misuses. So we would instead have a Efl.Part.Multibutton which implements a limited set of interfaces and methods.

Also, if you look at my current work on Efl.Ui.Win then you will see that background can be color_set() or file_set() (image file). More properties need to be added to this virtual part, but the key point here is that inside EDC there is not a single part, but two: a color rect and a swallow.

So, in the end a style subclass could expose a different list of parts than another subclass, as properties. Internally this should likely still use efl_part[*] so that we can extend at runtime.

  • The lifetime of an efl_part() object is defined to be one and only one function call. So as a property we would need to change this lifetime to be either permanent or volatile like eina_slstr (until the next main loop). I would rather use a permanent storage here, with a reference inside the real widget. efl_part would then return this same object and would not delete it after a function call (this is possible and currently in accordance with efl_part usage).

I don't get this... effectively we deal with the internal objects, thus why not return them? If they may be of different types, return the base type, or an interface/mixin (in ".eo", for usage it's always an Eo*).

Why do we need to create another instance that is more restricted and its sole work is to proxy data to the real object?

Well the point is the following (or where i see that it would be better): Lets say we have two widget classes (A,B), both are having the property "text_field". now you want a widget that basically composes those two widgets into a class C. So we take a class C and let it inherit from A,B. Now we have a problem here, since under one property we could find two textfields (other bindings than c are having not the full namespace!). With having the part api we could just override the efl_part_get function that returns the object in a efl_part(...) call, and decide which other part objects could be used.

Thats just one reason where efl_part is better than a property here ..., And in general we can keep the inheritance tree for widgets much smaller than they are currently. Also, with keeping the inheritance tree small and putting internal widgets into efl_part calls,we are getting away from the problem that two classes could have a property with the same name.

Performance

Neither the less i would strongly review the usages, since the part property would be much more expensive than a normal property.

Strings as keys

Also to what cedric said, i am also looking a bit worried about having strings as keys to get a property here. Strings are quite hard to remember, from my experience: "Oh was it 'clicked' 'mouse,down' or what was it?" Its quite easier to have some autocompleted something where your ide tells you "hey that does not exist" or at least at compile time, things would fail, so you see that you entered a wrong key

@bu5hm4n read what you wrote and you'll notice how hackish it reads:

With having the part api we could just override the efl_part_get function that returns the object in a efl_part(...) call, and decide which other part objects could be used.

This is EXACTLY the same as class C declaring its own "text_field" property. Which could "solve" for free the languages without lack of namespaces. However usually, for those, you can call B.text_field(o, ...) or A.text_field(o, ...), that is, be explicit which one you're trying to reach. Also note that this problem exists with or without "part_get", so it may happen in real life and we need to solve it.

putting internal widgets into efl_part calls,we are getting away from the problem that two classes could have a property with the same name

No you're not :-) The problem is still there and actually would cause a confusion on why properties and parts are resolved in different ways.

Other than that, it's providing only what I meant with __getitem__() is for python, or the JavaScript get property... If we want to address properties that are only know at runtime, then adding it more general is the best approach -- I read from Cedric that we'd isolate that to Efl_Model only, eventually we move it one level up. I'd not oppose to methods such property_get(), property_set(), property_del() and properties_get() (iterator over all properties), with nice default methods that would use compile time @property... which could be extended by calls to efl_super() or replaced entirely.

cedric added a comment.May 1 2017, 3:16 PM

The way I see efl_part is a clearer definition of lifecycle for an object property with less work to be made by developers. It also open a dynamic extension of the supported part. Some part my not be available depending most likely on the style you choose for a widget for example.

As you point out, they are inherited... except we can override that and define a new return type if we want. This is maybe a pattern that we may want to forbid, but not sure yet. Still, I don't think it is a problem if there is a collision with the uper class. But if you want to get the efl_part from the inherited parent class, you can just use an efl_cast and have it. The issue will be for language that do not provide cast... But as they just before, I don't think there will be a problem or collision as it would be pretty weird to return two completely different object for the same part depending on where you are in the class hierarchy. What I would expect would be more an extention, the efl_part of the children is extended compared to the class it inherit from. This could be enforced by eolian and would solve both problem.

jpeg added a comment.May 10 2017, 3:20 AM

I don't get this... effectively we deal with the internal objects, thus why not return them? If they may be of different types, return the base type, or an interface/mixin (in ".eo", for usage it's always an Eo*).

Why do we need to create another instance that is more restricted and its sole work is to proxy data to the real object?

I believe one of the mistakes made in EFL was to expose some internal objects. Eg. edje_object_part_object_get() or elm_image_object_get(). This allows the user to do API calls on those sub objects that conflict with the internal logic of the high-level widget (eg. manually hide a part of an edje layout). Also this exposes many "features" that we then can not change in the future or we will break compatibility. In the edje example the returned object isn't even consistent across part types (eg. GROUP is a rectangle object - the clipper).
We definitely do not want to expose internal objects.

Another thing is that by exposing internal objects, you can not do something like Efl.Ui.Win "background" part: it's either a solid color rectangle, an image object (note: some features still need to be added, of course) or even a swallow. Those are different parts in EDC. Also, you wouldn't be able to set a content to a swallow without something like efl_part (because a swallow is not an object). The only alternative would be the good old part apis (duplicates of many canvas APIs).

The main drawback with efl_part is that we have to implement support for each and every one of the properties of the sub object, and this can be tedious and useful stuff may be forgotten. But exposing internals is a big no, considering how many devs abuse our APIs. In some rare cases we could return the internal object in efl_part() but this should be avoided.

This discussion also mixes another topic which is property names as strings, and implies using Eina_Value/Model to pass arbitrary types of values. This is like the Qt meta property model, which is a fine concept, but IMO is an orthogonal point of discussion -- find another ticket for that.

@cedric @jpeg
Could you share any conclusion or decision on this task ?
As you know this is the super important concept decision for UI Widget Class with properties.

jpeg added a comment.Jun 20 2017, 10:18 PM

We will discuss this during EDD in Malta. This is something we most likely want to push forward.

cedric raised the priority of this task from TODO to High.Jul 10 2017, 3:44 PM

So this is needed for better bindings in general.

@q66 Did you had time looking at this ?

q66 added a comment.Jul 28 2017, 3:50 PM

yeah, i think we can just proceed with this, if there are no further objections

bu5hm4n moved this task from Backlog to Eolian on the efl board.Sep 30 2017, 1:47 AM
jpeg added a comment.Oct 23 2017, 12:16 AM

Hey has any progress been made on this front, @q66? This is crucial to the interfaces work...

q66 added a comment.Oct 23 2017, 2:28 AM

alright, let's get it in during this week

q66 added a comment.Oct 24 2017, 8:50 AM

i expect to have it done by tomorrow evening...

jpeg added a comment.Oct 24 2017, 7:29 PM

Very cool, thanks.

q66 closed this task as Resolved.Nov 1 2017, 5:13 AM

i guess i'll just close this since part information is already done since a few days ago...

the API is still missing but i'll add that soon

for now, the syntax is exactly as described in the ticket itself