Merge lp:~alecu/ubuntuone-client/highlight-search into lp:ubuntuone-client

[Alejandro J. Cura]

Highlight the strings that were found in the contacts search

Highlight the strings that were found in the contacts search

[John Lenton (chipaca)]

Yes, it does!

review: Approve

[Rodrigo Moya (rodrigo-moya)]

Looks great, very good code! Only one thing:

379 + while (*needle != NULL) {
380 + glong needle_len = g_utf8_strlen (*needle, -1);
381 + if (needle_len < 1) {
382 + needle++;
383 + continue;
384 + }
385 + gchar *search_start = folded_haystack;
386 + gchar *start_ptr = g_strstr_len (search_start,
387 + -1,
388 + *needle);

Don't declare variables after executed code, since that would make it fail when running strict compilation (C99). But not holding this branch for this, so fix it in another one, please

review: Approve

=== modified file '.bzrignore'
--- .bzrignore	2010-07-14 19:51:54 +0000
+++ .bzrignore	2010-07-20 16:05:59 +0000
@@ -49,6 +49,7 @@
 ubuntuone-marshallers.[ch]
 
 nautilus/test-contacts-picker
+nautilus/test-highlight
 
 libsyncdaemon/syncdaemon-marshal.[ch]
 libsyncdaemon/test-libsyncdaemon

=== modified file 'nautilus/Makefile.am'
--- nautilus/Makefile.am	2010-06-18 12:27:03 +0000
+++ nautilus/Makefile.am	2010-07-20 16:05:59 +0000
@@ -27,6 +27,8 @@
 	error-dialog.c		\
 	location-widget.c	\
 	location-widget.h	\
+	highlight.c \
+	highlight.h \
 	u1-contacts-picker.c	\
 	u1-contacts-picker.h	\
 	ubuntuone-marshallers.c \
@@ -40,7 +42,8 @@
 	$(NAUTILUS_LIBS)		\
 	$(top_builddir)/libsyncdaemon/libsyncdaemon-1.0.la
 
-noinst_PROGRAMS = test-contacts-picker
+noinst_PROGRAMS = test-contacts-picker test-highlight
+
 test_contacts_picker_CFLAGS =		\
 	$(NAUTILUS_CFLAGS)
 test_contacts_picker_SOURCES = 	\
@@ -49,10 +52,19 @@
 	add-contact-dialog.h	\
 	contacts-view.c		\
 	contacts-view.h		\
+	highlight.c \
+	highlight.h \
 	u1-contacts-picker.c	\
 	u1-contacts-picker.h
 test_contacts_picker_LDADD =  $(NAUTILUS_LIBS)
 
+test_highlight_CFLAGS = $(NAUTILUS_CFLAGS)
+test_highlight_SOURCES = \
+	highlight.c \
+	highlight.h \
+	test-highlight.c
+test_highlight_LDADD = $(NAUTILUS_LIBS)
+
 EXTRA_DIST =		\
 	ubuntuone-marshallers.list
 

=== modified file 'nautilus/contacts-view.c'
--- nautilus/contacts-view.c	2010-07-15 10:17:37 +0000
+++ nautilus/contacts-view.c	2010-07-20 16:05:59 +0000
@@ -22,6 +22,7 @@
 #include "config.h"
 #include <glib/gi18n.h>
 #include "contacts-view.h"
+#include "highlight.h"
 
 #ifdef BUILD_GRID_VIEW
 #define AVATAR_SIZE 64
@@ -40,6 +41,7 @@
 
 typedef struct {
 	gchar *name;
+	gchar *markedup_name;
 	gchar *email;
 	GdkPixbuf *pixbuf;
 } SelectedContactInfo;
@@ -166,6 +168,7 @@
 
 			sci = g_new0 (SelectedContactInfo, 1);
 			sci->name = g_strdup (name);
+			sci->markedup_name = g_markup_escape_text (name, -1);
 			sci->email = g_strdup (email);
 			sci->pixbuf = gdk_pixbuf_ref (icon);
 			g_hash_table_insert (cv->selection, g_strdup (name), sci);
@@ -188,7 +191,7 @@
 static void
 add_one_contact (ContactsView *cv,
 		 const gchar *name,
-		 const gchar *markup,
+		 const gchar *markedup_name,
 		 const gchar *email,
 		 EContact *contact,
 		 GHashTable *selection_hash)
@@ -197,7 +200,6 @@
 	EContactPhoto *photo = NULL;
 	GtkTreeModel *model;
 	GdkPixbuf *pixbuf;
-	gchar *escaped_markup;
 
 	/* Get the pixbuf for this contact */
 	if (contact != NULL) {
@@ -293,18 +295,16 @@
 	} else
 		gtk_list_store_append (GTK_LIST_STORE (model), &new_row);
 
-	escaped_markup = g_markup_escape_text (markup, strlen (markup));
 	gtk_list_store_set (GTK_LIST_STORE (model), &new_row,
 			    CONTACTS_VIEW_COLUMN_NAME, name,
-			    CONTACTS_VIEW_COLUMN_MARKUP, escaped_markup,
+			    CONTACTS_VIEW_COLUMN_MARKUP, markedup_name,
 			    CONTACTS_VIEW_COLUMN_EMAIL, email,
 			    CONTACTS_VIEW_COLUMN_PIXBUF, pixbuf,
 			    -1);
-	g_free (escaped_markup);
 }
 
 static void
-add_contacts (ContactsView *cv, GList *contacts, GHashTable *selection_hash)
+add_contacts (ContactsView *cv, GList *contacts, GHashTable *selection_hash, gchar *search_string)
 {
 	GList *l;
 	GtkTreeModel *model;
@@ -315,6 +315,7 @@
 		GList *emails_list, *al;
 		EContactName *contact_name;
 		gchar *full_name = NULL;
+		gchar *markedup_name = NULL;
 
 		contact_name = (EContactName *) e_contact_get (contact, E_CONTACT_NAME);
 		if (contact_name != NULL)
@@ -336,10 +337,12 @@
 				g_warning ("Contact without name or email addresses");
 		}
 
+		markedup_name = highlight_result (search_string, full_name);
+
 		/* We add the selected items when searching, so ignore them here */
 		if (!g_hash_table_lookup (selection_hash, (gconstpointer) full_name)) {
 			if (email != NULL) {
-				add_one_contact (cv, full_name, full_name, email, contact, selection_hash);
+				add_one_contact (cv, full_name, markedup_name, email, contact, selection_hash);
 				cv->matched_contacts += 1;
 			}
 		}
@@ -347,6 +350,7 @@
 		g_list_foreach (emails_list, (GFunc) e_vcard_attribute_free, NULL);
 		g_list_free (emails_list);
 		e_contact_name_free (contact_name);
+		g_free (markedup_name);
 		g_free (full_name);
 	}
 
@@ -361,7 +365,11 @@
 }
 
 static void
-append_selected_to_model (GtkWidget *view, const gchar *contact_name, const gchar *contact_email, GdkPixbuf *pixbuf)
+append_selected_to_model (GtkWidget *view,
+			  const gchar *contact_name,
+			  const gchar *contact_markedup_name,
+			  const gchar *contact_email,
+			  GdkPixbuf *pixbuf)
 {
 	GtkTreeIter new_row;
 	GtkListStore *model;
@@ -375,7 +383,7 @@
 	gtk_list_store_prepend (model, &new_row);
 	gtk_list_store_set (GTK_LIST_STORE (model), &new_row,
 			    CONTACTS_VIEW_COLUMN_NAME, contact_name,
-			    CONTACTS_VIEW_COLUMN_MARKUP, contact_name,
+			    CONTACTS_VIEW_COLUMN_MARKUP, contact_markedup_name,
 			    CONTACTS_VIEW_COLUMN_EMAIL, contact_email,
 			    CONTACTS_VIEW_COLUMN_PIXBUF, pixbuf,
 			    -1);
@@ -399,13 +407,14 @@
 	if (g_hash_table_lookup (cv->selection, sci->name) != NULL)
 		return;
 
-	append_selected_to_model (cv->contacts_list, sci->name, sci->email, sci->pixbuf);
+	append_selected_to_model (cv->contacts_list, sci->name, sci->markedup_name, sci->email, sci->pixbuf);
 }
 
 typedef struct {
 	ContactsView *cv;
 	GHashTable *selection_hash;
 	EBookQuery *query;
+	gchar *search_string;
 } GetContactsCallbackData;
 
 static void
@@ -414,7 +423,7 @@
 	GetContactsCallbackData *gccd = user_data;
 
 	if (status == E_BOOK_ERROR_OK) {
-		add_contacts (gccd->cv, contacts, gccd->selection_hash);
+		add_contacts (gccd->cv, contacts, gccd->selection_hash, gccd->search_string);
 		if (gccd->selection_hash != gccd->cv->selection) {
 			/* If it's a separate selection, add all contacts to the views */
 			g_hash_table_foreach (gccd->selection_hash, (GHFunc) foreach_selected_to_model_cb, gccd->cv);
@@ -426,6 +435,7 @@
 	}
 
 	e_book_query_unref (gccd->query);
+	g_free (gccd->search_string);
 	g_free (gccd);
 }
 
@@ -437,6 +447,7 @@
 	gccd = g_new0 (GetContactsCallbackData, 1);
 	gccd->cv = cv;
 	gccd->query = e_book_query_any_field_contains (search_string);
+	gccd->search_string = g_strdup (search_string);
 	if (selection_hash == cv->selection)
 		gccd->selection_hash = selection_hash;
 	else
@@ -471,6 +482,8 @@
 	if (sci != NULL) {
 		if (sci->name != NULL)
 			g_free (sci->name);
+		if (sci->markedup_name != NULL)
+			g_free (sci->markedup_name);
 		if (sci->email != NULL)
 			g_free (sci->email);
 		if (sci->pixbuf != NULL)
@@ -534,7 +547,7 @@
 	gtk_tree_view_insert_column_with_attributes (GTK_TREE_VIEW (cv->contacts_list), -1,
 						     "Name",
 						     gtk_cell_renderer_text_new (),
-						     "text", CONTACTS_VIEW_COLUMN_NAME,
+						     "markup", CONTACTS_VIEW_COLUMN_MARKUP,
 						     NULL);
 	gtk_tree_selection_set_mode (gtk_tree_view_get_selection (GTK_TREE_VIEW (cv->contacts_list)),
 				     GTK_SELECTION_MULTIPLE);
@@ -601,6 +614,7 @@
 
 		new_sci = g_new0 (SelectedContactInfo, 1);
 		new_sci->name = g_strdup (old_sci->name);
+		new_sci->markedup_name = g_markup_escape_text (old_sci->name, -1);
 		new_sci->email = g_strdup (old_sci->email);
 		new_sci->pixbuf = gdk_pixbuf_ref (old_sci->pixbuf);
 		g_hash_table_insert (tmp_selection, g_strdup (old_sci->name), new_sci);
@@ -686,6 +700,7 @@
 	/* First add the new contact to the list of selected ones */
 	sci = g_new0 (SelectedContactInfo, 1);
 	sci->name = g_strdup (contact_name);
+	sci->markedup_name = g_markup_escape_text (contact_name, -1);
 	sci->email = g_strdup (contact_email);
 	pixbuf = gtk_icon_theme_load_icon (icon_theme, "avatar-default", AVATAR_SIZE, 0, NULL);
 	sci->pixbuf = g_object_ref (pixbuf);
@@ -697,7 +712,7 @@
 	save_recently_used_list (cv);
 
 	/* And now add it to the icon views */
-	append_selected_to_model (cv->contacts_list, contact_name, contact_email, pixbuf);
+	append_selected_to_model (cv->contacts_list, contact_name, sci->markedup_name, contact_email, pixbuf);
 
 	g_object_unref (pixbuf);
 

=== added file 'nautilus/highlight.c'
--- nautilus/highlight.c	1970-01-01 00:00:00 +0000
+++ nautilus/highlight.c	2010-07-20 16:05:59 +0000
@@ -0,0 +1,161 @@
+/*
+ * Ubuntu One Nautilus plugin
+ *
+ * Authors: Alejandro J. Cura <[e-mail]>
+ *
+ * Copyright 2010 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 3, as published
+ * by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranties of
+ * MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+ * PURPOSE.  See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include <string.h>
+#include <glib.h>
+
+static gint
+compare_glongs (gconstpointer a,
+	        gconstpointer b,
+		gpointer user_data)
+{
+	return (glong) a - (glong) b;
+}
+
+
+static void
+tree_of_arrays_insert (GTree *tree,
+		       gpointer key,
+		       gpointer value)
+{
+	GPtrArray *array = g_tree_lookup (tree, key);
+	if (array == NULL) {
+		array = g_ptr_array_new ();
+		g_tree_insert (tree, key, array);
+	}
+	g_ptr_array_add (array, value);
+}
+
+
+static void
+destroy_tree_array (gpointer data)
+{
+	g_ptr_array_free (data, TRUE);
+}
+
+typedef struct {
+	GString *built_string;
+	gchar *source_string;
+	gchar *source_cursor;
+} BuildResultsData;
+
+
+static void
+append_array_strings (gpointer data,
+		      gpointer user_data)
+{
+	g_string_append (user_data, data);
+}
+
+
+static gboolean
+build_results (gpointer key,
+	       gpointer value,
+	       gpointer data)
+{
+	BuildResultsData* results_data = data;
+	glong tag_start = (glong)key;
+	gchar *tag_start_ptr = g_utf8_offset_to_pointer (results_data->source_string,
+							 tag_start);
+	glong len = tag_start_ptr - results_data->source_cursor;
+
+	gchar *escaped_str = g_markup_escape_text (results_data->source_cursor,
+						   len);
+	
+	g_string_append (results_data->built_string,
+			 escaped_str);
+	g_free (escaped_str);
+	results_data->source_cursor += len;
+
+	g_ptr_array_foreach (value,
+			     append_array_strings,
+			     results_data->built_string);
+	return FALSE;
+}
+
+
+gchar *
+highlight_result(gchar *needles, gchar *haystack)
+{
+	gchar **split_needles;
+	GTree *result_parts;
+	gchar *folded_needles;
+	gchar *folded_haystack;
+	BuildResultsData results_data;
+
+	folded_needles = g_utf8_casefold (needles, -1);
+	folded_haystack = g_utf8_casefold (haystack, -1);
+
+	results_data.built_string = g_string_new ("");
+	results_data.source_string = haystack;
+	results_data.source_cursor = haystack;
+
+	result_parts = g_tree_new_full (compare_glongs,
+					NULL,
+					NULL,
+					destroy_tree_array);
+	
+	split_needles = g_strsplit (folded_needles, " ", 0);
+	gchar **needle = split_needles;
+	while (*needle != NULL) {
+		glong needle_len = g_utf8_strlen (*needle, -1);
+		if (needle_len < 1) {
+			needle++;
+			continue;
+		}
+		gchar *search_start = folded_haystack;
+		gchar *start_ptr = g_strstr_len (search_start,
+						 -1,
+						 *needle);
+		while (start_ptr != NULL) {
+			glong start = g_utf8_pointer_to_offset (folded_haystack,
+								start_ptr);
+			glong end = start + g_utf8_strlen (*needle, -1);
+			tree_of_arrays_insert (result_parts,
+					       (gpointer) start,
+					       "<b>");
+			tree_of_arrays_insert (result_parts,
+					       (gpointer) end,
+					       "</b>");
+			search_start = g_utf8_next_char (start_ptr);
+			start_ptr = g_strstr_len (search_start,
+			                          -1,
+						  *needle);
+		}
+		needle++;
+	}
+	g_free (folded_needles);
+	g_free (folded_haystack);
+
+
+	g_tree_foreach (result_parts, build_results, &results_data);
+
+	gchar *escaped_str = g_markup_escape_text (results_data.source_cursor,
+						   -1);
+	g_string_append (results_data.built_string,
+			 escaped_str);
+	g_free (escaped_str);
+
+	g_tree_destroy (result_parts);
+	g_strfreev (split_needles);
+	return g_string_free (results_data.built_string,
+			      FALSE);
+}

=== added file 'nautilus/highlight.h'
--- nautilus/highlight.h	1970-01-01 00:00:00 +0000
+++ nautilus/highlight.h	2010-07-20 16:05:59 +0000
@@ -0,0 +1,29 @@
+/*
+ * Ubuntu One Nautilus plugin
+ *
+ * Authors: Alejandro J. Cura <[e-mail]>
+ *
+ * Copyright 2010 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 3, as published
+ * by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranties of
+ * MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+ * PURPOSE.  See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#ifndef __HIGHLIGHT_H__
+#define __HIGHLIGHT_H__
+
+#include <glib.h>
+
+gchar *highlight_result(gchar *search_string, gchar *result);
+
+#endif

=== added file 'nautilus/test-highlight.c'
--- nautilus/test-highlight.c	1970-01-01 00:00:00 +0000
+++ nautilus/test-highlight.c	2010-07-20 16:05:59 +0000
@@ -0,0 +1,93 @@
+/*
+ * Authors: Alejandro J. Cura <[e-mail]>
+ *
+ * Copyright 2010 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 3, as published
+ * by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranties of
+ * MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+ * PURPOSE.  See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "highlight.h"
+
+#include <glib.h>
+
+#define ASSERT_HIGHLIGHT(a,b,c) \
+	ret = highlight_result (a, b); \
+	g_assert_cmpstr (ret, ==, c); \
+	g_free (ret);
+
+static void
+test_highlight_matches (void)
+{
+	gchar *ret = NULL;
+	// match at beginning
+	ASSERT_HIGHLIGHT ("john", "john q. public", "<b>john</b> q. public");
+
+	// match in the middle
+	ASSERT_HIGHLIGHT ("john", "andrew john q. public", "andrew <b>john</b> q. public");
+
+	// no match
+	ASSERT_HIGHLIGHT ("john", "andrew q. public", "andrew q. public");
+
+	// double match
+	ASSERT_HIGHLIGHT ("jo sm", "john smith", "<b>jo</b>hn <b>sm</b>ith");
+	ASSERT_HIGHLIGHT ("john", "john johnson", "<b>john</b> <b>john</b>son");
+
+	// empty search
+	ASSERT_HIGHLIGHT ("", "john q. public", "john q. public");
+
+	// empty search with entities
+	ASSERT_HIGHLIGHT ("", "john & public", "john &amp; public");
+
+	// empty search term
+	ASSERT_HIGHLIGHT (" ", "john q. public", "john q. public");
+
+	// empty result
+	ASSERT_HIGHLIGHT ("john", "", "");
+
+	// case insensitive match
+	ASSERT_HIGHLIGHT ("john", "John Smith", "<b>John</b> Smith");
+	ASSERT_HIGHLIGHT ("john", "Brian JOHNSON", "Brian <b>JOHN</b>SON");
+	ASSERT_HIGHLIGHT ("br jo", "Brian JOHNSON", "<b>Br</b>ian <b>JO</b>HNSON");
+
+	// nested match
+	ASSERT_HIGHLIGHT ("edward wa", "edward wall", "<b>ed<b>wa</b>rd</b> <b>wa</b>ll");
+
+	// utf-8 strings
+	ASSERT_HIGHLIGHT ("ñandÚ", "Ñandú", "<b>Ñandú</b>");
+
+	// xml entities in the search
+	ASSERT_HIGHLIGHT ("john &", "john sons", "<b>john</b> sons");
+
+	// xml entities in the result
+	ASSERT_HIGHLIGHT ("john sons", "john & sons", "<b>john</b> &amp; <b>sons</b>");
+
+	// xml entities everywhere
+	ASSERT_HIGHLIGHT ("john & sons", "john & sons", "<b>john</b> <b>&amp;</b> <b>sons</b>");
+
+	// make sure the name of the entities is not highlighted
+	ASSERT_HIGHLIGHT ("john amp sons", "john & sons", "<b>john</b> &amp; <b>sons</b>");
+}
+
+int
+main (int argc, gchar *argv[])
+{
+	GMainLoop *main_loop;
+
+	g_type_init ();
+	g_test_init (&argc, &argv, NULL);
+
+	g_test_add_func ("/testcontacts/TestHighlightMatches", test_highlight_matches);
+
+	return g_test_run ();
+}