Re: Proposal for making the GtkFileChooser code asynchronous

[Tim Janik <timj imendio com>]

On Tue, 8 Nov 2005, Kristian Rietveld wrote:

> Hi,

> 1. Let's start out with a somewhat more general change.  We need to be able
>   to cancel currently running asynchronous operations.  For this we need
>   to have a handle on an operation, so we need to introduce a
>   GtkFileSystemHandle which would be defined the same way as GtkFileFolder --
>   each backend can provide it's own handle object.  Of course we add a
>   call which we can use to actually cancel an operation:
>
>   gboolean gtk_file_system_cancel_operation (GtkFileSystem        *file_system,
>                                              GtkFileSystemHandle  *handle,
>                                              GError              **error);

hm, since with GtkFileSystemHandle you already have an extra object/structure
per operation, couldn't you simply store the GtkFileSystem*file_system pointer
in it, so this can become (plus cancellation-cant-fail):

  void     gtk_file_system_cancel_operation (GtkFileSystemHandle  *handle);

and, if the handles are really only in place for asyncronous operations,
wouldn't naming it like that make more sense:

  void     gtk_file_async_handle_cancel_operation (GtkFileAsyncHandle  *handle);

oh, and is this actually going to be a ref-countable object?

>   The gboolean return value will indicate success or failure, in the latter
>   case the error will be provided via GError.
>
> 2. Addition of gtk_file_system_get_info(), so we are able to retrieve
>   information about a file without having to create a folder.  This is
>   useful in for example GtkPathBar. Prototypes:
>
>   typedef void (* GtkFileSystemGetInfoCallback) (GtkFileSystem       *file_system,
>                                                  GtkFileSystemHandle *handle,
>                                                  GtkFileInfo         *file_info,
>                                                  GError              *error,
>                                                  gpointer             data);
>
>   GtkFileSystemHandle *gtk_file_system_get_info (GtkFileSystem                 *file_system,
>                                                  const GtkFilePath             *path,
>                                                  GtkFileInfoType                types,
>                                                  GtkFileSystemGetInfoCallback   callback,
>                                                  gpointer                       data,
>                                                  GError                       **error);

i think this has been raised in some of the other comment emails already:
since the callback handler will have to deal with errors anyway, you could
make it easier on the caller if you removed the error argument from all
the gtk_file_system_*() functions and deliver the errors only through the
callback. that means the caller has to implement error handling only once.

>   typedef void (* GtkFileSystemGetFolderCallback) (GtkFileSystem       *file_system,
>                                                    GtkFileSystemHandle *handle,
>                                                    GtkFileFolder       *folder,
>                                                    GError              *error,
>                                                    gpointer             data);

> 4. gtk_file_system_create_folder() needs to be changed in order to be able
>   to operate asynchronously.  The new prototypes:
>
>   typedef void (* GtkFileSystemCreateFolderCallback) (GtkFileSystem     *file_system,
>                                                       const GtkFilePath *path
>                                                       GError            *error,
>                                                       gpointer           data);

shouldn't this have a GtkFileSystemHandle* ?

>   typedef void (* GtkFileSystemVolumeMountCallback) (GtkFileSystem       *file_system,
>                                                      GtkFileSystemVolume *volume,
>                                                      GError              *error,
>                                                      gpointer             data);

same here, shouldn't this have a GtkFileSystemHandle* ?

> 6. Right now gtk_file_system_render_icon() is synchronous, because it needs
>   some information about the file in order to be able to render a suitable
>   icon.  We think that loading icons asychronously, during the asynchronous
>   folder loading process, is the way to go.  Because we need a widget to
>   render icons for us, we need to add:
>
>   void gtk_file_system_set_widget (GtkFileSystem *file_system,
>                                    GtkWidget     *widget);

this will make it hard to render the icon for different widgets, say if you need
to display it in more than one place.

>   Then we will add an icon field to GtkFileInfo, appropriate get/set functions
>   and GTK_FILE_INFO_ICON.  We will remove gtk_file_system_render_icon().
>   Now we have gtk_file_system_set_widget(), we plan on changing
>   gtk_file_system_volume_render_icon() to:
>
>   GdkPixbuf *gtk_file_system_volume_get_icon (GtkFileSystem        *file_system,
>                                               GtkFileSystemVolume  *volume,
>                                               gint                  pixel_size,
>                                               GError              **error);

couldn't you just leave gtk_file_system_volume_render_icon() the way it is,
and introduce
  GdkPixbuf* gtk_file_info_render_icon (GtkFileInfo *file_info,
                                        GtkWidget   *widget,
                                        gint         pixel_size,
                                        GError     **error);
instead of introducing a _get variant that just takes the widget argument
implicitely from some place else?
i.e. i do understand that you need the widget handle for rendering the icon,
which is why it makes sense to pass it in to the rendering functions,
but i'd think you don't need the widget handle for loading the icon data
(asyncronously) internally, so having a widget setter on the file system
or file info (or file folder) doesn't really make sense to me.

> I am looking forward to your feedback.

> thanks,
>
> -kris.

---
ciaoTJ