GIO support

[Urizev]

Hi,

I have a patch to give support for adding remote files to the
application and I was really easy and clean. However, I have found a
trouble when application uploads the file due to flickcurl does not
support URIs. The flickcurl_photos_upload_params () method only
accepts a filepath but an URI is provided.

I think a solution could be to create a temporal file before uploading
the picture. But I do not know if it is a good practice. I am also
looking for a function in flickrcurl which lets me provide the data
instead of the path.

I think that is very useful to add remote support to the application.
What do you think? Any help?

[Mario Sanchez Prada]

Urizev wrote:
> Hi,
>
> I have a patch to give support for adding remote files to the
> application and I was really easy and clean. However, I have found a
> trouble when application uploads the file due to flickcurl does not
> support URIs. The flickcurl_photos_upload_params () method only
> accepts a filepath but an URI is provided.

If I'm understanding you properly, you mean to add support for things
like directly uploading pictures to flickr from remote sources such as a
cell-phone paired through BT, a "remote folder" mounted through ssh or
samba... and so forth

Please correct me if I'm wrong.

> I think a solution could be to create a temporal file before uploading
> the picture. But I do not know if it is a good practice. I am also
> looking for a function in flickrcurl which lets me provide the data
> instead of the path.

I think that creating a temporary local file under /tmp to workaround
that issue would not be too bad, whenever you do not keep more than one
temporary file "cached" at the same time, to avoid using too much space
for that.

In the other hand, I wonder if there could be any chance for adding URIs
support to flickcurl so we need no workaround there, although I'm afraid
that, even in the case it got accepted by Dave Beckett (flickcurl's
author) it could took too much time and your approach seems not bad at a
first glance either.

So, I would go ahead with your approach, bearing always in mind that we
should keep usage of temporary space always as low as possible.

> I think that is very useful to add remote support to the application.
> What do you think? Any help?

It sounds good. It would be great to directly upload pictures from a
cell phone through BT, for instance.

Thanks!
Mario

[Urizev]

On Fri, Aug 28, 2009 at 11:18 AM, Mario Sanchez Prada <msan...@igalia.com> wrote:

Urizev wrote:
> Hi,
>
> I have a patch to give support for adding remote files to the
> application and I was really easy and clean. However, I have found a
> trouble when application uploads the file due to flickcurl does not
> support URIs. The flickcurl_photos_upload_params () method only
> accepts a filepath but an URI is provided.

If I'm understanding you properly, you mean to add support for things
like directly uploading pictures to flickr from remote sources such as a
cell-phone paired through BT, a "remote folder" mounted through ssh or
samba... and so forth

Please correct me if I'm wrong.

Yes, that's right. It is what I mean.

> I think a solution could be to create a temporal file before uploading
> the picture. But I do not know if it is a good practice. I am also
> looking for a function in flickrcurl which lets me provide the data
> instead of the path.

I think that creating a temporary local file under /tmp to workaround
that issue would not be too bad, whenever you do not keep more than one
temporary file "cached" at the same time, to avoid using too much space
for that.

In the other hand, I wonder if there could be any chance for adding URIs
support to flickcurl so we need no workaround there, although I'm afraid
that, even in the case it got accepted by Dave Beckett (flickcurl's
author) it could took too much time and your approach seems not bad at a
first glance either.

So, I would go ahead with your approach, bearing always in mind that we
should keep usage of temporary space always as low as possible.

Of course, it should be removed as soon as possible. But maybe a cache system for pictures could be useful. For example, it will be necessary for editing flickr pictures. However, as the first approach, I will copy the file into a local one and then I will remove immediately after uploading.

> I think that is very useful to add remote support to the application.
> What do you think? Any help?

It sounds good. It would be great to directly upload pictures from a
cell phone through BT, for instance.

Thanks!
Mario

--
Saludos

Juan Carlos

[Mario Sanchez Prada]

Urizev wrote:
> [...]

> So, I would go ahead with your approach, bearing always in mind that we
> should keep usage of temporary space always as low as possible.
>
>
> Of course, it should be removed as soon as possible. But maybe a cache
> system for pictures could be useful. For example, it will be necessary
> for editing flickr pictures. However, as the first approach, I will copy
> the file into a local one and then I will remove immediately after
> uploading.

Yes, I agree it could be useful at some point, as you mention, but for
the time being I guess the most simple approach (while good enough)
would be to keep one file on disk, regardless of using a (very simple)
cache system or not.

That's up to you :-)

Mario

PS: btw, I commented your idea here in the office with a mate and his
comment about that, in perfect spanish, was "la ostia!". Just for you to
know :-)

[Urizev]

On Fri, Aug 28, 2009 at 11:33 AM, Mario Sanchez Prada <msan...@igalia.com> wrote:

Urizev wrote:
> [...]

>     So, I would go ahead with your approach, bearing always in mind that we
>     should keep usage of temporary space always as low as possible.
>
>
> Of course, it should be removed as soon as possible. But maybe a cache
> system for pictures could be useful. For example, it will be necessary
> for editing flickr pictures. However, as the first approach, I will copy
> the file into a local one and then I will remove immediately after
> uploading.

Yes, I agree it could be useful at some point, as you mention, but for
the time being I guess the most simple approach (while good enough)
would be to keep one file on disk, regardless of using a (very simple)
cache system or not.

That's up to you :-)

I only said that for recording the idea and thinking in the future. But I am going to implement the simplest one.
 

Mario

PS: btw, I commented your idea here in the office with a mate and his
comment about that, in perfect spanish, was "la ostia!". Just for you to
know :-)

Jajaja, thank you. That cheers me up.
 

--
Saludos

Juan Carlos

[Urizev]

Have a look and tell me

diff --git a/src/frogr-facade.c b/src/frogr-facade.c
index 3587daf..45bb252 100644
--- a/src/frogr-facade.c
+++ b/src/frogr-facade.c
@@ -89,12 +89,22 @@ _upload_picture_thread (upload_picture_st *up_st)
flickcurl_upload_params *uparams;
flickcurl_upload_status *ustatus;

- /* Upload picture and gather photo ID */
-
+ /* Creating cache file before uploading (for remote pictures like
BT, samba, etc) */
+ const gchar* local_path = frogr_picture_get_local_path (fpicture);
+ if (! local_path) {
+ g_warning ("There is no local path for %s...\n",
+ frogr_picture_get_title (fpicture));
+
+ g_idle_add ((GSourceFunc)_picture_uploaded, up_st);
+ return;
+ }
+
+ /* Upload picture and gather photo ID */
+
/* Prepare upload params */
uparams = g_slice_new (flickcurl_upload_params);
+ uparams->photo_file = local_path;
uparams->title = frogr_picture_get_title (fpicture);
- uparams->photo_file = frogr_picture_get_filepath (fpicture);
uparams->description = frogr_picture_get_description (fpicture);
uparams->tags = frogr_picture_get_tags (fpicture);
uparams->is_public = frogr_picture_is_public (fpicture);
diff --git a/src/frogr-main-window.c b/src/frogr-main-window.c
index 62ef4ab..e108989 100644
--- a/src/frogr-main-window.c
+++ b/src/frogr-main-window.c
@@ -40,7 +40,7 @@


enum {
- FILEPATH_COL,
+ URI_COL,
PIXBUF_COL,
FPICTURE_COL
};
@@ -282,19 +282,19 @@ _on_picture_loaded (FrogrMainWindow *fmainwin,
FrogrPicture *fpicture)
FrogrMainWindowPrivate *priv = FROGR_MAIN_WINDOW_GET_PRIVATE
(fmainwin);
GdkPixbuf *pixbuf;
GtkTreeIter iter;
- const gchar *filepath;
+ const gchar *uri;

/* Add to model */
g_object_ref (fpicture);
frogr_main_window_model_add_picture (priv->model, fpicture);

/* Add to GtkIconView */
- filepath = frogr_picture_get_filepath (fpicture);
+ uri = frogr_picture_get_uri (fpicture);
pixbuf = frogr_picture_get_pixbuf (fpicture);

gtk_list_store_append (GTK_LIST_STORE (priv->tree_model), &iter);
gtk_list_store_set (GTK_LIST_STORE (priv->tree_model), &iter,
- FILEPATH_COL, filepath,
+ URI_COL, uri,
PIXBUF_COL, pixbuf,
FPICTURE_COL, fpicture,
-1);
@@ -492,7 +492,7 @@ _on_icon_view_drag_data_received (GtkWidget
*widget,
gpointer data)
{
FrogrMainWindow *fmainwin = FROGR_MAIN_WINDOW (data);
- GSList *filepaths_list = NULL;
+ GSList *uris_list = NULL;
gchar *files_string;
gchar **fileuris_array = NULL;
gint i;
@@ -504,19 +504,27 @@ _on_icon_view_drag_data_received (GtkWidget
*widget,
files_string = ((gchar *) selection_data->data);

fileuris_array = g_strsplit (files_string, "\r\n", -1);
- for (i = 0; fileuris_array[i]; i++)
- {
- gchar *filepath = g_filename_from_uri (fileuris_array[i], NULL,
NULL);
- if (filepath && !g_str_equal (g_strstrip (filepath), ""))
- filepaths_list = g_slist_append (filepaths_list, filepath);
+ for (i = 0; fileuris_array[i]; i++) {
+ gchar* fileuri = g_strdup (fileuris_array[i]);
+
+ if (! fileuri) {
+ continue;
+ }
+
+ if (! g_str_equal (g_strstrip (fileuri), "")) {
+ uris_list = g_slist_append (uris_list, fileuri);
+ }
+ else {
+ g_free (fileuri);
}
+ }

/* Load pictures */
- if (filepaths_list != NULL)
+ if (uris_list != NULL)
{
FrogrPictureLoader *fploader;
fploader =
- frogr_picture_loader_new (filepaths_list,
+ frogr_picture_loader_new (uris_list,
(GFunc)_on_picture_loaded,
(GFunc)_on_pictures_loaded,
fmainwin);
@@ -530,7 +538,7 @@ _on_icon_view_drag_data_received (GtkWidget
*widget,

/* Free */
g_strfreev (fileuris_array);
- g_slist_free (filepaths_list);
+ g_slist_free (uris_list);
}

void
@@ -557,19 +565,20 @@ _on_add_button_clicked (GtkButton *widget,
gtk_file_filter_add_mime_type (filter, "image/gif");
gtk_file_filter_set_name (filter, "images");
gtk_file_chooser_add_filter (GTK_FILE_CHOOSER (dialog), filter);
+ gtk_file_chooser_set_local_only (GTK_FILE_CHOOSER (dialog), FALSE);
gtk_file_chooser_set_select_multiple (GTK_FILE_CHOOSER (dialog),
TRUE);

- if (gtk_dialog_run (GTK_DIALOG (dialog)) == GTK_RESPONSE_ACCEPT)
+ if (gtk_dialog_run (GTK_DIALOG (dialog)) == GTK_RESPONSE_ACCEPT)
{
- GSList *filepaths;
+ GSList *uris;

/* Add selected pictures to icon view area */
- filepaths = gtk_file_chooser_get_filenames (GTK_FILE_CHOOSER
(dialog));
- if (filepaths != NULL)
+ uris = gtk_file_chooser_get_uris (GTK_FILE_CHOOSER (dialog));
+ if (uris != NULL)
{
FrogrPictureLoader *fploader;
fploader =
- frogr_picture_loader_new (filepaths,
+ frogr_picture_loader_new (uris,
(GFunc)_on_picture_loaded,
(GFunc)_on_pictures_loaded,
fmainwin);
@@ -578,7 +587,7 @@ _on_add_button_clicked (GtkButton *widget,
}

/* Free memory */
- g_slist_free (filepaths);
+ g_slist_free (uris);
}

/* Close dialog */
diff --git a/src/frogr-picture-loader.c b/src/frogr-picture-loader.c
index 46572a4..93c4dff 100644
--- a/src/frogr-picture-loader.c
+++ b/src/frogr-picture-loader.c
@@ -41,7 +41,7 @@ typedef struct _FrogrPictureLoaderPrivate
FrogrPictureLoaderPrivate;
struct _FrogrPictureLoaderPrivate
{
FrogrMainWindow *mainwin;
- GSList *filepaths;
+ GSList *uris;
GSList *current;
guint index;
guint n_pictures;
@@ -81,8 +81,8 @@ _update_status_and_progress (FrogrPictureLoader
*fploader)

if (priv->current)
{
- const gchar *filepath = (const gchar *)priv->current->data;
- gchar *filename = g_path_get_basename (filepath);
+ const gchar *uri = (const gchar *)priv->current->data;
+ gchar *filename = g_path_get_basename (uri);

/* Update progress */
status_text = g_strdup_printf ("Loading '%s'...", filename);
@@ -141,18 +141,25 @@ _load_next_picture (FrogrPictureLoader
*fploader)
{
GFile *gfile = NULL;
GFileInfo *file_info;
- gchar *filepath = (gchar *)priv->current->data;
+ gchar *uri = (gchar *)priv->current->data;
const gchar *mime_type;
gboolean valid_mime = FALSE;
gint i;

+ GError* error = 0;
/* Get file info */
- gfile = g_file_new_for_path (filepath);
+ gfile = g_file_new_for_uri (uri);
file_info = g_file_query_info (gfile,

G_FILE_ATTRIBUTE_STANDARD_CONTENT_TYPE,
G_FILE_QUERY_INFO_NONE,
NULL,
- NULL);
+ &error);
+
+ if (error) {
+ g_warning ("Unable to load content type from %s: %s\n", uri, error-
>message);
+ return;
+ }
+
/* Check mimetype */
mime_type = g_file_info_get_content_type (file_info);
for (i = 0; valid_mimetypes[i]; i++)
@@ -164,7 +171,7 @@ _load_next_picture (FrogrPictureLoader *fploader)
}
}

- g_debug ("Adding file %s (%s)\n", filepath, mime_type);
+ g_debug ("Adding file %s (%s)\n", uri, mime_type);
g_object_unref (file_info);

/* Asynchronously load the picture if mime is valid */
@@ -223,11 +230,11 @@ _load_next_picture_cb (GObject *object,
{
GdkPixbuf *pixbuf;
GdkPixbuf *s_pixbuf;
- gchar *filepath;
+ gchar *uri;
gchar *filename;

/* Gather needed information */
- filepath = g_file_get_path (file);
+ uri = g_file_get_uri (file);
filename = g_file_get_basename (file);
gdk_pixbuf_loader_close (pixbuf_loader, NULL);
pixbuf = gdk_pixbuf_loader_get_pixbuf (pixbuf_loader);
@@ -236,12 +243,12 @@ _load_next_picture_cb (GObject *object,
s_pixbuf = _get_scaled_pixbuf (pixbuf);

/* Build the FrogrPicture and set pixbuf */
- fpicture = frogr_picture_new (filepath, filename, FALSE);
+ fpicture = frogr_picture_new (uri, filename, FALSE);
frogr_picture_set_pixbuf (fpicture, s_pixbuf);

/* Free */
g_object_unref (s_pixbuf);
- g_free (filepath);
+ g_free (uri);
g_free (filename);
}
else
@@ -293,8 +300,8 @@ _frogr_picture_loader_finalize (GObject* object)

/* Free */
g_object_unref (priv->mainwin);
- g_slist_foreach (priv->filepaths, (GFunc)g_free, NULL);
- g_slist_free (priv->filepaths);
+ g_slist_foreach (priv->uris, (GFunc)g_free, NULL);
+ g_slist_free (priv->uris);

G_OBJECT_CLASS (frogr_picture_loader_parent_class)->finalize
(object);
}
@@ -323,7 +330,7 @@ frogr_picture_loader_init (FrogrPictureLoader
*fploader)
g_object_unref (controller);

/* Init the rest of private data */
- priv->filepaths = NULL;
+ priv->uris = NULL;
priv->current = NULL;
priv->index = -1;
priv->n_pictures = 0;
@@ -332,7 +339,7 @@ frogr_picture_loader_init (FrogrPictureLoader
*fploader)
/* Public API */

FrogrPictureLoader *
-frogr_picture_loader_new (GSList *filepaths,
+frogr_picture_loader_new (GSList *uris,
GFunc picture_loaded_cb,
GFunc pictures_loaded_cb,
gpointer object)
@@ -344,10 +351,10 @@ frogr_picture_loader_new (GSList *filepaths,
FROGR_PICTURE_LOADER_GET_PRIVATE (fploader);

/* Internal data */
- priv->filepaths = g_slist_copy (filepaths);
- priv->current = priv->filepaths;
+ priv->uris = g_slist_copy (uris);
+ priv->current = priv->uris;
priv->index = 0;
- priv->n_pictures = g_slist_length (priv->filepaths);
+ priv->n_pictures = g_slist_length (priv->uris);

/* Callback data */
priv->picture_loaded_cb = picture_loaded_cb;
@@ -366,7 +373,7 @@ frogr_picture_loader_load (FrogrPictureLoader
*fploader)
FROGR_PICTURE_LOADER_GET_PRIVATE (fploader);

/* Check first whether there's something to load */
- if (priv->filepaths == NULL)
+ if (priv->uris == NULL)
return;

/* Set proper state */
diff --git a/src/frogr-picture-loader.h b/src/frogr-picture-loader.h
index edbc984..79ab6fb 100644
--- a/src/frogr-picture-loader.h
+++ b/src/frogr-picture-loader.h
@@ -51,7 +51,7 @@ struct _FrogrPictureLoaderClass

GType frogr_picture_loader_get_type(void) G_GNUC_CONST;

-FrogrPictureLoader *frogr_picture_loader_new (GSList *filepaths,
+FrogrPictureLoader *frogr_picture_loader_new (GSList *uri,
GFunc
picture_loaded_cb,
GFunc
pictures_loaded_cb,
gpointer object);
diff --git a/src/frogr-picture-uploader.c b/src/frogr-picture-
uploader.c
index 9d4ce55..477dea2 100644
--- a/src/frogr-picture-uploader.c
+++ b/src/frogr-picture-uploader.c
@@ -130,6 +130,10 @@ _upload_next_picture_cb (FrogrPictureUploader
*fpuploader,

/* Update internal status */
priv->current = g_slist_next (priv->current);
+
+ // XXX - urizev: When upload finishes, the local file is deleted.
Maybe this is not the best place to released.
+ frogr_picture_delete_local_path (fpicture);
+
priv->index++;

/* Update status and progress bars */
diff --git a/src/frogr-picture.c b/src/frogr-picture.c
index 532b7a4..cd86188 100644
--- a/src/frogr-picture.c
+++ b/src/frogr-picture.c
@@ -36,7 +36,8 @@ typedef struct _FrogrPicturePrivate
FrogrPicturePrivate;
struct _FrogrPicturePrivate
{
gchar *id;
- gchar *filepath;
+ gchar *uri;
+ gchar *local_path; // Cache file
gchar *title;
gchar *description;
gchar *tags_string;
@@ -53,7 +54,7 @@ struct _FrogrPicturePrivate
enum {
PROP_0,
PROP_ID,
- PROP_FILEPATH,
+ PROP_URI,
PROP_TITLE,
PROP_DESCRIPTION,
PROP_TAGS_STRING,
@@ -177,9 +178,9 @@ _frogr_picture_set_property (GObject *object,
g_free (priv->id);
priv->id = g_value_dup_string (value);
break;
- case PROP_FILEPATH:
- g_free (priv->filepath);
- priv->filepath = g_value_dup_string (value);
+ case PROP_URI:
+ g_free (priv->uri);
+ priv->uri = g_value_dup_string (value);
break;
case PROP_TITLE:
g_free (priv->title);
@@ -224,8 +225,8 @@ _frogr_picture_get_property (GObject *object,
case PROP_ID:
g_value_set_string (value, priv->id);
break;
- case PROP_FILEPATH:
- g_value_set_string (value, priv->filepath);
+ case PROP_URI:
+ g_value_set_string (value, priv->uri);
break;
case PROP_TITLE:
g_value_set_string (value, priv->title);
@@ -261,7 +262,7 @@ _frogr_picture_finalize (GObject* object)

/* free strings */
g_free (priv->id);
- g_free (priv->filepath);
+ g_free (priv->uri);
g_free (priv->title);
g_free (priv->description);
g_free (priv->tags_string);
@@ -298,11 +299,10 @@ frogr_picture_class_init(FrogrPictureClass
*klass)

G_PARAM_READWRITE));

g_object_class_install_property (obj_class,
- PROP_FILEPATH,
- g_param_spec_string ("filepath",
- "filepath",
- "Full
filepath at disk "
- "for the
picture",
+ PROP_URI,
+ g_param_spec_string ("uri",
+ "uri",
+ "Full URI of
the picture",
NULL,

G_PARAM_READWRITE));
g_object_class_install_property (obj_class,
@@ -372,7 +372,7 @@ frogr_picture_init (FrogrPicture *fpicture)

/* Default values */
priv->id = NULL;
- priv->filepath = NULL;
+ priv->uri = NULL;
priv->title = NULL;
priv->description = NULL;
priv->tags_string = NULL;
@@ -389,15 +389,15 @@ frogr_picture_init (FrogrPicture *fpicture)
/* Public API */

FrogrPicture *
-frogr_picture_new (const gchar *filepath,
+frogr_picture_new (const gchar *uri,
const gchar *title,
gboolean public)
{
- g_return_val_if_fail (filepath, NULL);
+ g_return_val_if_fail (uri, NULL);
g_return_val_if_fail (title, NULL);

GObject *new = g_object_new(FROGR_TYPE_PICTURE,
- "filepath", filepath,
+ "uri", uri,
"title", title,
"is-public", public,
NULL);
@@ -432,27 +432,85 @@ frogr_picture_set_id (FrogrPicture *fpicture,
}

const gchar *
-frogr_picture_get_filepath (FrogrPicture *fpicture)
+frogr_picture_get_uri (FrogrPicture *fpicture)
{
g_return_val_if_fail(FROGR_IS_PICTURE(fpicture), NULL);

FrogrPicturePrivate *priv =
FROGR_PICTURE_GET_PRIVATE (fpicture);

- return (const gchar *)priv->filepath;
+ return (const gchar *)priv->uri;
}

void
-frogr_picture_set_filepath (FrogrPicture *fpicture,
- const gchar *filepath)
+frogr_picture_set_uri (FrogrPicture *fpicture,
+ const gchar *uri)
{
g_return_if_fail(FROGR_IS_PICTURE(fpicture));

FrogrPicturePrivate *priv =
FROGR_PICTURE_GET_PRIVATE (fpicture);

- g_free (priv->filepath);
- priv->filepath = g_strdup (filepath);
+ g_free (priv->uri);
+ priv->uri = g_strdup (uri);
+}
+
+const gchar *
+frogr_picture_get_local_path (FrogrPicture *fpicture)
+{
+ g_return_if_fail(FROGR_IS_PICTURE(fpicture));
+
+ FrogrPicturePrivate *priv =
+ FROGR_PICTURE_GET_PRIVATE (fpicture);
+
+ if (priv->local_path) {
+ return priv->local_path;
+ }
+
+ GError* error = 0;
+
+ gint fd = g_file_open_tmp ("frogr-XXXXXX", &priv->local_path,
&error);
+ if (fd < 0) {
+ g_warning ("Unable to create a temporary file: %s\n", error-
>message);
+ return NULL;
+ }
+
+ close (fd);
+
+ GFile* local = g_file_new_for_path (priv->local_path);
+ GFile* remote = g_file_new_for_uri (priv->uri);
+
+ error = 0;
+ if (! g_file_copy (remote, local,
+ G_FILE_COPY_OVERWRITE |
+ G_FILE_COPY_NOFOLLOW_SYMLINKS |
+ G_FILE_COPY_ALL_METADATA,
+ NULL, NULL, NULL, &error)) {
+ g_warning ("Unable to copy the remote file '%s' to the local file
'%s': %s\n", priv->uri, priv->local_path, error->message);
+ // XXX - urizev: is it not necessary to release local and remote
variables?
+ return 0;
+ }
+
+ return priv->local_path;
+}
+
+void
+frogr_picture_delete_local_path (FrogrPicture *fpicture)
+{
+ g_return_if_fail(FROGR_IS_PICTURE(fpicture));
+
+ FrogrPicturePrivate *priv =
+ FROGR_PICTURE_GET_PRIVATE (fpicture);
+
+ if (! priv->local_path) {
+ return;
+ }
+
+ GError *error = 0;
+ GFile *file = g_file_new_for_path (priv->local_path);
+ if ((! g_file_delete (file, NULL, &error)) || error) {
+ g_warning ("Unable to delete temporary file: %s\n", priv-
>local_path);
+ }
}

const gchar *
diff --git a/src/frogr-picture.h b/src/frogr-picture.h
index d8180a6..8e3b4d4 100644
--- a/src/frogr-picture.h
+++ b/src/frogr-picture.h
@@ -53,7 +53,7 @@ struct _FrogrPictureClass
GType frogr_picture_get_type(void) G_GNUC_CONST;

/* Constructor */
-FrogrPicture *frogr_picture_new (const gchar *filepath,
+FrogrPicture *frogr_picture_new (const gchar *uri,
const gchar *title,
gboolean public);

@@ -71,9 +71,12 @@ const gchar *frogr_picture_get_description
(FrogrPicture *fpicture);
void frogr_picture_set_description (FrogrPicture *fpicture,
const gchar *description);

-const gchar *frogr_picture_get_filepath (FrogrPicture *fpicture);
-void frogr_picture_set_filepath (FrogrPicture *fpicture,
- const gchar *filepath);
+const gchar *frogr_picture_get_uri (FrogrPicture *fpicture);
+void frogr_picture_set_uri (FrogrPicture *fpicture,
+ const gchar *uri);
+
+const gchar *frogr_picture_get_local_path (FrogrPicture *fpicture);
+void frogr_picture_delete_local_path (FrogrPicture *fpicture);

const GSList *frogr_picture_get_tags_list (FrogrPicture *fpicture);
const gchar *frogr_picture_get_tags (FrogrPicture *fpicture);

[Urizev]

I tested it with samba, sftp and local files by adding and
drag'n'dropping.

> ...
>
> read more »

[Mario Sanchez Prada]

Urizev wrote:
> Have a look and tell me
>

I'm trying to apply this patches to master but it seems email screw them
up in some way and it's not an easy task to do.

Could you please send the patches again as attachments created with
git-format-patch? That way applying them to master would be
straightforward with git-am.

Supposing you had all your changes as commits in a separate local branch
it would be as easy as 'git format-patch master' and then attaching all
the .patch files generated by that command.

And in case you had all your changes as commits directly over master
branch, just doing 'git format-patch HEAD~n', where 'n' is the number of
commits you have, will do the trick as well.

I'll be taking a look over the code in the meanwhile.

Thanks!

[Mario Sanchez Prada]

Mario Sanchez Prada wrote:
> [...]

> I'll be taking a look over the code in the meanwhile.

It looks good and, although at a first glance I was not sure about the
place where you implemented the simple cache mechanism (inside
FrogrPicture), I think now it's perhaps the cleanest way to implement
such s simple mechanism at this early stage. Later on, when the cache
system gets bigger and more complex I guess the most sane thing will be
to take that out there thought.

I have just a question at this moment: I'm not completely sure but
reading your patch I have the feeling that you're always copying every
picture to a temporary location, regardless of being a remote or a local
file. If that's the case I'd bet for adding some extra code (it does not
look as a big deal, anyway) not to cache anything for local files. But
perhaps I'm missing something and I'm wrong and I'm just saying stupid
things here :-)

Could you confirm that?

At last, just to thank you again for your contributions. I'll be eagerly
waiting for the non-screwed-up patches to try this out.

Ah! forgot to tell you in my last mail that attaching a diff file
generated with git diff > output_file is also ok, even despite I'm a git
lover ;-).

Btw, if you finally use 'git format-patch', please take a look to some
of the last commits messages in the repository with git log, to make up
an idea of how we're writting those messages write now (one brief line +
blank line + ChangeLog style log). That way applying your commit,
keeping your log message and Author in the git repository would be
pretty easy :-)

Thanks!
Mario

[Urizev]

Patch attached.

Changelog:
  * Added GIO support

I write it here because I am not used to manage git. Sorry! I will learn to use.

On Fri, Aug 28, 2009 at 6:28 PM, Mario Sanchez Prada <msan...@igalia.com> wrote:

Mario Sanchez Prada wrote:
> [...]

> I'll be taking a look over the code in the meanwhile.

It looks good and, although at a first glance I was not sure about the
place where you implemented the simple cache mechanism (inside
FrogrPicture), I think now it's perhaps the cleanest way to implement
such s simple mechanism at this early stage. Later on, when the cache
system gets bigger and more complex I guess the most sane thing will be
to take that out there thought.

Yes. I prefer to develop by small approaches. I think that is isolated enough for future refactors.
 

I have just a question at this moment: I'm not completely sure but
reading your patch I have the feeling that you're always copying every
picture to a temporary location, regardless of being a remote or a local
file. If that's the case I'd bet for adding some extra code (it does not
look as a big deal, anyway) not to cache anything for local files. But
perhaps I'm missing something and I'm wrong and I'm just saying stupid
things here :-)

Could you confirm that?

Yes. You are right, but in the current patch attached it is fixed. Now, I check if the file is local before copying it.
 

At last, just to thank you again for your contributions. I'll be eagerly
waiting for the non-screwed-up patches to try this out.

Ah! forgot to tell you in my last mail that attaching a diff file
generated with git diff > output_file is also ok, even despite I'm a git
lover ;-).

As you can see, I dit it in that way. :(

Btw, if you finally use 'git format-patch', please take a look to some
of the last commits messages in the repository with git log, to make up
an idea of how we're writting those messages write now (one brief line +
blank line + ChangeLog style log). That way applying your commit,
keeping your log message and Author in the git repository would be
pretty easy :-)

Thanks!
Mario

--
Saludos

Juan Carlos

[Mario Sanchez Prada]

Urizev wrote:
> Patch attached.
>
Thanks! I've quickly tried out now (no much time right now, I did not
even read the patch) and I was able to upload pictures from a remote
folder through ssh and my cell phone through bluetooth at the same time.
Awesome!

> I write it here because I am not used to manage git. Sorry! I will learn to
> use.

No problem, I had no problem this time to apply your patch, and keeping
authorship is not a problem since I always can do 'git commit --author
"Your name <yo...@mail.here>"' :-)

> On Fri, Aug 28, 2009 at 6:28 PM, Mario Sanchez Prada <msan...@igalia.com>wrote:
>> I have just a question at this moment: I'm not completely sure but
>> reading your patch I have the feeling that you're always copying every
>> picture to a temporary location, regardless of being a remote or a local
>> file. If that's the case I'd bet for adding some extra code (it does not
>> look as a big deal, anyway) not to cache anything for local files. But
>> perhaps I'm missing something and I'm wrong and I'm just saying stupid
>> things here :-)
>>
>> Could you confirm that?
>>
>
> Yes. You are right, but in the current patch attached it is fixed. Now, I
> check if the file is local before copying it.

Great!

>> At last, just to thank you again for your contributions. I'll be eagerly
>> waiting for the non-screwed-up patches to try this out.
>>
>> Ah! forgot to tell you in my last mail that attaching a diff file
>> generated with git diff > output_file is also ok, even despite I'm a git
>> lover ;-).
>
>
> As you can see, I dit it in that way. :(

No place for ':(' here... I'm more like :D of having a git diff attached
:-). Keep doing it that way if it's better for you, although I'm sure as
soon you start using format-patch you'll fall in love with it ;-)

Btw, trying out your patch (did I already say I did it very quickly?) I
found some critical errors in the output as well as some problems
uploading local files ("No such file or directory") so I'm attaching you
the output log just in case you could (and wished to) take a look on it
before integrating it in master.

But if you could not don't worry, I'd take the look then as soon as
possible, although I'm afraid I won't be able to do it until next Monday
or, if lucky, tomorrow in the evening.

Thanks a lot! I'm sure this is a feature a lot of people (like me) will
love for sure!

Mario

[Urizev]

On Sun, Aug 30, 2009 at 12:55 AM, Mario Sanchez Prada
<msan...@igalia.com> wrote:
> Btw, trying out your patch (did I already say I did it very quickly?) I
> found some critical errors in the output as well as some problems
> uploading local files ("No such file or directory") so I'm attaching you
> the output log just in case you could (and wished to) take a look on it
> before integrating it in master.

Hey! I did not notice anything wrong (no critical errors). Could you
explain me how to repeat the error? Anyway, I think you should attach
the log. XD

>
> But if you could not don't worry, I'd take the look then as soon as
> possible, although I'm afraid I won't be able to do it until next Monday
> or, if lucky, tomorrow in the evening.

No problem. Now, I am focusing in the next feature.

>
> Thanks a lot! I'm sure this is a feature a lot of people (like me) will
> love for sure!

For sure. I think a good desktop integration is fundamental.

--
Saludos

Juan Carlos

[Mario Sanchez Prada]

Urizev wrote:
> On Sun, Aug 30, 2009 at 12:55 AM, Mario Sanchez Prada
> <msan...@igalia.com> wrote:
>> Btw, trying out your patch (did I already say I did it very quickly?) I
>> found some critical errors in the output as well as some problems
>> uploading local files ("No such file or directory") so I'm attaching you
>> the output log just in case you could (and wished to) take a look on it
>> before integrating it in master.
>
> Hey! I did not notice anything wrong (no critical errors). Could you
> explain me how to repeat the error? Anyway, I think you should attach
> the log. XD

Ooops! Sure... guess I'm too tired and I'd better go to bed right now.
Attaching now, btw :-)

The steps to reproduce the error are:

1. Open frogr
2. Drag & drop 3 pictures from 3 different sourceS:
- 1 picture from local path (this is the "no such file" error)
- 1 picture from ssh source (worked ok)
- 1 picture from bluetooth source (worked ok)
- Upload the pictures

EXPECTED: Everything should go fine

ACTUAL: the local file will throwh a "no such file" error and several
GLIB CRITICAl errores will appear on the terminal.

Others: I have the feeling that the "no such file" could have something
to do with the fact that the full path to the file has some "special
characters" in the middle, such as blank spaces or "ñ's".

In this case the full path is this:

"/home/mario/Pictures/Animales/En\ Iñás/000_0041.jpg"

... which is obviously different from what is printed out in the log:

"/home/mario/Pictures/Animales/En%20I%C3%B1%C3%A1s/000_0041.jpg"

Perhaps, some kind of conversion is still needed somehow... not sure,
though.

>> But if you could not don't worry, I'd take the look then as soon as
>> possible, although I'm afraid I won't be able to do it until next Monday
>> or, if lucky, tomorrow in the evening.
>
> No problem. Now, I am focusing in the next feature.

Ok.

>> Thanks a lot! I'm sure this is a feature a lot of people (like me) will
>> love for sure!
>
> For sure. I think a good desktop integration is fundamental.

Yep :-)

Thanks,
Mario

[Urizev]

Hi,

I have been working on the bug and I have fixed it. However, I have
found one more. I will send it when it was finished.

--
Saludos

Juan Carlos

[Mario Sanchez Prada]

Urizev wrote:
> Hi,
>
> I have been working on the bug and I have fixed it. However, I have
> found one more. I will send it when it was finished.

Great! There's no hurry btw, do take your time for that :-)

Mario