Note: Their isn't any ability to do something like a static array of
events at the moment. It might lead to large memory being used when it
wouldn't be necessary. If that was the case, we could fix it, but it
would require a lot of dynamic hash operation I think.
Depends on D7496
| Lint OK |
| No Unit Test Coverage |
| Buildable 8611 |
Will fix.
| src/lib/eo/efl_object.eo | ||
|---|---|---|
| 236–237 | Arf, I really need an .eo description for emacs. | |
| 244 | Priority is useless when removing a callback as the previously registered callbacks could have been moved due to the insertion of other callback. So you can only walk the entire thing to try to find the one you are looking for. | |
| src/lib/eo/efl_object.eo | ||
|---|---|---|
| 244 | well not really IMO. If you subscribe twice once before once after, and want to remove the after, so the before is still called, you need to unsubscribe twice, to subscribe again, maybe we should add the priority to all callbacks in eo ? | |
| src/lib/eo/efl_object.eo | ||
|---|---|---|
| 244 | So what happen if you do insert twice with before and now you want to remove the first one you inserted? The first one is not anymore the before one as it has been superseded by the second before insertion. As a caller, you have no clue what the state of the stack of event handler on a object look like at any point in time. So I don't see how you could rely on that information to find something. Basically if you where to implement it, you would realize that you need to ignore the order parameter to find the event handler in all case. | |
| src/lib/eo/efl_object.eo | ||
|---|---|---|
| 244 | Yeah but the difference is if i have two callbacks registered, with the identical parameters, then it does not matter which one is actaully removed. (Before just maps to a priority that is sorted.) However, if i have a "before" and a "after" registered, it is possible for the user to want to keep the "before" registration, while removing the "after" which *is* a different paramtered call... Also, we could use the priority as sort of entry into the callbacks array, so we *start* to search at the priority after, and iterate the full array by rolling over, this would serve the best usage for the user, while keeping the possibility of specifying which one to remove. :) | |
| src/lib/eo/efl_object.eo | ||
|---|---|---|
| 244 | You really can't use the priority as a sort of entry into the callbacks array as we have no clue what happened. Let see: BEFORE (A), BEFORE (B), AFTER (C), DEFAULT (D), AFTER (E), BEFORE (F) would result in FBACDE, now you use the priority to remove the AFTER (C). What do we do ? DEFAULT (D) is in between the two AFTER. You have to linearly walk the entire think. I don't see a way around that. As for making use of the PRIORITY as a key for all callback in our infra, I think this is a larger discussion than this patch as this is requesting a large change in our core object. Could you open a ticket to follow up on that discussion? | |
Maybe this is already existing elsewhere and not in the diff, but can a little more doc be added regarding the purpose of an event callback forwarder?
What would you like to see in the doc? I have mostly copy&pasted the existing doc when doing this API, so there isn't much anything different around.
We just should do it before we declare eo stable.
| src/lib/eo/efl_object.eo | ||
|---|---|---|
| 244 | Yeah you are right, lets move this to some other ticket :) | |
Yeah, I looked at the existing doc and it doesn't really say anything either. Maybe outside the scope of this patch, but it would be good to get a bit more detail here I think?
Add an event callback forwarder for an event and an object.
Is the description, but it could maybe be improved to something like 'Add an event callback forwarder for an event and an object which propagates the event to the passed new_obj' ?
@segfaultxavi may have ideas here since he docs...
To be clear, this is just discussion for another diff and my comments are not blocking the acceptance or merge-ability of this patch.
I'm too late for the party, let's just copy my comments to a new task.
| src/lib/eo/Eo.h | ||
|---|---|---|
| 2072 | It wouldn't hurt to specify the type of this parameter, since this is a macro (see the macro above, for example). | |
| src/lib/eo/efl_object.eo | ||
| 233 | I understand this was written like this before and it's not @cedric's responsibility to fix it... but this might be a good time anyway :) | |
| 236 | Haha, yeah, now this definitely needs some explanation. What is the priority? Why is it needed? What's the valid range? | |