GNOME Bugzilla – Bug 743321
Add a plugin to use the weboob abstraction layer to access public services
Last modified: 2018-09-24 09:32:02 UTC
Created attachment 295134 [details] [review] Patch to add weboob support inside grilo-plugins Since some month now, I started a plugin for Grilo in order to reuse the weboob project efforts. Weboob[1] is a set of application to process public web services from command line. Weboob mostly supports all actions of grilo: resolve, browse, search. [1] http://weboob.org/ I think my current state is stable enough to start a first review and possibly an integration inside grilo-plugins. My work can be found attached or in my github: https://github.com/guyou/grilo-plugins/tree/weboob PS: you can give an eye to my result as videos: https://www.youtube.com/watch?v=S4IV_fxqye4 https://www.youtube.com/watch?v=sWdY8KuFCHc https://www.youtube.com/watch?v=1PRhFUg1Gf4
Review of attachment 295134 [details] [review]: There's a few comments in there. The main question though is: why would we want this in grilo? We already have the lua factory, for writing this sort of plugins. What sources would it give us in addition to the ones already available through grilo-plugins (and the more sketchy ones in grilo-lua-sources in github.com)? Can we blacklist sources (the ones duplicating grilo ones, such as YouTube or Vimeo)? Isn't there a better way than spawning helpers? ::: src/weboob/Makefile.am @@ +24,3 @@ + +libgrlweboob_la_SOURCES = \ + grl-weboob-shared.h \ Please use tabs to indent. ::: src/weboob/README @@ +1,1 @@ +Weboob is a tool to appreciate the "web outside of the browser". There's no need for that file in here. If there were important information here, we'd put it in gtk-doc comments, so that they end up in the API documentation. @@ +20,3 @@ +In "backends" configuration key, set the list of backends to activate. + +The "all" keyword allow to create a source accessing Weboob without filtering on a given backend. I'd rather it created a unified source, and one for each backend, so that applications can filter them. ::: src/weboob/TODO @@ +1,1 @@ +- add support for audio (http://weboob.org/applications/radioob) This file really doesn't need to be here. ::: src/weboob/grl-weboob.c @@ +264,3 @@ + icon = weboob_module_get_icon (backend); + + source = GRL_WEBOOB_SOURCE (g_object_new (GRL_WEBOOB_SOURCE_TYPE, You probably also want to add source-tags, so that the sources get hidden when the internet isn't available. ::: src/weboob/videoob.c @@ +96,3 @@ + + if (content) { + // Update No C++ comments. @@ +98,3 @@ + // Update + media = content; + } if (NULL != path) { "else" missing, or a linefeed. @@ +225,3 @@ + else + g_debug ("Process videoob terminated with error!"); + stray linefeed. @@ +378,3 @@ + && *error == NULL) { +#define ENABLED_KEYWORD "Enabled:" + if (strncmp(line, ENABLED_KEYWORD, strlen(ENABLED_KEYWORD)) == 0) { g_str_has_prefix() ::: src/weboob/weboob.c @@ +196,3 @@ + + // Transfered + //g_free (name); That's not needed. @@ +220,3 @@ + } + + JsonNode *root = json_parser_get_root (parser); Declare variables at the top of the block. ::: tests/weboob/test_videoob.c @@ +27,3 @@ +#include <grilo.h> + +#include <../../src/weboob/grl-weboob-shared.h> Eek. No, that won't pass "make distcheck". Add -I$(top_srcdir)/src/weboob/
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/grilo/issues/56.