GNOME Bugzilla – Bug 725188
Lua-Factory: extend fetch operation
Last modified: 2014-02-26 09:31:09 UTC
Currently, the lua-source is able to fetch a single url doing: grl.fetch("url", "callback_fn") Sometimes is necessary to have multiples fetchs before dealing with the data in the callback_fn. Extend the grl.fetch from single url to an array of urls.
Created attachment 270344 [details] [review] Multiple urls to fetch as an array in grl.fetch The result of grl.fetch an array of urls is a array of data, where first data element corresponds to the first url element and so on.
Review of attachment 270344 [details] [review]: Looks good overall. ::: src/lua-factory/grl-lua-library.c @@ +215,3 @@ + for (i = 0; i < fo->num_urls; i++) + if (fo->data[i] == NULL) + goto fetch_op_cleanup; that doesn't tell me much about what's supposed to happen after "fetch_op_cleanup". @@ +482,3 @@ + num_urls = (lua_isstring (L, 1)) ? 1 : luaL_len (L, 1); + urls = g_malloc0 (sizeof (gchar *) * num_urls); use g_new0 instead of g_malloc0 @@ +540,3 @@ + /* shared data between urls */ + data = g_malloc0 (sizeof(gchar *) * num_urls); Ditto.
Review of attachment 270344 [details] [review]: ::: src/lua-factory/grl-lua-library.c @@ +219,3 @@ lua_getglobal (L, fo->lua_cb); + + if (fo->num_urls == 1) { Not quite good enough. If I want to avoid having to check whether the results are a table or a string in my code, I'd probably always pass a table with a single, or multiple URLs and expect the result to always be a table which I can iterate over. You'll need to add a member to the FetchOperation struct to remember whether the input is a table or not.
Fixed the niggles mentioned in the review and pushed.