Re: New filesel DnD patch

[Owen Taylor <otaylor redhat com>]

Alex Larsson <alexl redhat com> writes:

> Here is a new version of the fileselector drag and drop patch. This one 
> uses the API from the patch in the mail i sent earlier today 
> (Subject: Re: URIs vs. half-baked URIs [glib PATCH]).


> @@ -728,6 +735,147 @@ gtk_file_selection_init (GtkFileSelectio
>    gtk_widget_grab_focus (filesel->selection_entry);
>  }
>  
> +static gchar *
> +uri_list_extract_first_uri (const gchar* uri_list)
> +{
> +  const gchar *p, *q;
> +  gchar *retval;
> +  
> +  g_return_val_if_fail (uri_list != NULL, NULL);
> +  
> +  p = uri_list;
> +  /* We don't actually try to validate the URI according to RFC
> +   * 2396, or even check for allowed characters - we just ignore
> +   * comments and trim whitespace off the ends.  We also
> +   * allow LF delimination as well as the specified CRLF.
> +   */
> +  while (p)
> +    {
> +      if (*p != '#')
> +	{
> +	  while (isspace(*p))

Should use g_ascii_isspace().

> +	  if (q > p)
> +	    {
> +	      q--;
> +	      while (q > p && isspace (*q))
> +		q--;

Probably need to repeat the q > p check.

> +	      retval = g_malloc (q - p + 2);
> +	      strncpy (retval, p, q - p + 1);
> +	      retval[q - p + 1] = '\0';

You can use g_strndup() here.

> +	      return retval;
> +	    }
> +	}
> +      p = strchr (p, '\n');
> +      if (p)
> +	p++;
> +    }
> +  return NULL;


> +
> +  filename = g_filename_from_uri (uri, NULL, &error);
> +  g_free (uri);
> +  
> +  if (!filename)
> +    return;

Better free errror :-). I'd also g_warning() with
error->msg, since this indicates the drag source
has made a programming mistake 

> +  file = gtk_file_selection_get_filename (filesel);
> +
> +  if (file)
> +    {
> +      res = gethostname (hostname, 128);
> +      
> +      uri_list = g_filename_to_uri (file, (!res)?hostname:NULL, NULL);

I'd check errors here for sanity. gtk_file_selection_get_filename(),
can for instance, return "" if the filename is not convertable
into the filesystem encoding, which will produce an error.

> +      gtk_selection_data_set (selection_data,
> +			      selection_data->target, 8,
> +			      (void *)uri_list, strlen((char *)uri_list));
> +      g_free (uri_list);
> +    }

> +  else
> +    {
> +      gtk_selection_data_set (selection_data,
> +			      selection_data->target, 8,
> +			      NULL, 0);
> +    }
> +}

You should refuse the selection request in this case by
not calling gtk_selection_data_set(). This will give the
user snap-back feedback, indicating something went wrong.

(Actually, it doesn't look like gtk_file_selection_get_filename ()
will ever return NULL)

> +static void
> +file_selection_setup_dnd (GtkFileSelection *filesel)
> +{
> +  GtkWidget *eventbox;
> +  static GtkTargetEntry drag_types[] = {
> +    { "text/uri-list", 0, 0 }
> +  };
> +  static gint n_drag_types = sizeof(drag_types)/sizeof(drag_types[0]);
> +
> +  gtk_drag_dest_set (GTK_WIDGET (filesel),
> +		     GTK_DEST_DEFAULT_ALL,
> +		     drag_types, n_drag_types,
> +		     GDK_ACTION_COPY);
> +
> +  gtk_signal_connect (GTK_OBJECT(filesel), "drag_data_received",
> +		      GTK_SIGNAL_FUNC(filenames_dropped), NULL);
> +
> +  eventbox = gtk_widget_get_parent (filesel->selection_text);
> +  gtk_drag_source_set (eventbox,
> +		       GDK_BUTTON1_MASK,
> +		       drag_types, n_drag_types,
> +		       GDK_ACTION_COPY);
> +
> +  gtk_signal_connect (GTK_OBJECT (eventbox),
> +		      "drag_data_get",
> +		      GTK_SIGNAL_FUNC (filenames_drag_get),
> +		      filesel);
> +}

Would it make sense to also provide the filename as text
so you could drag it into, say, an entry?

Regards,
                                        Owen