[PATCH] Address-entry cleanups



	Hi all,
after having proposed my help to Jelmer I read through the code of
libbalsa/address-entry.c.
I had a few things to improve (better code, perhaps speedier), so here is
the patches (against 1.2.4 and cvs).

The changes are just better (IMHO) coding, no actual change of behaviour.
EG : g_list_free -> g_list_free_1, better memory management on string 
operations...

First comment on the code : it seems that it uses a lot of internals
functions from gtk (they seem to be
declared as "static" so that their code is just stolen from gtkentry.c). I
hope with 2.0 we don't need this duplication because this is the root of
bugs and it's just plain stupid to have to duplicate the code of the lib.
Perhaps if this is not corrected in 2.0 we should tell them about our
constraints?
Bye
Manu


--- ../balsa-1.2.4/libbalsa/address-entry.c	Tue Jan 15 20:47:56 2002
+++ balsa-1.2.4/libbalsa/address-entry.c	Mon Jan 28 15:12:48 2002
@@ -538,36 +538,25 @@
 libbalsa_strsplit(const gchar *str, gchar delimiter)
 {
     GList *glist;
-    gchar *data;
     const gchar *old, *current;
-    gint i, previous;
     gboolean quoted;
 
-    g_return_val_if_fail(str != NULL, NULL);
+    if (!str) return NULL;
 
     quoted = FALSE;
     glist = NULL;
-    previous = 0;
-    old = current = str;
-    for (i = 0; str[i]; i++) {
-	if (str[i] == '"') quoted = !quoted;
-	if ( (!quoted) && (str[i] == delimiter) ) {
-	    data = g_strndup(old, i - previous);
-	    glist = g_list_append(glist, data);
-	    previous = i + 1;
-	    old = current;
-	    old++;
+    old = str;
+    for (current=str;*current;current++) {
+	if (*current == '"') quoted = !quoted;
+	else if ( (!quoted) && (*current == delimiter) ) {
+	    glist = g_list_append(glist, g_strndup(old, current-old));
+	    old=current+1;
 	}
-	current++;
-    }
-    if (str) {
-	data = g_strndup(old, i - previous);
-	glist = g_list_append(glist, data);
     }
+    glist = g_list_append(glist, g_strndup(old, current-old));
     return glist;
 }
 
-
 /*************************************************************
  * libbalsa_length_of_address:
  *     Calculated how long an address would be when it is
@@ -587,13 +576,12 @@
 
     i = 0;
     if (addy->user)
-	i = i + strlen(addy->user);
+	i += strlen(addy->user);
     if (addy->match)
-	i = i + 3 + strlen(addy->match);
+	i += 3 + strlen(addy->match);
     return i;
 }
 
-
 /*************************************************************
  * libbalsa_make_address_string:
  *     Prints out the e-mail address in a newly allocated
@@ -641,7 +629,8 @@
 
     input = address_entry->input;
     addy = input->active->data;
-    if (addy->cursor < (tmp = strlen(addy->user))) {
+    tmp = strlen(addy->user);
+    if (addy->cursor < tmp) {
 	addy->cursor = tmp;
     } else {
 	libbalsa_force_no_match(addy);
@@ -706,7 +695,7 @@
 static inputData *
 libbalsa_fill_input(LibBalsaAddressEntry *address_entry)
 {
-    gint cursor = 0, size = 0, prev = 0;
+    gint cursor, size, prev=0;
     gchar *typed = NULL;
     GList *el, *current;
     GList *list = NULL;
@@ -730,8 +719,6 @@
      * Split the input string by comma, and store the result in
      * input->list.
      * str contains a list of e-mail addresses seperated by ','.
-     *
-     * FIXME: Breaks for '"Doe, John" <john@doe.com>'
      */
     el = libbalsa_strsplit(typed, ',');
     g_free(typed);
@@ -740,13 +727,13 @@
      */
     if (el != NULL) {
 	for (current = el;
-	     current != NULL;
+	     current;
 	     current = g_list_next(current)) {
 	    addy = libbalsa_emailData_new();
-	    addy->user = g_strdup((gchar *)current->data);
+	    addy->user = (gchar *)current->data;
 	    input->list = g_list_append(input->list, addy);
 	}
-	g_list_foreach(el, (GFunc)g_free, NULL);
+	g_list_free(el);
     } else {
        addy = libbalsa_emailData_new();
        addy->user = g_strdup("");
@@ -758,26 +745,25 @@
      * We have to match the cursor in GtkEntry to the list.
      */
     g_assert(input->list != NULL);
-    size = prev = 0;
-    for (list = g_list_first(input->list);
-	 list != NULL;
-	 list = g_list_next(list)) {
-	if (cursor >= size) {
-	    prev = size;
-	    input->active = list;
-	}
+    list = g_list_first(input->list);
+    size = 0;
+    while (list) {
+	prev=size;
 	addy = (emailData *)list->data;
-	size = size + strlen(addy->user) + 1;
+	size += strlen(addy->user) + 1;
 	addy->user = g_strchug(addy->user);
+	if (cursor<=size) break;
+	list=g_list_next(list);
     }
-
+    input->active = list;
     addy = (emailData *)input->active->data;
     addy->cursor = cursor - prev;
     if (input->active != input->list)
-	addy->cursor = addy->cursor - 1; /* Compensate for the ',' */
+	addy->cursor--; /* Compensate for the ',' */
     if (addy->cursor < 0) addy->cursor = 0;
-    if (addy->cursor > (tmp = strlen(addy->user)))
-        addy->cursor = tmp;
+    tmp = strlen(addy->user);
+    if (addy->cursor > tmp)
+	addy->cursor = tmp;
 
     return input;
 }
@@ -878,7 +864,7 @@
 	if (list != NULL) {
 	    input->list = g_list_remove_link(input->list, list);
 	    libbalsa_emailData_free(list->data);
-	    g_list_free(list);
+	    g_list_free_1(list);
 	}
 
     /*
@@ -894,7 +880,7 @@
 	    input->active = NULL;
 	input->list = g_list_remove_link(input->list, list);
 	libbalsa_emailData_free(list->data);
-	g_list_free(list);
+	g_list_free_1(list);
 	if (input->active != NULL) {
 	    addy = input->active->data;
 	    g_assert(addy != NULL);
@@ -964,11 +950,7 @@
 	}
 	input->list = g_list_remove_link(input->list, list);
 	libbalsa_emailData_free(list->data);
-	/*
-	list->next = NULL;
-	list->prev = NULL;
-	*/
-	g_list_free(list);
+	g_list_free_1(list);
 	if (input->active != NULL) {
 	    addy = input->active->data;
 	    g_assert(addy != NULL);
@@ -1019,19 +1001,12 @@
     /*
      * Free all the following data.
      */
-    for (list = g_list_next(input->active);
-	 list != NULL;
-	 list = g_list_next(input->active)) {
-	 /*
-	  * Concatenate the two e-mails.
-	  */
-	 libbalsa_emailData_free(list->data);
-	 input->list = g_list_remove_link(input->list, list);
-	 list->data = NULL;
-	 list->prev = NULL;
-	 list->next = NULL;
-	 g_list_free(list);
-    }
+    g_list_foreach(g_list_next(input->active),(GFunc)libbalsa_emailData_free,NULL);
+    g_list_free(g_list_next(input->active));
+
+    /* input->active must be the last address */
+    input->active->next=NULL;
+
     /* libbalsa_inputData_free(address_entry->input); 
      * look above: the line below is not necessary */
     /* address_entry->input = input; */
@@ -1362,7 +1337,7 @@
     g_return_if_fail(LIBBALSA_IS_ADDRESS_ENTRY(address_entry));
 
     libbalsa_address_entry_draw_cursor_on_drawable(address_entry,
-					GTK_ENTRY(address_entry)->text_area);
+						   GTK_ENTRY(address_entry)->text_area);
 }
 
 
@@ -1396,7 +1371,9 @@
 
 /*************************************************************
  * libbalsa_address_entry_adjust_scroll:
- *     ???
+ *     Just scroll the entry to have as must of the text as
+ *     possible (this is necessary when the text is too long
+ *     to fit in the entry space)
  *
  *   credits:
  *     This is stolen from gtk+-1.2.8/gtk/gtkentry.c
@@ -1545,7 +1522,7 @@
 {
     GtkEditable *editable;
     emailData *addy, *extra;
-    gchar *left, *right, *str;
+    gchar *p,* str;
     GList *list;
     unsigned i;
     inputData *input;
@@ -1567,7 +1544,7 @@
      * This is only valid if the user has hit tab.
      */
     if ((addy->cursor >= strlen(addy->user)) &&
-       (addy->match != NULL) && (addy->tabs > 0)) {
+       addy->match && (addy->tabs > 0)) {
        libbalsa_force_no_match(addy);
        return;
     }
@@ -1575,9 +1552,9 @@
     /*
      * Lets see if the user is at the beginning of an e-mail entry.
      */
-    if (addy->cursor == 0) {
+    if (!addy->cursor) {
        list = g_list_previous(input->active);
-       if (list != NULL) {
+       if (list) {
 	   /*
 	    * Concatenate the two e-mails.
 	    */
@@ -1592,8 +1569,7 @@
 	    */
 	   input->list = g_list_remove_link(input->list, input->active);
 	   libbalsa_emailData_free(addy);
-	   input->active->data = NULL;
-	   g_list_free(input->active);
+	   g_list_free_1(input->active);
 	   input->active = list;
        }
 
@@ -1601,18 +1577,12 @@
      * Normal character needs deleting.
      */
     } else {
-       left = g_strndup(addy->user, (addy->cursor - 1));
-       right = addy->user;
-       for (i = 0; i < addy->cursor; i++) right++;
-       str = g_strconcat(left, right, NULL);
-       g_free(addy->user);
-       g_free(left);
-       addy->user = str;
-       addy->cursor--;
-       if (*str == '\0')
-	   libbalsa_force_no_match(addy);
-       else if (address_entry->find_match)
-	   (*address_entry->find_match) (addy, TRUE);
+	addy->cursor--;
+	for (p=addy->user+addy->cursor;*p;p++) *p=*(p+1);
+	if (!*(addy->user))
+	    libbalsa_force_no_match(addy);
+	else if (address_entry->find_match)
+	    (*address_entry->find_match) (addy, TRUE);
     }
 }
 
@@ -1640,7 +1610,7 @@
 {
     GtkEditable *editable;
     emailData *addy, *extra;
-    gchar *left, *right, *str;
+    gchar *p, *str;
     GList *list;
     inputData *input;
     
@@ -1655,8 +1625,7 @@
     /*
      * Check if the user wanted to delete a match.
      */
-    if ((addy->cursor >= strlen(addy->user)) &&
-       (addy->match != NULL)) {
+    if ((addy->cursor >= strlen(addy->user)) && addy->match) {
        libbalsa_force_no_match(addy);
        return;
     }
@@ -1666,7 +1635,7 @@
      */
     if (addy->cursor >= strlen(addy->user)) {
 	list = g_list_next(input->active);
-	if (list != NULL) {
+	if (list) {
 	    /*
 	     * Concatenate the two e-mails.
 	     */
@@ -1680,22 +1649,14 @@
 	     */
 	    input->list = g_list_remove_link(input->list, list);
 	    libbalsa_emailData_free(extra);
-	    list->data = NULL;
-	    g_list_free(list);
+	    g_list_free_1(list);
 	}
     /*
      * Normal character needs deleting.
      */
     } else {
-	unsigned i;
-	left = g_strndup(addy->user, addy->cursor);
-	right = addy->user;
-	for (i = 0; i <= addy->cursor; i++) right++;
-	str = g_strconcat(left, right, NULL);
-	g_free(addy->user);
-	g_free(left);
-	addy->user = str;
-    }
+	for (p=addy->user+addy->cursor;*p;p++) *p=*(p+1);
+   }
 }
 
 
@@ -1765,14 +1726,14 @@
      * Else no match was found.  Check if there was a default
      * domain to add to e-mail addresses.
      */
-    if (address_entry->domain == NULL || *address_entry->domain == '\0')
+    if (address_entry->domain == NULL || !*address_entry->domain)
 	return;
 
     /*
      * There is a default domain to add.  Do we need to add it?
      */
     addy = address_entry->input->active->data;
-    if (libbalsa_is_an_email(addy->user) || *addy->user == '\0')
+    if (libbalsa_is_an_email(addy->user) || !*addy->user)
 	return;
 
     /*
@@ -1807,7 +1768,7 @@
 
 
 /*************************************************************
- * libbalsa_keystroke_down:
+ * libbalsa_keystroke_home:
  *     Moves the cursor to the beginning of the
  *     LibBalsaAddressEntry widget.
  *
--- balsa-cvs/balsa/libbalsa/address-entry.c	Mon Jan 21 15:16:06 2002
+++ test/balsa-cvs/balsa/libbalsa/address-entry.c	Mon Jan 28 15:12:40 2002
@@ -538,32 +538,22 @@
 libbalsa_strsplit(const gchar *str, gchar delimiter)
 {
     GList *glist;
-    gchar *data;
     const gchar *old, *current;
-    gint i, previous;
     gboolean quoted;
 
-    g_return_val_if_fail(str != NULL, NULL);
+    if (!str) return NULL;
 
     quoted = FALSE;
     glist = NULL;
-    previous = 0;
-    old = current = str;
-    for (i = 0; str[i]; i++) {
-	if (str[i] == '"') quoted = !quoted;
-	if ( (!quoted) && (str[i] == delimiter) ) {
-	    data = g_strndup(old, i - previous);
-	    glist = g_list_append(glist, data);
-	    previous = i + 1;
-	    old = current;
-	    old++;
+    old = str;
+    for (current=str;*current;current++) {
+	if (*current == '"') quoted = !quoted;
+	else if ( (!quoted) && (*current == delimiter) ) {
+	    glist = g_list_append(glist, g_strndup(old, current-old));
+	    old=current+1;
 	}
-	current++;
-    }
-    if (str) {
-	data = g_strndup(old, i - previous);
-	glist = g_list_append(glist, data);
     }
+    glist = g_list_append(glist, g_strndup(old, current-old));
     return glist;
 }
 
@@ -587,9 +577,9 @@
 
     i = 0;
     if (addy->user)
-	i = i + strlen(addy->user);
+	i += strlen(addy->user);
     if (addy->match)
-	i = i + 3 + strlen(addy->match);
+	i += 3 + strlen(addy->match);
     return i;
 }
 
@@ -641,7 +631,8 @@
 
     input = address_entry->input;
     addy = input->active->data;
-    if (addy->cursor < (tmp = strlen(addy->user))) {
+    tmp = strlen(addy->user);
+    if (addy->cursor < tmp) {
 	addy->cursor = tmp;
     } else {
 	libbalsa_force_no_match(addy);
@@ -706,7 +697,7 @@
 static inputData *
 libbalsa_fill_input(LibBalsaAddressEntry *address_entry)
 {
-    gint cursor = 0, size = 0, prev = 0;
+    gint cursor, size, prev=0;
     gchar *typed = NULL;
     GList *el, *current;
     GList *list = NULL;
@@ -730,8 +721,6 @@
      * Split the input string by comma, and store the result in
      * input->list.
      * str contains a list of e-mail addresses seperated by ','.
-     *
-     * FIXME: Breaks for '"Doe, John" <john@doe.com>'
      */
     el = libbalsa_strsplit(typed, ',');
     g_free(typed);
@@ -740,13 +729,13 @@
      */
     if (el != NULL) {
 	for (current = el;
-	     current != NULL;
+	     current;
 	     current = g_list_next(current)) {
 	    addy = libbalsa_emailData_new();
-	    addy->user = g_strdup((gchar *)current->data);
+	    addy->user = (gchar *)current->data;
 	    input->list = g_list_append(input->list, addy);
 	}
-	g_list_foreach(el, (GFunc)g_free, NULL);
+	g_list_free(el);
     } else {
        addy = libbalsa_emailData_new();
        addy->user = g_strdup("");
@@ -758,26 +747,25 @@
      * We have to match the cursor in GtkEntry to the list.
      */
     g_assert(input->list != NULL);
-    size = prev = 0;
-    for (list = g_list_first(input->list);
-	 list != NULL;
-	 list = g_list_next(list)) {
-	if (cursor >= size) {
-	    prev = size;
-	    input->active = list;
-	}
+    list = g_list_first(input->list);
+    size = 0;
+    while (list) {
+	prev=size;
 	addy = (emailData *)list->data;
-	size = size + strlen(addy->user) + 1;
+	size += strlen(addy->user) + 1;
 	addy->user = g_strchug(addy->user);
+	if (cursor<=size) break;
+	list=g_list_next(list);
     }
-
+    input->active = list;
     addy = (emailData *)input->active->data;
     addy->cursor = cursor - prev;
     if (input->active != input->list)
-	addy->cursor = addy->cursor - 1; /* Compensate for the ',' */
+	addy->cursor--; /* Compensate for the ',' */
     if (addy->cursor < 0) addy->cursor = 0;
-    if (addy->cursor > (tmp = strlen(addy->user)))
-        addy->cursor = tmp;
+    tmp = strlen(addy->user);
+    if (addy->cursor > tmp)
+	addy->cursor = tmp;
 
     return input;
 }
@@ -1015,16 +1003,12 @@
     /*
      * Free all the following data.
      */
-    for (list = g_list_next(input->active);
-	 list != NULL;
-	 list = g_list_next(input->active)) {
-	 /*
-	  * Concatenate the two e-mails.
-	  */
-	 libbalsa_emailData_free(list->data);
-	 input->list = g_list_remove_link(input->list, list);
-	 g_list_free_1(list);
-    }
+    g_list_foreach(g_list_next(input->active),(GFunc)libbalsa_emailData_free,NULL);
+    g_list_free(g_list_next(input->active));
+
+    /* input->active must be the last address */
+    input->active->next=NULL;
+
     /* libbalsa_inputData_free(address_entry->input); 
      * look above: the line below is not necessary */
     /* address_entry->input = input; */
@@ -1065,21 +1049,20 @@
 
 /*************************************************************
  * libbalsa_address_entry_find_position:
- *     Find the cursor position?
+ *     Find the offset of the char (in the text string) that has x as x-coord
  *
  *   credits:
  *     This is stolen from gtk+-1.2.8/gtk/gtkentry.c
  *
  *   arguments:
  *     address_entry: The LibBalsaAddressEntry to act on.
- *     x:             ???
+ *     x:             x-coord to transform
  *
  *   results:
- *     ???
+ *     The offset of the char
  *************************************************************/
 static gint
-libbalsa_address_entry_find_position(LibBalsaAddressEntry *address_entry,
-									gint x)
+libbalsa_address_entry_find_position(LibBalsaAddressEntry *address_entry,gint x)
 {
     GtkEntry *entry;
     gint start = 0;
@@ -1129,7 +1112,7 @@
  *************************************************************/
 static void
 libbalsa_address_entry_make_backing_pixmap(LibBalsaAddressEntry *address_entry,
-							gint width, gint height)
+					   gint width, gint height)
 {
     GtkEntry *entry;
     gint pixmap_width, pixmap_height;
@@ -1355,7 +1338,7 @@
     g_return_if_fail(LIBBALSA_IS_ADDRESS_ENTRY(address_entry));
 
     libbalsa_address_entry_draw_cursor_on_drawable(address_entry,
-					GTK_ENTRY(address_entry)->text_area);
+						   GTK_ENTRY(address_entry)->text_area);
 }
 
 
@@ -1538,7 +1521,7 @@
 {
     GtkEditable *editable;
     emailData *addy, *extra;
-    gchar *left, *right, *str;
+    gchar *p,* str;
     GList *list;
     unsigned i;
     inputData *input;
@@ -1560,7 +1543,7 @@
      * This is only valid if the user has hit tab.
      */
     if ((addy->cursor >= strlen(addy->user)) &&
-       (addy->match != NULL) && (addy->tabs > 0)) {
+       addy->match && (addy->tabs > 0)) {
        libbalsa_force_no_match(addy);
        return;
     }
@@ -1570,7 +1553,7 @@
      */
     if (addy->cursor == 0) {
        list = g_list_previous(input->active);
-       if (list != NULL) {
+       if (list) {
 	   /*
 	    * Concatenate the two e-mails.
 	    */
@@ -1593,18 +1576,12 @@
      * Normal character needs deleting.
      */
     } else {
-       left = g_strndup(addy->user, (addy->cursor - 1));
-       right = addy->user;
-       for (i = 0; i < addy->cursor; i++) right++;
-       str = g_strconcat(left, right, NULL);
-       g_free(addy->user);
-       g_free(left);
-       addy->user = str;
-       addy->cursor--;
-       if (*str == '\0')
-	   libbalsa_force_no_match(addy);
-       else if (address_entry->find_match)
-	   (*address_entry->find_match) (addy, TRUE);
+	addy->cursor--;
+	for (p=addy->user+addy->cursor;*p;p++) *p=*(p+1);
+	if (!*(addy->user))
+	    libbalsa_force_no_match(addy);
+	else if (address_entry->find_match)
+	    (*address_entry->find_match) (addy, TRUE);
     }
 }
 
@@ -1632,7 +1609,7 @@
 {
     GtkEditable *editable;
     emailData *addy, *extra;
-    gchar *left, *right, *str;
+    gchar *p, *str;
     GList *list;
     inputData *input;
     
@@ -1647,8 +1624,7 @@
     /*
      * Check if the user wanted to delete a match.
      */
-    if ((addy->cursor >= strlen(addy->user)) &&
-       (addy->match != NULL)) {
+    if ((addy->cursor >= strlen(addy->user)) && addy->match) {
        libbalsa_force_no_match(addy);
        return;
     }
@@ -1658,7 +1634,7 @@
      */
     if (addy->cursor >= strlen(addy->user)) {
 	list = g_list_next(input->active);
-	if (list != NULL) {
+	if (list) {
 	    /*
 	     * Concatenate the two e-mails.
 	     */
@@ -1678,14 +1654,7 @@
      * Normal character needs deleting.
      */
     } else {
-	unsigned i;
-	left = g_strndup(addy->user, addy->cursor);
-	right = addy->user;
-	for (i = 0; i <= addy->cursor; i++) right++;
-	str = g_strconcat(left, right, NULL);
-	g_free(addy->user);
-	g_free(left);
-	addy->user = str;
+	for (p=addy->user+addy->cursor;*p;p++) *p=*(p+1);
     }
 }
 
@@ -1756,7 +1725,7 @@
      * Else no match was found.  Check if there was a default
      * domain to add to e-mail addresses.
      */
-    if (address_entry->domain == NULL || *address_entry->domain == '\0')
+    if (address_entry->domain == NULL || !*address_entry->domain)
 	return;
 
     /*
@@ -1798,7 +1767,7 @@
 
 
 /*************************************************************
- * libbalsa_keystroke_down:
+ * libbalsa_keystroke_home:
  *     Moves the cursor to the beginning of the
  *     LibBalsaAddressEntry widget.
  *
@@ -2374,8 +2343,8 @@
 	     list != end;
 	     list = g_list_next(start)) {
 	    libbalsa_emailData_free(list->data);
+	    list->data = NULL;
 	    g_list_remove_link(address_entry->input->list, list);
-            g_list_free_1(list);
 	}
     }
 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]