Page MenuHomePhabricator

Fix efl_part issues in C#
Closed, ResolvedPublic

Description

efl_part suggest the existence of another namespace inside the object namespace, but implementation doesn't seems to reflect that. Proposed syntax in C#:

((Efl.File)obj["bg"]).file = "label";
obj.bg().file = "label";
obj.bg().file("toto");
obj.bg("toto");

Assuming that file is both a @constructor property and a part of bg. Clearly this is not always trivial to make it work. Alternative proposal for the syntax looks like :

obj.parts.bg.file = "toto";
obj.parts.bg.parts.file.text = "toto";

This is all hypothetic, but we will have conflict between property and part, it will be best to plan properly for it.

cedric created this task.Jan 24 2019, 10:06 PM
cedric triaged this task as High priority.
cedric added a subscriber: zmike.

Shouldn't we bring all the C# people to this discussion?

For the record, this is how parts are accessed right now from C#:

Efl.ContentConcrete.static_cast(vsplit.GetPart("first")).SetContent(box);

Because GetPart() returns an Efl.Object it has to be cast to Efl.Content, so it looks fugly.

Yes, this is exactly what we are trying to fix as it make setting things like the background color of a widget really ugly. It will stay ugly in some way for the fully dynamic case, but when the type is declared in the .eo we should have something nice. The additional question is, do we want to have the API look the same for the fully dynamic case and the known case from .eo ? I was inclined to say yes, but I don't know now.

@cedric, I don't remember exactly which case have some conflict problem regarding this,
so could you give some example to explain the problem?
this would very helpful to understand problem for @woohyun and @Jaehyun_Cho

So for example, if we start to use efl_part for property binding on object, we could end up with conflict on pretty much anything. Potential example would be just the bg part which has two property, file and color. This are also likely the property you would want to bind a model to. So you would have in C:

efl_model_property_bind(efl_part(efl_part(obj, "bg"), "file"), "path");

But in C#, JS, whatever, if we try to make efl_part in the same namespace as the rest of the object, we will end up with conflict like :

obj.bg().file().property_bind("path");

vs

obj.bg().file = "/some/where";

Now, how can your binding route things correctly without having conflict?

The long answer is the simpler to get things working, but not necessarily the best to work with:
obj.parts.bg().parts.file().property_bind("path");

This is just an example, but to make it simple, if a property name is the same as a part, we have potentially a conflict with some of the proposed syntax and we need to define the proper rules for that case and then enforce them.

cedric added a subscriber: q66.Jan 29 2019, 10:26 PM

I think we should agree that efl_part defined in .eo should not clash with any property or function defined in the .eo hierarchy. What do you think of implementing that rules and enforcing it in eolian? @zmike, @bu5hm4n and @q66 ?

I grepped a bit and only 7 eo files contain a parts section. Is that correct, or do we need to update a lot of eo files?

@cedric i would say that makes pretty much perfect sense :)

@segfaultxavi I haven't checked enough of the C files to be sure we do not have more hidden part, but it isn't a big deal I think as I would only consider stable API the one declared inside the .eo.

OK, just so I understand it, every object implementing the Efl.Part interface can have some of its parts retrieved with Efl.Object part_get(string name). But this is too generic because the user has to know which names are valid, and the returned part has to be cast to something usable (like Efl.Ui.Button, for example).
The parts section in some EO files list the valid part names and types, so at the very least these can be documented, but also allowing bindings to provide more confortable methods to access the parts.
We are now discussing how to make the C# bindings for parts more comfortable for the user. Correct?

Parts not declared in EO files are the ones you are calling "fully dynamic", is that correct too?

If I have scored 2 out of 2, then these are my two cents:

  • I agree making Eolian forbid name collisions between parts and properties would lead to the simplest possible C# usage: obj.Bg.File = "whatever";. I like this very much.
  • The only currently declared parts are indicator, tab, shadow, background, first, second, backwall, back_button, icon and end. I don't think fixing name collisions is going to be difficult.
  • However, I would make the generator add Part as a suffix (like it does for events), so it is more clear: obj.BgPart.File = "whatever";. In this way we do not need to modify Eolian.
  • For the fully dynamic case, I guess C# can do some black magic (introspection? extension methods?) and try to locate at runtime the part corresponding to a non-existing property: so if I wrote var p = obj.AwesomePart; and there is no property named AwesomePart in that object, the bindings can try to run `part_get("AwesomePart") and see if that works. But I am no expert.
  • If this is not possible, then I am OK with using a different access mechanism for declared parts (obj.BgPart) and undeclared ones (obj.Part("Awesome"), for example).

@segfaultxavi You're right on both questions.

About fully dynamic case, we can't have obj.AwesomePart syntax without using dynamic keyword, and that is not properly implemented in dotnet 2.x yet. And also makes all calls slower.

it seems issue is solved now.

lauromoura closed this task as Resolved.Sep 26 2019, 11:11 AM