Page MenuHomePhabricator

efl.loop
Closed, ResolvedPublic

Description

|class Efl.Loop
|├ (P) quit_when_all_windows_closed
|├ (P) throttle
|├ (P) time
|├ (M) iterate
|├ (M) iterate_may_block
|├ (M) begin
|├ (M) quit
|├ (M) job
|├ (M) idle
|├ (M) timeout
|├ (M) register
|├ (M) unregister
|├ (M) message_handler_get
|├ (E) idle,enter
|├ (E) idle,exit
|├ (E) idle
|├ (E) arguments
|├ (E) poll,high
|├ (E) poll,medium
|├ (E) poll,low
|├ (E) quit
zmike created this task.Jan 9 2019, 10:18 AM
zmike triaged this task as TODO priority.
zmike moved this task from Backlog to Evaluating on the efl: api board.Jan 17 2019, 11:09 AM
|├ (P) quit_when_all_windows_closed

I don't think this one landed yet. Except from that I am ok with the rest of the class.

I cannot comment on the API, since the relationship between Task, Loop and App is still unclear to me, however, I have some small concerns regarding the documentation for Efl.Loop. Parts of it could be made clearer.

bu5hm4n added a comment.EditedJan 19 2019, 3:26 AM

Undefined API!:

eolian: efl_loop.eo:7:1: partially implemented function 'closed' (originally defined at efl_io_closer.eo:35:13)
eolian: efl_loop.eo:7:1: unimplemented function 'can_read' (originally defined at efl_io_reader.eo:39:13)
eolian: efl_loop.eo:7:1: unimplemented function 'can_write' (originally defined at efl_io_writer.eo:40:13)
eolian: efl_loop.eo:7:1: unimplemented function 'close_on_exec' (originally defined at efl_io_closer.eo:52:13)
eolian: efl_loop.eo:7:1: unimplemented function 'close_on_invalidate' (originally defined at efl_io_closer.eo:68:13)
eolian: efl_loop.eo:7:1: unimplemented function 'close' (originally defined at efl_io_closer.eo:18:9)
eolian: efl_loop.eo:7:1: unimplemented function 'eos' (originally defined at efl_io_reader.eo:49:13)
eolian: efl_loop.eo:7:1: unimplemented function 'read' (originally defined at efl_io_reader.eo:19:9)
eolian: efl_loop.eo:7:1: unimplemented function 'write' (originally defined at efl_io_writer.eo:19:9)

Quoting from T7514:

  • arguments event should move to efl_app
  • is regular, should probebly be abstract ?
  • there is no need to have message_handler_get beeing @class

We should btw. things about a namespace ?

woohyun added a comment.EditedJan 22 2019, 6:17 PM

Undefined API!:

eolian: efl_loop.eo:7:1: partially implemented function 'closed' (originally defined at efl_io_closer.eo:35:13)
eolian: efl_loop.eo:7:1: unimplemented function 'can_read' (originally defined at efl_io_reader.eo:39:13)
eolian: efl_loop.eo:7:1: unimplemented function 'can_write' (originally defined at efl_io_writer.eo:40:13)
eolian: efl_loop.eo:7:1: unimplemented function 'close_on_exec' (originally defined at efl_io_closer.eo:52:13)
eolian: efl_loop.eo:7:1: unimplemented function 'close_on_invalidate' (originally defined at efl_io_closer.eo:68:13)
eolian: efl_loop.eo:7:1: unimplemented function 'close' (originally defined at efl_io_closer.eo:18:9)
eolian: efl_loop.eo:7:1: unimplemented function 'eos' (originally defined at efl_io_reader.eo:49:13)
eolian: efl_loop.eo:7:1: unimplemented function 'read' (originally defined at efl_io_reader.eo:19:9)
eolian: efl_loop.eo:7:1: unimplemented function 'write' (originally defined at efl_io_writer.eo:19:9)

I'm wondering how we will handle this unimplemented things.
What I can only think is that separating interface properly if those are not something that have plan to be implemented soon.
(Or removing some interfaces if necessary from the class)

With this case, I think removing interfaces from efl_task looks good for me.

Quoting from T7514:

  • is regular, should probebly be abstract ?

If this would be abstract, then, there will be no loop instance any more, right ?
I'm not sure this will not give any conceptual problem in EFL.

#1 The unimplemented things are most of the time handled by either moving a method from the interface, or just implementing them. Theoretically we can implement all those functions on efl_loop, it just does not seem to be that usefull. We can also drop the efl_io_* interfaces from efl_loop / efl_app. However, this means that you cannot read from the app object anymore, which seem to be one of the design decisions, (I don't have a personal favour here, if keeping or removing it).

#2 Making efl_loop abstract would partly solve this here, because the methods are then not unimplemented on this particular object. Conceptionally there would be no change to EFL since a efl_loop is never instantiated directly, what *is* instantiated is efl_app which inherits efl_loop. So making it abstract does not change something in that regard :)

@bu5hm4n

Thanks for giving an answer.
#1 As you described in T7514 like :
reading and writing to the task is feeling a bit weird, and is not really documented what it is supposed to to, in efl_app this is writing to the console, which is a lot complexer with c#&efl.io.writer, than a simpler Console.PrintLine("asfd")

I also feel little bit weird about the relation between efl_loop and efl_io_xxx interfaces.
So +1 to removing them from the class :)

@woohyun i added T7657, I can work on it when T5719 is done. If someone else does it, please claim the task :)

I am wondering if we should make the Efl_Loop_Arguments struct carry a accessor instead of an array. In an accessor we can implement a barrier to ensure its not mutable. So everyone that gets such an event, can access it, but we ensure that noone ever changes this data structure, and additionnaly, we don't leak a internal data structure, which we might want to change later on ... :)

The arguments in the event are defined in EO like this:

argv: const(array<const(stringshare)>);

Doesn't this mean they are inmutable? I don't know enough EO to answer that.

message_handler_get: this function is a bit weird, and is not often used, I would keep it beta, everyone happy with this ?

bu5hm4n moved this task from Evaluating to Stabilized on the efl: api board.Feb 21 2019, 10:19 AM
bu5hm4n raised the priority of this task from TODO to Normal.Feb 22 2019, 1:19 AM
zmike closed subtask T7598: efl.task as Resolved.Mar 11 2019, 10:46 AM
zmike closed this task as Resolved.
zmike claimed this task.