Page MenuHomePhabricator

Closed, ResolvedPublic


| |class Efl.Task
| |├ (P) command
| |├ (P) arg_count
| |├ (P) arg_value
| |├ (P) env
| |├ (P) priority
| |├ (P) exit_code
| |├ (P) flags
| |├ (M) arg_append
| |├ (M) arg_reset
| |├ (M) env_reset
| |├ (M) run
| |├ (M) end
zmike created this task.Jan 9 2019, 10:19 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

I think that arg_count, arg_append,arg_value and arg_reset are not a nice API for any binding as it look very foreign. Better API would be either to just have an Eina_Array or Eina_List based API. This would allow most binding to do something like :

app.args = [ 'titi', 'toto', 'tata' ];
app.args.foreach(function (arg) { console.log(arg); });

env{ set; get } and friends has the same inherent problem. Also general documentation is really unclear to me.

Flags needs documentation to be properly reviewed.

I also do not know if I can call "Run" multiple time. Documentation could be improved.

I agree on everything @cedric just said.

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

@cedric D7510 / D7516 is working on this.

The only difference here is that we have two fill operations, one for a array, one for a string. Reason for this is that sometimes it is usefull to have a string, sometimes its usefull to have an array.

I also moved things out of efl_task for the simple reason that it does not really below there IMO, but rather belong to a higher level object. String-based Arguments are nothing for a general purpose task abstraction, rather something for a application.

We should think about a namespace ?

zmike added a comment.Jan 22 2019, 9:33 AM

I think this one in particular will require more time to stabilize, so I'm expecting to leave this on the Evaluating board for much longer than any other task.

@bu5hm4n the command line seems to be generally going into the right direction for me. The env one, I don't know. It really feel like it should just be an Eina_Hash that you set once.

@cedric the reason i wanted to avoid a eina_hash is that we cannot see what is inside (could be a stringshare or string hash, we cannot really check), also, the API usage would be rather painfull, since using eina_hash in C is not that nice, (i am remembering a lot of broken things because somewhere a & is missing).

Additionally, for the usecase of the Env object that is mirrored to the environment, we cannot really use a hash table there without loosing a lot of performance, we would have to iterate everytime the full hash to check what is new / different to what we have, so this is also a lot easier with the env object. And last but not least, i am a really big fan of that fork call, its the same you have in meson on data obejcts / python on environment objects, and it is *really* usefull, and allows you to do cool things ... :)

I guess I see your point with Eina_Hash, especially not being nice in C (Mostly because it has quite a large API surface). I can agree to the Environment object, it isn't such a big deal. Will do a proper review later.

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