[PATCH] : Filters (against CVS)

[Emmanuel <e allaud wanadoo fr>]

	Hi all,
this patch corrects the save/load bugs for filters.
It replaces the preceding patch.
Bye
Manu
--- /home/manu/prog/balsa-cvs/balsa/libbalsa/filter-file.h	Mon Nov 26 10:07:00 2001
+++ balsa/libbalsa/filter-file.h	Tue Nov 27 11:11:25 2001
@@ -43,8 +43,11 @@
 
 /* Save conditions list using filter_section_name as prefix to create sections */
 
-void libbalsa_conditions_save_config(GSList * conds,gchar* prefix,
-                                     gchar * filter_section_name);
+void libbalsa_clean_condition_sections(const gchar * prefix,
+				       const gchar * filter_section_name);
+
+void libbalsa_conditions_save_config(GSList * conds,const gchar * prefix,
+                                     const gchar * filter_section_name);
 
 void libbalsa_filter_save_config(LibBalsaFilter * f);
 
@@ -59,9 +62,5 @@
 /* Saves the filters associated to the mailbox */
 
 void libbalsa_mailbox_filters_save_config(LibBalsaMailbox * mbox);
-
-/* Cleans all condition sections for a given filter section */
-
-void libbalsa_filter_clean_conditions(gchar *prefix,gchar* filter_section_name);
 
 #endif  /* __FILTER_FILE_H__ */
--- /home/manu/prog/balsa-cvs/balsa/libbalsa/filter-file.c	Mon Nov 26 10:42:17 2001
+++ balsa/libbalsa/filter-file.c	Tue Nov 27 11:08:29 2001
@@ -122,6 +122,27 @@
  * Position filter_errno
  */
 
+/* Temporary struct used to ensure that we keep the same order for conditions
+   as specified by the user */
+
+typedef struct {
+    LibBalsaCondition * cnd;
+    gint order;
+} LibBalsaTempCondition;
+
+/* You'll note that this comparison function is inversed
+   That's for a purpose ;-)
+   I use it to avoid a g_list_reverse call (because the order
+   will already be reversed because of the choice of the reversed
+   comparison func) */
+
+static gint compare_conditions_order(gconstpointer  a,gconstpointer  b)
+{
+    const LibBalsaTempCondition * t1=a;
+    const LibBalsaTempCondition * t2=b;
+    return t2->order-t1->order;
+}
+
 void
 libbalsa_conditions_new_from_config(gchar * prefix,
                                     gchar * filter_section_name,
@@ -132,6 +153,7 @@
     gchar *tmp,* condprefix,*key;
     gint pref_len=strlen(CONDITION_SECTION_PREFIX)+strlen(filter_section_name);
     gint err=FILTER_NOERR;
+    GList * tmp_list=NULL;
 
     FILTER_SETFLAG(fil,FILTER_VALID);
     FILTER_SETFLAG(fil,FILTER_COMPILED);
@@ -158,15 +180,13 @@
 		    filter_errno=FILTER_NOERR;
 		    libbalsa_condition_free(cond);
 		}
-		/* FIXME: Ugly hack to preserve condition order in a
-		   filter. It seems that gnome section iterator always
-		   retrieve section in reverse order so I prepend the
-		   conditions to have good order at the end for now
-		   this hack works, but we'll have to fix that in the
-		   future (gnome_config could change and break our
-		   assumption) and use order of condition section key
-		   to insure order of condition */
-		else libbalsa_filter_prepend_condition(fil,cond);
+		else {
+		    LibBalsaTempCondition * tmp=g_new(LibBalsaTempCondition,1);
+		    
+		    tmp->cnd=cond;
+		    tmp->order=atoi(strrchr(key,':')+1);
+		    tmp_list=g_list_prepend(tmp_list,tmp);
+		}
 	    }
 	    else FILTER_CLRFLAG(fil,FILTER_VALID);
 	    gnome_config_pop_prefix();
@@ -175,8 +195,23 @@
     }
     g_free(tmp);
     /* We position filter_errno to the last non-critical error */
-    if (filter_errno==FILTER_NOERR)
+    if (filter_errno==FILTER_NOERR) {
+	LibBalsaTempCondition * tmp;
+
 	filter_errno=err;
+	/* We sort the list of temp conditions, then
+	   we populate the conditions list of the filter */
+	tmp_list=g_list_sort(tmp_list,compare_conditions_order);
+	for (;tmp_list;) {
+	    tmp=(LibBalsaTempCondition *)(tmp_list->data);
+	    libbalsa_filter_prepend_condition(fil,tmp->cnd);
+	    g_free(tmp);
+	    tmp_list=g_list_remove_link(tmp_list,tmp_list);
+	}
+	/* We don't do a g_list_reverse because the comparison func
+	   which dictate the order of the list after the sort is already
+	   reversed */
+    }
 }
 
 /* End of helper functions */
@@ -228,11 +263,13 @@
     gnome_config_set_int("Match-fields",cond->match_fields);
 
     /* We clean all other keys, to have a clean config file */
-    gnome_config_clean_key("Match-string");
-    gnome_config_clean_key("Reg-exps");
-    gnome_config_clean_key("Low-date");
-    gnome_config_clean_key("High-date");
-    gnome_config_clean_key("Flags");
+    if (cond->type!=CONDITION_SIMPLE) gnome_config_clean_key("Match-string");
+    if (cond->type!=CONDITION_REGEX) gnome_config_clean_key("Reg-exps");
+    if (cond->type!=CONDITION_DATE) {
+	gnome_config_clean_key("Low-date");
+	gnome_config_clean_key("High-date");
+    }
+    if (cond->type!=CONDITION_FLAG) gnome_config_clean_key("Flags");
 
     switch(cond->type) {
     case CONDITION_SIMPLE:
@@ -267,47 +304,59 @@
     }
 }
 
+static void libbalsa_real_clean_condition_sections(gchar * buffer,gchar * num_pointer,gint num_len,gint begin)
+{
+    while (TRUE) {
+	snprintf(num_pointer,num_len,"%d/",begin++);
+	if (gnome_config_has_section(buffer)) {
+	    g_print("Cleaning section %s\n",buffer);
+	    gnome_config_clean_section(buffer);
+	}
+	else break;
+    }
+}
+
+#define CONDITION_SECTION_MAX "999"
+
+void libbalsa_clean_condition_sections(const gchar * prefix,const gchar * filter_section_name)
+{
+    gint nb=0,tmp_len=strlen(CONDITION_SECTION_MAX)+2;
+    gchar * buffer,* tmp;
+    
+    /* We allocate once for all a buffer to store conditions sections names */
+    buffer=g_strdup_printf("%s" CONDITION_SECTION_PREFIX "%s" ":%s/",prefix,filter_section_name,CONDITION_SECTION_MAX);
+    tmp=strrchr(buffer,':')+1;
+    libbalsa_real_clean_condition_sections(buffer,tmp,tmp_len,0);
+    g_free(buffer);
+}
+
 /* End of helper functions */
+
 /*
  * libbalsa_conditions_save_config saves a list of conditions using prefix and filter_section_name
  * to create the name of the section
  * We clean all preceding saved conditions to keep the config file clean and coherent
  */
-void libbalsa_conditions_save_config(GSList * conds,gchar * prefix,gchar * filter_section_name)
+
+void libbalsa_conditions_save_config(GSList * conds,const gchar * prefix,const gchar * filter_section_name)
 {
-    gint nb=0;
-    gchar * tmp;
+    gint nb=0,tmp_len=strlen(CONDITION_SECTION_MAX)+2;
+    gchar * buffer,* tmp;
+
+    /* We allocate once for all a buffer to store conditions sections names */
+    buffer=g_strdup_printf("%s" CONDITION_SECTION_PREFIX "%s" ":%s/",prefix,filter_section_name,CONDITION_SECTION_MAX);
+    tmp=strrchr(buffer,':')+1;
     for (;conds;conds=g_slist_next(conds),nb++) {
-	tmp=g_strdup_printf("%s" CONDITION_SECTION_PREFIX "%s" ":%d/",prefix,filter_section_name,nb);
-	gnome_config_push_prefix(tmp);
-	g_free(tmp);
-	libbalsa_condition_save_config((LibBalsaCondition*) conds->data);
+	snprintf(tmp,tmp_len,"%d/",nb);
+	g_print("Saving condition : %s \n",buffer,filter_section_name);
+	gnome_config_push_prefix(buffer);
+	libbalsa_condition_save_config((condition *) conds->data);
 	gnome_config_pop_prefix();
     }
+    libbalsa_real_clean_condition_sections(buffer,tmp,tmp_len,nb);
+    g_free(buffer);
 }
 
-/* Clean the old conditions section for a given filter section */
-
-void
-libbalsa_filter_clean_conditions(gchar *prefix,gchar * filter_section_name)
-{
-    void * iterator;
-    gchar *tmp,* condprefix,*key;
-    gint pref_len=strlen(CONDITION_SECTION_PREFIX)+strlen(filter_section_name);
-    
-    tmp=g_strconcat(CONDITION_SECTION_PREFIX,filter_section_name,NULL);
-    iterator = gnome_config_init_iterator_sections(prefix);
-
-    while ((iterator = gnome_config_iterator_next(iterator, &key, NULL))) {
-	
-	if (strncmp(key,tmp,pref_len)==0) {
-	    condprefix=g_strconcat(prefix,key,"/",NULL);
-	    gnome_config_clean_section(condprefix);
-	    g_free(condprefix);
-	}
-	g_free(key);
-    }
-}                           /* End of libbalsa_filter_clean_conditions() */
 /*
  * void libbalsa_filter_save_config(filter * f)
  *
@@ -344,12 +393,11 @@
     GSList * lst;
 
     /* We load the associated filters */
-    g_print("Loading filters to mailbox %s\n",mbox->name);
+    //g_print("Loading filters to mailbox %s\n",mbox->name);
     gnome_config_get_vector_with_default(MAILBOX_FILTERS_KEY,&nb_filters,&filters_names,&def);
     if (!def) {
 	for(i=0;i<nb_filters;i++) {
-	    g_print("Loading filter %s to mailbox %s\n",
-                    filters_names[i], mbox->name);
+	    //g_print("Loading filter %s to mailbox %s\n",filters_names[i], mbox->name);
 	    fil = libbalsa_filter_get_by_name(filters_names[i]);
 	    if (fil) {
 		LibBalsaMailboxFilter* mf = g_new(LibBalsaMailboxFilter,1);
@@ -362,7 +410,7 @@
 	    }
 	    else
 		libbalsa_information(LIBBALSA_INFORMATION_WARNING,
-				     "Invalid filters %s for mailbox %s",
+				     _("Invalid filters %s for mailbox %s"),
                                      filters_names[i], mbox->name);
 	}
 	mbox->filters=g_slist_reverse(mbox->filters);
@@ -410,7 +458,7 @@
     lst=names;
     for(i=0;i<nb_filters;i++) {
 	filters_names[i]=(gchar*)lst->data;
-	g_print("Saving filter %s to mailbox %s\n",filters_names[i],mbox->name);
+	//g_print("Saving filter %s to mailbox %s\n",filters_names[i],mbox->name);
 	lst=g_slist_next(lst);
     }
     g_slist_free(names);
--- /home/manu/prog/balsa-cvs/balsa/src/filter-edit-callbacks.c	Mon Nov 26 10:07:16 2001
+++ balsa/src/filter-edit-callbacks.c	Tue Nov 27 11:59:11 2001
@@ -182,7 +182,6 @@
 
 /**************** Conditions *************************/
 
-
 /* Callback function to fill the regex entry with the selected regex */
 
 void fe_regexs_select_row(GtkWidget * widget, gint row, gint column,
@@ -222,9 +221,9 @@
 fe_update_type_simple_label()
 {
     const static gchar normal_string[] = 
-      N_("One of the specified fields contains:");
+	N_("One of the specified fields contains:");
     const static gchar negate_string[]=
-      N_("None of the specified fields contains:");
+	N_("None of the specified fields contains:");
     
     if (!fe_conditions_list->selection) return;
 
@@ -235,8 +234,10 @@
  */
 static void fe_update_type_regex_label()
 {
-    static gchar normal_string[]=N_("One of the regular expressions matches");
-    static gchar negate_string[]=N_("None of the regular expressions matches");
+    const static gchar normal_string[] =
+	N_("One of the regular expressions matches");
+    const static gchar negate_string[] =
+	N_("None of the regular expressions matches");
     
     if (!fe_conditions_list->selection) return;
     
@@ -247,8 +248,10 @@
  */
 static void fe_update_type_date_label()
 {
-    static gchar normal_string[]=N_("Match when date is in the interval:");
-    static gchar negate_string[]=N_("Match when date is outside the interval:");
+    const static gchar normal_string[] =
+	N_("Match when date is in the interval:");
+    const static gchar negate_string[] =
+	N_("Match when date is outside the interval:");
     
     if (!fe_conditions_list->selection) return;
     
@@ -259,8 +262,10 @@
  */
 static void fe_update_type_flag_label()
 {
-    static gchar normal_string[]=N_("Match when one of these flags is set:");
-    static gchar negate_string[]=N_("Match when none of these flags is set:");
+    const static gchar normal_string[] =
+	N_("Match when one of these flags is set:");
+    const static gchar negate_string[] =
+	N_("Match when none of these flags is set:");
     
     if (!fe_conditions_list->selection) return;
     
@@ -592,9 +597,8 @@
 	    new_cnd = libbalsa_condition_new();
             if (!condition_validate(new_cnd))
 		return;
-            /* No error occured, condition is valid, so change/add it
-             * based on is_new_condition and only if something has
-             * changed of course
+            /* No error occured, condition is valid, so change/add it based on is_new_condition
+             * and only if something has changed of course
              */
             if (condition_has_changed) {
 		if (!is_new_condition) {
@@ -1059,6 +1063,8 @@
 				//g_print("Removed\n");
 				for (j=i;j<nb_filters-1;j++)
 				    filters_names[j]=filters_names[j+1];
+				/* We put NULL to be sure that g_strfreev does not
+				   free already freed memory */
 				filters_names[--nb_filters]=NULL;
 			    }
 			}
@@ -1156,13 +1162,6 @@
 			G_LEVEL_ORDER,
 			G_TRAVERSE_ALL, 10, update_filters_mailbox, NULL);
 
-	/* Update filters config (remove sections of removed filters) */
-	for (names_lst=filters_names_changes;names_lst;names_lst=g_list_next(names_lst))
-	    if (!((filters_names_rec *)names_lst->data)->new_name) {
-		// g_print("Cleaning section for %s\n",((filters_names_rec *)names_lst->data)->old_name);
-		clean_filter_config_section(((filters_names_rec *)names_lst->data)->old_name);
-	    }
-	
 	gnome_dialog_close(GNOME_DIALOG(dialog));
 	config_filters_save();
 	break;
@@ -1401,7 +1400,6 @@
  *
  * Builds a new filter from the data provided, check for correctness (regex compil...)
  * and sticks it where the selection is in the clist
- * FIXME : Only basic checks are made
  * we can have a filter with unvalid conditions
  */
 void
--- /home/manu/prog/balsa-cvs/balsa/src/save-restore.c	Mon Nov 26 10:07:17 2001
+++ balsa/src/save-restore.c	Tue Nov 27 11:46:07 2001
@@ -1285,32 +1285,52 @@
 	filter_errno=non_critical_error;
 }
 
+#define FILTER_SECTION_MAX "9999"
+
 void
 config_filters_save(void)
 {
     GSList *list;
     LibBalsaFilter* fil;
-    gchar * tmp,* section_name;
-    gint i=0;
+    gchar * buffer,* tmp,* section_name;
+    gint i,nb=0,tmp_len=strlen(FILTER_SECTION_MAX)+2;
+
+    /* We allocate once for all a buffer to store conditions sections names */
+    buffer=g_strdup_printf(BALSA_CONFIG_PREFIX FILTER_SECTION_PREFIX "%s/",FILTER_SECTION_MAX);
+    /* section_name points to the beginning of the filter section name */
+    section_name=buffer+strlen(BALSA_CONFIG_PREFIX);
+    /* tmp points to the space where filter number is appended */
+    tmp=section_name+strlen(FILTER_SECTION_PREFIX);
 
     for(list = balsa_app.filters; list; list = g_slist_next(list)) {
 	fil = (LibBalsaFilter*)(list->data);
-	section_name=g_strdup_printf(FILTER_SECTION_PREFIX"%d",i);
-	tmp=g_strconcat(BALSA_CONFIG_PREFIX,section_name,"/",NULL);
-	gnome_config_push_prefix(tmp);
-	g_free(tmp);
+	i=snprintf(tmp,tmp_len,"%d/",nb++);
+	gnome_config_push_prefix(buffer);
 	libbalsa_filter_save_config(fil);
 	gnome_config_pop_prefix();
 
-	libbalsa_filter_clean_conditions(BALSA_CONFIG_PREFIX,section_name);
-
-	if (fil->conditions)
-	    libbalsa_conditions_save_config(fil->conditions,BALSA_CONFIG_PREFIX,section_name);
-
-	g_free(section_name);
-	gnome_config_sync();
-	i++;
+	/* We suppress the final "/", this is necessary in order that
+	 * libbalsa_conditions_save_config can construct the condition section name */
+	tmp[i-1]='\0';
+	//g_print("Section name = %s\n",section_name);
+	libbalsa_conditions_save_config(fil->conditions,BALSA_CONFIG_PREFIX,section_name);
+    }
+    gnome_config_sync();
+    /* This loop takes care of cleaning up old filter sections */
+    while (TRUE) {
+	i=snprintf(tmp,tmp_len,"%d/",nb++);
+	if (gnome_config_has_section(buffer)) {
+	    //g_print("Cleaning filter section %s\n",buffer);
+	    gnome_config_clean_section(buffer);
+	    /* We suppress the final "/", this is necessary in order that
+	     * libbalsa_clean_condition_sections can construct the condition section name */
+	    tmp[i-1]='\0';
+	    libbalsa_clean_condition_sections(BALSA_CONFIG_PREFIX,section_name);
+	}
+	else break;
     }
+    gnome_config_sync();
+    g_free(buffer);
 }
 
 /* Looks for a mailbox filters section with MBOX_URL field equals to mbox->url
@@ -1318,8 +1338,7 @@
  * The returned string has to be freed by the caller
  */
 
-gchar*
-mailbox_filters_section_lookup(const gchar * url)
+gchar * mailbox_filters_section_lookup(const gchar * url)
 {
     gint pref_len=strlen(MAILBOX_FILTERS_SECTION_PREFIX);
     guint url_len;
@@ -1347,41 +1366,7 @@
     return NULL;
 }
 
-
-/* Clean the section holding the content of filter and all conditions sections related to it
- */
-void
-clean_filter_config_section(const gchar* name)
-{
-    void *iterator;
-    gchar * key,* section,* fil_name;
-    gint pref_len = strlen(FILTER_SECTION_PREFIX);
-    gboolean found=FALSE;
-
-    iterator = gnome_config_init_iterator_sections(BALSA_CONFIG_PREFIX);
-    while (!found && 
-           (iterator = gnome_config_iterator_next(iterator, &key, NULL))) {
-
-	if (strncmp(key, FILTER_SECTION_PREFIX, pref_len) == 0) {
-	    section=g_strconcat(BALSA_CONFIG_PREFIX,key,"/",NULL);
-	    gnome_config_push_prefix(section);
-	    fil_name=gnome_config_get_string("Name");
-	    gnome_config_pop_prefix();
-	    if (strcmp(fil_name,name)==0) {
-		libbalsa_filter_clean_conditions(BALSA_CONFIG_PREFIX,key);
-		gnome_config_clean_section(section);
-		found=TRUE;
-	    }
-	    g_free(section);
-	    g_free(fil_name);
-	}
-	g_free(key);
-    }
-	
-}
-
-void
-config_mailbox_filters_save(LibBalsaMailbox * mbox)
+void config_mailbox_filters_save(LibBalsaMailbox * mbox)
 {
     gchar * tmp;
 
@@ -1410,8 +1395,7 @@
     gnome_config_sync();
 }
 
-void
-config_mailbox_filters_load(LibBalsaMailbox * mbox)
+void config_mailbox_filters_load(LibBalsaMailbox * mbox)
 {
     gchar * section;
 
@@ -1498,6 +1482,4 @@
             printf("Setting converted signature as executable.\n");
         }
     }
-} 
-                                                               
-
+}