Page MenuHomePhabricator

efl_ui_widget: introduce a new API
ClosedPublic

Authored by bu5hm4n on Feb 24 2019, 3:56 AM.

Details

Summary

this new API can be used to get a easy to use iterator, to iterate
through all the children of a specific widget.

ref T7553

Diff Detail

Repository
rEFL core/efl
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
bu5hm4n created this revision.Feb 24 2019, 3:56 AM

It seems that this patch has no reviewers specified. If you are unsure who can review your patch, please check this wiki page and see if anyone can be added: https://phab.enlightenment.org/w/maintainers_reviewers/

bu5hm4n requested review of this revision.Feb 24 2019, 3:56 AM
bu5hm4n updated this revision to Diff 19638.Feb 24 2019, 12:03 PM
bu5hm4n edited the summary of this revision. (Show Details)

added file for testsuite

segfaultxavi requested changes to this revision.Feb 25 2019, 3:56 AM

Not much to say, the API makes sense, and I guess this is meant to simplify things. Some questions, though:

  • Why the new files efl_ui_widget_common.*? Couldn't this be just added to Efl.Ui.Widget?
  • I assume this will be used in a later commit but can you give a use case for this new API?
src/lib/elementary/efl_ui_widget_common.h
14

What is the type of the objects inside the iterator?

23

What is the type of the objects inside the iterator? Do I need to cast them to Efl.Ui.Widget?

34

What is the type of the objects inside the iterator?

This revision now requires changes to proceed.Feb 25 2019, 3:56 AM

Not much to say, the API makes sense, and I guess this is meant to simplify things. Some questions, though:

  • Why the new files efl_ui_widget_common.*? Couldn't this be just added to Efl.Ui.Widget?

Because Efl_Ui_Widget has roughly 6000LOC which is a mixture of two classes eo api and legacy API, nothing where i want to add more stuff to tbh.
And they are in a seperated .h file, because i don't want them to be overwritable, i *could* add them as a class function that is true, but i am not too sure we want to enable this to the bindings etc. not even sure if we shoudln't just keep this internally.

  • I assume this will be used in a later commit but can you give a use case for this new API?

Walk over all widgets so sync up a property? walk up all widgets and search a specific one ? see D8015 as an example.

bu5hm4n updated this revision to Diff 19661.Feb 25 2019, 6:19 AM
bu5hm4n edited the summary of this revision. (Show Details)

more comments

segfaultxavi accepted this revision.Feb 25 2019, 10:39 AM

I rest convinced.
Remember to always use links to classes in docs as in @Efl.Ui.Widget.
Also, do you need to say anything about ownership in the iterators? Or is that a known thing?

This revision is now accepted and ready to land.Feb 25 2019, 10:39 AM
woohyun added inline comments.
src/lib/elementary/efl_ui_widget_common.h
14

@segfaultxavi
I'm wondering which is correct in this description - Efl.Gfx.Entity or @Efl.Gfx.Entity ?

23

@segfaultxavi
I'm wondering which is correct in this description - Efl.Ui.Widget or @Efl.Ui.Widget ?

segfaultxavi added inline comments.Feb 26 2019, 1:47 AM
src/lib/elementary/efl_ui_widget_common.h
14

@Efl.Gfx.Entity will create a link, so I prefer it over Efl.Gfx.Entity which will only create plain text.

However, I am tired of repeating this every time, so I let the author decide, and I can always add the links later :)

23

Same as above :)

bu5hm4n added inline comments.Feb 26 2019, 1:51 AM
src/lib/elementary/efl_ui_widget_common.h
14

Thats not true, @Efl.Gfx.Entity will cause a link to Efl.Gfx.Entity in .eo files, but we are here in plain header files, which would require me to add @ref Efl.Gfx.Entity. However, this does not work out here, because we don't do doxygen for eo api, right ?

segfaultxavi added inline comments.Feb 26 2019, 1:56 AM
src/lib/elementary/efl_ui_widget_common.h
14

YOU ARE CORRECT! SORRY!
This is not an EO file. This is why I didn't ask you to add the @ initially... 🙄

bu5hm4n updated this revision to Diff 19686.Feb 26 2019, 2:36 AM

update & fixes

Closed by commit rEFLaef19e9017c4: efl_ui_widget: introduce a new API (authored by Marcel Hollerbach <mail@marcel-hollerbach.de>, committed by zmike). · Explain WhyFeb 27 2019, 10:31 AM
This revision was automatically updated to reflect the committed changes.