GNOME.org
 

Re: [Nautilus-list] [PATCH] location bar autocompletion

Wow.  You weren't kidding when you said you were going to try and live
up to the "legendary code reviews of Darin"... ;)

On Sat, 2002-02-23 at 18:09, Alex Larsson wrote:
> You need some more changes to handle the fact that all strings (even 
> filenames) in Nautilus now are in UTF8. This means that all arguments to 
> GtkEntry and GtkEditable that specify positions in the strings are 
> measured in number of characters from the start (called position), not the 
> number of bytes from the start (called index).
[snip]

Done.  I only changed the code for "user visable strings" so uri's and
strings created by gnome_vfs are still assumed to be straight ASCII.
 
> You can't change this to a normal signal, since we need to peek on the 
> return value from the entry to see if it handled the event, otherwise we 
> shouldn't touch it. Our signahandler must also be connected "after" in 
> order to run after the entry handler.
> 

There still appears to be a problem here.  When this callback is set to
run "after", only non visible key events (e.g. GDK_Tab, GDK_Shift) cause
the editable_key_press_callback to be invoked.  When I change the signal
to run during "stage 2" rather than "stage 4" things mostly work.

I am guessing that this bug might related to the following:
"In GTK+ 1.2, if an event handler returned TRUE it prevented propagation
of that event to parent widgets. That is, the event signal would not be
emitted on parent widgets. In GTK+ 2.0, if an event handler returns
TRUE, the current signal emission on the current widget is immediately
stopped. That is, other callbacks connected to the signal will not be
invoked." (GTK 1.2->2.0 Porting Guide)

In a few days I will take another look at NautilusEntry to see if I can
figure out where things have gone wrong...

Thanks a lot for your review,
David

Index: ./src/nautilus-location-bar.c
===================================================================
RCS file: /cvs/gnome/nautilus/src/nautilus-location-bar.c,v
retrieving revision 1.81
diff -p -u -r1.81 nautilus-location-bar.c
--- ./src/nautilus-location-bar.c	2002/02/19 22:53:43	1.81
+++ ./src/nautilus-location-bar.c	2002/02/25 15:00:14
@@ -1,4 +1,3 @@
-
 /* -*- Mode: C; indent-tabs-mode: t; c-basic-offset: 8; tab-width: 8 -*- */
 
 /*
@@ -98,18 +97,17 @@ static GtkTargetEntry drop_types [] = {
 static char *nautilus_location_bar_get_location     (NautilusNavigationBar    *navigation_bar); 
 static void  nautilus_location_bar_set_location     (NautilusNavigationBar    *navigation_bar,
 						     const char               *location);
-static void  nautilus_location_bar_class_init (NautilusLocationBarClass *class);
-static void  nautilus_location_bar_init       (NautilusLocationBar      *bar);
+static void  nautilus_location_bar_class_init       (NautilusLocationBarClass *class);
+static void  nautilus_location_bar_init             (NautilusLocationBar      *bar);
 static void  nautilus_location_bar_update_label     (NautilusLocationBar      *bar);
 
 EEL_CLASS_BOILERPLATE (NautilusLocationBar,
-				   nautilus_location_bar,
-				   NAUTILUS_TYPE_NAVIGATION_BAR)
+		       nautilus_location_bar,
+		       NAUTILUS_TYPE_NAVIGATION_BAR)
 
 static NautilusWindow *
 nautilus_location_bar_get_window (GtkWidget *bar)
 {
-
 	return NAUTILUS_WINDOW (gtk_widget_get_ancestor (bar, NAUTILUS_TYPE_WINDOW));
 }
 
@@ -239,13 +237,11 @@ style_set_handler (GtkWidget *widget, Gt
 #endif
 }
 
-#if GNOME2_CONVERSION_COMPLETE
-
 /* utility routine to determine the string to expand to.  If we don't have anything yet, accept
    the whole string, otherwise accept the largest part common to both */
 
 static char *
-accumulate_name(char *full_name, char *candidate_name)
+accumulate_name (char *full_name, char *candidate_name)
 {
 	char *result_name, *str1, *str2;
 
@@ -256,11 +252,15 @@ accumulate_name(char *full_name, char *c
 		if (!eel_str_has_prefix (full_name, candidate_name)) {
 			str1 = full_name;
 			str2 = candidate_name;
-			while ((*str1++ == *str2++)) { }
-			*--str1 = '\0';
+
+			while ((g_utf8_get_char (str1) == g_utf8_get_char (str2))) {
+				str1 = g_utf8_next_char (str1);
+				str2 = g_utf8_next_char (str2);
+			}
+			*str1 = '\0';
 		}
 	}
-	
+
 	return result_name;
 }
 
@@ -275,7 +275,7 @@ get_file_info_list (NautilusLocationBar 
 			gnome_vfs_file_info_list_free (bar->details->file_info_list);	
 			bar->details->file_info_list = NULL;		
 		}
-		
+
 		bar->details->current_directory = g_strdup (dir_name);
 		result = gnome_vfs_directory_list_load (&bar->details->file_info_list, dir_name,
 							GNOME_VFS_FILE_INFO_DEFAULT);
@@ -289,8 +289,8 @@ get_file_info_list (NautilusLocationBar 
 }
 
 /* routine that performs the tab expansion using gnome-vfs.  Extract the directory name and
-  incomplete basename, then iterate through the directory trying to complete it.  If we
-  find something, add it to the entry */
+   incomplete basename, then iterate through the directory trying to complete it.  If we
+   find something, add it to the entry */
   
 static gboolean
 try_to_expand_path (gpointer callback_data)
@@ -301,10 +301,7 @@ try_to_expand_path (gpointer callback_da
 	GList *element;
 	GnomeVFSURI *uri;
 	GtkEditable *editable;
-	
-	uint base_length;
-	int current_path_length;
-	int offset;
+
 	char *base_name_uri_escaped;
 	char *base_name;
 	char *user_location;
@@ -313,33 +310,41 @@ try_to_expand_path (gpointer callback_da
 	char *expand_text;
 	char *expand_name;
 
-	bar = NAUTILUS_LOCATION_BAR (callback_data);
+	uint base_name_length;
+	uint user_location_length;
+	uint current_path_length;
+	uint expand_text_length;
+	uint offset;
 
+	printf ("idle callback: try_to_expand_path\n");
+
+	bar = NAUTILUS_LOCATION_BAR (callback_data);
 	editable = GTK_EDITABLE (bar->details->entry);
 	user_location = gtk_editable_get_chars (editable, 0, -1);
 	bar->details->idle_id = 0;
-	
+
 	/* if it's just '~' or '~/', don't expand because slash shouldn't be appended */
 	if (eel_strcmp (user_location, "~") == 0
 	    || eel_strcmp (user_location, "~/") == 0) {
 		g_free (user_location);
 		return FALSE;
 	}
-	current_path = eel_make_uri_from_input (user_location);
 
+	current_path = eel_make_uri_from_input (user_location);
 	if (!eel_istr_has_prefix (current_path, "file://")) {
 		g_free (user_location);
 		g_free (current_path);
 		return FALSE;
 	}
 
-	current_path_length = strlen (current_path);	
-	offset = current_path_length - strlen (user_location);
+	current_path_length = strlen (current_path);
+	user_location_length = g_utf8_strlen (user_location, -1);
+	offset = current_path_length - user_location_length;
 
 	g_free (user_location);
 
 	uri = gnome_vfs_uri_new (current_path);
-	
+
 	base_name_uri_escaped = gnome_vfs_uri_extract_short_name (uri);
 	if (base_name_uri_escaped == NULL) {
 		base_name = NULL;
@@ -351,14 +356,15 @@ try_to_expand_path (gpointer callback_da
 	if (base_name == NULL) {
 		gnome_vfs_uri_unref (uri);
 		g_free (current_path);
-		return FALSE;	
+		return FALSE;
 	}
 
-	base_length = strlen (base_name);
+	base_name_length = strlen (base_name);
+
 	dir_name = gnome_vfs_uri_extract_dirname (uri);
 
 	/* get file info for the directory, if it hasn't changed since last time */
-	get_file_info_list (bar, dir_name);	
+	get_file_info_list (bar, dir_name);
 	if (bar->details->file_info_list == NULL) {
 		g_free (dir_name);
 		g_free (base_name);
@@ -369,7 +375,6 @@ try_to_expand_path (gpointer callback_da
 
 	/* iterate through the directory, keeping the intersection of all the names that
 	   have the current basename as a prefix. */
-
 	expand_text = NULL;
 	for (element = bar->details->file_info_list; element != NULL; element = element->next) {
 		current_file_info = element->data;
@@ -383,15 +388,21 @@ try_to_expand_path (gpointer callback_da
 			g_free (expand_name);
 		}
 	}
-	
-	/* if we've got something, add it to the entry */	
-	if (expand_text != NULL
-	    && !eel_str_has_suffix (current_path, expand_text)
-	    && base_length < strlen (expand_text)) {
-		gtk_entry_append_text (GTK_ENTRY (editable), expand_text + base_length);
-		gtk_entry_select_region (GTK_ENTRY (editable),
-					 current_path_length - offset,
-					 current_path_length - offset + strlen (expand_text) - base_length);
+
+	/* if we've got something, add it to the entry */
+	if (expand_text != NULL) {
+		expand_text_length = g_utf8_strlen (expand_text, -1);
+		if (!eel_str_has_suffix (current_path, expand_text)
+		    && base_name_length < expand_text_length) {
+			gtk_editable_insert_text (editable,
+						  g_utf8_offset_to_pointer (expand_text, base_name_length),
+						  expand_text_length - base_name_length,
+						  &user_location_length);
+
+			gtk_editable_select_region (editable,
+						    current_path_length - offset,
+						    current_path_length - offset + expand_text_length - base_name_length);
+		}
 	}
 	g_free (expand_text);
 
@@ -435,13 +446,13 @@ entry_would_have_inserted_characters (co
 }
 
 static int
-get_editable_length (GtkEditable *editable)
+get_editable_number_of_chars (GtkEditable *editable)
 {
 	char *text;
 	int length;
 
 	text = gtk_editable_get_chars (editable, 0, -1);
-	length = strlen (text);
+	length = g_utf8_strlen (text, -1);
 	g_free (text);
 	return length;
 }
@@ -453,8 +464,8 @@ has_exactly_one_slash (GtkEditable *edit
 	gboolean exactly_one;
 
 	text = gtk_editable_get_chars (editable, 0, -1);
-	slash = strchr (text, '/');
-	exactly_one = slash != NULL && strchr (slash + 1, '/') == NULL;
+	slash = g_utf8_strchr (text, -1, '/');
+	exactly_one = slash != NULL && g_utf8_strchr (g_utf8_next_char (slash), -1, '/') == NULL;
 	g_free (text);
 
 	return exactly_one;
@@ -465,7 +476,7 @@ set_position_and_selection_to_end (GtkEd
 {
 	int end;
 
-	end = get_editable_length (editable);
+	end = get_editable_number_of_chars (editable);
 	gtk_editable_select_region (editable, end, end);
 	gtk_editable_set_position (editable, end);
 }
@@ -476,7 +487,7 @@ position_and_selection_are_at_end (GtkEd
 	int end;
 	int start_sel, end_sel;
 	
-	end = get_editable_length (editable);
+	end = get_editable_number_of_chars (editable);
 	if (gtk_editable_get_selection_bounds (editable, &start_sel, &end_sel)) {
 		if (start_sel != end || end_sel != end) {
 			return FALSE;
@@ -489,26 +500,27 @@ position_and_selection_are_at_end (GtkEd
  * callback so that we don't mess up the return value.
  */
 static void
-editable_key_press_callback (GtkObject *object,
-			     gpointer data,
-			     guint n_args,
-			     GtkArg *args)
+editable_key_press_callback (GClosure     *closure,
+			     GValue       *return_value,
+			     guint         n_param_values,
+			     const GValue *param_values,
+			     gpointer      invocation_hint,
+			     gpointer      data)
 {
+	NautilusLocationBar *bar;
 	GtkEditable *editable;
 	GdkEventKey *event;
-	gboolean *return_value_location;
-	NautilusLocationBar *bar;
 	const char *unexpanded_text;
 	char *expanded_text;
-	
-	g_assert (n_args == 1);
-	g_assert (args != NULL);
+
+	printf ("key press callback: editable_key_press_callback\n");
 
-	bar = NAUTILUS_LOCATION_BAR (data);
+	g_assert (n_param_values == 2);
+	g_assert (param_values != NULL);
 
-	editable = GTK_EDITABLE (object);
-	event = GTK_VALUE_POINTER (args[0]);
-	return_value_location = GTK_RETLOC_BOOL (args[1]);
+	bar = NAUTILUS_LOCATION_BAR (closure->data);
+	editable = GTK_EDITABLE (bar->details->entry);
+	event = (GdkEventKey *)g_value_peek_pointer (param_values);
 
 	/* After typing the right arrow key we move the selection to
 	 * the end.
@@ -530,8 +542,8 @@ editable_key_press_callback (GtkObject *
 	 * when we type a key that would have inserted characters.
 	 */
 	if (position_and_selection_are_at_end (editable)) {
-		if (*return_value_location
-		    && entry_would_have_inserted_characters (event)) {
+		if (g_value_get_boolean (return_value)
+		      && entry_would_have_inserted_characters (event)) {
 			if (event->keyval == GDK_slash
 			    && has_exactly_one_slash (editable)) {
 				/* It's OK for us to call
@@ -564,7 +576,6 @@ editable_key_press_callback (GtkObject *
 
 	nautilus_location_bar_update_label (bar);
 }
-#endif
 
 static void
 real_activate (NautilusNavigationBar *navigation_bar)
@@ -646,6 +657,7 @@ nautilus_location_bar_init (NautilusLoca
 	GtkWidget *entry;
 	GtkWidget *event_box;
 	GtkWidget *hbox;
+	GClosure *closure;
 
 	bar->details = g_new0 (NautilusLocationBarDetails, 1);
 
@@ -659,10 +671,10 @@ nautilus_location_bar_init (NautilusLoca
 	gtk_label_set_justify (GTK_LABEL (label), GTK_JUSTIFY_RIGHT);
 	gtk_misc_set_alignment (GTK_MISC (label), 1, 0.5);
 	g_signal_connect (label, "style_set", 
-			    G_CALLBACK (style_set_handler), NULL);
+			  G_CALLBACK (style_set_handler), NULL);
 
-	gtk_box_pack_start  (GTK_BOX (hbox), event_box, FALSE, TRUE,
-			     GNOME_PAD_SMALL);
+	gtk_box_pack_start (GTK_BOX (hbox), event_box, FALSE, TRUE,
+			    GNOME_PAD_SMALL);
 
 	entry = nautilus_entry_new ();
 
@@ -672,38 +684,35 @@ nautilus_location_bar_init (NautilusLoca
 				  G_CALLBACK (nautilus_navigation_bar_location_changed),
 				  bar);
 
-#if GNOME2_CONVERSION_COMPLETE
 	/* The callback uses the marshal interface directly
 	 * so it can both read and write the return value.
 	 */
-	gtk_signal_connect_full (GTK_OBJECT (entry), "key_press_event",
-				 NULL, editable_key_press_callback,
-				 bar, NULL,
-				 FALSE, TRUE);
-#endif
-	
+	closure = g_closure_new_simple (sizeof (GCClosure), bar);
+	g_closure_set_marshal (closure, editable_key_press_callback);
+	g_signal_connect_closure (GTK_OBJECT (entry), "key_press_event",
+				  closure, TRUE);
+
 	gtk_box_pack_start (GTK_BOX (hbox), entry, TRUE, TRUE, 0);
 
 	gtk_container_add (GTK_CONTAINER (bar), hbox);
 
-
 	/* Drag source */
 	gtk_drag_source_set (GTK_WIDGET (event_box), 
 			     GDK_BUTTON1_MASK | GDK_BUTTON3_MASK,
 			     drag_types, G_N_ELEMENTS (drag_types),
 			     GDK_ACTION_LINK);
-	g_signal_connect  (event_box, "drag_data_get",
-			   G_CALLBACK (drag_data_get_callback),
-			   bar);
+	g_signal_connect (event_box, "drag_data_get",
+			  G_CALLBACK (drag_data_get_callback),
+			  bar);
 
 	/* Drag dest. */
-	gtk_drag_dest_set  (GTK_WIDGET (bar),
-			    GTK_DEST_DEFAULT_ALL,
-			    drop_types, G_N_ELEMENTS (drop_types),
-			    GDK_ACTION_COPY | GDK_ACTION_MOVE | GDK_ACTION_LINK);
+	gtk_drag_dest_set (GTK_WIDGET (bar),
+			   GTK_DEST_DEFAULT_ALL,
+			   drop_types, G_N_ELEMENTS (drop_types),
+			   GDK_ACTION_COPY | GDK_ACTION_MOVE | GDK_ACTION_LINK);
 	g_signal_connect (bar, "drag_data_received",
-			    G_CALLBACK (drag_data_received_callback),
-			    NULL);
+			  G_CALLBACK (drag_data_received_callback),
+			  NULL);
 
 	gtk_widget_show_all (hbox);
 
@@ -711,7 +720,6 @@ nautilus_location_bar_init (NautilusLoca
 	bar->details->entry = NAUTILUS_ENTRY (entry);	
 }
 
-
 GtkWidget *
 nautilus_location_bar_new (NautilusWindow *window)
 {
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]