GNOME.org
 

Re: [PATCH] config: rework code to use GKeyFile instead of GHashTable+GValue

Hmmm... I'm not sure that make sense.
A plugin can either use the config framework proposed by grilo or choose
to ignore it. In that case, he's free to use whatever setting system he
wants (or even none).

Regards,

--
Lionel

On Tue, 2011-01-18 at 15:03 +0000, Fabien Lebaillif - Delamare wrote:
> Hi,
> 
> Personally, the problem I see with this type of implementation, is that 
> we are forcing the backend on developers. Using GKeyFile as the core for 
> configuration makes it very hard to implement something based on gconf 
> or dconf.
> 
> We should let the developer decides where the configuration is coming 
> from and not dictate its form "it has to be a .ini type file"
> 
> Fabien
> 
> 
> On 1/18/2011 1:14 PM, llandwerlin gmail com wrote:
> > From: Lionel Landwerlin<lionel g landwerlin linux intel com>
> >
> > Signed-off-by: Lionel Landwerlin<lionel g landwerlin linux intel com>
> > ---
> >   src/data/grl-config.c     |  118 +++++++++++++++++---------------------------
> >   src/data/grl-config.h     |   11 ++--
> >   src/grl-plugin-registry.c |    8 ++-
> >   3 files changed, 56 insertions(+), 81 deletions(-)
> >
> > diff --git a/src/data/grl-config.c b/src/data/grl-config.c
> > index 07866b5..62a4fe5 100644
> > --- a/src/data/grl-config.c
> > +++ b/src/data/grl-config.c
> > @@ -32,6 +32,8 @@
> >   #include "grl-config.h"
> >   #include "grl-log.h"
> >
> > +#define GROUP_NAME "none"
> > +
> >   #define GRL_LOG_DOMAIN_DEFAULT  config_log_domain
> >   GRL_LOG_DOMAIN(config_log_domain);
> >
> > @@ -39,7 +41,7 @@ GRL_LOG_DOMAIN(config_log_domain);
> >     (G_TYPE_INSTANCE_GET_PRIVATE ((o), GRL_TYPE_CONFIG, GrlConfigPrivate))
> >
> >   struct _GrlConfigPrivate {
> > -  GHashTable *config;
> > +  GKeyFile *config;
> >   };
> >
> >   static void grl_config_dispose (GObject *object);
> > @@ -48,15 +50,6 @@ static void grl_config_finalize (GObject *object);
> >   G_DEFINE_TYPE (GrlConfig, grl_config, G_TYPE_OBJECT);
> >
> >   static void
> > -free_val (GValue *val)
> > -{
> > -  if (val) {
> > -    g_value_unset (val);
> > -    g_free (val);
> > -  }
> > -}
> > -
> > -static void
> >   grl_config_class_init (GrlConfigClass *klass)
> >   {
> >     GObjectClass *gobject_class = (GObjectClass *)klass;
> > @@ -71,10 +64,9 @@ static void
> >   grl_config_init (GrlConfig *self)
> >   {
> >     self->priv = GRL_CONFIG_GET_PRIVATE (self);
> > -  self->priv->config = g_hash_table_new_full (g_str_hash,
> > -					      g_str_equal,
> > -					      g_free,
> > -					      (GDestroyNotify) free_val);
> > +  self->priv->config = g_key_file_new ();
> > +
> > +  g_key_file_load_from_data (self->priv->config, "[]\n", -1, G_KEY_FILE_NONE, NULL);
> >   }
> >
> >   static void
> > @@ -124,105 +116,87 @@ grl_config_new (const gchar *plugin, const gchar *source)
> >   void
> >   grl_config_set (GrlConfig *config, const gchar *param, const GValue *value)
> >   {
> > -  GValue *copy;
> >     g_return_if_fail (GRL_IS_CONFIG (config));
> > -  copy = g_new0 (GValue, 1);
> > -  g_value_init (copy, G_VALUE_TYPE (value));
> > -  g_value_copy (value, copy);
> > -  g_hash_table_insert (config->priv->config, g_strdup (param), copy);
> > +
> > +  switch (G_VALUE_TYPE (value)) {
> > +  case G_TYPE_STRING:
> > +    g_key_file_set_string (config->priv->config, GROUP_NAME, param,
> > +                           g_value_get_string (value));
> > +    break;
> > +
> > +  case G_TYPE_FLOAT:
> > +    g_key_file_set_double (config->priv->config, GROUP_NAME, param,
> > +                           g_value_get_double (value));
> > +    break;
> > +
> > +  case G_TYPE_INT:
> > +    g_key_file_set_integer (config->priv->config, GROUP_NAME, param,
> > +                            g_value_get_int (value));
> > +    break;
> > +
> > +  case G_TYPE_BOOLEAN:
> > +    g_key_file_set_boolean (config->priv->config, GROUP_NAME, param,
> > +                            g_value_get_boolean (value));
> > +    break;
> > +
> > +  default:
> > +    g_return_if_reached ();
> > +    break;
> > +  }
> >   }
> >
> >   void
> >   grl_config_set_string (GrlConfig *config, const gchar *param, const gchar *value)
> >   {
> > -  GValue v = { 0 };
> > -  g_value_init (&v, G_TYPE_STRING);
> > -  g_value_set_string (&v, value);
> > -  grl_config_set (config, param,&v);
> > -  g_value_unset (&v);
> > +  g_key_file_set_string (config->priv->config, GROUP_NAME, param, value);
> >   }
> >
> >   void
> >   grl_config_set_int (GrlConfig *config, const gchar *param, gint value)
> >   {
> > -  GValue v = { 0 };
> > -  g_value_init (&v, G_TYPE_INT);
> > -  g_value_set_int (&v, value);
> > -  grl_config_set (config, param,&v);
> > +  g_key_file_set_integer (config->priv->config, GROUP_NAME, param, value);
> >   }
> >
> >
> >   void
> >   grl_config_set_float (GrlConfig *config, const gchar *param, gfloat value)
> >   {
> > -  GValue v = { 0 };
> > -  g_value_init (&v, G_TYPE_FLOAT);
> > -  g_value_set_float (&v, value);
> > -  grl_config_set (config, param,&v);
> > +  g_key_file_set_double (config->priv->config, GROUP_NAME, param, (gdouble) value);
> >   }
> >
> >   void
> >   grl_config_set_boolean (GrlConfig *config, const gchar *param, gboolean value)
> >   {
> > -  GValue v = { 0 };
> > -  g_value_init (&v, G_TYPE_BOOLEAN);
> > -  g_value_set_boolean (&v, value);
> > -  grl_config_set (config, param,&v);
> > -}
> > -
> > -const GValue *
> > -grl_config_get (GrlConfig *config, const gchar *param)
> > -{
> > -  g_return_val_if_fail (GRL_IS_CONFIG (config), NULL);
> > -  return g_hash_table_lookup (config->priv->config, param);
> > +  g_key_file_set_boolean (config->priv->config, GROUP_NAME, param, value);
> >   }
> >
> > -const gchar *
> > +gchar *
> >   grl_config_get_string (GrlConfig *config, const gchar *param)
> >   {
> >     g_return_val_if_fail (GRL_IS_CONFIG (config), NULL);
> > -  const GValue *value = grl_config_get (config, param);
> > -  if (!value || !G_VALUE_HOLDS_STRING (value)) {
> > -    return NULL;
> > -  } else {
> > -    return g_value_get_string (value);
> > -  }
> > +  return g_key_file_get_string (config->priv->config, GROUP_NAME, param, NULL);
> >   }
> >
> >   gint
> >   grl_config_get_int (GrlConfig *config, const gchar *param)
> >   {
> >     g_return_val_if_fail (GRL_IS_CONFIG (config), 0);
> > -  const GValue *value = grl_config_get (config, param);
> > -  if (!value || !G_VALUE_HOLDS_INT (value)) {
> > -    return 0;
> > -  } else {
> > -    return g_value_get_int (value);
> > -  }
> > +  return g_key_file_get_integer (config->priv->config, GROUP_NAME, param, NULL);
> >   }
> >
> >   gfloat
> >   grl_config_get_float (GrlConfig *config, const gchar *param)
> >   {
> >     g_return_val_if_fail (GRL_IS_CONFIG (config), 0.0);
> > -  const GValue *value = grl_config_get (config, param);
> > -  if (!value || !G_VALUE_HOLDS_FLOAT (value)) {
> > -    return 0.0;
> > -  } else {
> > -    return g_value_get_float (value);
> > -  }
> > +  return (gfloat) g_key_file_get_double (config->priv->config, GROUP_NAME,
> > +                                         param, NULL);
> >   }
> >
> >   gboolean
> >   grl_config_get_boolean (GrlConfig *config, const gchar *param)
> >   {
> >     g_return_val_if_fail (GRL_IS_CONFIG (config), FALSE);
> > -  const GValue *value = grl_config_get (config, param);
> > -  if (!value || !G_VALUE_HOLDS_BOOLEAN (value)) {
> > -    return FALSE;
> > -  } else {
> > -    return g_value_get_boolean (value);
> > -  }
> > +  return g_key_file_get_boolean (config->priv->config, GROUP_NAME, param, NULL);
> >   }
> >
> >   /**
> > @@ -348,7 +322,7 @@ grl_config_set_password(GrlConfig *config, const gchar *password)
> >    *
> >    * Since: 0.1.4
> >    */
> > -const gchar *
> > +gchar *
> >   grl_config_get_plugin (GrlConfig *config)
> >   {
> >     return grl_config_get_string (GRL_CONFIG (config),
> > @@ -363,7 +337,7 @@ grl_config_get_plugin (GrlConfig *config)
> >    *
> >    * Since: 0.1.4
> >    */
> > -const gchar *
> > +gchar *
> >   grl_config_get_api_key (GrlConfig *config)
> >   {
> >     return grl_config_get_string (GRL_CONFIG (config),
> > @@ -378,7 +352,7 @@ grl_config_get_api_key (GrlConfig *config)
> >    *
> >    * Since: 0.1.4
> >    */
> > -const gchar *
> > +gchar *
> >   grl_config_get_api_token (GrlConfig *config)
> >   {
> >     return grl_config_get_string (GRL_CONFIG (config),
> > @@ -393,7 +367,7 @@ grl_config_get_api_token (GrlConfig *config)
> >    *
> >    * Since: 0.1.4
> >    */
> > -const gchar *
> > +gchar *
> >   grl_config_get_api_secret (GrlConfig *config)
> >   {
> >     return grl_config_get_string (GRL_CONFIG (config),
> > diff --git a/src/data/grl-config.h b/src/data/grl-config.h
> > index b2a0f63..f0f46d2 100644
> > --- a/src/data/grl-config.h
> > +++ b/src/data/grl-config.h
> > @@ -112,13 +112,13 @@ void grl_config_set_username (GrlConfig *config, const gchar *secret);
> >
> >   void grl_config_set_password (GrlConfig *config, const gchar *secret);
> >
> > -const gchar *grl_config_get_plugin (GrlConfig *config);
> > +gchar *grl_config_get_plugin (GrlConfig *config);
> >
> > -const gchar *grl_config_get_api_key (GrlConfig *config);
> > +gchar *grl_config_get_api_key (GrlConfig *config);
> >
> > -const gchar *grl_config_get_api_token (GrlConfig *config);
> > +gchar *grl_config_get_api_token (GrlConfig *config);
> >
> > -const gchar *grl_config_get_api_secret (GrlConfig *config);
> > +gchar *grl_config_get_api_secret (GrlConfig *config);
> >
> >   const gchar *grl_config_get_username (GrlConfig *config);
> >
> > @@ -139,9 +139,8 @@ void grl_config_set_float (GrlConfig *config, const gchar *param, gfloat value);
> >
> >   void grl_config_set_boolean (GrlConfig *config, const gchar *param, gboolean value);
> >
> > -const GValue *grl_config_get (GrlConfig *config, const gchar *param);
> >
> > -const gchar *grl_config_get_string (GrlConfig *config, const gchar *param);
> > +gchar *grl_config_get_string (GrlConfig *config, const gchar *param);
> >
> >   gint grl_config_get_int (GrlConfig *config, const gchar *param);
> >
> > diff --git a/src/grl-plugin-registry.c b/src/grl-plugin-registry.c
> > index c33657f..6e20f0d 100644
> > --- a/src/grl-plugin-registry.c
> > +++ b/src/grl-plugin-registry.c
> > @@ -584,7 +584,7 @@ grl_plugin_registry_load_all (GrlPluginRegistry *registry, GError **error)
> >                             "All configured plugin paths are invalid. "   \
> >                             "Failed to load plugins.");
> >     }
> > -
> > +
> >     return loaded_one;
> >   }
> >
> > @@ -882,7 +882,7 @@ grl_plugin_registry_add_config (GrlPluginRegistry *registry,
> >                                   GrlConfig *config,
> >                                   GError **error)
> >   {
> > -  const gchar *plugin_id;
> > +  gchar *plugin_id;
> >     GList *configs = NULL;
> >
> >     g_return_val_if_fail (config != NULL, FALSE);
> > @@ -899,7 +899,7 @@ grl_plugin_registry_add_config (GrlPluginRegistry *registry,
> >       }
> >       return FALSE;
> >     }
> > -
> > +
> >     configs = g_hash_table_lookup (registry->priv->configs, plugin_id);
> >     if (configs) {
> >       /* Notice that we are using g_list_append on purpose to avoid
> > @@ -912,6 +912,8 @@ grl_plugin_registry_add_config (GrlPluginRegistry *registry,
> >   			 configs);
> >     }
> >
> > +  g_free (plugin_id);
> > +
> >     return TRUE;
> >   }
> >
> 
>
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]