Hey,
Sorry for the delay, and thanks for your feedback! Comments are inline.
Just one thing though, while you provided a lot of very valuable
comments, you haven't solved my issue with splitting interfaces, which
is the main thing blocking my work ("the advice"). Any comments on that?
On 27/09/2019 15:22, Marcel Hollerbach wrote:
> Hi,
>
> after a long read:
Sorry, there's a lot there. :)
> - efl2_text_attribute_factory:
> - Why having a struct here as handle, i fear that bindings will have
> a very hard time with this, where you actually pass in a struct to the
> remove method, and this one is dead after it.
What's your suggested alternative? Anyhow, why would bindings have a
hard time? Structs have associated free functions and ref/unref, no? I'm
happy with an alternative, but I'd need a lightweight handle for this.
> - Its also not clear to me how explicit remove plays well with unref.
Remove: removes it from the text object. This means the handle is still
alive, just disassociated from the text object so it doesn't affect it
anymore.
Unref: reduces the refcount in case you reffed it before. Used for when
you kept your own copies.
> - the `insert` method should return a owned struct handle ? Or is
> the user never really owning any of the structs there ?
It's not owned. If you want to use it, you need to immediately ref it
after it's returned from there. It's done this way so you can safely
ignore the return value when you don't want it, but also use it when you do.
> - efl2_text_font_properties / style_properties: (I have criticized the
> names before, i like the new names.) However, hard to comment more, as i
> do not know much about text property stuff itself.
No worries, these ones are actually less of a problem for me.
> - efl2_text_wrap_properties: Can you document what impact ellipsis and
> wrap do have ?
As in: you have questions, or just commenting about the lack of docs?
> - efl2_text_markup:
> - I am not entirely sure how item_factory works here. Is the
> item_factory knowing to which Efl2.Text_Markup they belong ? If so,
> shoudnt that be expressed somehow ?
Not sure I understand your question.
> - efl2_text_item_factory:
> - Again, why having a struct here, same as above applies to this.
Same answer as before. :)
> - efl2_text_raw_editable:
> - Where does this object belong ? We normally only have objects in
> efl.canvas / ui / layout. But not in efl. itself.
Happy to rename it, though dunno what to. It's somewhere between canvas
and ui. It's above canvas as it handles input and deals with X, but
below UI because it doesn't have a theme.
> - Is it doing undo/redo or only emitting the events ?
At the moment just events, but Ali is going to fix this.
> - efl2_ui_text:
> - Why are here cnp related events ? Isnt that just a normal
> insertion / copy operation ? The cut and copy operation could be on a
> object that handles cnp, the paste operation as well, paired with a
> changed,user event?
Fair comment, need to re-evaluate that.
>
> General notes:
> - you used a lot of ptr(...), you cannot use them when you remove @beta
> from the file. These files should not use ptr, nor void_ptr.
I know. :(
I created my branch before there was an alternative (very recently), and
haven't updated anything yet.
> - You have a few places where you have explicitly x,y,w,h (x,y) in your
> API, can you replace them with rect (position) handles from eina, that
> would make it more aligned with the rest of efl.
>
Could you please provide an example?
Thanks!
--
Tom
> Greetings,
> bu5hm4n
>
> On 9/19/19 4:15 PM, Tom Hacohen wrote:
>> Hey,
>>
>> As most of you (at least the IRC lurkers) know, I've been recently
>> working on the text interfaces. Trying to clean them up and stabilise
>> them.
>> The discussion and work has been covered on phab at:
>> https://phab.enlightenment.org/T8151
>>
>> And the new (suggested) interfaces are all the files starting with
>> "efl2_" in my branch:
>> https://git.enlightenment.org/core/efl.git/tree/?h=devs/tasn/ifaces
>>
>> I'd love to get your feedback and let me know if there's anything I've
>> missed. All feedback is welcomed, including bike shedding.
>> Some interfaces still have massive FIXMEs at the top, so obviously read
>> those before replying to avoid repeating what we already know.
>>
>>
>> As for the advice I mentioned in the title: due to composite object
>> regressions as described in T8184, I'm forced to break up the classes
>> into interfaces. As discussed at length in the ticket, these interfaces
>> would have to be very specific to the classes and not really reusable
>> ("cursor_new" is quite specific, obviously).
>>
>> I can either just do as I said in the ticket, and for every class do a
>> big interface, so Efl.Canvas.Text -> Efl.Canvas.Text +
>> Efl.Canvas.Text_Interface. This is one way. It's obviously very ugly.
>> The other way is to split to a lot of smaller, probably 1/2 property
>> interfaces, which is also ugly and quite inefficient (classes/interfaces
>> are not free).
>>
>> I'd love to get your input, to what interfaces would you split up these
>> two classes:
>> 1.
>> https://git.enlightenment.org/core/efl.git/tree/src/lib/evas/canvas/efl2_canvas_text.eo?h=devs/tasn/ifaces
>>
>> 2.
>> https://git.enlightenment.org/core/efl.git/tree/src/lib/elementary/efl2_text_raw_editable.eo?h=devs/tasn/ifaces
>>
>>
>>
>> Thanks a lot for your help and feedback!
>>
>> --
>> Tom
>>
>>
>> _______________________________________________
>> enlightenment-devel mailing list
>> [email protected]
>> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
>>
>
>
> _______________________________________________
> enlightenment-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
_______________________________________________
enlightenment-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
