On Sun, 6 Jan 2013 00:08:38 +0000
Michael Blumenkrantz <michael.blumenkra...@gmail.com> wrote:
> On Sun, 6 Jan 2013 03:59:17 +0400
> Igor Murzov <intergalactic.anonym...@gmail.com> wrote:
>
> > On Fri, 4 Jan 2013 16:29:56 +0900
> > Carsten Haitzler (The Rasterman) <ras...@rasterman.com> wrote:
> >
> > > On Tue, 1 Jan 2013 23:30:49 +0400 Igor Murzov
> > > <intergalactic.anonym...@gmail.com> said:
> > >
> > > > Happy new year everyone!
> > > >
> > > > While i was playing with eweather, i found out that when its gadget
> > > > is removed and module is unloaded, configuration gets lost.
> > > > At first i thought that this issue is specific to eweather and
> > > > that's some stupid bug, that went unnoticed.
> > > > But it looks like configs are handled this way intentionally.
> > > > If some gadget can have multiple instances, every instance gets
> > > > its own id. When user adds a new instance, NULL is passed to
> > > > _gc_init() as id. The same happens, if user unloaded a module in the
> > > > past for some reason, then changed his mind and loaded the module once
> > > > again. And you may expect that the last configuration is loaded in
> > > > this case, but NO... id gets incremented and you'll get the default
> > > > settings. That's very strange -- configurations are stored and never
> > > > reused again.
> > > > That doesn't feel like an expected behaviour to me. I would
> > > > expect GADCON_CLIENT_CONFIG_GET() to load the last available
> > > > configuration if id is NULL. Or maybe even last id but one when the
> > > > gadget that has the last id already exists. So some smart solution
> > > > may be required to fix the issue. Any ideas?
> > > >
> > >
> > > hmm module unload SHOULD be separate to the gadcon client config. i
> > > designed
> > > them to be separate so u can "unload" and "load" a module and magically
> > > the
> > > gadget appears where it was configured for before. of course each module
> > > is in
> > > charge of remembering its OWN list of "instances" and names/id's for them
> > > and
> > > corresponding config for that instance. the gadcon entry is separate and
> > > when
> > > the gadcon inits it asks for a provider of that name/class - and if it
> > > finds
> > > one (the module provides it) it asks it to init the gadget...
> >
> > What you describing here works fine. Unloading module doesn't change
> > gagdgets
> > position or configuration.
> > But if gadget is removed and module is unloaded, then gadget's configuration
> > gets lost.
> >
> > Let me explain.
> >
> > If some module has only one configuration item in its config, then gadget
> > loads the config and uses settings stored in it, and everything is fine.
> >
> > Things are different with modules that have multiple configurations in
> > their configs.
> > If some module has multiple configuration items in its config, then gadget
> > loads the config and then it searches for a specific configuration item that
> > has a specific id.
> >
> > The thing is that if the gadget was removed, then next time when you decide
> > to add the new instance, that new instance gets NULL as its id in
> > _gc_init().
> > And _gc_init() will call GADCON_CLIENT_CONFIG_GET(..., id) to get a
> > configuration item. But GADCON_CLIENT_CONFIG_GET(..., NULL) won't get ANY of
> > the existing configurations in this case, it will simply generate a new id,
> > which
> > will result in getting the default configuration. And that is not what
> > users expect.
> >
> > > the point was that we could ship default config FULLY populated with every
> > > gadget and then only the modules u want provide the gadgets and
> > > non-provided
> > > modules are just left out with the user non-the-wiser.
> >
> > I have no idea what you are talking about here :D
> >
> > > something has gone wrong here. what you think should work... should.
> >
>
> if an instance is removed by the user, its config is deleted. this is
> intentional.
To make things clearer, I changed config_item_get() in EWeather to demonstrate
how i think the function should look like. I've tried to comment everything,
I hope this will help to understand the code. Here it is:
----------------------------------------------------------
EINTERN Config_Item *
_weather_config_item_get(Instance *inst, const char *id)
{
Config_Item *ci;
char buf[128];
/* try to find an item with the requested id */
if (id)
{
Eina_List *l;
EINA_LIST_FOREACH(weather_cfg->items, l, ci)
{
if (!ci->id) continue;
if (!strcmp(ci->id, id))
{
ci->inst = inst;
return ci; /* got it. return configuration item */
}
}
/* uh-oh, no such item in config */
id = NULL;
}
/* id is NULL, but we found previous configurations, so
/* reuse the last one instead of setting default values */
if (!id && weather_cfg->items)
{
const char *p;
int num = 0;
ci = eina_list_last(weather_cfg->items)->data;
p = strrchr(ci->id, '.');
if (p) num = atoi(p + 1) + 1; /* set unique id to avoid collision */
snprintf(buf, sizeof(buf), "%s.%d", _gc_name(), num);
ci->id = eina_stringshare_add(id);
return ci;
}
/* no usable previous configurations found. set the default values */
snprintf(buf, sizeof(buf), "%s.0", _gc_name());
ci = E_NEW(Config_Item, 1);
ci->id = eina_stringshare_add(buf);
ci->celcius = 0;
ci->location = eina_stringshare_add("623164");
ci->google = eina_stringshare_add("Paris France");
ci->inst = inst;
ci->plugin = eina_stringshare_add("Yahoo");
ci->poll_time = 30 * 60; // 30 minutes
weather_cfg->items = eina_list_append(weather_cfg->items, ci);
return ci;
}
----------------------------------------------------------
-- Igor
>From 8bc1453b618fcc95ba0594771c7a3fe1ce2da0ba Mon Sep 17 00:00:00 2001
From: Igor Murzov <e-m...@date.by>
Date: Sun, 6 Jan 2013 18:34:19 +0400
Subject: [PATCH] eweather: Reuse previous configurations when possible
---
eweather/src/module/e_mod_config.c | 58 +++++++++++++++++++++-----------------
1 file changed, 32 insertions(+), 26 deletions(-)
diff --git a/eweather/src/module/e_mod_config.c b/eweather/src/module/e_mod_config.c
index 4f14376..ddc1a50 100644
--- a/eweather/src/module/e_mod_config.c
+++ b/eweather/src/module/e_mod_config.c
@@ -10,46 +10,52 @@ EINTERN Config_Item *
_weather_config_item_get(Instance *inst, const char *id)
{
Config_Item *ci;
+ char buf[128];
- if (!id)
- {
- char buf[128];
- int num = 0;
-
- /* Create id */
- if (weather_cfg->items)
- {
- const char *p;
-
- ci = eina_list_last(weather_cfg->items)->data;
- p = strrchr(ci->id, '.');
- if (p) num = atoi(p + 1) + 1;
- }
- snprintf(buf, sizeof(buf), "%s.%d", _gc_name(), num);
- id = buf;
- }
- else
+ /* try to find an item with the requested id */
+ if (id)
{
Eina_List *l;
EINA_LIST_FOREACH(weather_cfg->items, l, ci)
- {
- if (!ci->id) continue;
- if (!strcmp(ci->id, id))
+ {
+ if (!ci->id) continue;
+ if (!strcmp(ci->id, id))
{
ci->inst = inst;
- return ci;
+ return ci; /* got it. return configuration item */
}
- }
+ }
+ /* uh-oh, no such item in config */
+ id = NULL;
}
+
+ /* id is NULL, but we found previous configurations, so
+ /* reuse the last one instead of setting default values */
+ if (!id && weather_cfg->items)
+ {
+ const char *p;
+ int num = 0;
+
+ ci = eina_list_last(weather_cfg->items)->data;
+ p = strrchr(ci->id, '.');
+ if (p) num = atoi(p + 1) + 1; /* set unique id to avoid collision */
+ snprintf(buf, sizeof(buf), "%s.%d", _gc_name(), num);
+ ci->id = eina_stringshare_add(id);
+ return ci;
+ }
+
+ /* no usable previous configurations found. set the default values */
+ snprintf(buf, sizeof(buf), "%s.0", _gc_name());
+
ci = E_NEW(Config_Item, 1);
- ci->id = eina_stringshare_add(id);
+ ci->id = eina_stringshare_add(buf);
ci->celcius = 0;
ci->location = eina_stringshare_add("623164");
ci->google = eina_stringshare_add("Paris France");
ci->inst = inst;
- ci->plugin = eina_stringshare_add("yahoo");
- ci->poll_time = 30*60; //30 minutes
+ ci->plugin = eina_stringshare_add("Yahoo");
+ ci->poll_time = 30 * 60; //30 minutes
weather_cfg->items = eina_list_append(weather_cfg->items, ci);
return ci;
--
1.7.12.1
------------------------------------------------------------------------------
Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS,
MVC, Windows 8 Apps, JavaScript and much more. Keep your skills current
with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft
MVPs and experts. ON SALE this month only -- learn more at:
http://p.sf.net/sfu/learnmore_123012
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
