Re: lua-factory: the analysis of the changes proposed in #753141

[George Sedov <radist morse gmail com>]

Hi,

> > > So, this changes a lot. What I like least is that you are
> > > splitting
> > > functions provided by grl-lua-library. I do like grl as a library
> > > -
> > > globally accessible.
> >
> > > aside of `callback` which is the only one I think it makes sense
> > > to
> > > keep as an argument here.
> >
> > Why is callback so different from options in your opinion? They are
> > both context-dependent in the very same way. They both do not make
> > any
> > sense outside of their respective scopes for which they were
> > created.
> > And they both do not belong to the globally accessible library,
> > IMO.
>
> Callback makes more sense to me to have as parameters then the
> options
> as one could simply say that callback per se is not part of Grl
> library
> - Plugin developer should keep reference to the callback (as userdata
>   on async functions) in order to finish a operation later on
>
> As functions, get_media_keys or get_requested_keys should always be
> reacheable when a operation is ongoing IMHO.
> After three or four fetch() the user might want to get those keys to
> only write the keys that were requested from application or only
> write the keys that has no value yet. Passing a table of utility
> functions around for that does not seem right to me.

Still do not see how it is different from the callback. Callback
finishes the operation, so the user should pass it as a userdata even
after 3 or 4 or whatever fetch()'s, just like options(), if he wants
some info regarding the operation. These two functions are
complimenting each other in a way: you call options() when you want to
get some info about the operation, and you call callback() when you
want to return some info: media or (I hope in the future it'll be
implemented) error.

> I might prefer to have an actual GrlMedia as parameter and maybe the
> operations options as argument (as table) as well. I think this would
> be enough to remove context-dependend functions -- aside of callback.

Funny you should say that, because that's exactly how I ended up
implementing it. :) Well, at least for the media I did.

> <snip>
> grl_source_search (query, media, options, callback)
>   local userdata = {}
>   if options.count < 5 then
>     grl.callback("fail!")
>     callback()
>   end
>   -- save what you want here
>   userdata.media = media
>   userdata.options = options.req_keys
>   userdata.callback = callback
>   grl.fetch("something", fetch_done, userdata)
> </snip>
>
> What do you think?

search() is a bad example, since it doesn't have access to any kind of
media at all, but here's how the browse() now looks in vk.lua (you can
see the code in #744238)

function grl_source_browse (media, options, callback)
  local skip = options ("skip")
  local count = options ("count")

  if (not media) or (not media.id) then
    ### root folder ###
  else
    local media_id = split (media.id, "_")
    ### work further with media-id ###

And now you want to make the same thing for the options() as well. Yes,
it will make the API clearer, but I wonder if there will be some
performance implications. Here's the list of keys we provide in
get_options:

"type" (like I don't know what function I'm currently writing)
"count"
"skip"
"flags"
"key-filter" (with an additional argument "key name")
"range-filter" (same as previous)
"operation-id"
"media-id" "query" "search" (mutually exclusive, and, IMO, redundant)
"requested-keys"

As you can see, the "key-filter" and "range-filter" are the biggest
problem. Unfortunately, I'm not quite sure how filtering works. Is it a
set of additional restrictions aimed to narrow the results? Then we
probably should provide all of them that are listed in the respective
***_list() functions. How much of them can there be? How much time it
will take to retrieve all of them for every operation?

Also, about the "flags". Right now it returns an int, which is pretty
useless unless you decode it. But even if we return a set of flags
(namely "normal" "full" "idle_loop" and "fast_only") what exactly lua
plugin can use it for? "normal" is a default, when nothing else is on.
"full" means that grilo should use all the plugins available in order
to retrieve the needed info. Pointless in lua. "idle_loop" means (and I
quote) "use idle loop to relay results". Whatever this means, it has
nothing to do with lua. And finally "fast_only". And while yes, lua
could use this info, it is redundant since we already have a list of
requested-keys. And if we provided the list of slow-keys correctly,
then we should be fine.

Cheers,
        George