[Users] [PATCH 2/2] Clean up summary comparison functions

Alexander Lyons Harkness me at bearbin.net
Sun Aug 25 17:53:30 CEST 2019


All functions now have the requisite error checks and behaviour is
consistent between the different comparison algorithms (unset is
now always counted as a maximal value for sorting). Comparison
functions never return 0 unless the two messages compared are in
fact the same.

Signed-off-by: Alexander Lyons Harkness <me at bearbin.net>
---
This patch depends on the previous one, although trivially so. If I
should resubmit without the dependency, please let me know. Also, this
changes the existing behaviour of sorting by date - previously
messages (or threads) without date set were not placed in a well
defined position (although it would typically be at the bottom). Now,
such messages are placed at the top of the list to be consistent with
the other sorting methods.

 src/summaryview.c | 210 +++++++++++++++++++++++++++++-----------------
 1 file changed, 131 insertions(+), 79 deletions(-)

diff --git a/src/summaryview.c b/src/summaryview.c
index 37142d86b..db94a4531 100644
--- a/src/summaryview.c
+++ b/src/summaryview.c
@@ -7737,21 +7737,6 @@ static void summary_drag_data_received(GtkWidget        *widget,
 
 /* custom compare functions for sorting */
 
-static gint summary_cmp_by_date(GtkCMCList *clist,
-		      gconstpointer ptr1, gconstpointer ptr2)
-{
-	MsgInfo *msginfo1 = ((GtkCMCListRow *)ptr1)->data;
-	MsgInfo *msginfo2 = ((GtkCMCListRow *)ptr2)->data;
-	gint res;
-	if (!msginfo1 || !msginfo2)
-		return -1;
-
-	res = (msginfo1->date_t - msginfo2->date_t);
-	if (res == 0)
-		res = msginfo1->msgnum - msginfo2->msgnum;
-	return res;
-}
-
 #define CMP_FUNC_DEF(func_name, val)					 \
 static gint func_name(GtkCMCList *clist,				 \
 		      gconstpointer ptr1, gconstpointer ptr2)		 \
@@ -7759,8 +7744,8 @@ static gint func_name(GtkCMCList *clist,				 \
 	MsgInfo *msginfo1 = ((GtkCMCListRow *)ptr1)->data;		 \
 	MsgInfo *msginfo2 = ((GtkCMCListRow *)ptr2)->data;		 \
 	gint res;							 \
-	if (!msginfo1 || !msginfo2)					 \
-		return -1;						 \
+	                                                                 \
+	cm_return_val_if_fail(msginfo1 != NULL && msginfo2 != NULL, -1); \
 									 \
 	res = (val);							 \
 	return (res != 0) ? res:summary_cmp_by_date(clist, ptr1, ptr2);	 \
@@ -7784,6 +7769,30 @@ CMP_FUNC_DEF(summary_cmp_by_size, msginfo1->size - msginfo2->size)
 
 #undef CMP_FUNC_DEF
 
+static gint summary_cmp_by_date(GtkCMCList *clist,
+				gconstpointer ptr1,
+				gconstpointer ptr2)
+{
+	MsgInfo *msginfo1 = ((GtkCMCListRow *)ptr1)->data;
+	MsgInfo *msginfo2 = ((GtkCMCListRow *)ptr2)->data;
+	gint res;
+
+	cm_return_val_if_fail(msginfo1 != NULL && msginfo2 != NULL, -1);
+
+	if (msginfo1->date_t != 0 && msginfo2->date_t != 0)
+		res = (msginfo1->date_t - msginfo2->date_t);
+	else if (msginfo1->date_t != 0 && msginfo2->date_t == 0)
+		res = -1;
+	else if (msginfo1->date_t == 0 && msginfo2->date_t != 0)
+		res = 1;
+	else
+		res = 0;
+
+	if (res == 0)
+		res = summary_cmp_by_num(clist, ptr1, ptr2);
+	return res;
+}
+
 static gint summary_cmp_by_subject(GtkCMCList *clist,
 				   gconstpointer ptr1,
 				   gconstpointer ptr2)
@@ -7792,14 +7801,21 @@ static gint summary_cmp_by_subject(GtkCMCList *clist,
 	MsgInfo *msginfo2 = ((GtkCMCListRow *)ptr2)->data;
 	gint res;
 
-	if (!msginfo1->subject)
-		return (msginfo2->subject != NULL);
-	if (!msginfo2->subject)
-		return -1;
+	cm_return_val_if_fail(msginfo1 != NULL && msginfo2 != NULL, -1);
 
-	res = subject_compare_for_sort
-		(msginfo1->subject, msginfo2->subject);
-	return (res != 0)? res: summary_cmp_by_date(clist, ptr1, ptr2);
+	if (msginfo1->subject != NULL && msginfo2->subject != NULL)
+		res = subject_compare_for_sort
+			(msginfo1->subject, msginfo2->subject);
+	else if (msginfo1->subject != NULL && msginfo2->subject == NULL)
+		res = -1;
+	else if (msginfo1->subject == NULL && msginfo2->subject != NULL)
+		res = 1;
+	else
+		res = 0;
+
+	if (res == 0)
+		res = summary_cmp_by_date(clist, ptr1, ptr2);
+	return res;
 }
 
 static gint summary_cmp_by_thread_num(GtkCMCList *clist,
@@ -7809,11 +7825,12 @@ static gint summary_cmp_by_thread_num(GtkCMCList *clist,
 	MsgInfo *msginfo1 = ((GtkCMCListRow *)ptr1)->data;
 	MsgInfo *msginfo2 = ((GtkCMCListRow *)ptr2)->data;
 	gint res;
-	if (!msginfo1 || !msginfo2)
-		return -1;
+
+	cm_return_val_if_fail(msginfo1 != NULL && msginfo2 != NULL, -1);
 
 	res = (msginfo1->thread_msgnum - msginfo2->thread_msgnum);
 
+	/* if both thread_msgnums are set, res is guaranteed not to be 0 */
 	if (!msginfo1->thread_msgnum || !msginfo2->thread_msgnum)
 		res = summary_cmp_by_num(clist, ptr1, ptr2);
 	return res;
@@ -7825,15 +7842,22 @@ static gint summary_cmp_by_thread_date(GtkCMCList *clist,
 {
 	MsgInfo *msginfo1 = ((GtkCMCListRow *)ptr1)->data;
 	MsgInfo *msginfo2 = ((GtkCMCListRow *)ptr2)->data;
-	gint thread_diff = msginfo1->thread_date - msginfo2->thread_date;
-	
-	if (msginfo1->thread_date > 0 && msginfo2->thread_date > 0)
-		return thread_diff;
-	else 
-		return msginfo1->date_t - msginfo2->date_t;
+	gint res;
+
+	cm_return_val_if_fail(msginfo1 != NULL && msginfo2 != NULL, -1);
+
+	res = msginfo1->thread_date - msginfo2->thread_date;
+
+	if (!msginfo1->thread_date || !msginfo2->thread_date)
+		res = summary_cmp_by_date(clist, ptr1, ptr2);
+
+	if (res == 0)
+		res = summary_cmp_by_thread_num(clist, ptr1, ptr2);
+	return res;
 }
 
-static gint summary_cmp_by_from(GtkCMCList *clist, gconstpointer ptr1,
+static gint summary_cmp_by_from(GtkCMCList *clist,
+				gconstpointer ptr1,
 				gconstpointer ptr2)
 {
 	const gchar *str1, *str2;
@@ -7845,6 +7869,8 @@ static gint summary_cmp_by_from(GtkCMCList *clist, gconstpointer ptr1,
 	gint res;
 
 	cm_return_val_if_fail(sv, -1);
+	cm_return_val_if_fail(msginfo1 != NULL && msginfo2 != NULL, -1);
+
 	if (sv->col_state[sv->col_pos[S_COL_FROM]].visible) {
 		str1 = GTK_CMCELL_TEXT(r1->cell[sv->col_pos[S_COL_FROM]])->text;
 		str2 = GTK_CMCELL_TEXT(r2->cell[sv->col_pos[S_COL_FROM]])->text;
@@ -7853,14 +7879,18 @@ static gint summary_cmp_by_from(GtkCMCList *clist, gconstpointer ptr1,
 		str2 = msginfo2->from;
 	}
 
-	if (!str1)
-		return str2 != NULL;
- 
-	if (!str2)
- 		return -1;
- 
-	res = g_utf8_collate(str1, str2);
-	return (res != 0)? res: summary_cmp_by_date(clist, ptr1, ptr2);
+	if (str1 != NULL && str2 != NULL)
+		res = g_utf8_collate(str1, str2);
+	else if (str1 != NULL && str2 == NULL)
+		res = -1;
+	else if (str1 == NULL && str2 != NULL)
+		res = 1;
+	else
+		res = 0;
+
+	if (res == 0)
+		res = summary_cmp_by_date(clist, ptr1, ptr2);
+	return res;
 }
  
 static gint summary_cmp_by_to(GtkCMCList *clist, gconstpointer ptr1,
@@ -7873,8 +7903,10 @@ static gint summary_cmp_by_to(GtkCMCList *clist, gconstpointer ptr1,
 	MsgInfo *msginfo2 = ((GtkCMCListRow *)ptr2)->data;
 	const SummaryView *sv = g_object_get_data(G_OBJECT(clist), "summaryview");
 	gint res;
+
 	cm_return_val_if_fail(sv, -1);
-	
+	cm_return_val_if_fail(msginfo1 != NULL && msginfo2 != NULL, -1);
+
 	if (sv->col_state[sv->col_pos[S_COL_TO]].visible) {
 		str1 = GTK_CMCELL_TEXT(r1->cell[sv->col_pos[S_COL_TO]])->text;
 		str2 = GTK_CMCELL_TEXT(r2->cell[sv->col_pos[S_COL_TO]])->text;
@@ -7883,14 +7915,18 @@ static gint summary_cmp_by_to(GtkCMCList *clist, gconstpointer ptr1,
 		str2 = msginfo2->to;
 	}
 
-	if (!str1)
-		return str2 != NULL;
- 
-	if (!str2)
- 		return -1;
- 
-	res = g_utf8_collate(str1, str2);
-	return (res != 0)? res: summary_cmp_by_date(clist, ptr1, ptr2);
+	if (str1 != NULL && str2 != NULL)
+		res = g_utf8_collate(str1, str2);
+	else if (str1 != NULL && str2 == NULL)
+		res = -1;
+	else if (str1 == NULL && str2 != NULL)
+		res = 1;
+	else
+		res = 0;
+
+	if (res == 0)
+		res = summary_cmp_by_date(clist, ptr1, ptr2);
+	return res;
 }
  
 static gint summary_cmp_by_tags(GtkCMCList *clist, gconstpointer ptr1,
@@ -7903,8 +7939,10 @@ static gint summary_cmp_by_tags(GtkCMCList *clist, gconstpointer ptr1,
 	MsgInfo *msginfo1 = ((GtkCMCListRow *)ptr1)->data;
 	MsgInfo *msginfo2 = ((GtkCMCListRow *)ptr2)->data;
 	gint res;
+
 	cm_return_val_if_fail(sv, -1);
-	
+	cm_return_val_if_fail(msginfo1 != NULL && msginfo2 != NULL, -1);
+
 	if (sv->col_state[sv->col_pos[S_COL_TAGS]].visible) {
 		str1 = g_strdup(GTK_CMCELL_TEXT(r1->cell[sv->col_pos[S_COL_TAGS]])->text);
 		str2 = g_strdup(GTK_CMCELL_TEXT(r2->cell[sv->col_pos[S_COL_TAGS]])->text);
@@ -7913,20 +7951,23 @@ static gint summary_cmp_by_tags(GtkCMCList *clist, gconstpointer ptr1,
 		str2 = procmsg_msginfo_get_tags_str(msginfo2);
 	}
 
-	if (!str1) {
-		res = (str2 != NULL);
-		g_free(str2);
-		return res;
-	}
-	if (!str2) {
+	if (str1 != NULL && str2 != NULL)
+		res = g_utf8_collate(str1, str2);
+	else if (str1 != NULL && str2 == NULL)
+		res = -1;
+	else if (str1 == NULL && str2 != NULL)
+		res = 1;
+	else
+		res = 0;
+
+	if (str1 != NULL)
 		g_free(str1);
- 		return -1;
-	}
- 
-	res = g_utf8_collate(str1, str2);
-	g_free(str1);
-	g_free(str2);
-	return (res != 0)? res: summary_cmp_by_date(clist, ptr1, ptr2);
+	if (str2 != NULL)
+		g_free(str2);
+
+	if (res == 0)
+		res = summary_cmp_by_date(clist, ptr1, ptr2);
+	return res;
 }
  
 static gint summary_cmp_by_simplified_subject
@@ -7943,7 +7984,7 @@ static gint summary_cmp_by_simplified_subject
 
 	cm_return_val_if_fail(sv, -1);
 	cm_return_val_if_fail(msginfo1 != NULL && msginfo2 != NULL, -1);
-	
+
 	if (sv->col_state[sv->col_pos[S_COL_SUBJECT]].visible) {
 		str1 = GTK_CMCELL_TEXT(r1->cell[sv->col_pos[S_COL_SUBJECT]])->text;
 		str2 = GTK_CMCELL_TEXT(r2->cell[sv->col_pos[S_COL_SUBJECT]])->text;
@@ -7952,20 +7993,24 @@ static gint summary_cmp_by_simplified_subject
 		str2 = msginfo2->subject;
 	}
 
-	if (!str1)
-		return str2 != NULL;
-
-	if (!str2)
-		return -1;
-
 	prefs = msginfo1->folder->prefs;
 	if (!prefs)
 		prefs = msginfo2->folder->prefs;
 	if (!prefs)
 		return -1;
+
+	if (str1 != NULL && str2 != NULL)
+		res = subject_compare_for_sort(str1, str2);
+	else if (str1 != NULL && str2 == NULL)
+		res = -1;
+	else if (str1 == NULL && str2 != NULL)
+		res = 1;
+	else
+		res = 0;
 	
-	res = subject_compare_for_sort(str1, str2);
-	return (res != 0)? res: summary_cmp_by_date(clist, ptr1, ptr2);
+	if (res == 0)
+		res = summary_cmp_by_date(clist, ptr1, ptr2);
+	return res;
 }
 
 static gint summary_cmp_by_score(GtkCMCList *clist,
@@ -7973,15 +8018,22 @@ static gint summary_cmp_by_score(GtkCMCList *clist,
 {
 	MsgInfo *msginfo1 = ((GtkCMCListRow *)ptr1)->data;
 	MsgInfo *msginfo2 = ((GtkCMCListRow *)ptr2)->data;
-	int diff;
+	gint res;
 
-	/* if score are equal, sort by date */
+	cm_return_val_if_fail(msginfo1 != NULL && msginfo2 != NULL, -1);
 
-	diff = msginfo1->score - msginfo2->score;
-	if (diff != 0)
-		return diff;
+	if (msginfo1->score != 0 && msginfo2->score != 0)
+		res = msginfo1->score - msginfo2->score;
+	else if (msginfo1->score != 0 && msginfo2->score == 0)
+		res = -1;
+	else if (msginfo1->score == 0 && msginfo2->score != 0)
+		res = 1;
 	else
-		return summary_cmp_by_date(clist, ptr1, ptr2);
+		res = 0;
+
+	if (res == 0)
+		res = summary_cmp_by_date(clist, ptr1, ptr2);
+	return res;
 }
 
 static void summary_ignore_thread_func_mark_unread(GtkCMCTree *ctree, GtkCMCTreeNode *row, gpointer data)
-- 
2.23.0.rc1



More information about the Users mailing list