On 08/06/17 10:38, Victor Toso wrote:
> Hi,
>
> On Thu, Jun 08, 2017 at 03:08:02PM +0200, Pavel Grunt wrote:
>> Use .png as the default
>>
>> Related: rhbz#1455832
>
> Small clarification of the rationale behind would be good (#1455832) -
> Still, not a full solution, is it?
>
> This will check if we are replacing the image file at
> gtk-file-chooser-dialog level. After that we still don't check if the
> file exists and it seems that gdk_pixbuf_save() don't care about it.
>
> IMHO the full solution would be running g_file_test() with
> G_FILE_TEST_EXISTS before gdk_pixbuf_save() and maybe a yes-no dialog if
> file exist.
>
> Example (with your patch):
> - Rename "Screenshot.png" to "fedora25-wayland"
> - As we don't have '.png', if fedora25-wayland.png exists it would be
> overwritten
>
>> ---
>> src/virt-viewer-window.c | 7 +++++--
>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/virt-viewer-window.c b/src/virt-viewer-window.c
>> index 3ba4e2c..9531f15 100644
>> --- a/src/virt-viewer-window.c
>> +++ b/src/virt-viewer-window.c
>> @@ -974,7 +974,8 @@ virt_viewer_window_menu_file_screenshot(GtkWidget *menu G_GNUC_UNUSED,
>> {
>> GtkWidget *dialog;
>> VirtViewerWindowPrivate *priv = self->priv;
>> - const char *image_dir;
>> + const gchar *image_dir;
>
> You did not touch image_dir, any special reason to change its type?
>
> Cheers,
> toso
>
>> + gchar *save_name;
>>
>> g_return_if_fail(priv->display != NULL);
>>
>> @@ -990,7 +991,9 @@ virt_viewer_window_menu_file_screenshot(GtkWidget *menu G_GNUC_UNUSED,
>> image_dir = g_get_user_special_dir(G_USER_DIRECTORY_PICTURES);
>> if (image_dir != NULL)
>> gtk_file_chooser_set_current_folder(GTK_FILE_CHOOSER (dialog), image_dir);
>> - gtk_file_chooser_set_current_name(GTK_FILE_CHOOSER (dialog), _("Screenshot"));
>> + save_name = g_strdup_printf("%s.png", _("Screenshot"));
For simple concatenation, g_strconcat should be faster.
--
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etrunko@xxxxxxxxxx
_______________________________________________
virt-tools-list mailing list
virt-tools-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/virt-tools-list
