GNOME.org
 

Re: [evolution-patches] #54338:Problem with vFolders of same name.

Just FYI, i've attached the fix I ended up doing for this, as well as an
example of how the vfolders.xml file format has changed.  It
transparently loads the old file too.

It fixes it 'better', because it fixes a basic bug in the vfolder-rule
code to start with, reduces the code, and simplifies the api too.
 Michael


On Fri, 2004-02-20 at 09:28 +0800, Not Zed wrote:

> On Thu, 2004-02-19 at 19:16 +0800, Bill Zhu wrote:
> 
> > Hi, Not Zed
> > 在2004年02月15日的19:07，Not Zed写道：
> > > But that does'nt make any sense either, for vfolders the source should
> > > be NULL (i think ...), or if not, all the same.  searches will have
> > > different sources though.
> > > 
> > > Why is that bit of code not finding a match, is it passing NULL as
> > > source, and the find method isn't dealing with it appropriately, or is
> > > it passing a different source when it isn't supposed to?  Or is the
> > > problem somewhere completely different (e.g. mail-vfolder.c).
> > 
> > It is not passing NULL. the sources of vfolders are marked as:       
> > "specific", "local", "remote_active" and "local_remote_active".
> > As your said, they should be all NULL or all the same, shouldn't they?
> > but they looks like specified on purpose, is this a mistake?
> 
> Ahh, finally we find the real source of the of the problem (pardon the
> pun!).
> 
> So I incorrectly and stupidly re-used the filter-rule source attribute
> as a short-cut to defining the vFolder source-type as well ... the fewl
> me.
> 
> Two solutions I guess:
>  - make a bunch of rule finding stuff on rule-context virtual, so the
> vfolder stuff can make it behave as though source is always NULL
>     - no change required to vfolders.xml format
>     - kinda messy, a lot of non-obvious functionality going on
> 
>  - make the vfolder code store it in a separate attribute instead
>     - vfolders.xml format changes slightly
>     - will need either upgrade code, or old-version handling of the file
> (the later would be the simplest/easiest/most robust).
> 
> I'll go for the second one.  Since i've already investigated it, I might
> hack it up this morning, shouldn't take too long.
> 
> Thanks for finding the base problem here, now a 'more correct' fix can
> be done.
> 
>  Michael
> 
> 
> _______________________________________________
> Evolution-patches mailing list
> Evolution-patches lists ximian com
> http://lists.ximian.com/mailman/listinfo/evolution-patches

Index: mail/ChangeLog
===================================================================
RCS file: /cvs/gnome/evolution/mail/ChangeLog,v
retrieving revision 1.3115
diff -u -3 -r1.3115 ChangeLog
--- mail/ChangeLog	20 Feb 2004 07:30:35 -0000	1.3115
+++ mail/ChangeLog	20 Feb 2004 09:14:58 -0000
@@ -1,5 +1,8 @@
 2004-02-20  Not Zed  <NotZed Ximian com>
 
+	* mail-vfolder.c (mail_vfolder_add_uri): fix for vfolder-rule api
+	changes.
+
 	* mail-folder-cache.c (unset_folder_info, setup_folder)
 	(rename_folders): switch the sense of the no select checks.  TEST!
 
Index: mail/mail-vfolder.c
===================================================================
RCS file: /cvs/gnome/evolution/mail/mail-vfolder.c,v
retrieving revision 1.116
diff -u -3 -r1.116 mail-vfolder.c
--- mail/mail-vfolder.c	5 Feb 2004 05:14:59 -0000	1.116
+++ mail/mail-vfolder.c	20 Feb 2004 09:15:00 -0000
@@ -427,9 +427,9 @@
 		/* dont auto-add any sent/drafts folders etc, they must be explictly listed as a source */
 		if (rule->source
 		    && !is_ignore
-		    && ((!strcmp(rule->source, "local") && !remote)
-			|| (!strcmp(rule->source, "remote_active") && remote)
-			|| (!strcmp(rule->source, "local_remote_active"))))
+		    && ((((VfolderRule *)rule)->with == VFOLDER_RULE_WITH_LOCAL && !remote)
+			|| (((VfolderRule *)rule)->with == VFOLDER_RULE_WITH_REMOTE_ACTIVE && remote)
+			|| (((VfolderRule *)rule)->with == VFOLDER_RULE_WITH_LOCAL_REMOTE_ACTIVE)))
 			found = TRUE;
 		
 		/* we check using the store uri_cmp since its more accurate */
@@ -660,9 +660,11 @@
 	if (rule->source) {
 		LOCK();
 		for (i=0;i<2;i++) {
-			if (i==0 && (!strcmp(rule->source, "local") || !strcmp(rule->source, "local_remote_active")))
+			if (i==0 && (((VfolderRule *)rule)->with == VFOLDER_RULE_WITH_LOCAL
+				     || ((VfolderRule *)rule)->with == VFOLDER_RULE_WITH_LOCAL_REMOTE_ACTIVE))
 				l = source_folders_local;
-			else if (i==1 && (!strcmp(rule->source, "remote_active") || !strcmp(rule->source, "local_remote_active")))
+			else if (i==1 && (((VfolderRule *)rule)->with == VFOLDER_RULE_WITH_REMOTE_ACTIVE
+				     || ((VfolderRule *)rule)->with == VFOLDER_RULE_WITH_LOCAL_REMOTE_ACTIVE))
 				l = source_folders_remote;
 			else
 				l = NULL;
Index: filter/ChangeLog
===================================================================
RCS file: /cvs/gnome/evolution/filter/ChangeLog,v
retrieving revision 1.391
diff -u -3 -r1.391 ChangeLog
--- filter/ChangeLog	17 Feb 2004 07:01:12 -0000	1.391
+++ filter/ChangeLog	20 Feb 2004 09:15:01 -0000
@@ -1,3 +1,18 @@
+2004-02-20  Not Zed  <NotZed Ximian com>
+
+	** See bug #54338.
+
+	* vfolder-rule.c: instead of overriding the filter-rule's 'source'
+	attribute for 'with sources', add a 'with' enum.
+	(xml_decode): handle loading old-format files, as well as new
+	ones, where the with is a prop on the sources xml.
+	(xml_encode): set 'with' on save.
+	(rule_copy): copy across with value.
+	(get_widget): just hook onto option menu changed, dont bother
+	setting up any per-item callbacks.
+	(select_source_with_changed): callback to monitor source 'with'
+	type.
+
 2004-02-17  Not Zed  <NotZed Ximian com>
 
 	* filter-int.c (int_clone): implement, since we dont store the
Index: filter/vfolder-editor.c
===================================================================
RCS file: /cvs/gnome/evolution/filter/vfolder-editor.c,v
retrieving revision 1.23
diff -u -3 -r1.23 vfolder-editor.c
--- filter/vfolder-editor.c	11 Apr 2003 19:11:50 -0000	1.23
+++ filter/vfolder-editor.c	20 Feb 2004 09:15:02 -0000
@@ -39,10 +39,8 @@
 static void vfolder_editor_init (VfolderEditor *ve);
 static void vfolder_editor_finalise (GObject *obj);
 
-
 static RuleEditorClass *parent_class = NULL;
 
-
 GtkType
 vfolder_editor_get_type (void)
 {
Index: filter/vfolder-rule.c
===================================================================
RCS file: /cvs/gnome/evolution/filter/vfolder-rule.c,v
retrieving revision 1.42
diff -u -3 -r1.42 vfolder-rule.c
--- filter/vfolder-rule.c	16 Jan 2004 07:00:22 -0000	1.42
+++ filter/vfolder-rule.c	20 Feb 2004 09:15:03 -0000
@@ -52,10 +52,16 @@
 static void vfolder_rule_init (VfolderRule *vr);
 static void vfolder_rule_finalise (GObject *obj);
 
+/* DO NOT internationalise these strings */
+const char *with_names[] = {
+	"specific",
+	"local",
+	"remote_active",
+	"local_remote_active"
+};
 
 static FilterRuleClass *parent_class = NULL;
 
-
 GType
 vfolder_rule_get_type (void)
 {
@@ -103,7 +109,7 @@
 static void
 vfolder_rule_init (VfolderRule *vr)
 {
-	;
+	vr->with = VFOLDER_RULE_WITH_SPECIFIC;
 }
 
 static void
@@ -216,7 +222,7 @@
 	
 	/* We have to have at least one source set in the "specific" case.
 	   Do not translate this string! */
-	if (fr->source && !strcmp (fr->source, "specific") && VFOLDER_RULE (fr)->sources == NULL) {
+	if (((VfolderRule *)fr)->with == VFOLDER_RULE_WITH_SPECIFIC && ((VfolderRule *)fr)->sources == NULL) {
 		/* FIXME: set a parent window? */
 		dialog = gtk_message_dialog_new (NULL, GTK_DIALOG_DESTROY_WITH_PARENT,
 						 GTK_MESSAGE_ERROR, GTK_BUTTONS_CLOSE,
@@ -263,8 +269,10 @@
 	
         node = FILTER_RULE_CLASS (parent_class)->xml_encode (fr);
 	g_assert(node != NULL);
+	g_assert(vr->with >= 0 && vr->with < sizeof(with_names)/sizeof(with_names[0]));
 	set = xmlNewNode (NULL, "sources");
 	xmlAddChild (node, set);
+	xmlSetProp(set, "with", with_names[vr->with]);
 	l = vr->sources;
 	while (l) {
 		work = xmlNewNode (NULL, "folder");
@@ -276,28 +284,55 @@
 	return node;
 }
 
+static void
+set_with(VfolderRule *vr, const char *name)
+{
+	int i;
+
+	for (i=0;i<sizeof(with_names)/sizeof(with_names[0]);i++) {
+		if (!strcmp(name, with_names[i])) {
+			vr->with = i;
+			return;
+		}
+	}
+
+	vr->with = 0;
+}
+
 static int
 xml_decode (FilterRule *fr, xmlNodePtr node, struct _RuleContext *f)
 {
 	xmlNodePtr set, work;
 	int result;
 	VfolderRule *vr = (VfolderRule *)fr;
-	char *uri;
+	char *tmp;
 	
         result = FILTER_RULE_CLASS (parent_class)->xml_decode (fr, node, f);
 	if (result != 0)
 		return result;
 	
+	/* handle old format file, vfolder source is in filterrule */
+	if (strcmp(fr->source, "incoming") != 0) {
+		set_with(vr, fr->source);
+		g_free(fr->source);
+		fr->source = g_strdup("incoming");
+	}
+
 	set = node->children;
 	while (set) {
-		if (!strcmp (set->name, "sources")) {
+		if (!strcmp(set->name, "sources")) {
+			tmp = xmlGetProp(set, "with");
+			if (tmp) {
+				set_with(vr, tmp);
+				xmlFree(tmp);
+			}
 			work = set->children;
 			while (work) {
-				if (!strcmp (work->name, "folder")) {
-					uri = xmlGetProp (work, "uri");
-					if (uri) {
-						vr->sources = g_list_append (vr->sources, g_strdup (uri));
-						xmlFree (uri);
+				if (!strcmp(work->name, "folder")) {
+					tmp = xmlGetProp(work, "uri");
+					if (tmp) {
+						vr->sources = g_list_append(vr->sources, g_strdup(tmp));
+						xmlFree(tmp);
 					}
 				}
 				work = work->next;
@@ -330,11 +365,12 @@
 		vdest->sources = g_list_append (vdest->sources, g_strdup (uri));
 		node = node->next;
 	}
-	
+
+	vdest->with = vsrc->with;
+
 	FILTER_RULE_CLASS (parent_class)->copy (dest, src);
 }
 
-
 enum {
 	BUTTON_ADD,
 	BUTTON_REMOVE,
@@ -385,11 +421,14 @@
 }
 
 static void
-select_source_with (GtkWidget *widget, struct _source_data *data)
+select_source_with_changed(GtkWidget *widget, struct _source_data *data)
 {
-	char *source = g_object_get_data ((GObject *) widget, "source");
-	
-	filter_rule_set_source ((FilterRule *) data->vr, source);
+	vfolder_rule_with_t with;
+
+	with = gtk_option_menu_get_history((GtkOptionMenu *)widget);
+	if (with < VFOLDER_RULE_WITH_SPECIFIC || with > VFOLDER_RULE_WITH_LOCAL_REMOTE_ACTIVE)
+		with = 0;
+	data->vr->with = with;
 }
 
 /* attempt to make a 'nice' folder name out of the raw uri */
@@ -560,15 +599,6 @@
 	return scrolled;
 }
 
-
-/* DO NOT internationalise these strings */
-const char *source_names[] = {
-	"specific",
-	"local",
-	"remote_active",
-	"local_remote_active"
-};
-
 static GtkWidget *
 get_widget (FilterRule *fr, RuleContext *rc)
 {
@@ -579,8 +609,7 @@
 	const char *source;
 	GtkTreeIter iter;
 	GladeXML *gui;
-	int i, row;
-	GList *l;
+	int i;
 	
         widget = FILTER_RULE_CLASS (parent_class)->get_widget (fr, rc);
 	
@@ -614,31 +643,8 @@
 	g_signal_connect (data->list, "cursor-changed", G_CALLBACK (select_source), data);
 	
 	omenu = (GtkOptionMenu *) glade_xml_get_widget (gui, "source_option");
-	l = GTK_MENU_SHELL (omenu->menu)->children;
-	i = 0;
-	row = 0;
-	while (l) {
-		GtkWidget *item = GTK_WIDGET (l->data);
-		
-		/* make sure that the glade is in sync with the source list! */
-		if (i < sizeof (source_names) / sizeof (source_names[0])) {
-			g_object_set_data ((GObject *) item, "source", (char *) source_names[i]);
-			if (fr->source && strcmp (source_names[i], fr->source) == 0) {
-				row = i;
-			}
-		} else {
-			g_warning ("Glade file " FILTER_GLADEDIR "/filter.glade out of sync with editor code");
-		}
-		
-		g_signal_connect (item, "activate", G_CALLBACK (select_source_with), data);
-		
-		i++;
-		l = l->next;
-	}
-	
-	gtk_option_menu_set_history (omenu, row);
-	if (fr->source == NULL)
-		filter_rule_set_source (fr, (char *) source_names[row]);
+	gtk_option_menu_set_history(omenu, vr->with);
+	g_signal_connect(omenu, "changed", G_CALLBACK(select_source_with_changed), data);
 	
 	set_sensitive (data);
 		
Index: filter/vfolder-rule.h
===================================================================
RCS file: /cvs/gnome/evolution/filter/vfolder-rule.h,v
retrieving revision 1.6
diff -u -3 -r1.6 vfolder-rule.h
--- filter/vfolder-rule.h	5 Nov 2002 22:17:16 -0000	1.6
+++ filter/vfolder-rule.h	20 Feb 2004 09:15:03 -0000
@@ -33,12 +33,23 @@
 #define IS_VFOLDER_RULE_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), VFOLDER_TYPE_RULE))
 #define VFOLDER_RULE_GET_CLASS(obj)  (G_TYPE_INSTANCE_GET_CLASS ((obj), VFOLDER_TYPE_RULE, VfolderRuleClass))
 
+/* perhaps should be bits? */
+enum _vfolder_rule_with_t {
+	VFOLDER_RULE_WITH_SPECIFIC,
+	VFOLDER_RULE_WITH_LOCAL,
+	VFOLDER_RULE_WITH_REMOTE_ACTIVE,
+	VFOLDER_RULE_WITH_LOCAL_REMOTE_ACTIVE,
+};
+
 typedef struct _VfolderRule VfolderRule;
 typedef struct _VfolderRuleClass VfolderRuleClass;
 
+typedef enum _vfolder_rule_with_t vfolder_rule_with_t;
+
 struct _VfolderRule {
-	FilterRule parent_object;
+	FilterRule rule;
 	
+	vfolder_rule_with_t with;
 	GList *sources;		/* uri's of the source folders */
 };
 

--- .evolution/mail/vfolders.xml.save	2004-02-20 14:07:54.000000000 +0800
+++ .evolution/mail/vfolders.xml	2004-02-20 16:46:46.295348072 +0800
@@ -1,7 +1,7 @@
 <?xml version="1.0"?>
 <filteroptions>
   <ruleset>
-    <rule grouping="all" source="local">
+    <rule grouping="all" source="incoming">
       <title>Important mail (local)</title>
       <partset>
         <part name="status">
@@ -9,9 +9,9 @@
           <value name="flag" type="option" value="Flagged"/>
         </part>
       </partset>
-      <sources/>
+      <sources with="local"/>
     </rule>
-    <rule grouping="all" source="local">
+    <rule grouping="all" source="incoming">
       <title>Unread mail (local)</title>
       <partset>
         <part name="status">
@@ -19,9 +19,9 @@
           <value name="flag" type="option" value="Seen"/>
         </part>
       </partset>
-      <sources/>
+      <sources with="local"/>
     </rule>
-    <rule grouping="all" source="specific">
+    <rule grouping="all" source="incoming">
       <title>Important copy</title>
       <partset>
         <part name="sexp">
@@ -30,7 +30,7 @@
           </value>
         </part>
       </partset>
-      <sources>
+      <sources with="specific">
         <folder uri="email://vfolder local/Important%20mail%20(local)"/>
       </sources>
     </rule>
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]