This patch addresses some of the column sorting bugs in the recent
flurry of grouped searches updates, mostly to do with passive searches.
Namely, it does:
[GTK1] Count value is now consistently updated for all child nodes
[GTK1] A passive search won't mess up count values for other searches
[GTK1] Sorting by count works as expected in a passive search after
results have maxed out
Richard: It's possible this will fix those weird sorting results you
were getting but I'm not holding my breath.
Some gory details: I had to do something messy that I'm not that excited
about but couldn't see another way around. Basically I had to make a
special case in the sorting algorithms for sorting by "#" in a passive
search. This is because once a passive search maxes out we are no
longer adding nodes to that tree but if the other searches aren't maxed
the count values can continue to be increased for the search results
(but the # column is not updated in the passive results tree). Then
when we sort by count in the passive search, it sorts by the real count
value for the regular search tree. The solution I chose was to just use
the text displayed in the column for passive searches instead of the
count value stored in the search result record. This is what the user
expects but involves a bunch of checks and ctree retrievals. Sorting is
slow enough as is so I didn't want to use this for all sorts by count,
only for passive searches. Hopefully there's not too many more "special
cases".
So, sorting by count in a passive search will be a little slower than
sorting by count in a regular search, but it should behave correctly.
Also, this raised an interesting question. Now that we've added child
nodes, should they count towards the "max search results" value?
Imagine someone has max search results set to 100, they search and they
get only 5 different results (with 20 sources each). Is this expected?
Or should it refer to 100 parent nodes instead?
Emile
--
Computer Troubleshooting, New Computer Advice, Virus Clean-up,
Hardware/Software Installs, Web Design, Linux Installations: Attune
Consulting can help.
http://www.attuneconsulting.com
[EMAIL PROTECTED]
? gtk-gnutella-current/src/Makefile
? gtk-gnutella-current/src/getdate.c
? gtk-gnutella-current/src/gtk-gnutella
Index: gtk-gnutella-current/src/search_gui.c
===================================================================
RCS file: /cvsroot/gtk-gnutella/gtk-gnutella-current/src/search_gui.c,v
retrieving revision 1.85
diff -u -r1.85 search_gui.c
--- gtk-gnutella-current/src/search_gui.c 18 Jan 2004 19:16:17 -0000 1.85
+++ gtk-gnutella-current/src/search_gui.c 26 Jan 2004 05:42:41 -0000
@@ -513,7 +513,51 @@
}
-/* Searches results */
+/* Search results */
+
+
+/*
+ * search_gui_compare_passive_count
+ *
+ * If the count column for n1 is greater than that for n2, return +1
+ * 0 if they're equal, and -1 if less than n2
+ */
+gint search_gui_compare_passive_count(GtkCTree *ctree, GtkCTreeNode *n1,
+ GtkCTreeNode *n2)
+{
+ gchar *temp_str = NULL;
+ gint count1 = 0;
+ gint count2 = 0;
+ gint result = 0;
+
+ if ((NULL == n1) || (NULL == n2))
+ return result;
+
+ if (gtk_ctree_node_get_text(ctree, n1, c_sr_count, &temp_str)) {
+
+ if (0 == strcmp(temp_str, ""))
+ count1 = 1;
+ else
+ count1 = atoi(temp_str);
+ }
+
+ if (gtk_ctree_node_get_text(ctree, n2, c_sr_count, &temp_str)){
+
+ if (0 == strcmp(temp_str, ""))
+ count2 = 1;
+ else
+ count2 = atoi(temp_str);
+ }
+
+ if ((0 != count1) && (0 != count2)) {
+ if (count1 == count2)
+ result = 0;
+ else
+ result = (count1 > count2) ? +1 : -1;
+ }
+
+ return result;
+}
/*
@@ -628,7 +672,7 @@
* time critical code, some code duplication is intentional.
*/
GList *search_gui_insert_with_sort(GList *list, GtkCTreeNode *node,
- GtkCTree *ctree, gboolean ascending, gint sort_col)
+ GtkCTree *ctree, gboolean ascending, gint sort_col, gboolean is_passive)
{
GList *l;
gint i;
@@ -642,10 +686,18 @@
if (ascending) {
for (i = 0, l=list;; l = g_list_next(l), i++) {
-
- rlist = gtk_ctree_node_get_row_data(ctree, l->data);
- result = search_gui_compare_records(sort_col, rkey, rlist);
-
+
+ /* In the case that this is a passive search and we're sorting by
+ * count we use the text from the gui column, not the count value
+ * held in the record as these two can get out of sync. --Emile
+ */
+ if ((c_sr_count == sort_col) && (is_passive))
+ result = search_gui_compare_passive_count(ctree, node, l->data);
+ else {
+ rlist = gtk_ctree_node_get_row_data(ctree, l->data);
+ result = search_gui_compare_records(sort_col, rkey, rlist);
+ }
+
if (result <= 0) {
/* Our entry is "less" than the current (l)*/
@@ -689,8 +741,16 @@
for (i = 0, l=list;; l = g_list_next(l), i++) {
- rlist = gtk_ctree_node_get_row_data(ctree, l->data);
- result = search_gui_compare_records(sort_col, rkey, rlist);
+ /* In the case that this is a passive search and we're sorting by
+ * count we use the text from the gui column, not the count value
+ * held in the record as these two can get out of sync. --Emile
+ */
+ if ((c_sr_count == sort_col) && (is_passive))
+ result = search_gui_compare_passive_count(ctree, node, l->data);
+ else {
+ rlist = gtk_ctree_node_get_row_data(ctree, l->data);
+ result = search_gui_compare_records(sort_col, rkey, rlist);
+ }
if (result >= 0) {
/* Entry is "greater" than the current (l)*/
@@ -762,8 +822,9 @@
* intentional.
*/
void search_gui_quick_sort(GArray *array, gint beg, gint end,
- GtkCTree *ctree, gboolean ascending, gint sort_col)
+ GtkCTree *ctree, gboolean ascending, gint sort_col, gboolean is_passive)
{
+ GtkCTreeNode *node;
record_t *rbeg;
record_t *ri;
gint i;
@@ -776,15 +837,24 @@
/* Choose the item in the middle for the pivot, shift it to the front */
search_gui_quick_sort_array_swap(array, beg, pivot_index);
- rbeg = gtk_ctree_node_get_row_data(ctree,
- g_array_index(array, GtkCTreeNode *, beg));
+ node = g_array_index(array, GtkCTreeNode *, beg);
+ rbeg = gtk_ctree_node_get_row_data(ctree, node);
if (ascending) {
for (pivot_index = beg, i = beg + 1; i <= end; i++) {
- ri = gtk_ctree_node_get_row_data(ctree,
- g_array_index(array, GtkCTreeNode *, i));
- result = search_gui_compare_records(sort_col, rbeg, ri);
+ /* In the case that this is a passive search and we're sorting by
+ * count we use the text from the gui column, not the count value
+ * held in the record as these two can get out of sync. --Emile
+ */
+ if ((c_sr_count == sort_col) && (is_passive))
+ result = search_gui_compare_passive_count(ctree, node,
+ g_array_index(array, GtkCTreeNode *, i));
+ else {
+ ri = gtk_ctree_node_get_row_data(ctree,
+ g_array_index(array, GtkCTreeNode *, i));
+ result = search_gui_compare_records(sort_col, rbeg, ri);
+ }
if (result == 1)
search_gui_quick_sort_array_swap(array, ++pivot_index, i);
@@ -792,9 +862,18 @@
} else {
for (pivot_index = beg, i = beg + 1; i <= end; i++) {
- ri = gtk_ctree_node_get_row_data(ctree,
- g_array_index(array, GtkCTreeNode *, i));
- result = search_gui_compare_records(sort_col, rbeg, ri);
+ /* In the case that this is a passive search and we're sorting by
+ * count we use the text from the gui column, not the count value
+ * held in the record as these two can get out of sync. --Emile
+ */
+ if ((c_sr_count == sort_col) && (is_passive))
+ result = search_gui_compare_passive_count(ctree, node,
+ g_array_index(array, GtkCTreeNode *, i));
+ else {
+ ri = gtk_ctree_node_get_row_data(ctree,
+ g_array_index(array, GtkCTreeNode *, i));
+ result = search_gui_compare_records(sort_col, rbeg, ri);
+ }
if (result == -1)
search_gui_quick_sort_array_swap(array, ++pivot_index, i);
@@ -806,9 +885,9 @@
search_gui_quick_sort_array_swap(array, beg, pivot_index);
search_gui_quick_sort(array, beg, pivot_index - 1, ctree,
- ascending, sort_col);
+ ascending, sort_col, is_passive);
search_gui_quick_sort(array, pivot_index + 1, end, ctree,
- ascending, sort_col);
+ ascending, sort_col, is_passive);
}
@@ -922,7 +1001,8 @@
* insertion sort will be needed, ie. one that makes an almost ordered list
* followed by a cleanup algorithm.
*/
-void search_gui_perform_sort(GtkCTree *ctree, gboolean ascending, gint sort_col)
+void search_gui_perform_sort(GtkCTree *ctree, gboolean ascending, gint sort_col,
+ gboolean is_passive)
{
GtkCTreeNode *cur_node;
GtkCTreeNode *prev_node = NULL;
@@ -993,7 +1073,7 @@
cur_node = GTK_CTREE_NODE_SIBLING(cur_node)
) {
temp_list = search_gui_insert_with_sort(temp_list,
- cur_node, ctree, ascending, sort_col);
+ cur_node, ctree, ascending, sort_col, is_passive);
}
/* Now order the CTree using the list O(n) */
@@ -1043,7 +1123,7 @@
/* Sort the array O(nlogn) */
search_gui_quick_sort(
- array, 0, array->len - 1, ctree, ascending, sort_col);
+ array, 0, array->len - 1, ctree, ascending, sort_col, is_passive);
/* Use the sorted array to sort the ctree widget O(n) */
gtk_clist_freeze(GTK_CLIST(ctree));
@@ -1134,7 +1214,8 @@
gtk_box_reorder_child(GTK_BOX(cw), search->arrow, 0);
gtk_widget_show(search->arrow);
}
- search_gui_perform_sort(search->ctree, ascending, column);
+ search_gui_perform_sort(search->ctree, ascending, column,
+ (0 == strcmp(search->query, "Passive")));
search->sort = TRUE;
} else {
@@ -1144,6 +1225,32 @@
/*
+ * search_gui_update_node_count
+ *
+ * Updates the count value associated with the record for given parent node
+ * and all of it's children nodes.
+ */
+search_gui_update_node_count(GtkCTree *ctree, GtkCTreeNode *parent,
+ gint new_count)
+{
+ GtkCTreeNode *child;
+ record_t *rc;
+
+ rc = gtk_ctree_node_get_row_data(ctree, parent);
+ rc->count = new_count;
+
+ for (child = GTK_CTREE_ROW(parent)->children; NULL != child;
+ child = GTK_CTREE_ROW(child)->sibling) {
+
+ rc = gtk_ctree_node_get_row_data(ctree, child);
+ rc->count = new_count;
+ }
+}
+
+
+
+
+/*
* search_gui_add_record
*
* Adds the record to gth GtkCTree for this search.
@@ -1168,7 +1275,8 @@
GtkCTreeNode *cur_node;
GtkCTreeNode *sibling;
GtkCTreeNode *auto_node;
- GtkCTreeRow *row, *parent_row;
+ GtkCTreeRow *row;
+ GtkCTreeRow *parent_row;
GtkCTree *ctree = GTK_CTREE(sch->ctree);
info = g_string_assign(info, "");
@@ -1230,16 +1338,19 @@
/* A parent exists with that sha1, add as child to that parent */
node = gtk_ctree_insert_node(ctree, parent, NULL, titles, 5,
NULL, NULL, NULL, NULL, 0, 0);
+ gtk_ctree_node_set_row_data(ctree, node, (gpointer) rc);
/*Update the "#" column of the parent, +1 for parent */
count = count_node_children(ctree, parent) + 1;
gm_snprintf(tmpstr, sizeof(tmpstr), "%u", count);
gtk_ctree_node_set_text(ctree, parent, c_sr_count, tmpstr);
- /* Update count in the records (use for column sorting) */
- rc->count = count;
- parent_rc = gtk_ctree_node_get_row_data(ctree, parent);
- parent_rc->count = count;
+ /* Update count in the records (use for column sorting).
+ * Passive searches duplicate results already added in another
+ * search so we don't update the count value for them.
+ */
+ if (0 != strcmp(sch->query, "Passive"))
+ search_gui_update_node_count(ctree, parent, count);
is_parent = FALSE;
} else {
key = atom_sha1_get(rc->sha1); /* New parent, need new atom ref */
@@ -1251,8 +1362,12 @@
NULL, NULL, NULL, NULL, 0, 0);
add_parent_with_sha1(sch->parents, key, node);
- /* Update count in the records (use for column sorting) */
- rc->count = 1;
+ /* Update count in the records (use for column sorting).
+ * Passive searches duplicate results already added in another
+ * search so we don't update the count value for them.
+ */
+ if (0 != strcmp(sch->query, "Passive"))
+ rc->count = 1;
is_parent = TRUE;
}
@@ -1261,16 +1376,22 @@
node = gtk_ctree_insert_node(ctree, parent = NULL, NULL, titles, 5,
NULL, NULL, NULL, NULL, 0, 0);
- /* Update count in the records (use for column sorting) */
- rc->count = 1;
+
+ /* Update count in the records (use for column sorting).
+ * Passive searches duplicate results already added in another
+ * search so we don't update the count value for them.
+ */
+ if (0 != strcmp(sch->query, "Passive"))
+ rc->count = 1;
is_parent = TRUE;
}
atom_str_free(titles[c_sr_speed]);
- search_gui_ref_record(rc);
+ if (is_parent) /* This was already done if it's a child */
+ gtk_ctree_node_set_row_data(ctree, node, (gpointer) rc);
- gtk_ctree_node_set_row_data(ctree, node, (gpointer) rc);
+ search_gui_ref_record(rc);
if (sch->sort) {
/*
@@ -1453,9 +1574,6 @@
else
*tmpstr = '\0';
- /* Update record count, child_rc will become the rc for the parent*/
- child_rc->count = n;
-
/* Now actually modify the old parent node */
gtk_ctree_node_set_text(ctree, node, c_sr_filename, filename);
gtk_ctree_node_set_text(ctree, node, c_sr_info, info);
@@ -1469,6 +1587,13 @@
/* Delete the 1st child node, now that we've copied the data */
gtk_ctree_remove_node(ctree, child_node);
+ /* Update count in the records (use for column sorting).
+ * Passive searches duplicate results already added in another
+ * search so we don't update the count value for them.
+ */
+ if (0 != strcmp(current_search->query, "Passive"))
+ search_gui_update_node_count(ctree, node, n);
+
atom_str_free(speed);
} else {
@@ -1495,8 +1620,12 @@
*tmpstr = '\0';
gtk_ctree_node_set_text(ctree, old_parent, c_sr_count, tmpstr);
- parent_rc = gtk_ctree_node_get_row_data(ctree, old_parent);
- parent_rc->count = n;
+ /* Update count in the records (use for column sorting).
+ * Passive searches duplicate results already added in another
+ * search so we don't update the count value for them.
+ */
+ if (0 != strcmp(current_search->query, "Passive"))
+ search_gui_update_node_count(ctree, old_parent, n);
}
/*
@@ -1505,7 +1634,7 @@
* One is a gui reference, the other is the dups hash table reference
* (Note that GTK2 hashtables allow us to define an auto remove function,
* not so with GTK1, so we have to do it ourselves)
- */
+ */
g_hash_table_remove(current_search->dups, rc);
search_gui_unref_record(rc);
search_gui_unref_record(rc);
