Hi,
after a long read:
- 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.
- Its also not clear to me how explicit remove plays well with unref.
- the `insert` method should return a owned struct handle ? Or is
the user never really owning any of the structs there ?
- 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.
- efl2_text_wrap_properties: Can you document what impact ellipsis and
wrap do have ?
- 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 ?
- efl2_text_item_factory:
- Again, why having a struct here, same as above applies to this.
- efl2_text_raw_editable:
- Where does this object belong ? We normally only have objects in
efl.canvas / ui / layout. But not in efl. itself.
- Is it doing undo/redo or only emitting the events ?
- 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?
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.
- 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.
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
