Page MenuHomePhabricator

eo: improve documentation for event forwarder.
ClosedPublic

Authored by cedric on Jan 2 2019, 3:30 PM.

Diff Detail

Repository
rEFL core/efl
Branch
devs/cedric/forwarddoc
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 8671
cedric created this revision.Jan 2 2019, 3:30 PM
cedric requested review of this revision.Jan 2 2019, 3:30 PM
segfaultxavi requested changes to this revision.Jan 3 2019, 3:41 AM

Added some inline comments. The whole event forwarding thing seems to be previously-undocumented API, so it is important we put clear docs :)

src/lib/eo/efl_object.eo
252–264

It's still not clear to me in which direction the event is forwarded. How about:
"Add an event callback forwarder which will make another object emit some of the events this object is emitting."

Things that I am missing:

  • Are we still emitting the event after it is forwarded?
  • When multiple forwarders are installed, all of them emit the events? Can any forwarder break the chain (stop the event from being forwarded further)?
  • What's the point of all this? Why doesn't the app listen to this object instead of new_obj? A use case would be great (just one sentence, something like "This simplifies genlists because the app can listen to the genlist object instead of each individual list item", for example).
268

BEFORE and AFTER what?
Also, "shortcut" is a single word.

270

I cannot find a definition for Efl_Callback_Priority in any EO file.
I cannot find any of the EFL_CALLBACK_PRIORITY_* constants either.
They should be in EO files if they are to be used by bindings...

Once they are available, they should be referenced with @ (hyperlink) instead of $ (monospaced font), and using the proper EO syntax, which will probably be @Efl.Callback_Priority.

This revision now requires changes to proceed.Jan 3 2019, 3:41 AM
cedric updated this revision to Diff 18228.Jan 4 2019, 1:51 PM

Updated after a rebase with requested improvement.

cedric added a comment.Jan 4 2019, 1:51 PM

I have also added some more text on the new Efl.Callback_Priority type too.

src/lib/eo/efl_object.eo
270

I didn't even know that was possible! Nice improvement indeed.

segfaultxavi added inline comments.Jan 7 2019, 5:09 AM
src/lib/eo/efl_object.eo
252

This is still a bit obscure to me... how about:
Add an event callback forwarder that will make this object emit an event whenever another object ($new_obj) emits it. The event is said to be forwarded from $new_obj to this object.

254

So confusing! So the event is unaffected, but it can be intercepted? How about:

$new_obj still emits the original event. Whether this object emits the forwarded event before or after $new_obj can be controlled by the $priority parameter (see below).
In order to prevent $new_obj from emitting the initial event, register the forwarder using the @Efl.Callback_Priority_Before and then stop further propagation of the event by ... (explain briefly how you do that, because I don't know. Add link to where that is explained. It should be https://www.enlightenment.org/develop/guides/c/core/events.md but that part is missing).

266

Wow, I didn't know @cref. What does it do?
Also, it needs @in.

267

Do not change the name of the thing! We have been calling it "event forwarder", so keep calling it "event forwarder" (not "redirection handler").

272

I would call this original_obj, and the comment should be [[The object which emits the initial event]]

Change my previous comments accordingly if you call this parameter something other than new_obj.

cedric updated this revision to Diff 18307.Jan 9 2019, 6:50 PM

Rebase and take comments into account.

cedric added inline comments.
src/lib/eo/efl_object.eo
254

I am removing completely the reference to event interception as this is confusing and unrelated to forwarder (all events can be blocked from propagating there event further with efl_event_callback_stop).

266

To my understanding @cref is a const pointer to an object that the caller keep control of after the call. I would for example not expect this call to keep a reference somewhere to desc once the call is done. Arguably, this is one of this things where onle @q66 and @felipealmeida will be able to give the exact definition.

It also imply @in as const pointer can not be modified and so can't be an out.

segfaultxavi requested changes to this revision.Jan 10 2019, 2:25 AM

Nice! Fix the indentation (TABS!) and this is good to go for me!

This revision now requires changes to proceed.Jan 10 2019, 2:25 AM
cedric updated this revision to Diff 18334.Jan 10 2019, 10:20 AM

Rebase and fix tab.

segfaultxavi accepted this revision.Jan 10 2019, 10:23 AM
This revision is now accepted and ready to land.Jan 10 2019, 10:23 AM
This revision was automatically updated to reflect the committed changes.