Re: [virt-tools-list] [virt-viewer v2] Use GResource for loading ui files

2016-03-02 Thread Eduardo Lima (Etrunko)
On 03/02/2016 05:24 PM, Fabiano Fidêncio wrote:
> On Wed, Mar 2, 2016 at 8:05 PM, Eduardo Lima (Etrunko)
>  wrote:
>> On 03/02/2016 02:46 PM, Eduardo Lima (Etrunko) wrote:
>>> On 03/02/2016 12:23 PM, Fabiano Fidêncio wrote:
>>>> Let's take advantage of GResource for loading ui files in a better and
>>>> cleaner way than virt_viewer_util_load_ui() was doing.
>>>> It also brings the benefit, at least for developers, of being able to
>>>> test ui changes without having to "make install" virt-viewer.
>>>>
>>>> Signed-off-by: Fabiano Fidêncio 
>>>> ---
>>>> Changes since v1:
>>>>  - Adressed all comments from Eduardo and Jonathon.
>>>> ---
>>>>  mingw-virt-viewer.spec.in | 18 ++--
>>>>  src/Makefile.am   | 21 +--
>>>>  src/virt-viewer-about.xml |  1 -
>>>>  src/virt-viewer-app.c |  5 +
>>>>  src/virt-viewer-util.c| 48 
>>>> +--
>>>>  src/virt-viewer-util.h|  1 +
>>>>  src/virt-viewer-window.c  | 20 ++
>>>>  src/virt-viewer.gresource.xml | 19 +
>>>>  virt-viewer.spec.in   |  9 
>>>>  9 files changed, 63 insertions(+), 79 deletions(-)
>>>>  create mode 100644 src/virt-viewer.gresource.xml
>>>>
>>>> diff --git a/mingw-virt-viewer.spec.in b/mingw-virt-viewer.spec.in
>>>> index b200db7..ddea296 100644
>>>> --- a/mingw-virt-viewer.spec.in
>>>> +++ b/mingw-virt-viewer.spec.in
>>>> @@ -114,10 +114,12 @@ MinGW Windows virt-viewer MSI
>>>>  %mingw_make_install DESTDIR=$RPM_BUILD_ROOT
>>>>
>>>>  %if 0%{?mingw_build_win32} == 1
>>>> +mkdir $RPM_BUILD_ROOT/%{mingw32_datadir}/virt-viewer
>>>>  cp build_win32$MINGW_BUILDDIR_SUFFIX/data/virt-viewer-x86-@VERSION@.msi 
>>>> $RPM_BUILD_ROOT/%{mingw32_datadir}/virt-viewer
>>>>  %endif
>>>>
>>>>  %if 0%{?mingw_build_win64} == 1
>>>> +mkdir $RPM_BUILD_ROOT/%{mingw64_datadir}/virt-viewer
>>>>  cp build_win64$MINGW_BUILDDIR_SUFFIX/data/virt-viewer-x64-@VERSION@.msi 
>>>> $RPM_BUILD_ROOT/%{mingw64_datadir}/virt-viewer
>>>>  %endif
>>>>
>>>> @@ -138,14 +140,6 @@ rm -rf $RPM_BUILD_ROOT
>>>>  %{mingw32_bindir}/debug-helper.exe
>>>>
>>>>  %dir %{mingw32_datadir}/virt-viewer/
>>>> -%dir %{mingw32_datadir}/virt-viewer/ui/
>>>> -%{mingw32_datadir}/virt-viewer/ui/virt-viewer.xml
>>>> -%{mingw32_datadir}/virt-viewer/ui/virt-viewer-about.xml
>>>> -%{mingw32_datadir}/virt-viewer/ui/virt-viewer-auth.xml
>>>> -%{mingw32_datadir}/virt-viewer/ui/virt-viewer-guest-details.xml
>>>> -%{mingw32_datadir}/virt-viewer/ui/virt-viewer-vm-connection.xml
>>>> -%{mingw32_datadir}/virt-viewer/ui/virt-viewer-preferences.xml
>>>> -%{mingw32_datadir}/virt-viewer/ui/remote-viewer-connect.xml
>>>>  %{mingw32_datadir}/icons/hicolor/*/apps/*
>>>>  %{mingw32_datadir}/icons/hicolor/*/devices/*
>>>>
>>>> @@ -163,14 +157,6 @@ rm -rf $RPM_BUILD_ROOT
>>>>  %{mingw64_bindir}/debug-helper.exe
>>>>
>>>>  %dir %{mingw64_datadir}/virt-viewer/
>>>> -%dir %{mingw64_datadir}/virt-viewer/ui/
>>>> -%{mingw64_datadir}/virt-viewer/ui/virt-viewer.xml
>>>> -%{mingw64_datadir}/virt-viewer/ui/virt-viewer-about.xml
>>>> -%{mingw64_datadir}/virt-viewer/ui/virt-viewer-auth.xml
>>>> -%{mingw64_datadir}/virt-viewer/ui/virt-viewer-guest-details.xml
>>>> -%{mingw64_datadir}/virt-viewer/ui/virt-viewer-vm-connection.xml
>>>> -%{mingw64_datadir}/virt-viewer/ui/virt-viewer-preferences.xml
>>>> -%{mingw64_datadir}/virt-viewer/ui/remote-viewer-connect.xml
>>>>  %{mingw64_datadir}/icons/hicolor/*/apps/*
>>>>  %{mingw64_datadir}/icons/hicolor/*/devices/*
>>>>
>>>> diff --git a/src/Makefile.am b/src/Makefile.am
>>>> index f42a7bf..a42c01e 100644
>>>> --- a/src/Makefile.am
>>>> +++ b/src/Makefile.am
>>>> @@ -5,8 +5,7 @@ bin_PROGRAMS =
>>>>
>>>>  noinst_LTLIBRARIES = libvirt-viewer.la
>>>>
>>>> -builderxmldir = $(pkgdatadir)/ui
>>>> -builderxml_DATA =   \
>>>> +noinst_DATA =   \
>>>>  virt-viewer.x

Re: [virt-tools-list] [virt-viewer] app: Remove useless libxml includes

2016-03-03 Thread Eduardo Lima (Etrunko)
On 03/02/2016 12:16 PM, Fabiano Fidêncio wrote:
> ---
>  src/virt-viewer-app.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/src/virt-viewer-app.c b/src/virt-viewer-app.c
> index 3fa80a5..b51bf4f 100644
> --- a/src/virt-viewer-app.c
> +++ b/src/virt-viewer-app.c
> @@ -36,9 +36,6 @@
>  #include 
>  #include 
>  
> -#include 
> -#include 
> -
>  #ifdef HAVE_SYS_SOCKET_H
>  #include 
>  #endif
> 
Acked-by: Eduardo Lima (Etrunko) 

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


Re: [virt-tools-list] [PATCH virt-viewer 2/2] Move tests under /tests directory

2016-03-11 Thread Eduardo Lima (Etrunko)
On 03/11/2016 11:44 AM, Fabiano Fidêncio wrote:
[snip]
>> diff --git a/tests/Makefile.am b/tests/Makefile.am
>> new file mode 100644
>> index 000..470733b
>> --- /dev/null
>> +++ b/tests/Makefile.am
>> @@ -0,0 +1,30 @@
>> +NULL =
>> +
>> +AM_CPPFLAGS =   
>>\
>> +   -DLOCALE_DIR=\""$(datadir)/locale"\"\
>> +   -I$(top_srcdir)/src/\
>> +   -I$(top_srcdir)/tests/  \
>> +   $(GLIB2_CFLAGS)  
>>\
>> +   $(GTK_CFLAGS)
>>\
>> +   $(LIBXML2_CFLAGS)
>>\
>> +   $(WARN_CFLAGS)   
>>\
>> +   $(NULL)
> 
> Pavel, I know you basically copied & pasted the old code here. But do
> we need {GTK,LIBXML2}_CFLAGS here? Our tests are not using none of
> those if I'm not mistaken.
> 
> I would accept a first patch removing these unneeded CPPFLAGS/LIBS.
> 
> Also, can we have the "\" aligned?
> 
>> +
>> +LDADD=  
>>\
>> +   $(top_builddir)/src/libvirt-viewer-util.la  \
>> +   $(GLIB2_LIBS)
>>\
>> +   $(GTK_LIBS)  
>>\
>> +   $(LIBXML2_LIBS)  
>>\
>> +   $(NULL)
>> +
> 
> Same here.
> 

The problem with having backslash symbols aligned is that each one's
preferred editor might have different setups. Makefiles use to require
tab indentation (not sure if still a hard requirement), so different
tabstops may result in different alignment.

My suggestion is to just use a single space after the file/variable and
then the backslash symbol, while the begin of each line is still tab
indented. For instance:

+LDADD= \
+   $(top_builddir)/src/libvirt-viewer-util.la \
+   $(GLIB2_LIBS) \
+   $(GTK_LIBS) \
+   $(LIBXML2_LIBS) \
+   $(NULL)

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list

Re: [virt-tools-list] [PATCH virt-viewer 1/2] Add libvirt-viewer-util library an use it in tests

2016-03-11 Thread Eduardo Lima (Etrunko)
On 03/11/2016 11:40 AM, Fabiano Fidêncio wrote:
> On Wed, Mar 9, 2016 at 2:21 PM, Pavel Grunt  wrote:
>> ---
>>  src/Makefile.am | 28 +---
>>  1 file changed, 21 insertions(+), 7 deletions(-)
>>
>> diff --git a/src/Makefile.am b/src/Makefile.am
>> index 2cd9001..aaa111d 100644
>> --- a/src/Makefile.am
>> +++ b/src/Makefile.am
>> @@ -3,7 +3,7 @@ LDADD =
>>  MAINTAINERCLEANFILES =
>>  bin_PROGRAMS =
>>
>> -noinst_LTLIBRARIES = libvirt-viewer.la
>> +noinst_LTLIBRARIES = libvirt-viewer-util.la libvirt-viewer.la
>>
>>  noinst_DATA =  \
>> virt-viewer.xml \
>> @@ -45,10 +45,13 @@ CLEANFILES = \
>> $(BUILT_SOURCES)\
>> $(NULL)
>>
>> -libvirt_viewer_la_SOURCES =\
>> -   $(BUILT_SOURCES)\
>> +libvirt_viewer_util_la_SOURCES =   \
>> virt-viewer-util.h  \
>> virt-viewer-util.c  \
>> +   $(NULL)
>> +
>> +libvirt_viewer_la_SOURCES =\
>> +   $(BUILT_SOURCES)\
>> virt-viewer-auth.h  \
>> virt-viewer-auth.c  \
>> virt-viewer-app.h   \
>> @@ -120,6 +123,13 @@ COMMON_CFLAGS = \
>> $(WARN_CFLAGS) \
>> $(NULL)
>>
>> +libvirt_viewer_util_la_LIBADD = \
>> +   $(COMMON_LIBS) \
>> +   $(NULL)
>> +
>> +libvirt_viewer_util_la_CFLAGS = \
>> +   $(COMMON_CFLAGS)
>> +   $(NULL)
>>
>>  libvirt_viewer_la_LIBADD = \
>> $(COMMON_LIBS) \
>> @@ -132,8 +142,6 @@ libvirt_viewer_la_CFLAGS = \
>>  check_PROGRAMS = test-version-compare test-monitor-mapping
>>  TESTS = $(check_PROGRAMS)
>>  test_version_compare_SOURCES = \
>> -   virt-viewer-util.c  \
>> -   virt-viewer-util.h  \
>> test-version-compare.c  \
>> $(NULL)
>>  test_version_compare_LDFLAGS = \
>> @@ -148,10 +156,11 @@ test_version_compare_CFLAGS =  
>>\
>> $(LIBXML2_CFLAGS)   \
>> $(WARN_CFLAGS)  \
>> $(NULL)
>> +test_version_compare_LDADD =   \
>> +   libvirt-viewer-util.la  \
>> +   $(NULL)
>>
>>  test_monitor_mapping_SOURCES = \
>> -   virt-viewer-util.c  \
>> -   virt-viewer-util.h  \
>> test-monitor-mapping.c  \
>> $(NULL)
>>  test_monitor_mapping_LDFLAGS = \
>> @@ -166,6 +175,9 @@ test_monitor_mapping_CFLAGS =
>>\
>> $(LIBXML2_CFLAGS)   \
>> $(WARN_CFLAGS)  \
>> $(NULL)
>> +test_monitor_mapping_LDADD =   \
>> +   libvirt-viewer-util.la  \
>> +   $(NULL)
>>
>>  if HAVE_LIBVIRT
>>  bin_PROGRAMS += virt-viewer
>> @@ -183,6 +195,7 @@ virt_viewer_CFLAGS =\
>> $(LIBVIRT_CFLAGS)   \
>> $(NULL)
>>  virt_viewer_LDADD = \
>> +   libvirt-viewer-util.la  \
>> libvirt-viewer.la \
>> $(NULL)
>>  endif
>> @@ -205,6 +218,7 @@ remote_viewer_CFLAGS =  \
>> $(SPICE_CONTROLLER_CFLAGS)  \
>> $(NULL)
>>  remote_viewer_LDADD = \
>> +   libvirt-viewer-util.la  \
>> libvirt-viewer.la \
>> $(NULL)
>>
>> --
>> 2.5.0
>>
>> ___
>> virt-tools-list mailing list
>> virt-tools-list@redhat.com
>> https://www.redhat.com/mailman/listinfo/virt-tools-list
> 
> ACK from me.
> Eduardo, can you also take a look on this one?
> 
> Best Regards,
> --
> Fabiano Fidêncio
> 
> ___
> virt-tools-list mailing list
> virt-tools-list@redhat.com
> https://www.redhat.com/mailman/listinfo/virt-tools-list
> 

Acked-by: Eduardo Lima (Etrunko) 

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list

Re: [virt-tools-list] [PATCH 2/2] Fix spice includes

2016-03-15 Thread Eduardo Lima (Etrunko)
On 11/11/2015 09:09 AM, Eduardo Lima (Etrunko) wrote:
> On 11/11/2015 05:11 AM, Pavel Grunt wrote:
>> Hi Eduardo,
>>
>> these warnings were introduced after spice-gtk v0.30 release, virt-viewer
>> currently depends on spice-gtk v0.29.35. You can make the code conditional or
>> wait for the spice-gtk v0.31 release.
>>
> 
> Alright, I guess it is better to wait then.
> 
> Regards. Eduardo.
> 

I waited and it happened. Is it time to submit this patch again?

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


[virt-tools-list] [PATCH 1/2] Fix spice includes

2016-03-15 Thread Eduardo Lima (Etrunko)
Recent spice commit requires that only spice-client.h or
spice-client-gtk.h should be included directly. As a result,
compilation is now throwing warnings like:

warning: #warning "Only  can be included directly" [-Wcpp]
warning: #warning "Only  can be included directly" [-Wcpp]

Signed-off-by: Eduardo Lima (Etrunko) 
---
 src/virt-viewer-display-spice.c | 2 +-
 src/virt-viewer-display-spice.h | 3 +--
 src/virt-viewer-session-spice.c | 5 +
 src/virt-viewer-session-spice.h | 3 +--
 4 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/src/virt-viewer-display-spice.c b/src/virt-viewer-display-spice.c
index 5114833..4abab2c 100644
--- a/src/virt-viewer-display-spice.c
+++ b/src/virt-viewer-display-spice.c
@@ -25,7 +25,7 @@
 #include 
 
 #include 
-#include 
+#include 
 
 #include 
 
diff --git a/src/virt-viewer-display-spice.h b/src/virt-viewer-display-spice.h
index 3c30270..598a1b7 100644
--- a/src/virt-viewer-display-spice.h
+++ b/src/virt-viewer-display-spice.h
@@ -25,8 +25,7 @@
 #define _VIRT_VIEWER_DISPLAY_SPICE_H
 
 #include 
-#include 
-#include 
+#include 
 
 #include "virt-viewer-display.h"
 #include "virt-viewer-session-spice.h"
diff --git a/src/virt-viewer-session-spice.c b/src/virt-viewer-session-spice.c
index dc0c1ff..f7f2dc9 100644
--- a/src/virt-viewer-session-spice.c
+++ b/src/virt-viewer-session-spice.c
@@ -24,12 +24,9 @@
 
 #include 
 
-#include 
 #include 
 
-#include 
-#include 
-#include 
+#include 
 
 #include 
 #include "virt-viewer-file.h"
diff --git a/src/virt-viewer-session-spice.h b/src/virt-viewer-session-spice.h
index 95bdcdf..aa1f7e8 100644
--- a/src/virt-viewer-session-spice.h
+++ b/src/virt-viewer-session-spice.h
@@ -25,8 +25,7 @@
 #define _VIRT_VIEWER_SESSION_SPICE_H
 
 #include 
-#include 
-#include 
+#include 
 
 #include "virt-viewer-session.h"
 
-- 
2.5.0

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


[virt-tools-list] [PATCH 2/2] configure: cleanup {GLIB2, GTK}_CFLAGS

2016-03-15 Thread Eduardo Lima (Etrunko)
Also, remove unecessary AC_SUBST calls

Signed-off-by: Eduardo Lima (Etrunko) 
---
 configure.ac | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/configure.ac b/configure.ac
index 6d8475b..ee9dc73 100644
--- a/configure.ac
+++ b/configure.ac
@@ -102,10 +102,8 @@ GLIB_MKENUMS=`$PKG_CONFIG --variable=glib_mkenums glib-2.0`
 AC_SUBST(GLIB_MKENUMS)
 
 PKG_CHECK_MODULES(GLIB2, glib-2.0 >= $GLIB2_REQUIRED gio-2.0 gthread-2.0 
gmodule-export-2.0)
-GLIB2_CFLAGS="$GLIB2_CFLAGS -DGLIB_VERSION_MIN_REQUIRED=$GLIB2_ENCODED_VERSION"
-GLIB2_CFLAGS="$GLIB2_CFLAGS -DGLIB_VERSION_MAX_ALLOWED=$GLIB2_ENCODED_VERSION"
+GLIB2_CFLAGS="$GLIB2_CFLAGS -DGLIB_VERSION_MIN_REQUIRED=$GLIB2_ENCODED_VERSION 
-DGLIB_VERSION_MAX_ALLOWED=$GLIB2_ENCODED_VERSION"
 AC_SUBST(GLIB2_CFLAGS)
-AC_SUBST(GLIB2_LIBS)
 
 AC_ARG_VAR([GLIB_COMPILE_RESOURCES],[the glib-compile-resources program])
 AC_PATH_PROG([GLIB_COMPILE_RESOURCES],[glib-compile-resources],[])
@@ -137,10 +135,8 @@ AC_CHECK_LIB([virt],
 LIBS=$old_LIBS
 
 PKG_CHECK_MODULES(GTK, gtk+-3.0 >= $GTK_REQUIRED)
-GTK_CFLAGS="$GTK_CFLAGS -DGDK_VERSION_MIN_REQUIRED=$GTK_ENCODED_VERSION"
-GTK_CFLAGS="$GTK_CFLAGS -DGDK_VERSION_MAX_ALLOWED=$GTK_ENCODED_VERSION"
+GTK_CFLAGS="$GTK_CFLAGS -DGDK_VERSION_MIN_REQUIRED=$GTK_ENCODED_VERSION 
-DGDK_VERSION_MAX_ALLOWED=$GTK_ENCODED_VERSION"
 AC_SUBST(GTK_CFLAGS)
-AC_SUBST(GTK_LIBS)
 
 AC_ARG_WITH([gtk-vnc],
 AS_HELP_STRING([--without-gtk-vnc], [Ignore presence of gtk-vnc and 
disable it]))
-- 
2.5.0

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


Re: [virt-tools-list] [PATCH 2/2] Fix spice includes

2016-03-15 Thread Eduardo Lima (Etrunko)
On 03/15/2016 11:08 AM, Pavel Grunt wrote:
> On Tue, 2016-03-15 at 10:57 -0300, Eduardo Lima (Etrunko) wrote:
>> On 11/11/2015 09:09 AM, Eduardo Lima (Etrunko) wrote:
>>>
>>> On 11/11/2015 05:11 AM, Pavel Grunt wrote:
>>>>
>>>> Hi Eduardo,
>>>>
>>>> these warnings were introduced after spice-gtk v0.30 release,
>>>> virt-viewer
>>>> currently depends on spice-gtk v0.29.35. You can make the code
>>>> conditional or
>>>> wait for the spice-gtk v0.31 release.
>>>>
>>> Alright, I guess it is better to wait then.
>>>
>>> Regards. Eduardo.
>>>
>> I waited and it happened. Is it time to submit this patch again?
>>
> It is :)
> 
> Pavel
> 

Ok, I'll send a new patch rebased on top of latest master, as it
currently does not apply cleanly.

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


Re: [virt-tools-list] [PATCH 1/2] Fix spice includes

2016-03-22 Thread Eduardo Lima (Etrunko)
On 03/22/2016 07:57 AM, Pavel Grunt wrote:
> Hi,
> 
> you need to bump spice-gtk dependency to v0.31, because these headers
> are not in v0.30 and virt-viewer currently depends on v0.30.
> 

Oh, I missed that. Thanks for the note, I will send a new version with
the new dependency.

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


Re: [virt-tools-list] [PATCH 2/2] configure: cleanup {GLIB2, GTK}_CFLAGS

2016-03-22 Thread Eduardo Lima (Etrunko)
On 03/22/2016 08:02 AM, Pavel Grunt wrote:
> Hi,
> 
> imo with the change the line is too long
> 
> About the AC_SUBST calls - iirc they are defined automatically when
> pkg-config is >= 0.24 (not the case of rhel6).
> Or is it about something else (why just _LIBS and not _CFLAGS) ?
> 

About line being to long it is easily fixed by breaking the line. This
patch is only here because when I reviewed the original patch, it wal
already merged.

About calling AC_SUBST for _LIBS, only _CFLAGS varible is touched, so it
is only required for the latter. I will provide more details in the
commit message.

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


Re: [virt-tools-list] [PATCH 2/2] configure: cleanup {GLIB2, GTK}_CFLAGS

2016-03-22 Thread Eduardo Lima (Etrunko)
On 03/22/2016 10:42 AM, Eduardo Lima (Etrunko) wrote:
> On 03/22/2016 08:02 AM, Pavel Grunt wrote:
>> Hi,
>>
>> imo with the change the line is too long
>>
>> About the AC_SUBST calls - iirc they are defined automatically when
>> pkg-config is >= 0.24 (not the case of rhel6).
>> Or is it about something else (why just _LIBS and not _CFLAGS) ?
>>
> 
> About line being to long it is easily fixed by breaking the line. This
> patch is only here because when I reviewed the original patch, it wal
> already merged.
> 
> About calling AC_SUBST for _LIBS, only _CFLAGS varible is touched, so it
> is only required for the latter. I will provide more details in the
> commit message.
> 

Forgot to add that about RHEL6, it does not matter anyway, as
virt-viewer dropped Gtk+ 2.0 support recently.

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


[virt-tools-list] [PATCH v2 virt-viewer 1/2] Fix spice includes

2016-03-22 Thread Eduardo Lima (Etrunko)
Spice release version 0.31 requires that only spice-client.h or
spice-client-gtk.h should be included directly. As a result,
compilation is now throwing warnings like:

warning: #warning "Only  can be included directly" [-Wcpp]
warning: #warning "Only  can be included directly" [-Wcpp]

This patch also bumps spice version requirement to 0.31, to ensure
those files are available.

Signed-off-by: Eduardo Lima (Etrunko) 
---
 configure.ac| 2 +-
 src/virt-viewer-display-spice.c | 2 +-
 src/virt-viewer-display-spice.h | 3 +--
 src/virt-viewer-session-spice.c | 5 +
 src/virt-viewer-session-spice.h | 3 +--
 5 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/configure.ac b/configure.ac
index 6d8475b..115ad81 100644
--- a/configure.ac
+++ b/configure.ac
@@ -24,7 +24,7 @@ LIBXML2_REQUIRED="2.6.0"
 LIBVIRT_REQUIRED="0.10.0"
 LIBVIRT_GLIB_REQUIRED="0.1.8"
 GTK_VNC_REQUIRED="0.4.0"
-SPICE_GTK_REQUIRED="0.30"
+SPICE_GTK_REQUIRED="0.31"
 SPICE_PROTOCOL_REQUIRED="0.12.7"
 GOVIRT_REQUIRED="0.3.2"
 
diff --git a/src/virt-viewer-display-spice.c b/src/virt-viewer-display-spice.c
index 5114833..4abab2c 100644
--- a/src/virt-viewer-display-spice.c
+++ b/src/virt-viewer-display-spice.c
@@ -25,7 +25,7 @@
 #include 
 
 #include 
-#include 
+#include 
 
 #include 
 
diff --git a/src/virt-viewer-display-spice.h b/src/virt-viewer-display-spice.h
index 3c30270..598a1b7 100644
--- a/src/virt-viewer-display-spice.h
+++ b/src/virt-viewer-display-spice.h
@@ -25,8 +25,7 @@
 #define _VIRT_VIEWER_DISPLAY_SPICE_H
 
 #include 
-#include 
-#include 
+#include 
 
 #include "virt-viewer-display.h"
 #include "virt-viewer-session-spice.h"
diff --git a/src/virt-viewer-session-spice.c b/src/virt-viewer-session-spice.c
index dc0c1ff..f7f2dc9 100644
--- a/src/virt-viewer-session-spice.c
+++ b/src/virt-viewer-session-spice.c
@@ -24,12 +24,9 @@
 
 #include 
 
-#include 
 #include 
 
-#include 
-#include 
-#include 
+#include 
 
 #include 
 #include "virt-viewer-file.h"
diff --git a/src/virt-viewer-session-spice.h b/src/virt-viewer-session-spice.h
index 95bdcdf..aa1f7e8 100644
--- a/src/virt-viewer-session-spice.h
+++ b/src/virt-viewer-session-spice.h
@@ -25,8 +25,7 @@
 #define _VIRT_VIEWER_SESSION_SPICE_H
 
 #include 
-#include 
-#include 
+#include 
 
 #include "virt-viewer-session.h"
 
-- 
2.5.5

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


[virt-tools-list] [PATCH v2 virt-viewer 2/2] configure: cleanup {GLIB2, GTK}_CFLAGS

2016-03-22 Thread Eduardo Lima (Etrunko)
Also, remove unecessary AC_SUBST calls, as {GLIB2,GTK}_LIBS are never
touched.

Signed-off-by: Eduardo Lima (Etrunko) 
---
 configure.ac | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/configure.ac b/configure.ac
index 115ad81..4f7087e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -102,10 +102,9 @@ GLIB_MKENUMS=`$PKG_CONFIG --variable=glib_mkenums glib-2.0`
 AC_SUBST(GLIB_MKENUMS)
 
 PKG_CHECK_MODULES(GLIB2, glib-2.0 >= $GLIB2_REQUIRED gio-2.0 gthread-2.0 
gmodule-export-2.0)
-GLIB2_CFLAGS="$GLIB2_CFLAGS -DGLIB_VERSION_MIN_REQUIRED=$GLIB2_ENCODED_VERSION"
-GLIB2_CFLAGS="$GLIB2_CFLAGS -DGLIB_VERSION_MAX_ALLOWED=$GLIB2_ENCODED_VERSION"
+GLIB2_CFLAGS="$GLIB2_CFLAGS -DGLIB_VERSION_MIN_REQUIRED=$GLIB2_ENCODED_VERSION 
\
+-DGLIB_VERSION_MAX_ALLOWED=$GLIB2_ENCODED_VERSION"
 AC_SUBST(GLIB2_CFLAGS)
-AC_SUBST(GLIB2_LIBS)
 
 AC_ARG_VAR([GLIB_COMPILE_RESOURCES],[the glib-compile-resources program])
 AC_PATH_PROG([GLIB_COMPILE_RESOURCES],[glib-compile-resources],[])
@@ -137,10 +136,9 @@ AC_CHECK_LIB([virt],
 LIBS=$old_LIBS
 
 PKG_CHECK_MODULES(GTK, gtk+-3.0 >= $GTK_REQUIRED)
-GTK_CFLAGS="$GTK_CFLAGS -DGDK_VERSION_MIN_REQUIRED=$GTK_ENCODED_VERSION"
-GTK_CFLAGS="$GTK_CFLAGS -DGDK_VERSION_MAX_ALLOWED=$GTK_ENCODED_VERSION"
+GTK_CFLAGS="$GTK_CFLAGS -DGDK_VERSION_MIN_REQUIRED=$GTK_ENCODED_VERSION \
+-DGDK_VERSION_MAX_ALLOWED=$GTK_ENCODED_VERSION"
 AC_SUBST(GTK_CFLAGS)
-AC_SUBST(GTK_LIBS)
 
 AC_ARG_WITH([gtk-vnc],
 AS_HELP_STRING([--without-gtk-vnc], [Ignore presence of gtk-vnc and 
disable it]))
-- 
2.5.5

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


Re: [virt-tools-list] [virt-viewer 2/5] remote-viewer: Remove unneeded g_application_hold() call

2016-06-14 Thread Eduardo Lima (Etrunko)
On 06/14/2016 06:40 PM, Fabiano Fidêncio wrote:
> This call was added as hack during the GApplication's port, as the
> remote-viewer window was closed as soon as the connection happened.
> 
> This behaviour hapened because we didn't chain-up to the parent's
> window_added() method, which already calls g_application_hold()..
> 
> Signed-off-by: Fabiano Fidêncio 
> ---
>  src/virt-viewer-app.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/src/virt-viewer-app.c b/src/virt-viewer-app.c
> index b99ed32..4c3a373 100644
> --- a/src/virt-viewer-app.c
> +++ b/src/virt-viewer-app.c
> @@ -1825,8 +1825,6 @@ virt_viewer_app_on_application_startup(GApplication 
> *app)
>  g_application_quit(app);
>  return;
>  }
> -
> -g_application_hold(app);
>  }
>  
>  static gboolean
> 

Thanks for finding the real problem here. So, this one could be squashed
with previous one maybe?

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list

Re: [virt-tools-list] [virt-viewer 3/5] app: Set the resource base path

2016-06-14 Thread Eduardo Lima (Etrunko)

On 06/14/2016 06:40 PM, Fabiano Fidêncio wrote:
> GApplication's resource base path is based on the application-id, for
> instamce:

type (instance)

>  - virt-viewer's resource base path: /org/virt-manager/virt-viewer, as
>the virt-viewer's application id is: org.virt-manager.virt-viewer.
>  - remote-viewer's resource bash path: /org/virt-manager/remote-viewer
>as remote-viewer's application id is: org.virt-manager.remote-viewer
> 
> It's a issue because our resources have /org/virt-manager/virt-viewer
> and Gtk, when trying to automatically load ui files (as done for
> gtk/menu.ui, gtk/menus-appmenu.ui, gtk/menus-tradicional.ui and
> gtk/help-overlay.ui), searches for these files in the base path.
> 
> For solving this issue, we can basically set the resource path using

s/For solving/To solve

> g_application_set_resource_base_path() method. A check could be done and
> this method could be called only when running remote-viewer, but as it's
> a simple call, called only when the application starts I decided to go
> without the application-id's check.
> 
> g_application_set_resource_base_path() was introduced in GLib 2.42 and
> that's the reason I'm also bumping GLib dependency's version. Currently
> it makes impossible to build virt-viewer on SLES 12 SP1 as it still has
> GLib 2.38, so postponing this patch till SLES 12 SP2 release is
> desirable.
> 
> Signed-off-by: Fabiano Fidêncio 
> ---
>  configure.ac  | 4 ++--
>  src/virt-viewer-app.c | 2 ++
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index 9426350..f7e537d 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -13,8 +13,8 @@ m4_ifndef([AM_SILENT_RULES], 
> [m4_define([AM_SILENT_RULES],[])])
>  AM_SILENT_RULES([yes])
>  
>  # Keep these two definitions in agreement.
> -GLIB2_REQUIRED="2.38"
> -GLIB2_ENCODED_VERSION="GLIB_VERSION_2_38"
> +GLIB2_REQUIRED="2.42"
> +GLIB2_ENCODED_VERSION="GLIB_VERSION_2_42"
>  
>  # Keep these two definitions in agreement.
>  GTK_REQUIRED="3.10"
> diff --git a/src/virt-viewer-app.c b/src/virt-viewer-app.c
> index 4c3a373..d9f46ab 100644
> --- a/src/virt-viewer-app.c
> +++ b/src/virt-viewer-app.c
> @@ -1784,6 +1784,8 @@ virt_viewer_app_on_application_startup(GApplication 
> *app)
>  VirtViewerApp *self = VIRT_VIEWER_APP(app);
>  GError *error = NULL;
>  
> +g_application_set_resource_base_path(app, 
> "/org/virt-manager/virt-viewer");
> +
>  G_APPLICATION_CLASS(virt_viewer_app_parent_class)->startup(app);
>  
>  self->priv->resource = virt_viewer_get_resource();
> 

Other than that, patch is good. Does anyone know when the new SLES is due?

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list

Re: [virt-tools-list] [virt-viewer 0/5] Preparing the ground for UI changes

2016-06-16 Thread Eduardo Lima (Etrunko)
On 06/15/2016 07:20 PM, Jim Fehlig wrote:
> On 06/15/2016 06:42 AM, Pavel Grunt wrote:
>> Hi,
>>
>> On Tue, 2016-06-14 at 23:40 +0200, Fabiano Fidêncio wrote:
>>> These small series has as objective start "preparing the ground" for
>>> the work Sagar Huge has been done on re-design/re-write virt-viewer's
>>> UI in a way to have it looking as something from this decade. :-) [0]
>> I really like the new UI.
>>> There is still a lot of work to do on our (Sagar's and mine) side and
>>> once they are fixed he will send the patches and ask for comments and
>>> reviews. So far I'm keeping his patches rebased on top of current
>>> master on my personal gitlab[1] and you also can find his work on his
>>> personal's github[2].
>>>
>>> As I said, there's still a lot of work to do and we are trying to do it
>>> as soon as possible. Our target is to have it in for Fedora25 and maybe
>>> we will need a virt-viewer release before starting, actually, doing the
>>> UI changes (for those who want to stick to the old UI/not bump GLib/Gtk
>>> dependencies' versions).
>> F25 sounds like a reasonable goal
>>
>>> This small series of patch can also be found on another branch of my
>>> personal gitlab repo[3] and can be waiting there till we have SLES 12
>>> SP2 release, as the 3rd patch of this series will bump GLib dependency
>>> version, making impossible to build virt-viewer in a stock SLES 12 SP1.
> 
> That's a bummer, but...
> 
>> I don't think it is reasonable to wait for SLES to be released.
> 
> agree that upstream development shouldn't come to a halt due to an old version
> of glib in SLES12 SP1. For the record, here are glib versions in relevant SUSE
> distros
> 
> openSUSE 13.2 (becoming rather old openSUSE release): 2.42
> openSUSE Leap 42.1 (latest stable openSUSE release): 2.44
> openSUSE Tumbleweed (rolling openSUSE release): 2.48
> SLES12 SP1 (latest stable enterprise release): 2.38
> SLES12 SP2 (enterprise release scheduled for Oct 2016): 2.48
> 
>>  As you said a
>> release can be done before merging Sagar's and yours works upstream.
> 
> Ah, so there will be a release before these changes go in? And a release with
> the changes isn't planned until later in the year, e.g. October or later? If 
> so,
> the latest release will always work with the latest SUSE Linux Enterprise. 
> Only
> developers would be affected, but they should be using something newer anyhow.
> 

I think this is a good plan. Release first and then bump requirements
for the next one.

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list

Re: [virt-tools-list] [virt-viewer 1/2] window, util: Fix resource path for icons

2016-06-21 Thread Eduardo Lima (Etrunko)
On 06/21/2016 05:04 PM, Fabiano Fidêncio wrote:
> Since commit 1f6f1a48 the resource path for icons has been broken.
> The reason is that when moving the .ui files to $(srcdir)/resources/ui,
> the define used for the resources was changed to reflect the new
> directory. However, this change wasn't needed by the icons and ended up
> with virt-viewer not displaying a few icons.
> 
> Let's fix the issue by adding a new define for the icon's resource path.
> 
> Signed-off-by: Fabiano Fidêncio 
> ---
>  src/virt-viewer-util.h   | 1 +
>  src/virt-viewer-window.c | 4 ++--
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/src/virt-viewer-util.h b/src/virt-viewer-util.h
> index 14a477a..7b90052 100644
> --- a/src/virt-viewer-util.h
> +++ b/src/virt-viewer-util.h
> @@ -35,6 +35,7 @@ enum {
>  
>  #define VIRT_VIEWER_ERROR virt_viewer_error_quark ()
>  #define VIRT_VIEWER_RESOURCE_PREFIX "/org/virt-manager/virt-viewer/ui"
> +#define VIRT_VIEWER_RESOURCE_ICONS_PREFIX "/org/virt-manager/virt-viewer"
>  

I would prefer if you kept using VIRT_VIEWER_RESOUCE_PREFIX instead of
adding a new define. It could be "/org/virt-manager/virt-viewer" and
then you would adjust other occurrences accordingly (the only one i see
is in virt-viewer-util.c).

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list

Re: [virt-tools-list] [virt-viewer 2/2] window: Replace autoDrawer with native Gtk widgets

2016-06-21 Thread Eduardo Lima (Etrunko)
On 06/21/2016 05:04 PM, Fabiano Fidêncio wrote:
> GtkRevealer was intrudced in Gtk+ 3.10 and, combined with Gtk Overlay
> (intoduced in Gtk+ 3.2), can provide a more sustainably implementation
> of the AutoDrawer functionality.
> 
> This approach is completely based on the approach taken by virt-manager:
> https://github.com/virt-manager/virt-manager/commit/dc05600324f6b9a82b68581fc0a9c145f9889ce9
> 
> Resolves: https://bugs.freedesktop.org/show_bug.cgi?id=94495
> 
> Signed-off-by: Fabiano Fidêncio 
> ---
>  src/Makefile.am   |   8 +-
>  src/resources/ui/virt-viewer.ui   | 401 +++---
>  src/view/autoDrawer.c | 991 
> --
>  src/view/autoDrawer.h |  91 
>  src/view/drawer.c | 366 -
>  src/view/drawer.h |  83 ---
>  src/view/ovBox.c  | 946 
>  src/view/ovBox.h  | 103 
>  src/view/virt-viewer-timed-revealer.c | 224 
>  src/view/virt-viewer-timed-revealer.h |  74 +++
>  src/virt-viewer-window.c  |  36 +-
>  11 files changed, 521 insertions(+), 2802 deletions(-)

The joys of cleaning up code. I have some general questions before going
deep on the review.

1) Do you think the new files could go under src/ instead of src/view/
directory? Those seemed to be used only for the old widget.

2) Even better, it seems that timed-revealed has more boilerplate code
than code that really does something useful. Any specific reason why
those functions could not be part of virt-viewer-window.c ?

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list

Re: [virt-tools-list] [virt-viewer v2] util: Fix resource path

2016-06-21 Thread Eduardo Lima (Etrunko)
On 06/21/2016 05:28 PM, Fabiano Fidêncio wrote:
> Since commit 1f6f1a48 the resource path for icons has been broken.
> The reason is that when moving the .ui files to $(srcdir)/resources/ui
> the define used for the resources was changed to reflect the new
> directory. However, this change wasn't needed by the icons and ended
> up with virt-viewer not displaying a few icons.
> 
> Let's fix it by setting back the define to the previous one and then
> tweak the virt_viewer_util_load_ui() to add "ui" to the resource path,
> for loading the ui files.
> ---
>  src/virt-viewer-util.c | 3 ++-
>  src/virt-viewer-util.h | 2 +-
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/src/virt-viewer-util.c b/src/virt-viewer-util.c
> index 9e52b87..7c95583 100644
> --- a/src/virt-viewer-util.c
> +++ b/src/virt-viewer-util.c
> @@ -50,8 +50,9 @@ virt_viewer_error_quark(void)
>  GtkBuilder *virt_viewer_util_load_ui(const char *name)
>  {
>  GtkBuilder *builder;
> -gchar *resource = g_strdup_printf("%s/%s",
> +gchar *resource = g_strdup_printf("%s/%s/%s",

Minor:

"%s/ui/%s" ???

Acked-by: Eduardo Lima (Etrunko) 

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


Re: [virt-tools-list] [virt-viewer 2/2] window: Replace autoDrawer with native Gtk widgets

2016-06-21 Thread Eduardo Lima (Etrunko)
On 06/21/2016 05:04 PM, Fabiano Fidêncio wrote:

> diff --git a/src/view/virt-viewer-timed-revealer.c 
> b/src/view/virt-viewer-timed-revealer.c
> new file mode 100644
> index 000..bba363c
> --- /dev/null
> +++ b/src/view/virt-viewer-timed-revealer.c
> @@ -0,0 +1,224 @@
> +/*
> + * Virt Viewer: A virtual machine console viewer
> + *
> + * Copyright (c) 2016 Red Hat, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
> + *
> + * Author: Cole Robinson 
> + * Author: Fabiano Fidêncio 
> + */
> +
> +#include 
> +
> +#include "virt-viewer-timed-revealer.h"
> +
> +G_DEFINE_TYPE (VirtViewerTimedRevealer, virt_viewer_timed_revealer, 
> G_TYPE_OBJECT)
> +
> +#define VIRT_VIEWER_TIMED_REVEALER_GET_PRIVATE(obj) \
> +(G_TYPE_INSTANCE_GET_PRIVATE ((obj), VIRT_VIEWER_TYPE_TIMED_REVEALER, 
> VirtViewerTimedRevealerPrivate))
> +
> +struct _VirtViewerTimedRevealerPrivate
> +{
> +gboolean fullscreen;
> +guint timeout_id;
> +
> +GtkWidget *revealer;
> +GtkWidget *evBox;
> +};
> +
> +static void
> +virt_viewer_timed_revealer_unregister_timeout(VirtViewerTimedRevealer *self)
> +{
> +VirtViewerTimedRevealerPrivate *priv;
> +
> +priv = self->priv;
> +

I can see you don't like using self->priv->, so maybe you could just
initialize priv with the declaration on the line above?

> +if (priv->timeout_id) {
> +g_source_remove(priv->timeout_id);
> +priv->timeout_id = 0;
> +}
> +}
> +
> +static gboolean
> +schedule_unreveal_timeout_cb(VirtViewerTimedRevealer *self)
> +{
> +VirtViewerTimedRevealerPrivate *priv;
> +
> +priv = self->priv;
> +

Here too.

> +gtk_revealer_set_reveal_child(GTK_REVEALER(priv->revealer), FALSE);
> +priv->timeout_id = 0;
> +
> +return FALSE;
> +}
> +
> +static void
> +virt_viewer_timed_revealer_schedule_unreveal_timeout(VirtViewerTimedRevealer 
> *self,
> + guint timeout)
> +{
> +VirtViewerTimedRevealerPrivate *priv;
> +
> +priv = self->priv;
> +

And here.

> +if (priv->timeout_id != 0)
> +return;
> +
> +priv->timeout_id = g_timeout_add(timeout,
> + 
> (GSourceFunc)schedule_unreveal_timeout_cb,
> + self);
> +}
> +
> +static gboolean
> +virt_viewer_timed_revealer_enter_leave_notify(GtkWidget *evBox G_GNUC_UNUSED,
> +  GdkEventCrossing *event 
> G_GNUC_UNUSED,

event is marked G_GNC_UNUSED, but it is used on the function in a couple
of places.

> +  VirtViewerTimedRevealer *self)
> +{
> +VirtViewerTimedRevealerPrivate *priv;
> +GdkDevice *device;
> +GtkAllocation allocation;
> +gint x, y;
> +gboolean entered;
> +
> +priv = self->priv;

Same about initializing.

> +
> +device = gdk_event_get_device((GdkEvent *)event);
> +
> +gdk_window_get_device_position(event->window, device, &x, &y, 0);
> +gtk_widget_get_allocation(priv->evBox, &allocation);
> +
> +entered = !!(x >= 0 && y >= 0 && x < allocation.width && y < 
> allocation.height);
> +
> +if (!priv->fullscreen)
> +return FALSE;
> +

You should test for fullscreen the first thing in the function.

> +/*
> + * Pointer exited the toolbar, and toolbar is revealed. Schedule
> + * a timeout to close it, if one isn't already scheduled.
> + */
> +if (!entered && 
> gtk_revealer_get_reveal_child(GTK_REVEALER(priv->revealer))) {
> +virt_viewer_timed_revealer_schedule_unreveal_timeout(self, 1000);
> +return FALSE;
> +}
> +
> +virt_viewer_timed_revealer_unregister_timeout(self);
> +if (entered && 
> !gtk_revealer_get_reveal_child(GTK_REVEALER(priv->revealer))) {
> +gtk_revealer_set_reveal_child(GTK_REVEALER(priv->revealer), TRUE);
> +}
> +
> +return FALSE;
> +}
> +
> +static void
> +virt_viewer_timed_revealer_init(VirtViewerTimedRevealer *self)
> +{
> +self->priv = VIRT_VIEWER_TIMED_REVEALER_GET_PRIVATE(self);
> +}
> +
> +static void
> +virt_viewer_timed_revealer_dispose(GObject *object)
> +{
> +VirtViewerTimedRevealer *self;
> +VirtViewerTimedRevealerPrivate *priv;
> +
> +self = VIRT_VIEWER_TIMED_REVEALER(object);
> +priv = self->priv

Re: [virt-tools-list] [virt-viewer 2/2] window: Replace autoDrawer with native Gtk widgets

2016-06-21 Thread Eduardo Lima (Etrunko)
On 06/21/2016 05:50 PM, Fabiano Fidêncio wrote:
> On Tue, Jun 21, 2016 at 10:39 PM, Eduardo Lima (Etrunko)
>  wrote:
>> On 06/21/2016 05:04 PM, Fabiano Fidêncio wrote:
>>> GtkRevealer was intrudced in Gtk+ 3.10 and, combined with Gtk Overlay
>>> (intoduced in Gtk+ 3.2), can provide a more sustainably implementation
>>> of the AutoDrawer functionality.
>>>
>>> This approach is completely based on the approach taken by virt-manager:
>>> https://github.com/virt-manager/virt-manager/commit/dc05600324f6b9a82b68581fc0a9c145f9889ce9
>>>
>>> Resolves: https://bugs.freedesktop.org/show_bug.cgi?id=94495
>>>
>>> Signed-off-by: Fabiano Fidêncio 
>>> ---
>>>  src/Makefile.am   |   8 +-
>>>  src/resources/ui/virt-viewer.ui   | 401 +++---
>>>  src/view/autoDrawer.c | 991 
>>> --
>>>  src/view/autoDrawer.h |  91 
>>>  src/view/drawer.c | 366 -
>>>  src/view/drawer.h |  83 ---
>>>  src/view/ovBox.c  | 946 
>>> 
>>>  src/view/ovBox.h  | 103 
>>>  src/view/virt-viewer-timed-revealer.c | 224 
>>>  src/view/virt-viewer-timed-revealer.h |  74 +++
>>>  src/virt-viewer-window.c  |  36 +-
>>>  11 files changed, 521 insertions(+), 2802 deletions(-)
>>
>> The joys of cleaning up code. I have some general questions before going
>> deep on the review.
>>
>> 1) Do you think the new files could go under src/ instead of src/view/
>> directory? Those seemed to be used only for the old widget.
> 
> Well, I personally don't have a strong opinion about where those files
> are placed. Just left there because the files used for the old widget
> were there.

Yeah, if you want my suggestion, get rid of that directory. :P

> 
>>
>> 2) Even better, it seems that timed-revealed has more boilerplate code
>> than code that really does something useful. Any specific reason why
>> those functions could not be part of virt-viewer-window.c ?
> 
> Just because I preferred to keep the implementation details as close
> as possible to the one used in virt-manager. The functions could be
> moved to the virt-viewer-window.c, but I, particularly, fail to see a
> good reason for doing this. Actually, IMHO, it seems more organized
> the way it's now than moving the functions to the virt-viewer-window.c
> file.

Okay, I just read the code, I like it the way it is. I have posted a few
comments about it.

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list

Re: [virt-tools-list] [PATCH virt-viewer 4/4] Fix missing field initializers

2016-06-22 Thread Eduardo Lima (Etrunko)
This is not actually necessary as of C99. You only need to initialize
any field of a structure to get all other fields initialized too.

On 06/22/2016 03:17 AM, Pavel Grunt wrote:
> ---
>  src/virt-viewer-display-spice.c | 2 +-
>  src/virt-viewer-display-vnc.c   | 2 +-
>  src/virt-viewer-window.c| 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/virt-viewer-display-spice.c b/src/virt-viewer-display-spice.c
> index ee07507..a604230 100644
> --- a/src/virt-viewer-display-spice.c
> +++ b/src/virt-viewer-display-spice.c
> @@ -243,7 +243,7 @@ enable_accel_changed(VirtViewerApp *app,
>   GParamSpec *pspec G_GNUC_UNUSED,
>   VirtViewerDisplaySpice *self)
>  {
> -GtkAccelKey key = { 0 };
> +GtkAccelKey key = {0, 0, 0};
>  if (virt_viewer_app_get_enable_accel(app))
>  gtk_accel_map_lookup_entry("/view/release-cursor", 
> &key);
>  
> diff --git a/src/virt-viewer-display-vnc.c b/src/virt-viewer-display-vnc.c
> index 390c366..cb45c23 100644
> --- a/src/virt-viewer-display-vnc.c
> +++ b/src/virt-viewer-display-vnc.c
> @@ -190,7 +190,7 @@ enable_accel_changed(VirtViewerApp *app,
>   GParamSpec *pspec G_GNUC_UNUSED,
>   VncDisplay *vnc)
>  {
> -GtkAccelKey key = { 0 };
> +GtkAccelKey key = {0, 0, 0};
>  if (virt_viewer_app_get_enable_accel(app))
>  gtk_accel_map_lookup_entry("/view/release-cursor", 
> &key);
>  
> diff --git a/src/virt-viewer-window.c b/src/virt-viewer-window.c
> index 6bf0a2e..c828916 100644
> --- a/src/virt-viewer-window.c
> +++ b/src/virt-viewer-window.c
> @@ -1176,7 +1176,7 @@ virt_viewer_window_update_title(VirtViewerWindow *self)
>  
>  if (priv->grabbed) {
>  gchar *label;
> -GtkAccelKey key = { 0 };
> +GtkAccelKey key = {0, 0, 0};
>  
>  if (virt_viewer_app_get_enable_accel(priv->app))
>  gtk_accel_map_lookup_entry("/view/release-cursor", 
> &key);
> 


-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


[virt-tools-list] [PATCH 1/2] Avoid creating ovirt foreign menu item manually

2016-06-23 Thread Eduardo Lima (Etrunko)
This patch is a preparation for a upcoming UI change that will present
the ISO list in a separate dialog, instead of a submenu.

Signed-off-by: Eduardo Lima (Etrunko) 
---
 src/remote-viewer.c | 26 --
 src/resources/ui/virt-viewer.ui |  8 
 2 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/src/remote-viewer.c b/src/remote-viewer.c
index 71723cf..16c9d22 100644
--- a/src/remote-viewer.c
+++ b/src/remote-viewer.c
@@ -737,30 +737,28 @@ ovirt_foreign_menu_update(GtkApplication *gtkapp, 
GtkWindow *gtkwin, G_GNUC_UNUS
 RemoteViewer *app = REMOTE_VIEWER(gtkapp);
 VirtViewerWindow *win = g_object_get_data(G_OBJECT(gtkwin), 
"virt-viewer-window");
 GtkWidget *menu = g_object_get_data(G_OBJECT(win), "foreign-menu");
-GtkWidget *submenu;
-GtkMenuShell *shell = 
GTK_MENU_SHELL(gtk_builder_get_object(virt_viewer_window_get_builder(win), 
"top-menu"));
+GtkWidget *submenu = 
ovirt_foreign_menu_get_gtk_menu(app->priv->ovirt_foreign_menu);
 
 if (app->priv->ovirt_foreign_menu == NULL) {
 /* nothing to do */
 return;
 }
-if (menu == NULL) {
-menu = gtk_menu_item_new_with_label(_("_Change CD"));
-gtk_menu_item_set_use_underline(GTK_MENU_ITEM(menu), TRUE);
-gtk_menu_shell_append(shell, menu);
-g_object_set_data_full(G_OBJECT(win), "foreign-menu",
-   g_object_ref(menu),
-   (GDestroyNotify)gtk_widget_destroy);
-}
 
-submenu = ovirt_foreign_menu_get_gtk_menu(app->priv->ovirt_foreign_menu);
-if (submenu != NULL) {
-gtk_menu_item_set_submenu(GTK_MENU_ITEM(menu), submenu);
-} else {
+if (submenu == NULL) {
 /* No items to show, no point in showing the menu */
+if (menu != NULL)
+   gtk_widget_set_visible(menu, FALSE);
 g_object_set_data(G_OBJECT(win), "foreign-menu", NULL);
+return;
+}
+
+if (menu == NULL) {
+menu = 
GTK_WIDGET(gtk_builder_get_object(virt_viewer_window_get_builder(win), 
"menu-change-cd"));
+g_object_set_data(G_OBJECT(win), "foreign-menu", menu);
+gtk_widget_set_visible(menu, TRUE);
 }
 
+gtk_menu_item_set_submenu(GTK_MENU_ITEM(menu), submenu);
 gtk_widget_show_all(menu);
 }
 
diff --git a/src/resources/ui/virt-viewer.ui b/src/resources/ui/virt-viewer.ui
index 830a451..219e0af 100644
--- a/src/resources/ui/virt-viewer.ui
+++ b/src/resources/ui/virt-viewer.ui
@@ -243,6 +243,14 @@
 
   
 
+
+  
+False
+False
+_Change CD
+True
+  
+
   
   
 False
-- 
2.5.5

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


[virt-tools-list] [PATCH 2/2] Avoid unecessary debug message if returning NULL

2016-06-23 Thread Eduardo Lima (Etrunko)
Signed-off-by: Eduardo Lima (Etrunko) 
---
 src/ovirt-foreign-menu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c
index 9b552eb..959804d 100644
--- a/src/ovirt-foreign-menu.c
+++ b/src/ovirt-foreign-menu.c
@@ -465,10 +465,10 @@ GtkWidget 
*ovirt_foreign_menu_get_gtk_menu(OvirtForeignMenu *foreign_menu)
 GList *it;
 char *current_iso;
 
-g_debug("Creating GtkMenu for foreign menu");
 if (foreign_menu->priv->iso_names == NULL) {
 return NULL;
 }
+g_debug("Creating GtkMenu for foreign menu");
 current_iso = ovirt_foreign_menu_get_current_iso_name(foreign_menu);
 gtk_menu = gtk_menu_new();
 for (it = foreign_menu->priv->iso_names; it != NULL; it = it->next) {
-- 
2.5.5

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


Re: [virt-tools-list] [PATCH virt-viewer 4/4] Fix missing field initializers

2016-06-23 Thread Eduardo Lima (Etrunko)
On 06/22/2016 05:16 PM, Pavel Grunt wrote:
> On Wed, 2016-06-22 at 14:59 -0300, Eduardo Lima (Etrunko) wrote:
>> This is not actually necessary as of C99. You only need to initialize
>> any field of a structure to get all other fields initialized too.
> 
> yes, I was just annoyed by the warning..


Interesting, can you share what was exactly the warning message? Clang
as a modern compiler should be able to handle this case.


-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


Re: [virt-tools-list] [PATCH virt-viewer 4/4] Fix missing field initializers

2016-06-23 Thread Eduardo Lima (Etrunko)
On 06/23/2016 10:26 AM, Pavel Grunt wrote:
> On Thu, 2016-06-23 at 10:15 -0300, Eduardo Lima (Etrunko) wrote:
>> On 06/22/2016 05:16 PM, Pavel Grunt wrote:
>>> On Wed, 2016-06-22 at 14:59 -0300, Eduardo Lima (Etrunko) wrote:
>>>> This is not actually necessary as of C99. You only need to initialize
>>>> any field of a structure to get all other fields initialized too.
>>>
>>> yes, I was just annoyed by the warning..
>>
>>
>> Interesting, can you share what was exactly the warning message? Clang
>> as a modern compiler should be able to handle this case.
>>
> 
> virt-viewer-window.c:1164:31: warning: missing field 'accel_mods' initializer
>   [-Wmissing-field-initializers]
> GtkAccelKey key = { 0 };
> 
> maybe it is needed to pass something like "--std=c99" to the compiler
> 

I suspect it is because accel_mods is a enum (GdkModifierType) and the
compiler would not know a default value for that one.


-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


[virt-tools-list] [PATCH v2] Avoid creating ovirt foreign menu item manually

2016-06-23 Thread Eduardo Lima (Etrunko)
This patch is a preparation for a upcoming UI change that will present
the ISO list in a separate dialog, instead of a submenu.

Signed-off-by: Eduardo Lima (Etrunko) 
---
 src/remote-viewer.c | 23 +++
 src/resources/ui/virt-viewer.ui |  8 
 2 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/src/remote-viewer.c b/src/remote-viewer.c
index 71723cf..6d29bf2 100644
--- a/src/remote-viewer.c
+++ b/src/remote-viewer.c
@@ -738,29 +738,28 @@ ovirt_foreign_menu_update(GtkApplication *gtkapp, 
GtkWindow *gtkwin, G_GNUC_UNUS
 VirtViewerWindow *win = g_object_get_data(G_OBJECT(gtkwin), 
"virt-viewer-window");
 GtkWidget *menu = g_object_get_data(G_OBJECT(win), "foreign-menu");
 GtkWidget *submenu;
-GtkMenuShell *shell = 
GTK_MENU_SHELL(gtk_builder_get_object(virt_viewer_window_get_builder(win), 
"top-menu"));
 
 if (app->priv->ovirt_foreign_menu == NULL) {
 /* nothing to do */
 return;
 }
-if (menu == NULL) {
-menu = gtk_menu_item_new_with_label(_("_Change CD"));
-gtk_menu_item_set_use_underline(GTK_MENU_ITEM(menu), TRUE);
-gtk_menu_shell_append(shell, menu);
-g_object_set_data_full(G_OBJECT(win), "foreign-menu",
-   g_object_ref(menu),
-   (GDestroyNotify)gtk_widget_destroy);
-}
 
 submenu = ovirt_foreign_menu_get_gtk_menu(app->priv->ovirt_foreign_menu);
-if (submenu != NULL) {
-gtk_menu_item_set_submenu(GTK_MENU_ITEM(menu), submenu);
-} else {
+if (submenu == NULL) {
 /* No items to show, no point in showing the menu */
+if (menu != NULL)
+   gtk_widget_set_visible(menu, FALSE);
 g_object_set_data(G_OBJECT(win), "foreign-menu", NULL);
+return;
+}
+
+if (menu == NULL) {
+menu = 
GTK_WIDGET(gtk_builder_get_object(virt_viewer_window_get_builder(win), 
"menu-change-cd"));
+g_object_set_data(G_OBJECT(win), "foreign-menu", menu);
+gtk_widget_set_visible(menu, TRUE);
 }
 
+gtk_menu_item_set_submenu(GTK_MENU_ITEM(menu), submenu);
 gtk_widget_show_all(menu);
 }
 
diff --git a/src/resources/ui/virt-viewer.ui b/src/resources/ui/virt-viewer.ui
index 830a451..219e0af 100644
--- a/src/resources/ui/virt-viewer.ui
+++ b/src/resources/ui/virt-viewer.ui
@@ -243,6 +243,14 @@
 
   
 
+
+  
+False
+False
+_Change CD
+True
+  
+
   
   
 False
-- 
2.5.5

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


Re: [virt-tools-list] [virt-viewer] mingw, spec: Bump msitools version

2016-06-23 Thread Eduardo Lima (Etrunko)
On 06/22/2016 07:59 AM, Fabiano Fidêncio wrote:
> Fedora 24 has GLib 2.48.0, which brings a new dependency: PCRE.
> The new dependency is already added to the wxi file (in msitools) and a
> new msitools build including the fix is already done [0].
> 
> Let's just bump the version in our spec file and make sure we will be
> using the msitools which includes the fix.
> 
> [0]: https://bodhi.fedoraproject.org/updates/FEDORA-2016-a7a2db6109
> ---
>  mingw-virt-viewer.spec.in | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mingw-virt-viewer.spec.in b/mingw-virt-viewer.spec.in
> index 3109f5f..06f1992 100644
> --- a/mingw-virt-viewer.spec.in
> +++ b/mingw-virt-viewer.spec.in
> @@ -65,7 +65,7 @@ BuildRequires:  icoutils
>  BuildRequires:  dos2unix
>  BuildRequires:  hicolor-icon-theme
>  BuildRequires:  hwdata
> -BuildRequires:  msitools >= 0.95-4
> +BuildRequires:  msitools >= 0.95-5
>  
>  BuildArch:  noarch
>  
> 
Acked-by: Eduardo Lima (Etrunko) 


-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


Re: [virt-tools-list] [PATCH 2/2] Avoid unecessary debug message if returning NULL

2016-06-23 Thread Eduardo Lima (Etrunko)
On 06/23/2016 01:36 PM, Christophe Fergeau wrote:
> On Thu, Jun 23, 2016 at 10:13:25AM -0300, Eduardo Lima (Etrunko) wrote:
>> Signed-off-by: Eduardo Lima (Etrunko) 
>> ---
>>  src/ovirt-foreign-menu.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c
>> index 9b552eb..959804d 100644
>> --- a/src/ovirt-foreign-menu.c
>> +++ b/src/ovirt-foreign-menu.c
>> @@ -465,10 +465,10 @@ GtkWidget 
>> *ovirt_foreign_menu_get_gtk_menu(OvirtForeignMenu *foreign_menu)
>>  GList *it;
>>  char *current_iso;
>>  
>> -g_debug("Creating GtkMenu for foreign menu");
>>  if (foreign_menu->priv->iso_names == NULL) {
>>  return NULL;
>>  }
>> +g_debug("Creating GtkMenu for foreign menu");
>>  current_iso = ovirt_foreign_menu_get_current_iso_name(foreign_menu);
>>  gtk_menu = gtk_menu_new();
>>  for (it = foreign_menu->priv->iso_names; it != NULL; it = it->next) {
> 
> Fine with me. Actually it might be more useful to have a log when the
> Change CD menu does not show up because the iso name list is empty
> rather than having a log when something is going to be shown. So maybe
> add an additional g_debug() when NULL is returned?

Sure, does not hurt.

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


Re: [virt-tools-list] [PATCH v2] Avoid creating ovirt foreign menu item manually

2016-06-23 Thread Eduardo Lima (Etrunko)
On 06/23/2016 01:35 PM, Christophe Fergeau wrote:
> This looks fine to me, however I'd rephrase the shortlog (or add to the
> long log) that we get the Change CD menu from the UI file rather than
> manually creating it. Before looking at the commit, I was not sure what
> this was about.

Okay fixed.

> 
> Christophe
> 
> On Thu, Jun 23, 2016 at 11:23:11AM -0300, Eduardo Lima (Etrunko) wrote:
>> This patch is a preparation for a upcoming UI change that will present
>> the ISO list in a separate dialog, instead of a submenu.
>>
>> Signed-off-by: Eduardo Lima (Etrunko) 
>> ---
>>  src/remote-viewer.c | 23 +++
>>  src/resources/ui/virt-viewer.ui |  8 
>>  2 files changed, 19 insertions(+), 12 deletions(-)
>>
>> diff --git a/src/remote-viewer.c b/src/remote-viewer.c
>> index 71723cf..6d29bf2 100644
>> --- a/src/remote-viewer.c
>> +++ b/src/remote-viewer.c
>> @@ -738,29 +738,28 @@ ovirt_foreign_menu_update(GtkApplication *gtkapp, 
>> GtkWindow *gtkwin, G_GNUC_UNUS
>>  VirtViewerWindow *win = g_object_get_data(G_OBJECT(gtkwin), 
>> "virt-viewer-window");
>>  GtkWidget *menu = g_object_get_data(G_OBJECT(win), "foreign-menu");
>>  GtkWidget *submenu;
>> -GtkMenuShell *shell = 
>> GTK_MENU_SHELL(gtk_builder_get_object(virt_viewer_window_get_builder(win), 
>> "top-menu"));
>>  
>>  if (app->priv->ovirt_foreign_menu == NULL) {
>>  /* nothing to do */
>>  return;
>>  }
>> -if (menu == NULL) {
>> -menu = gtk_menu_item_new_with_label(_("_Change CD"));
>> -gtk_menu_item_set_use_underline(GTK_MENU_ITEM(menu), TRUE);
>> -gtk_menu_shell_append(shell, menu);
>> -g_object_set_data_full(G_OBJECT(win), "foreign-menu",
>> -   g_object_ref(menu),
>> -   (GDestroyNotify)gtk_widget_destroy);
>> -}
>>  
>>  submenu = 
>> ovirt_foreign_menu_get_gtk_menu(app->priv->ovirt_foreign_menu);
>> -if (submenu != NULL) {
>> -gtk_menu_item_set_submenu(GTK_MENU_ITEM(menu), submenu);
>> -} else {
>> +if (submenu == NULL) {
>>  /* No items to show, no point in showing the menu */
>> +if (menu != NULL)
>> +   gtk_widget_set_visible(menu, FALSE);
>>  g_object_set_data(G_OBJECT(win), "foreign-menu", NULL);
>> +return;
>> +}
>> +
>> +if (menu == NULL) {
>> +menu = 
>> GTK_WIDGET(gtk_builder_get_object(virt_viewer_window_get_builder(win), 
>> "menu-change-cd"));
>> +g_object_set_data(G_OBJECT(win), "foreign-menu", menu);
>> +gtk_widget_set_visible(menu, TRUE);
>>  }
>>  
>> +gtk_menu_item_set_submenu(GTK_MENU_ITEM(menu), submenu);
>>  gtk_widget_show_all(menu);
>>  }
>>  
>> diff --git a/src/resources/ui/virt-viewer.ui 
>> b/src/resources/ui/virt-viewer.ui
>> index 830a451..219e0af 100644
>> --- a/src/resources/ui/virt-viewer.ui
>> +++ b/src/resources/ui/virt-viewer.ui
>> @@ -243,6 +243,14 @@
>>  
>>
>>  
>> +
>> +  
>> +False
>> +False
>> +_Change 
>> CD
>> +True
>> +  
>> +
>>
>>
>>  False
>> -- 
>> 2.5.5
>>
>> ___
>> virt-tools-list mailing list
>> virt-tools-list@redhat.com
>> https://www.redhat.com/mailman/listinfo/virt-tools-list


-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


[virt-tools-list] [PATCH virt-viewer v2 0/2] Changes to ovirt-foreign menu

2016-06-23 Thread Eduardo Lima (Etrunko)
In this version:
 - Rebase to latest master
 - Better commit messages
 - Additional debug message if returning NULL in patch 2

Eduardo Lima (Etrunko) (2):
  Get ovirt foreign menu item from UI file
  Use more accurate debug messages for foreign menu

 src/ovirt-foreign-menu.c|  3 ++-
 src/remote-viewer.c | 23 +++
 src/resources/ui/virt-viewer.ui |  8 
 3 files changed, 21 insertions(+), 13 deletions(-)

-- 
2.5.5

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


[virt-tools-list] [PATCH virt-viewer v2 2/2] Use more accurate debug messages for foreign menu

2016-06-23 Thread Eduardo Lima (Etrunko)
Signed-off-by: Eduardo Lima (Etrunko) 
---
 src/ovirt-foreign-menu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c
index 9b552eb..2d286fb 100644
--- a/src/ovirt-foreign-menu.c
+++ b/src/ovirt-foreign-menu.c
@@ -465,10 +465,11 @@ GtkWidget 
*ovirt_foreign_menu_get_gtk_menu(OvirtForeignMenu *foreign_menu)
 GList *it;
 char *current_iso;
 
-g_debug("Creating GtkMenu for foreign menu");
 if (foreign_menu->priv->iso_names == NULL) {
+g_debug("ISO list is empty, no menu to show");
 return NULL;
 }
+g_debug("Creating GtkMenu for foreign menu");
 current_iso = ovirt_foreign_menu_get_current_iso_name(foreign_menu);
 gtk_menu = gtk_menu_new();
 for (it = foreign_menu->priv->iso_names; it != NULL; it = it->next) {
-- 
2.5.5

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


[virt-tools-list] [PATCH virt-viewer v2 1/2] Get ovirt foreign menu item from UI file

2016-06-23 Thread Eduardo Lima (Etrunko)
Currently the menu item is created manually, while this patch changes it to be
retrieved from the UI file, and decides if it needs to be shown or hidden if the
ISO list is received from ovirt.

This a preparation for a upcoming UI change that will present the ISO list in a
separate dialog, instead of a submenu.

Signed-off-by: Eduardo Lima (Etrunko) 
---
 src/remote-viewer.c | 23 +++
 src/resources/ui/virt-viewer.ui |  8 
 2 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/src/remote-viewer.c b/src/remote-viewer.c
index 71723cf..6d29bf2 100644
--- a/src/remote-viewer.c
+++ b/src/remote-viewer.c
@@ -738,29 +738,28 @@ ovirt_foreign_menu_update(GtkApplication *gtkapp, 
GtkWindow *gtkwin, G_GNUC_UNUS
 VirtViewerWindow *win = g_object_get_data(G_OBJECT(gtkwin), 
"virt-viewer-window");
 GtkWidget *menu = g_object_get_data(G_OBJECT(win), "foreign-menu");
 GtkWidget *submenu;
-GtkMenuShell *shell = 
GTK_MENU_SHELL(gtk_builder_get_object(virt_viewer_window_get_builder(win), 
"top-menu"));
 
 if (app->priv->ovirt_foreign_menu == NULL) {
 /* nothing to do */
 return;
 }
-if (menu == NULL) {
-menu = gtk_menu_item_new_with_label(_("_Change CD"));
-gtk_menu_item_set_use_underline(GTK_MENU_ITEM(menu), TRUE);
-gtk_menu_shell_append(shell, menu);
-g_object_set_data_full(G_OBJECT(win), "foreign-menu",
-   g_object_ref(menu),
-   (GDestroyNotify)gtk_widget_destroy);
-}
 
 submenu = ovirt_foreign_menu_get_gtk_menu(app->priv->ovirt_foreign_menu);
-if (submenu != NULL) {
-gtk_menu_item_set_submenu(GTK_MENU_ITEM(menu), submenu);
-} else {
+if (submenu == NULL) {
 /* No items to show, no point in showing the menu */
+if (menu != NULL)
+   gtk_widget_set_visible(menu, FALSE);
 g_object_set_data(G_OBJECT(win), "foreign-menu", NULL);
+return;
+}
+
+if (menu == NULL) {
+menu = 
GTK_WIDGET(gtk_builder_get_object(virt_viewer_window_get_builder(win), 
"menu-change-cd"));
+g_object_set_data(G_OBJECT(win), "foreign-menu", menu);
+gtk_widget_set_visible(menu, TRUE);
 }
 
+gtk_menu_item_set_submenu(GTK_MENU_ITEM(menu), submenu);
 gtk_widget_show_all(menu);
 }
 
diff --git a/src/resources/ui/virt-viewer.ui b/src/resources/ui/virt-viewer.ui
index 5f767d1..6e3c5ad 100644
--- a/src/resources/ui/virt-viewer.ui
+++ b/src/resources/ui/virt-viewer.ui
@@ -247,6 +247,14 @@
 
   
 
+
+  
+False
+False
+_Change 
CD
+True
+  
+
   
   
 False
-- 
2.5.5

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


Re: [virt-tools-list] [PATCH virt-viewer v2 0/2] Changes to ovirt-foreign menu

2016-06-24 Thread Eduardo Lima (Etrunko)
On 06/24/2016 04:16 AM, Christophe Fergeau wrote:
> Hey,
> 
> On Fri, Jun 24, 2016 at 02:44:07AM +0200, Fabiano Fidêncio wrote:
>> On Thu, Jun 23, 2016 at 7:15 PM, Eduardo Lima (Etrunko)
>>  wrote:
>>> In this version:
>>>  - Rebase to latest master
>>>  - Better commit messages
>>>  - Additional debug message if returning NULL in patch 2
>>>
>>> Eduardo Lima (Etrunko) (2):
>>>   Get ovirt foreign menu item from UI file
>>>   Use more accurate debug messages for foreign menu
>>>
>>>  src/ovirt-foreign-menu.c|  3 ++-
>>>  src/remote-viewer.c | 23 +++
>>>  src/resources/ui/virt-viewer.ui |  8 
>>>  3 files changed, 21 insertions(+), 13 deletions(-)
>>>
>>> --
>>> 2.5.5
>>>
>>> ___
>>> virt-tools-list mailing list
>>> virt-tools-list@redhat.com
>>> https://www.redhat.com/mailman/listinfo/virt-tools-list
>>
>> Both patches look good.
>> Acked-by: Fabiano Fidêncio 
>>
>> Please, wait till Christophe's Ack before pushing, as he did the first
>> round of review.
> 
> Same for me,
> 
> Acked-by: Christophe Fergeau 
> 


Thanks, pushed

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


[virt-tools-list] [PATCH] Get rid of deprecated functions to customize widget colors

2016-06-27 Thread Eduardo Lima (Etrunko)
Fixes https://bugs.freedesktop.org/show_bug.cgi?id=94276

Signed-off-by: Eduardo Lima (Etrunko) 
---

As a result of commit cc455b7f916110d7cfae6b7af753349e070c9494, setting
custom color for background does not work anymore on my Fedora 23
system. The status label still rendered with white color, but with the
background with default theme color (usually light grey), it has become
hard to read the text from the label.

I talked to Fabiano who told me that everything was working as expected
with his recently upgraded Fedora 24 system. While trying to track and
fix the issue, I noticed that it will only happen if the notebook tabs
are hidden. If tabs are shown, the background color will be properly
set.

I tracked down to Gtk+ some changes to GtkNotebook in recently released
version 3.20, which fixed the rendering of the custom background color,
with tabs hidden, but those could not be easily backported. Even though
it is a change of behavior in virt-viewer, I think it is really a minor
issue, and I decided to not spent too much time on this, so I put a
check for Gtk+ version to decide whether or not set the custom colors.

Some screenshots to illustrate:

Gtk+ > 3.20:
  http://imgur.com/gpuMukA

Gtk+ < 3.20:

  without this patch.: http://imgur.com/RdirSoX
  with this patch: http://imgur.com/9LJNeNI

---
 src/virt-viewer-notebook.c | 25 ++---
 src/virt-viewer-window.c   | 10 --
 2 files changed, 14 insertions(+), 21 deletions(-)

diff --git a/src/virt-viewer-notebook.c b/src/virt-viewer-notebook.c
index 420c914..f02779c 100644
--- a/src/virt-viewer-notebook.c
+++ b/src/virt-viewer-notebook.c
@@ -71,25 +71,28 @@ static void
 virt_viewer_notebook_init (VirtViewerNotebook *self)
 {
 VirtViewerNotebookPrivate *priv;
-GdkRGBA color;
 
 self->priv = GET_PRIVATE(self);
 priv = self->priv;
 
-priv->status = gtk_label_new("");
+/* Check for Gtk+ 3.20 to set the custom colors, because with older 
versions
+ * it the background color will not be set correctly, so we will end up 
with
+ * the default theme color for background (usually light grey), while the
+ * foreground text color is white.
+ */
+if (gtk_check_version(3,20,0) == NULL) {
+GtkStyleContext *style = 
gtk_widget_get_style_context(GTK_WIDGET(self));
+GtkCssProvider *css = gtk_css_provider_new();
+gtk_css_provider_load_from_data(css, "* { background-color: black; 
color: white; }", -1, NULL);
+gtk_style_context_add_provider(style, GTK_STYLE_PROVIDER(css), 
GTK_STYLE_PROVIDER_PRIORITY_APPLICATION);
+}
+
 gtk_notebook_set_show_tabs(GTK_NOTEBOOK(self), FALSE);
 gtk_notebook_set_show_border(GTK_NOTEBOOK(self), FALSE);
+
+priv->status = gtk_label_new("");
 gtk_widget_show_all(priv->status);
 gtk_notebook_append_page(GTK_NOTEBOOK(self), priv->status, NULL);
-gdk_rgba_parse(&color, "white");
-/* FIXME:
- * This method has been deprecated in 3.16.
- * For more details on how to deal with this in the future, please, see:
- * 
https://developer.gnome.org/gtk3/stable/GtkWidget.html#gtk-widget-override-color
- * For the bug report about this deprecated function, please, see:
- * https://bugs.freedesktop.org/show_bug.cgi?id=94276
- */
-gtk_widget_override_color(priv->status, GTK_STATE_FLAG_NORMAL, &color);
 }
 
 void
diff --git a/src/virt-viewer-window.c b/src/virt-viewer-window.c
index 1ebb423..c59fff5 100644
--- a/src/virt-viewer-window.c
+++ b/src/virt-viewer-window.c
@@ -297,7 +297,6 @@ virt_viewer_window_init (VirtViewerWindow *self)
 {
 VirtViewerWindowPrivate *priv;
 GtkWidget *vbox;
-GdkRGBA color;
 GSList *accels;
 
 self->priv = GET_PRIVATE(self);
@@ -340,15 +339,6 @@ virt_viewer_window_init (VirtViewerWindow *self)
 virt_viewer_window_toolbar_setup(self);
 
 gtk_box_pack_end(GTK_BOX(vbox), GTK_WIDGET(priv->notebook), TRUE, TRUE, 0);
-gdk_rgba_parse(&color, "black");
-/* FIXME:
- * This method has been deprecated in 3.16.
- * For more details on how to deal with this in the future, please, see:
- * 
https://developer.gnome.org/gtk3/stable/GtkWidget.html#gtk-widget-override-background-color
- * For the bug report about this deprecated function, please, see:
- * https://bugs.freedesktop.org/show_bug.cgi?id=94276
- */
-gtk_widget_override_background_color(GTK_WIDGET(priv->notebook), 
GTK_STATE_FLAG_NORMAL, &color);
 
 priv->window = GTK_WIDGET(gtk_builder_get_object(priv->builder, "viewer"));
 gtk_window_add_accel_group(GTK_WINDOW(priv->window), priv->accel_group);
-- 
2.5.5

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


Re: [virt-tools-list] [PATCH] Get rid of deprecated functions to customize widget colors

2016-06-28 Thread Eduardo Lima (Etrunko)
On 06/28/2016 07:21 AM, Fabiano Fidêncio wrote:
> On Tue, Jun 28, 2016 at 12:08 PM, Pavel Grunt  wrote:
>> Hi Eduardo,
>>
>> On Mon, 2016-06-27 at 18:00 -0300, Eduardo Lima (Etrunko) wrote:
>>> Fixes https://bugs.freedesktop.org/show_bug.cgi?id=94276
>>>
>>> Signed-off-by: Eduardo Lima (Etrunko) 
>>> ---
>>>
>>> As a result of commit cc455b7f916110d7cfae6b7af753349e070c9494, setting
>>> custom color for background does not work anymore on my Fedora 23
>>> system. The status label still rendered with white color, but with the
>>> background with default theme color (usually light grey), it has become
>>> hard to read the text from the label.
>>>
>>> I talked to Fabiano who told me that everything was working as expected
>>> with his recently upgraded Fedora 24 system. While trying to track and
>>> fix the issue, I noticed that it will only happen if the notebook tabs
>>> are hidden. If tabs are shown, the background color will be properly
>>> set.
>>>
>>> I tracked down to Gtk+ some changes to GtkNotebook in recently released
>>> version 3.20, which fixed the rendering of the custom background color,
>>> with tabs hidden, but those could not be easily backported. Even though
>>> it is a change of behavior in virt-viewer, I think it is really a minor
>>> issue, and I decided to not spent too much time on this, so I put a
>>> check for Gtk+ version to decide whether or not set the custom colors.
>>>
>>> Some screenshots to illustrate:
>>>
>>> Gtk+ > 3.20:
>>>   http://imgur.com/gpuMukA
>>>
>>> Gtk+ < 3.20:
>>>
>>>   without this patch.: http://imgur.com/RdirSoX
>>>   with this patch: http://imgur.com/9LJNeNI
>>
>> I would make it more simple, stick with the system theme
>> (ie http://imgur.com/9LJNeNI for all gtk versions) instead of introducing 
>> some
>> css just for 3.20 (is it stable btw ;-) ?). It would simplify the code, imho 
>> it
>> looks better and another gui tool from the family - virt-manager - uses it.
>>
>> What do you think ?
> 
> Hmm. This solution is quite okay for me.
> If the other people agree, I'd say to proceed with your suggestion then.
> 

Oh yes, I am all in favor of keeping the default theme colors.

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list

[virt-tools-list] [PATCH v2 virt-viewer] Get rid of deprecated functions to customize widget colors

2016-06-28 Thread Eduardo Lima (Etrunko)
Let's just stick with default theme colors.

Fixes https://bugs.freedesktop.org/show_bug.cgi?id=94276

Signed-off-by: Eduardo Lima (Etrunko) 
---
 src/virt-viewer-notebook.c | 13 ++---
 src/virt-viewer-window.c   | 10 --
 2 files changed, 2 insertions(+), 21 deletions(-)

diff --git a/src/virt-viewer-notebook.c b/src/virt-viewer-notebook.c
index 420c914..8bb9977 100644
--- a/src/virt-viewer-notebook.c
+++ b/src/virt-viewer-notebook.c
@@ -71,25 +71,16 @@ static void
 virt_viewer_notebook_init (VirtViewerNotebook *self)
 {
 VirtViewerNotebookPrivate *priv;
-GdkRGBA color;
 
 self->priv = GET_PRIVATE(self);
 priv = self->priv;
 
-priv->status = gtk_label_new("");
 gtk_notebook_set_show_tabs(GTK_NOTEBOOK(self), FALSE);
 gtk_notebook_set_show_border(GTK_NOTEBOOK(self), FALSE);
+
+priv->status = gtk_label_new("");
 gtk_widget_show_all(priv->status);
 gtk_notebook_append_page(GTK_NOTEBOOK(self), priv->status, NULL);
-gdk_rgba_parse(&color, "white");
-/* FIXME:
- * This method has been deprecated in 3.16.
- * For more details on how to deal with this in the future, please, see:
- * 
https://developer.gnome.org/gtk3/stable/GtkWidget.html#gtk-widget-override-color
- * For the bug report about this deprecated function, please, see:
- * https://bugs.freedesktop.org/show_bug.cgi?id=94276
- */
-gtk_widget_override_color(priv->status, GTK_STATE_FLAG_NORMAL, &color);
 }
 
 void
diff --git a/src/virt-viewer-window.c b/src/virt-viewer-window.c
index 1ebb423..c59fff5 100644
--- a/src/virt-viewer-window.c
+++ b/src/virt-viewer-window.c
@@ -297,7 +297,6 @@ virt_viewer_window_init (VirtViewerWindow *self)
 {
 VirtViewerWindowPrivate *priv;
 GtkWidget *vbox;
-GdkRGBA color;
 GSList *accels;
 
 self->priv = GET_PRIVATE(self);
@@ -340,15 +339,6 @@ virt_viewer_window_init (VirtViewerWindow *self)
 virt_viewer_window_toolbar_setup(self);
 
 gtk_box_pack_end(GTK_BOX(vbox), GTK_WIDGET(priv->notebook), TRUE, TRUE, 0);
-gdk_rgba_parse(&color, "black");
-/* FIXME:
- * This method has been deprecated in 3.16.
- * For more details on how to deal with this in the future, please, see:
- * 
https://developer.gnome.org/gtk3/stable/GtkWidget.html#gtk-widget-override-background-color
- * For the bug report about this deprecated function, please, see:
- * https://bugs.freedesktop.org/show_bug.cgi?id=94276
- */
-gtk_widget_override_background_color(GTK_WIDGET(priv->notebook), 
GTK_STATE_FLAG_NORMAL, &color);
 
 priv->window = GTK_WIDGET(gtk_builder_get_object(priv->builder, "viewer"));
 gtk_window_add_accel_group(GTK_WINDOW(priv->window), priv->accel_group);
-- 
2.5.5

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


Re: [virt-tools-list] [PATCH virt-viewer] window: Factor out common code for toolbar items

2016-06-28 Thread Eduardo Lima (Etrunko)
On 06/28/2016 12:16 PM, Fabiano Fidêncio wrote:
> On Tue, Jun 28, 2016 at 5:10 PM, Pavel Grunt  wrote:
>> Create toolbar widget in the loop
> 
> NACK from my side.
> There is any gain on re-factoring a code that will be removed as soon
> as we do the release.
> Actually, it just makes my life harder in order to rebase Sagar's
> patches on top of this change.

Agreed, only a small comment below.

> 
>> ---
>>  src/virt-viewer-window.c | 121 
>> ---
>>  1 file changed, 83 insertions(+), 38 deletions(-)
>>
>> diff --git a/src/virt-viewer-window.c b/src/virt-viewer-window.c
>> index 1ebb423..b276ae8 100644
>> --- a/src/virt-viewer-window.c
>> +++ b/src/virt-viewer-window.c
>> @@ -1062,56 +1062,101 @@ virt_viewer_window_menu_help_about(GtkWidget *menu 
>> G_GNUC_UNUSED,
>>  g_object_unref(G_OBJECT(about));
>>  }
>>
>> +typedef struct {
>> +GtkWidget *icon;
>> +const gchar *icon_name;
>> +const gchar *label;
>> +const gchar *tooltip;
>> +const gboolean sensitive;
>> +const gboolean show_label;
>> +const GCallback callback;
>> +} VirtViewerToolbarButton;
>> +
>> +static void
>> +virt_viewer_window_toolbar_add_button(VirtViewerWindow *self,
>> +  const VirtViewerToolbarButton 
>> *tb_button,
>> +  GtkWidget **dest_widget)
>> +{
>> +VirtViewerWindowPrivate *priv = self->priv;
>> +GtkToolItem *button = gtk_tool_button_new(tb_button->icon, 
>> tb_button->label);
>> +
>> +gtk_tool_button_set_icon_name(GTK_TOOL_BUTTON(button), 
>> tb_button->icon_name);
>> +gtk_tool_item_set_tooltip_text(button, tb_button->tooltip);
>> +gtk_tool_item_set_is_important(button, tb_button->show_label);
>> +gtk_widget_set_sensitive(GTK_WIDGET(button), tb_button->sensitive);
>> +gtk_widget_show_all(GTK_WIDGET(button));
>> +gtk_toolbar_insert(GTK_TOOLBAR(priv->toolbar), button, 0);
>> +g_signal_connect(button, "clicked", tb_button->callback, self);
>> +
>> +if (dest_widget != NULL)
>> +*dest_widget = GTK_WIDGET(button);
>> +}
>>
>>  static void
>>  virt_viewer_window_toolbar_setup(VirtViewerWindow *self)
>>  {
>> -GtkWidget *button;
>>  GtkWidget *overlay;
>>  VirtViewerWindowPrivate *priv = self->priv;
>> +guint i;
>> +
>> +const struct {
>> +const VirtViewerToolbarButton tb_button;
>> +GtkWidget **dest_widget;
>> +} toolbar_buttons[] = {
>> +{   /* Close connection */
>> +{
>> +NULL,
>> +"window-close",
>> +NULL,
>> +_("Disconnect"),
>> +TRUE,
>> +FALSE,
>> +G_CALLBACK(virt_viewer_window_menu_file_quit),
>> +},
>> +NULL,
>> +},{ /* USB Device selection */
>> +{
>> +
>> gtk_image_new_from_resource(VIRT_VIEWER_RESOURCE_PREFIX"/icons/24x24/virt-viewer-usb.png"),
>> +NULL,
>> +_("USB device selection"),
>> +_("USB device selection"),
>> +TRUE,
>> +FALSE,
>> +
>> G_CALLBACK(virt_viewer_window_menu_file_usb_device_selection),
>> +},
>> +&priv->toolbar_usb_device_selection,
>> +},{ /* Send key */
>> +{
>> +NULL,
>> +"preferences-desktop-keyboard-shortcuts",
>> +NULL,
>> +_("Send key combination"),
>> +FALSE,
>> +FALSE,
>> +G_CALLBACK(virt_viewer_window_toolbar_send_key),
>> +},
>> +&priv->toolbar_send_key,
>> +},{ /* Leave fullscreen */
>> +{
>> +NULL,
>> +"view-restore",
>> +_("Leave fullscreen"),
>> +_("Leave fullscreen"),
>> +TRUE,
>> +TRUE,
>> +G_CALLBACK(virt_viewer_window_toolbar_leave_fullscreen),
>> +},
>> +NULL,
>> +},
>> +};

In the case we did not have patches in the queue that makes this
obsolete, it would be a good idea to have explicit field initializers
for the items in the list. Besides being easier to read, you could save
some lines of code for the NULL and FALSE ones.

Regards, Eduardo.

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


Re: [virt-tools-list] [PATCH v2 virt-viewer] Get rid of deprecated functions to customize widget colors

2016-06-29 Thread Eduardo Lima (Etrunko)
On 06/29/2016 05:08 AM, Pavel Grunt wrote:
> Hi,
> 
> Imo the commit message should say that there are issues with the font /
> background colour since commit ... in some gtk versions - like you said in the
> comment for v1, because that is the reason for this patch. Getting rid of
> deprecated functions is a "side effect".
> 

Well, if you notice the previous email, the message was after the line
starting with '---' meaning it would be ignored. I wanted to explain why
I was doing it only for 3.20 and thought it would not matter in the
commit message. Anyway, I will add more information to the commit
message and push it.


-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


Re: [virt-tools-list] [PATCH] timed-revealer: listen to the "grab-notify" signal

2016-06-29 Thread Eduardo Lima (Etrunko)
On 06/29/2016 10:36 AM, Fabiano Fidêncio wrote:
> The "grab-notify" signal lets us know when our widget becomes
> shadowed by a Gtk+ grab on another widget, or when it becomes unshadowed
> due to a grab being removed.
> 
> That's exactly the case we face when dealing with "usb-redirection" and
> "close" items of the fullscreen toolbar. And, currently, when these
> widgets get closed the timed-revealer stays there forever. From now on
> let's take advantage of the "grab-notify" signal and schedule a timeout
> for the revealer when these widgets' windows get closed.
> 


Reading the docs about the 'grab-notify' signal, it indeed seems the
right thing to do (TM), but would be nice to have Christophe opinion
here too, as he was involved in reviewing the refactor of this widget.

Regards, Eduardo.

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list

[virt-tools-list] [PATCH] Adjust timer to refresh ovirt foreign menu

2016-06-30 Thread Eduardo Lima (Etrunko)
This is a temporary solution, as discussed in the bug. We will adjust
the timer to refresh the ISO list from 15 seconds to 5 minutes (300
seconds), while reworking in the UI to replace the menu with a dialog,
which seems a saner way to display the list.

Resolves: rhbz#1347726

Signed-off-by: Eduardo Lima (Etrunko) 
---
 src/ovirt-foreign-menu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c
index 2d286fb..33ff4f1 100644
--- a/src/ovirt-foreign-menu.c
+++ b/src/ovirt-foreign-menu.c
@@ -797,7 +797,7 @@ static void iso_list_fetched_cb(GObject *source_object,
 ovirt_foreign_menu_set_files(OVIRT_FOREIGN_MENU(user_data), files);
 g_list_free(files);
 
-g_timeout_add_seconds(15, ovirt_foreign_menu_refresh_iso_list, user_data);
+g_timeout_add_seconds(300, ovirt_foreign_menu_refresh_iso_list, user_data);
 }
 
 
-- 
2.5.5

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


Re: [virt-tools-list] [PATCH] Adjust timer to refresh ovirt foreign menu

2016-06-30 Thread Eduardo Lima (Etrunko)
On 06/30/2016 09:03 AM, Fabiano Fidêncio wrote:
> On Thu, Jun 30, 2016 at 2:01 PM, Eduardo Lima (Etrunko)
>  wrote:
>> This is a temporary solution, as discussed in the bug. We will adjust
>> the timer to refresh the ISO list from 15 seconds to 5 minutes (300
>> seconds), while reworking in the UI to replace the menu with a dialog,
>> which seems a saner way to display the list.
>>
>> Resolves: rhbz#1347726
>>
>> Signed-off-by: Eduardo Lima (Etrunko) 
>> ---
>>  src/ovirt-foreign-menu.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c
>> index 2d286fb..33ff4f1 100644
>> --- a/src/ovirt-foreign-menu.c
>> +++ b/src/ovirt-foreign-menu.c
>> @@ -797,7 +797,7 @@ static void iso_list_fetched_cb(GObject *source_object,
>>  ovirt_foreign_menu_set_files(OVIRT_FOREIGN_MENU(user_data), files);
>>  g_list_free(files);
>>
>> -g_timeout_add_seconds(15, ovirt_foreign_menu_refresh_iso_list, 
>> user_data);
>> +g_timeout_add_seconds(300, ovirt_foreign_menu_refresh_iso_list, 
>> user_data);
>>  }
>>
>>
>> --
>> 2.5.5
>>
>> ___
>> virt-tools-list mailing list
>> virt-tools-list@redhat.com
>> https://www.redhat.com/mailman/listinfo/virt-tools-list
> 
> 
> Acked-by: Fabiano Fidêncio 

Thanks, pushed.

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list

[virt-tools-list] [PATCH virt-viewer 00/11] Replace oVirt foreign menu with dedicated dialog

2016-07-17 Thread Eduardo Lima (Etrunko)
After analyzing the causes of the bug reported in rhbz #1310624, we came
to the conclusion that the whole idea of having a menu to display a list
remote files seemed wrong. A temporary solution has been provided until
the code was reworked, and this is the result.

Most of the changes is in ovirt-foreign menu, which does not handle any
UI widgets anymore, and some accessors were added so that other objects
can interface with it.

The dialog itself is pretty simple, and displays the list of files using
a GtkTreeView widget. It was designed using glade, which made the
development easier than programming the whole UI manually.

Eduardo Lima (Etrunko) (11):
  ovirt-foreign-menu: Rework states logic
  ovirt-foreign-menu: Use g_clear_pointer/g_clear_object
  ovirt-foreign-menu: Remove timer used to refresh iso list
  ovirt-foreign-menu: Add accessors for current iso and iso list
  ovirt-foreign-menu: Remove GtkMenu related functions
  ovirt-foreign-menu: Notify of new files even if list did not change
  UI: Make 'Change CD' menu item a submenu under 'File' toplevel menu
  Introduce ISO List dialog
  Run iso-dialog when 'Change CD' menu is activated
  remote-viewer: Make ovirt-foreign-menu a property
  iso-dialog: Implement functionality provided by oVirt foreign menu

 src/Makefile.am|   3 +
 src/ovirt-foreign-menu.c   | 290 +
 src/ovirt-foreign-menu.h   |   5 +-
 src/remote-viewer-iso-list-dialog.c| 274 +++
 src/remote-viewer-iso-list-dialog.h|  59 ++
 src/remote-viewer.c|  88 -
 src/resources/ui/remote-viewer-iso-list.ui | 176 +
 src/resources/ui/virt-viewer.ui|  58 +++---
 src/resources/virt-viewer.gresource.xml|   1 +
 src/virt-viewer-window.c   |  43 +
 10 files changed, 715 insertions(+), 282 deletions(-)
 create mode 100644 src/remote-viewer-iso-list-dialog.c
 create mode 100644 src/remote-viewer-iso-list-dialog.h
 create mode 100644 src/resources/ui/remote-viewer-iso-list.ui

-- 
2.7.4

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


[virt-tools-list] [PATCH virt-viewer 06/11] ovirt-foreign-menu: Notify of new files even if list did not change

2016-07-17 Thread Eduardo Lima (Etrunko)
Signed-off-by: Eduardo Lima (Etrunko) 
---
 src/ovirt-foreign-menu.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c
index 24b5af2..75154bb 100644
--- a/src/ovirt-foreign-menu.c
+++ b/src/ovirt-foreign-menu.c
@@ -428,12 +428,15 @@ static void ovirt_foreign_menu_set_files(OvirtForeignMenu 
*menu,
 
 if ((it == NULL) && (it2 == NULL)) {
 /* sorted_files and menu->priv->files content was the same */
+g_debug("Iso list unchanged");
 g_list_free_full(sorted_files, (GDestroyNotify)g_free);
-return;
+goto end;
 }
 
 g_list_free_full(menu->priv->iso_names, (GDestroyNotify)g_free);
 menu->priv->iso_names = sorted_files;
+
+end:
 g_object_notify(G_OBJECT(menu), "files");
 }
 
-- 
2.7.4

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


[virt-tools-list] [PATCH virt-viewer 04/11] ovirt-foreign-menu: Add accessors for current iso and iso list

2016-07-17 Thread Eduardo Lima (Etrunko)
Signed-off-by: Eduardo Lima (Etrunko) 
---
 src/ovirt-foreign-menu.c | 49 ++--
 src/ovirt-foreign-menu.h |  5 -
 2 files changed, 39 insertions(+), 15 deletions(-)

diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c
index b071e27..2446239 100644
--- a/src/ovirt-foreign-menu.c
+++ b/src/ovirt-foreign-menu.c
@@ -47,6 +47,7 @@ static void 
ovirt_foreign_menu_fetch_storage_domain_async(OvirtForeignMenu *menu
 static void ovirt_foreign_menu_fetch_vm_cdrom_async(OvirtForeignMenu *menu);
 static void ovirt_foreign_menu_refresh_cdrom_file_async(OvirtForeignMenu 
*menu);
 static void ovirt_foreign_menu_fetch_iso_list_async(OvirtForeignMenu *menu);
+static void updated_cdrom_cb(GObject *source_object, GAsyncResult *result, 
gpointer user_data);
 
 G_DEFINE_TYPE (OvirtForeignMenu, ovirt_foreign_menu, G_TYPE_OBJECT)
 
@@ -85,7 +86,7 @@ enum {
 };
 
 
-static char *
+char *
 ovirt_foreign_menu_get_current_iso_name(OvirtForeignMenu *foreign_menu)
 {
 char *name;
@@ -100,6 +101,36 @@ ovirt_foreign_menu_get_current_iso_name(OvirtForeignMenu 
*foreign_menu)
 }
 
 
+void
+ovirt_foreign_menu_set_current_iso_name(OvirtForeignMenu *foreign_menu, char 
*name)
+{
+g_return_if_fail(foreign_menu->priv->cdrom != NULL);
+g_return_if_fail(foreign_menu->priv->next_iso_name == NULL);
+
+if (name) {
+g_debug("Updating VM cdrom image to '%s'", name);
+foreign_menu->priv->next_iso_name = g_strdup(name);
+} else {
+g_debug("Removing current cdrom image");
+foreign_menu->priv->next_iso_name = NULL;
+}
+
+g_object_set(foreign_menu->priv->cdrom,
+ "file", name,
+ NULL);
+ovirt_cdrom_update_async(foreign_menu->priv->cdrom, TRUE,
+ foreign_menu->priv->proxy, NULL,
+ updated_cdrom_cb, foreign_menu);
+}
+
+
+GList*
+ovirt_foreign_menu_get_iso_names(OvirtForeignMenu *foreign_menu)
+{
+return foreign_menu->priv->iso_names;
+}
+
+
 static void
 ovirt_foreign_menu_get_property(GObject *object, guint property_id,
GValue *value, GParamSpec *pspec)
@@ -385,7 +416,7 @@ static void
 ovirt_foreign_menu_activate_item_cb(GtkMenuItem *menuitem, gpointer user_data)
 {
 OvirtForeignMenu *foreign_menu;
-const char *iso_name;
+const char *iso_name = NULL;
 gboolean checked;
 
 checked = gtk_check_menu_item_get_active(GTK_CHECK_MENU_ITEM(menuitem));
@@ -405,19 +436,9 @@ ovirt_foreign_menu_activate_item_cb(GtkMenuItem *menuitem, 
gpointer user_data)
 
 if (checked) {
 iso_name = gtk_menu_item_get_label(menuitem);
-g_debug("Updating VM cdrom image to '%s'", iso_name);
-foreign_menu->priv->next_iso_name = g_strdup(iso_name);
-} else {
-g_debug("Removing current cdrom image");
-iso_name = NULL;
-foreign_menu->priv->next_iso_name = NULL;
 }
-g_object_set(foreign_menu->priv->cdrom,
- "file", iso_name,
- NULL);
-ovirt_cdrom_update_async(foreign_menu->priv->cdrom, TRUE,
- foreign_menu->priv->proxy, NULL,
- updated_cdrom_cb, foreign_menu);
+
+ovirt_foreign_menu_set_current_iso_name(foreign_menu, iso_name);
 }
 
 
diff --git a/src/ovirt-foreign-menu.h b/src/ovirt-foreign-menu.h
index cf18b52..f1a1ddb 100644
--- a/src/ovirt-foreign-menu.h
+++ b/src/ovirt-foreign-menu.h
@@ -70,7 +70,10 @@ OvirtForeignMenu* ovirt_foreign_menu_new(OvirtProxy *proxy);
 OvirtForeignMenu *ovirt_foreign_menu_new_from_file(VirtViewerFile *self);
 void ovirt_foreign_menu_start(OvirtForeignMenu *menu);
 
-GtkWidget *ovirt_foreign_menu_get_gtk_menu(OvirtForeignMenu *foreign_menu);
+char *ovirt_foreign_menu_get_current_iso_name(OvirtForeignMenu *menu);
+void ovirt_foreign_menu_set_current_iso_name(OvirtForeignMenu *menu, char 
*name);
+
+GList *ovirt_foreign_menu_get_iso_names(OvirtForeignMenu *menu);
 
 G_END_DECLS
 
-- 
2.7.4

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


[virt-tools-list] [PATCH virt-viewer 08/11] Introduce ISO List dialog

2016-07-17 Thread Eduardo Lima (Etrunko)
Signed-off-by: Eduardo Lima (Etrunko) 
---
 src/Makefile.am|   3 +
 src/remote-viewer-iso-list-dialog.c| 115 +++
 src/remote-viewer-iso-list-dialog.h|  58 ++
 src/resources/ui/remote-viewer-iso-list.ui | 174 +
 src/resources/virt-viewer.gresource.xml|   1 +
 5 files changed, 351 insertions(+)
 create mode 100644 src/remote-viewer-iso-list-dialog.c
 create mode 100644 src/remote-viewer-iso-list-dialog.h
 create mode 100644 src/resources/ui/remote-viewer-iso-list.ui

diff --git a/src/Makefile.am b/src/Makefile.am
index 0c48e40..66a73f8 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -13,6 +13,7 @@ noinst_DATA = \
resources/ui/virt-viewer-vm-connection.ui \
resources/ui/virt-viewer-preferences.ui \
resources/ui/remote-viewer-connect.ui \
+   resources/ui/remote-viewer-iso-list.ui \
$(NULL)
 
 EXTRA_DIST =   \
@@ -96,6 +97,8 @@ if HAVE_OVIRT
 libvirt_viewer_la_SOURCES += \
ovirt-foreign-menu.h \
ovirt-foreign-menu.c \
+   remote-viewer-iso-list-dialog.c \
+   remote-viewer-iso-list-dialog.h \
$(NULL)
 endif
 
diff --git a/src/remote-viewer-iso-list-dialog.c 
b/src/remote-viewer-iso-list-dialog.c
new file mode 100644
index 000..b3972ac
--- /dev/null
+++ b/src/remote-viewer-iso-list-dialog.c
@@ -0,0 +1,115 @@
+/*
+ * Virt Viewer: A virtual machine console viewer
+ *
+ * Copyright (C) 2016 Red Hat, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#include 
+
+#include 
+
+#include "remote-viewer-iso-list-dialog.h"
+#include "virt-viewer-util.h"
+
+G_DEFINE_TYPE(RemoteViewerISOListDialog, remote_viewer_iso_list_dialog, 
GTK_TYPE_DIALOG)
+
+#define DIALOG_PRIVATE(o) \
+(G_TYPE_INSTANCE_GET_PRIVATE((o), REMOTE_VIEWER_TYPE_ISO_LIST_DIALOG, 
RemoteViewerISOListDialogPrivate))
+
+struct _RemoteViewerISOListDialogPrivate
+{
+GtkBuilder *builder;
+GtkWidget *notebook;
+};
+
+enum RemoteViewerIsoListDialogPages
+{
+STATUS_PAGE = 0,
+ISO_LIST_PAGE,
+};
+
+static void
+remote_viewer_iso_list_dialog_dispose(GObject *object)
+{
+RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(object);
+RemoteViewerISOListDialogPrivate *priv = self->priv;
+
+g_clear_object(&priv->builder);
+
+
G_OBJECT_CLASS(remote_viewer_iso_list_dialog_parent_class)->dispose(object);
+}
+
+static void
+remote_viewer_iso_list_dialog_class_init(RemoteViewerISOListDialogClass *klass)
+{
+GObjectClass *object_class = G_OBJECT_CLASS(klass);
+
+g_type_class_add_private(klass, sizeof(RemoteViewerISOListDialogPrivate));
+
+object_class->dispose = remote_viewer_iso_list_dialog_dispose;
+}
+
+static void
+remote_viewer_iso_list_dialog_response(GtkDialog *dialog,
+   gint response_id,
+   gpointer user_data G_GNUC_UNUSED)
+{
+RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(dialog);
+RemoteViewerISOListDialogPrivate *priv = self->priv;
+
+if (response_id != GTK_RESPONSE_NONE)
+return;
+
+gtk_notebook_set_current_page(GTK_NOTEBOOK(priv->notebook), STATUS_PAGE);
+gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, 
FALSE);
+gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_CLOSE, 
FALSE);
+}
+
+static void
+remote_viewer_iso_list_dialog_init(RemoteViewerISOListDialog *self)
+{
+GtkWidget *content = gtk_dialog_get_content_area(GTK_DIALOG(self));
+RemoteViewerISOListDialogPrivate *priv = self->priv = DIALOG_PRIVATE(self);
+
+gtk_widget_set_size_request(GTK_WIDGET(self), 400, 300);
+gtk_box_set_spacing(GTK_BOX(content), 6);
+
+priv->builder = virt_viewer_util_load_ui("remote-viewer-iso-list.ui");
+gtk_builder_connect_signals(priv->builder, self);
+
+priv->notebook = GTK_WIDGET(gtk_builder_get_object(priv->builder, 
"notebook"));
+gtk_box_pack_start(GTK_BOX(content), priv->notebook, TRUE, TRUE, 0);
+
+gtk_dialog_add_buttons(GTK_DIALOG(self),
+   _("Refresh"), GTK_RESPONSE_NONE,
+   _("Close&q

[virt-tools-list] [PATCH virt-viewer 02/11] ovirt-foreign-menu: Use g_clear_pointer/g_clear_object

2016-07-17 Thread Eduardo Lima (Etrunko)
Signed-off-by: Eduardo Lima (Etrunko) 
---
 src/ovirt-foreign-menu.c | 68 
 1 file changed, 16 insertions(+), 52 deletions(-)

diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c
index b0b8fec..889e7bc 100644
--- a/src/ovirt-foreign-menu.c
+++ b/src/ovirt-foreign-menu.c
@@ -143,33 +143,23 @@ ovirt_foreign_menu_set_property(GObject *object, guint 
property_id,
 
 switch (property_id) {
 case PROP_PROXY:
-if (priv->proxy != NULL) {
-g_object_unref(priv->proxy);
-}
+g_clear_object(&priv->proxy);
 priv->proxy = g_value_dup_object(value);
 break;
 case PROP_API:
-if (priv->api != NULL) {
-g_object_unref(priv->api);
-}
+g_clear_object(&priv->api);
 priv->api = g_value_dup_object(value);
 break;
 case PROP_VM:
-if (priv->vm != NULL) {
-g_object_unref(priv->vm);
-}
+g_clear_object(&priv->vm);
 priv->vm = g_value_dup_object(value);
-g_free(priv->vm_guid);
-priv->vm_guid = NULL;
+g_clear_pointer(&priv->vm_guid, g_free);
 if (priv->vm != NULL) {
 g_object_get(G_OBJECT(priv->vm), "guid", &priv->vm_guid, NULL);
 }
 break;
 case PROP_VM_GUID:
-if (priv->vm != NULL) {
-g_object_unref(priv->vm);
-priv->vm = NULL;
-}
+g_clear_object(&priv->vm);
 g_free(priv->vm_guid);
 priv->vm_guid = g_value_dup_string(value);
 break;
@@ -184,44 +174,20 @@ ovirt_foreign_menu_dispose(GObject *obj)
 {
 OvirtForeignMenu *self = OVIRT_FOREIGN_MENU(obj);
 
-if (self->priv->proxy) {
-g_object_unref(self->priv->proxy);
-self->priv->proxy = NULL;
-}
-
-if (self->priv->api != NULL) {
-g_object_unref(self->priv->api);
-self->priv->api = NULL;
-}
-
-if (self->priv->vm) {
-g_object_unref(self->priv->vm);
-self->priv->vm = NULL;
-}
-
-g_free(self->priv->vm_guid);
-self->priv->vm_guid = NULL;
-
-if (self->priv->files) {
-g_object_unref(self->priv->files);
-self->priv->files = NULL;
-}
-
-if (self->priv->cdrom) {
-g_object_unref(self->priv->cdrom);
-self->priv->cdrom = NULL;
-}
+g_clear_object(&self->priv->proxy);
+g_clear_object(&self->priv->api);
+g_clear_object(&self->priv->vm);
+g_clear_pointer(&self->priv->vm_guid, g_free);
+g_clear_object(&self->priv->files);
+g_clear_object(&self->priv->cdrom);
 
 if (self->priv->iso_names) {
 g_list_free_full(self->priv->iso_names, (GDestroyNotify)g_free);
 self->priv->iso_names = NULL;
 }
 
-g_free(self->priv->current_iso_name);
-self->priv->current_iso_name = NULL;
-
-g_free(self->priv->next_iso_name);
-self->priv->next_iso_name = NULL;
+g_clear_pointer(&self->priv->current_iso_name, g_free);
+g_clear_pointer(&self->priv->next_iso_name, g_free);
 
 G_OBJECT_CLASS(ovirt_foreign_menu_parent_class)->dispose(obj);
 }
@@ -410,8 +376,8 @@ static void updated_cdrom_cb(GObject *source_object,
 current_file?current_file:NULL);
 g_object_set(foreign_menu->priv->cdrom, "file", current_file, NULL);
 }
-g_free(foreign_menu->priv->next_iso_name);
-foreign_menu->priv->next_iso_name = NULL;
+
+g_clear_pointer(&foreign_menu->priv->next_iso_name, g_free);
 }
 
 
@@ -546,13 +512,11 @@ static void cdrom_file_refreshed_cb(GObject 
*source_object,
 }
 
 /* Content of OvirtCdrom is now current */
-g_free(menu->priv->current_iso_name);
+g_clear_pointer(&menu->priv->current_iso_name, g_free);
 if (menu->priv->cdrom != NULL) {
 g_object_get(G_OBJECT(menu->priv->cdrom),
  "file", &menu->priv->current_iso_name,
  NULL);
-} else {
-menu->priv->current_iso_name = NULL;
 }
 g_object_notify(G_OBJECT(menu), "file");
 if (menu->priv->cdrom != NULL) {
-- 
2.7.4

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


[virt-tools-list] [PATCH virt-viewer 01/11] ovirt-foreign-menu: Rework states logic

2016-07-17 Thread Eduardo Lima (Etrunko)
Use switch/case instead of lots of conditional blocks

Signed-off-by: Eduardo Lima (Etrunko) 
---
 src/ovirt-foreign-menu.c | 76 +++-
 1 file changed, 36 insertions(+), 40 deletions(-)

diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c
index 33ff4f1..b0b8fec 100644
--- a/src/ovirt-foreign-menu.c
+++ b/src/ovirt-foreign-menu.c
@@ -312,51 +312,47 @@ ovirt_foreign_menu_next_async_step(OvirtForeignMenu *menu,
 g_return_if_fail(current_state >= STATE_0);
 g_return_if_fail(current_state < STATE_ISOS);
 
-current_state++;
-
-if (current_state == STATE_API) {
-if (menu->priv->api == NULL) {
-ovirt_foreign_menu_fetch_api_async(menu);
-} else {
-current_state++;
+switch (++current_state) {
+case STATE_API: {
+if (menu->priv->api == NULL) {
+ovirt_foreign_menu_fetch_api_async(menu);
+break;
+}
 }
-}
-
-if (current_state == STATE_VM) {
-if (menu->priv->vm == NULL) {
-ovirt_foreign_menu_fetch_vm_async(menu);
-} else {
-current_state++;
+case STATE_VM: {
+if (menu->priv->vm == NULL) {
+ovirt_foreign_menu_fetch_vm_async(menu);
+break;
+}
 }
-}
-
-if (current_state == STATE_STORAGE_DOMAIN) {
-if (menu->priv->files == NULL) {
-ovirt_foreign_menu_fetch_storage_domain_async(menu);
-} else {
-current_state++;
+case STATE_STORAGE_DOMAIN: {
+if (menu->priv->files == NULL) {
+ovirt_foreign_menu_fetch_storage_domain_async(menu);
+break;
+}
 }
-}
-
-if (current_state == STATE_VM_CDROM) {
-if (menu->priv->cdrom == NULL) {
-ovirt_foreign_menu_fetch_vm_cdrom_async(menu);
-} else {
-current_state++;
+case STATE_VM_CDROM: {
+if (menu->priv->cdrom == NULL) {
+ovirt_foreign_menu_fetch_vm_cdrom_async(menu);
+break;
+}
 }
-}
-
-if (current_state == STATE_CDROM_FILE) {
-ovirt_foreign_menu_refresh_cdrom_file_async(menu);
-}
-
-if (current_state == STATE_ISOS) {
-g_warn_if_fail(menu->priv->api != NULL);
-g_warn_if_fail(menu->priv->vm != NULL);
-g_warn_if_fail(menu->priv->files != NULL);
-g_warn_if_fail(menu->priv->cdrom != NULL);
+case STATE_CDROM_FILE: {
+ovirt_foreign_menu_refresh_cdrom_file_async(menu);
+break;
+}
+case STATE_ISOS: {
+g_warn_if_fail(menu->priv->api != NULL);
+g_warn_if_fail(menu->priv->vm != NULL);
+g_warn_if_fail(menu->priv->files != NULL);
+g_warn_if_fail(menu->priv->cdrom != NULL);
 
-ovirt_foreign_menu_refresh_iso_list(menu);
+ovirt_foreign_menu_refresh_iso_list(menu);
+break;
+}
+default: {
+g_warn_if_reached();
+}
 }
 }
 
-- 
2.7.4

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


[virt-tools-list] [PATCH virt-viewer 10/11] remote-viewer: Make ovirt-foreign-menu a property

2016-07-17 Thread Eduardo Lima (Etrunko)
The OvirtForeignMenu pointer is needed by the new ISO list dialog, and
we make it acessible via property to avoid interdependency between
objects.

Signed-off-by: Eduardo Lima (Etrunko) 
---
 src/remote-viewer-iso-list-dialog.c | 20 +++-
 src/remote-viewer-iso-list-dialog.h |  3 ++-
 src/remote-viewer.c | 36 
 src/virt-viewer-window.c| 16 +++-
 4 files changed, 68 insertions(+), 7 deletions(-)

diff --git a/src/remote-viewer-iso-list-dialog.c 
b/src/remote-viewer-iso-list-dialog.c
index b3972ac..996c1f2 100644
--- a/src/remote-viewer-iso-list-dialog.c
+++ b/src/remote-viewer-iso-list-dialog.c
@@ -34,6 +34,7 @@ struct _RemoteViewerISOListDialogPrivate
 {
 GtkBuilder *builder;
 GtkWidget *notebook;
+OvirtForeignMenu *foreign_menu;
 };
 
 enum RemoteViewerIsoListDialogPages
@@ -106,10 +107,19 @@ 
remote_viewer_iso_list_dialog_init(RemoteViewerISOListDialog *self)
 }
 
 GtkWidget *
-remote_viewer_iso_list_dialog_new(GtkWindow *parent)
+remote_viewer_iso_list_dialog_new(GtkWindow *parent, OvirtForeignMenu 
*foreign_menu)
 {
-return g_object_new(REMOTE_VIEWER_TYPE_ISO_LIST_DIALOG,
-"title", _("Change CD"),
-"transient-for", parent,
-NULL);
+GtkWidget *dialog;
+RemoteViewerISOListDialog *self;
+
+g_return_val_if_fail(foreign_menu != NULL, NULL);
+
+dialog = g_object_new(REMOTE_VIEWER_TYPE_ISO_LIST_DIALOG,
+ "title", _("Change CD"),
+ "transient-for", parent,
+ NULL);
+
+self = REMOTE_VIEWER_ISO_LIST_DIALOG(dialog);
+self->priv->foreign_menu = foreign_menu;
+return dialog;
 }
diff --git a/src/remote-viewer-iso-list-dialog.h 
b/src/remote-viewer-iso-list-dialog.h
index def841b..ee7c70f 100644
--- a/src/remote-viewer-iso-list-dialog.h
+++ b/src/remote-viewer-iso-list-dialog.h
@@ -22,6 +22,7 @@
 #define __REMOTE_VIEWER_ISO_LIST_DIALOG_H__
 
 #include 
+#include "ovirt-foreign-menu.h"
 
 G_BEGIN_DECLS
 
@@ -51,7 +52,7 @@ struct _RemoteViewerISOListDialogClass
 
 GType remote_viewer_iso_list_dialog_get_type(void) G_GNUC_CONST;
 
-GtkWidget *remote_viewer_iso_list_dialog_new(GtkWindow *parent);
+GtkWidget *remote_viewer_iso_list_dialog_new(GtkWindow *parent, 
OvirtForeignMenu *foreign_menu);
 
 G_END_DECLS
 
diff --git a/src/remote-viewer.c b/src/remote-viewer.c
index 95130dc..e4e41c4 100644
--- a/src/remote-viewer.c
+++ b/src/remote-viewer.c
@@ -67,6 +67,13 @@ G_DEFINE_TYPE (RemoteViewer, remote_viewer, 
VIRT_VIEWER_TYPE_APP)
 #define GET_PRIVATE(o)\
 (G_TYPE_INSTANCE_GET_PRIVATE ((o), REMOTE_VIEWER_TYPE, 
RemoteViewerPrivate))
 
+enum RemoteViewerProperties {
+PROP_0,
+#ifdef HAVE_OVIRT
+PROP_OVIRT_FOREIGN_MENU,
+#endif
+};
+
 #ifdef HAVE_OVIRT
 static OvirtVm * choose_vm(GtkWindow *main_window,
char **vm_name,
@@ -213,6 +220,25 @@ end:
 }
 
 static void
+remote_viewer_get_property(GObject *object, guint property_id,
+   GValue *value, GParamSpec *pspec)
+{
+RemoteViewer *self = REMOTE_VIEWER(object);
+RemoteViewerPrivate *priv = self->priv;
+
+switch (property_id) {
+#ifdef HAVE_OVIRT
+case PROP_OVIRT_FOREIGN_MENU:
+g_value_set_pointer(value, priv->ovirt_foreign_menu);
+break;
+#endif
+
+default:
+G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec);
+}
+}
+
+static void
 remote_viewer_class_init (RemoteViewerClass *klass)
 {
 GObjectClass *object_class = G_OBJECT_CLASS (klass);
@@ -222,6 +248,7 @@ remote_viewer_class_init (RemoteViewerClass *klass)
 
 g_type_class_add_private (klass, sizeof (RemoteViewerPrivate));
 
+object_class->get_property = remote_viewer_get_property;
 object_class->dispose = remote_viewer_dispose;
 
 g_app_class->local_command_line = remote_viewer_local_command_line;
@@ -235,6 +262,15 @@ remote_viewer_class_init (RemoteViewerClass *klass)
 #else
 (void) gtk_app_class;
 #endif
+
+#ifdef HAVE_OVIRT
+g_object_class_install_property(object_class,
+PROP_OVIRT_FOREIGN_MENU,
+g_param_spec_pointer("ovirt-foreign-menu",
+ "oVirt Foreign Menu",
+ "Object which is used 
as interface to oVirt",
+ G_PARAM_READABLE | 
G_PARAM_STATIC_STRINGS));
+#endif
 }
 
 static void
diff --git a/src/virt-viewer-window.c b/src/virt-viewer-window.c
index 867fb86..76fe80f 100644
--- a/src/virt-viewer-window.c
+++ b/src/virt-viewer-window.c
@@ -1072,11 +10

[virt-tools-list] [PATCH virt-viewer 05/11] ovirt-foreign-menu: Remove GtkMenu related functions

2016-07-17 Thread Eduardo Lima (Etrunko)
With this commit, we finish cleaning up ovirt foreign menu code, which
only deals with data, leaving the UI bits to be handled properly in the
new ISO list dialog.

This patch also updates remote-viewer to reflect the change.

Signed-off-by: Eduardo Lima (Etrunko) 
---
 src/ovirt-foreign-menu.c | 79 
 src/remote-viewer.c  | 52 +--
 2 files changed, 8 insertions(+), 123 deletions(-)

diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c
index 2446239..24b5af2 100644
--- a/src/ovirt-foreign-menu.c
+++ b/src/ovirt-foreign-menu.c
@@ -361,22 +361,6 @@ ovirt_foreign_menu_start(OvirtForeignMenu *menu)
 }
 
 
-static void
-ovirt_foreign_menu_activate_item_cb(GtkMenuItem *menuitem, gpointer user_data);
-
-
-static void
-menu_item_set_active_no_signal(GtkMenuItem *menuitem,
-   gboolean active,
-   GCallback callback,
-   gpointer user_data)
-{
-g_signal_handlers_block_by_func(menuitem, callback, user_data);
-gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(menuitem), active);
-g_signal_handlers_unblock_by_func(menuitem, callback, user_data);
-}
-
-
 static void updated_cdrom_cb(GObject *source_object,
  GAsyncResult *result,
  gpointer user_data)
@@ -412,69 +396,6 @@ static void updated_cdrom_cb(GObject *source_object,
 }
 
 
-static void
-ovirt_foreign_menu_activate_item_cb(GtkMenuItem *menuitem, gpointer user_data)
-{
-OvirtForeignMenu *foreign_menu;
-const char *iso_name = NULL;
-gboolean checked;
-
-checked = gtk_check_menu_item_get_active(GTK_CHECK_MENU_ITEM(menuitem));
-foreign_menu = OVIRT_FOREIGN_MENU(user_data);
-g_return_if_fail(foreign_menu->priv->cdrom != NULL);
-g_return_if_fail(foreign_menu->priv->next_iso_name == NULL);
-
-g_debug("'%s' clicked", gtk_menu_item_get_label(menuitem));
-
-/* We only want to move the check mark for the currently selected ISO
- * when ovirt_cdrom_update_async() is successful, so for now we move
- * the check mark back to where it was before
- */
-menu_item_set_active_no_signal(menuitem, !checked,
-   
(GCallback)ovirt_foreign_menu_activate_item_cb,
-   foreign_menu);
-
-if (checked) {
-iso_name = gtk_menu_item_get_label(menuitem);
-}
-
-ovirt_foreign_menu_set_current_iso_name(foreign_menu, iso_name);
-}
-
-
-GtkWidget *ovirt_foreign_menu_get_gtk_menu(OvirtForeignMenu *foreign_menu)
-{
-GtkWidget *gtk_menu;
-GList *it;
-char *current_iso;
-
-if (foreign_menu->priv->iso_names == NULL) {
-g_debug("ISO list is empty, no menu to show");
-return NULL;
-}
-g_debug("Creating GtkMenu for foreign menu");
-current_iso = ovirt_foreign_menu_get_current_iso_name(foreign_menu);
-gtk_menu = gtk_menu_new();
-for (it = foreign_menu->priv->iso_names; it != NULL; it = it->next) {
-GtkWidget *menuitem;
-
-menuitem = gtk_check_menu_item_new_with_label((char *)it->data);
-if (g_strcmp0((char *)it->data, current_iso) == 0) {
-g_warn_if_fail(g_strcmp0(current_iso, 
foreign_menu->priv->current_iso_name) == 0);
-gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(menuitem),
-   TRUE);
-}
-g_signal_connect(menuitem, "activate",
- G_CALLBACK(ovirt_foreign_menu_activate_item_cb),
- foreign_menu);
-gtk_menu_shell_append(GTK_MENU_SHELL(gtk_menu), menuitem);
-}
-g_free(current_iso);
-
-return gtk_menu;
-}
-
-
 static void ovirt_foreign_menu_set_files(OvirtForeignMenu *menu,
  const GList *files)
 {
diff --git a/src/remote-viewer.c b/src/remote-viewer.c
index 6d29bf2..95130dc 100644
--- a/src/remote-viewer.c
+++ b/src/remote-viewer.c
@@ -734,33 +734,11 @@ authenticate_cb(RestProxy *proxy, G_GNUC_UNUSED 
RestProxyAuth *auth,
 static void
 ovirt_foreign_menu_update(GtkApplication *gtkapp, GtkWindow *gtkwin, 
G_GNUC_UNUSED gpointer data)
 {
-RemoteViewer *app = REMOTE_VIEWER(gtkapp);
+RemoteViewer *self = REMOTE_VIEWER(gtkapp);
 VirtViewerWindow *win = g_object_get_data(G_OBJECT(gtkwin), 
"virt-viewer-window");
-GtkWidget *menu = g_object_get_data(G_OBJECT(win), "foreign-menu");
-GtkWidget *submenu;
-
-if (app->priv->ovirt_foreign_menu == NULL) {
-/* nothing to do */
-return;
-}
-
-submenu = ovirt_foreign_menu_get_gtk_menu(app->priv->ovirt_foreign_menu);
-if (submenu == NULL) {
-/* No items to show, no point in showing the menu */
-if (menu != NULL)
-   gtk_widget_set_visible(menu,

[virt-tools-list] [PATCH virt-viewer 03/11] ovirt-foreign-menu: Remove timer used to refresh iso list

2016-07-17 Thread Eduardo Lima (Etrunko)
With the new ISO dialog, the user triggers the refresh manually.

Signed-off-by: Eduardo Lima (Etrunko) 
---
 src/ovirt-foreign-menu.c | 23 +++
 1 file changed, 3 insertions(+), 20 deletions(-)

diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c
index 889e7bc..b071e27 100644
--- a/src/ovirt-foreign-menu.c
+++ b/src/ovirt-foreign-menu.c
@@ -46,7 +46,7 @@ static void 
ovirt_foreign_menu_fetch_vm_async(OvirtForeignMenu *menu);
 static void ovirt_foreign_menu_fetch_storage_domain_async(OvirtForeignMenu 
*menu);
 static void ovirt_foreign_menu_fetch_vm_cdrom_async(OvirtForeignMenu *menu);
 static void ovirt_foreign_menu_refresh_cdrom_file_async(OvirtForeignMenu 
*menu);
-static gboolean ovirt_foreign_menu_refresh_iso_list(gpointer user_data);
+static void ovirt_foreign_menu_fetch_iso_list_async(OvirtForeignMenu *menu);
 
 G_DEFINE_TYPE (OvirtForeignMenu, ovirt_foreign_menu, G_TYPE_OBJECT)
 
@@ -313,7 +313,7 @@ ovirt_foreign_menu_next_async_step(OvirtForeignMenu *menu,
 g_warn_if_fail(menu->priv->files != NULL);
 g_warn_if_fail(menu->priv->cdrom != NULL);
 
-ovirt_foreign_menu_refresh_iso_list(menu);
+ovirt_foreign_menu_fetch_iso_list_async(menu);
 break;
 }
 default: {
@@ -756,8 +756,6 @@ static void iso_list_fetched_cb(GObject *source_object,
 files = 
g_hash_table_get_values(ovirt_collection_get_resources(collection));
 ovirt_foreign_menu_set_files(OVIRT_FOREIGN_MENU(user_data), files);
 g_list_free(files);
-
-g_timeout_add_seconds(300, ovirt_foreign_menu_refresh_iso_list, user_data);
 }
 
 
@@ -767,27 +765,12 @@ static void 
ovirt_foreign_menu_fetch_iso_list_async(OvirtForeignMenu *menu)
 return;
 }
 
+g_debug("Refreshing foreign menu iso list");
 ovirt_collection_fetch_async(menu->priv->files, menu->priv->proxy,
  NULL, iso_list_fetched_cb, menu);
 }
 
 
-static gboolean ovirt_foreign_menu_refresh_iso_list(gpointer user_data)
-{
-OvirtForeignMenu *menu;
-
-g_debug("Refreshing foreign menu iso list");
-menu = OVIRT_FOREIGN_MENU(user_data);
-ovirt_foreign_menu_fetch_iso_list_async(menu);
-
-/* ovirt_foreign_menu_fetch_iso_list_async() will schedule a new call to
- * that function through iso_list_fetched_cb() when it has finished
- * fetching the iso list
- */
-return G_SOURCE_REMOVE;
-}
-
-
 OvirtForeignMenu *ovirt_foreign_menu_new_from_file(VirtViewerFile *file)
 {
 OvirtProxy *proxy = NULL;
-- 
2.7.4

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


[virt-tools-list] [PATCH virt-viewer 09/11] Run iso-dialog when 'Change CD' menu is activated

2016-07-17 Thread Eduardo Lima (Etrunko)
Signed-off-by: Eduardo Lima (Etrunko) 
---
 src/resources/ui/virt-viewer.ui |  1 +
 src/virt-viewer-window.c| 29 +
 2 files changed, 30 insertions(+)

diff --git a/src/resources/ui/virt-viewer.ui b/src/resources/ui/virt-viewer.ui
index ee23ba5..e27ee77 100644
--- a/src/resources/ui/virt-viewer.ui
+++ b/src/resources/ui/virt-viewer.ui
@@ -73,6 +73,7 @@
 False
 _Change 
CD
 True
+
   
 
 
diff --git a/src/virt-viewer-window.c b/src/virt-viewer-window.c
index 7e6b93f..867fb86 100644
--- a/src/virt-viewer-window.c
+++ b/src/virt-viewer-window.c
@@ -43,6 +43,8 @@
 #include "virt-viewer-util.h"
 #include "virt-viewer-timed-revealer.h"
 
+#include "remote-viewer-iso-list-dialog.h"
+
 #define ZOOM_STEP 10
 
 /* Signal handlers for main window (move in a VirtViewerMainWindow?) */
@@ -62,6 +64,7 @@ void virt_viewer_window_menu_file_smartcard_insert(GtkWidget 
*menu, VirtViewerWi
 void virt_viewer_window_menu_file_smartcard_remove(GtkWidget *menu, 
VirtViewerWindow *self);
 void virt_viewer_window_menu_view_release_cursor(GtkWidget *menu, 
VirtViewerWindow *self);
 void virt_viewer_window_menu_preferences_cb(GtkWidget *menu, VirtViewerWindow 
*self);
+void virt_viewer_window_menu_change_cd_activate(GtkWidget *menu, 
VirtViewerWindow *self);
 
 
 /* Internal methods */
@@ -1052,6 +1055,32 @@ virt_viewer_window_menu_help_about(GtkWidget *menu 
G_GNUC_UNUSED,
 g_object_unref(G_OBJECT(about));
 }
 
+static void
+iso_dialog_response(GtkDialog *dialog,
+gint response_id,
+GtkWidget **dialog_ptr) {
+if (response_id == GTK_RESPONSE_NONE)
+return;
+
+gtk_widget_destroy(GTK_WIDGET(dialog));
+*dialog_ptr = NULL;
+}
+
+void
+virt_viewer_window_menu_change_cd_activate(GtkWidget *menu G_GNUC_UNUSED,
+   VirtViewerWindow *self)
+{
+VirtViewerWindowPrivate *priv = self->priv;
+static GtkWidget *dialog = NULL;
+
+if (dialog)
+return;
+
+dialog = remote_viewer_iso_list_dialog_new(GTK_WINDOW(priv->window));
+g_signal_connect(dialog, "response", G_CALLBACK(iso_dialog_response), 
&dialog);
+gtk_widget_show_all(dialog);
+gtk_dialog_run(GTK_DIALOG(dialog));
+}
 
 static void
 virt_viewer_window_toolbar_setup(VirtViewerWindow *self)
-- 
2.7.4

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


[virt-tools-list] [PATCH virt-viewer 07/11] UI: Make 'Change CD' menu item a submenu under 'File' toplevel menu

2016-07-17 Thread Eduardo Lima (Etrunko)
Signed-off-by: Eduardo Lima (Etrunko) 
---
 src/resources/ui/virt-viewer.ui | 57 +++--
 1 file changed, 21 insertions(+), 36 deletions(-)

diff --git a/src/resources/ui/virt-viewer.ui b/src/resources/ui/virt-viewer.ui
index 6e3c5ad..ee23ba5 100644
--- a/src/resources/ui/virt-viewer.ui
+++ b/src/resources/ui/virt-viewer.ui
@@ -1,6 +1,7 @@
 
+
 
-  
+  
   
   
 False
@@ -23,7 +24,6 @@
   
 True
 False
-False
 _File
 True
 
@@ -35,7 +35,6 @@
   
 True
 False
-False
 _Screenshot
 True
 
@@ -46,7 +45,6 @@
 True
 False
 False
-False
 _USB 
device selection
 True
 
@@ -55,7 +53,6 @@
 
   
 False
-False
 <virt-viewer>/file/smartcard-insert
 Smartcard insertion
 True
@@ -65,7 +62,6 @@
 
   
 False
-False
 <virt-viewer>/file/smartcard-remove
 Smartcard removal
 True
@@ -73,6 +69,13 @@
   
 
 
+  
+False
+_Change 
CD
+True
+  
+
+
   
 True
 False
@@ -89,13 +92,12 @@
 
 
   
-_Quit
 True
 False
-False
+_Quit
 True
-
 
+
   
 
   
@@ -106,7 +108,6 @@
   
 True
 False
-False
 _View
 True
 
@@ -118,7 +119,6 @@
   
 True
 False
-False
 <virt-viewer>/view/toggle-fullscreen
 _Full 
screen
 True
@@ -129,7 +129,6 @@
   
 True
 False
-False
 _Zoom
 True
 
@@ -139,22 +138,22 @@
 accelgroup
 
   
-<virt-viewer>/view/zoom-in
-Zoom _In
+False
 True
 False
-False
+<virt-viewer>/view/zoom-in
+Zoom _In
 True
 
   
 
 
   
-<virt-viewer>/view/zoom-out
-Zoom _Out
+False
 True
 False
-False
+<virt-viewer>/view/zoom-out
+Zoom _Out
 True
 
   
@@ -167,11 +166,11 @@
 
 
   
-<virt-viewer>/view/zoom-reset
-_Normal Size
+False

[virt-tools-list] [PATCH virt-viewer 11/11] iso-dialog: Implement functionality provided by oVirt foreign menu

2016-07-17 Thread Eduardo Lima (Etrunko)
Signed-off-by: Eduardo Lima (Etrunko) 
---
 src/remote-viewer-iso-list-dialog.c| 149 +
 src/resources/ui/remote-viewer-iso-list.ui |   4 +-
 2 files changed, 152 insertions(+), 1 deletion(-)

diff --git a/src/remote-viewer-iso-list-dialog.c 
b/src/remote-viewer-iso-list-dialog.c
index 996c1f2..3b2e187 100644
--- a/src/remote-viewer-iso-list-dialog.c
+++ b/src/remote-viewer-iso-list-dialog.c
@@ -20,6 +20,7 @@
 
 #include 
 
+#include 
 #include 
 
 #include "remote-viewer-iso-list-dialog.h"
@@ -34,7 +35,9 @@ struct _RemoteViewerISOListDialogPrivate
 {
 GtkBuilder *builder;
 GtkWidget *notebook;
+GtkWidget *treeview;
 OvirtForeignMenu *foreign_menu;
+GtkListStore *list_store;
 };
 
 enum RemoteViewerIsoListDialogPages
@@ -43,6 +46,15 @@ enum RemoteViewerIsoListDialogPages
 ISO_LIST_PAGE,
 };
 
+enum RemoteViewerIsoListDialogModel
+{
+ISO_IS_ACTIVE = 0,
+ISO_NAME,
+FONT_WEIGHT,
+};
+
+void remote_viewer_iso_list_dialog_toggled(GtkCellRendererToggle 
*cell_renderer, gchar *path, gpointer user_data);
+
 static void
 remote_viewer_iso_list_dialog_dispose(GObject *object)
 {
@@ -65,6 +77,52 @@ 
remote_viewer_iso_list_dialog_class_init(RemoteViewerISOListDialogClass *klass)
 }
 
 static void
+remote_viewer_iso_list_dialog_show_files(RemoteViewerISOListDialog *self)
+{
+RemoteViewerISOListDialogPrivate *priv = self->priv = DIALOG_PRIVATE(self);
+gtk_notebook_set_current_page(GTK_NOTEBOOK(priv->notebook), ISO_LIST_PAGE);
+gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, 
TRUE);
+gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_CLOSE, 
TRUE);
+}
+
+static void
+remote_viewer_iso_list_dialog_foreach(char *name, RemoteViewerISOListDialog 
*self)
+{
+RemoteViewerISOListDialogPrivate *priv = self->priv;
+char * current_iso = 
ovirt_foreign_menu_get_current_iso_name(self->priv->foreign_menu);
+gboolean active = g_strcmp0(current_iso, name) == 0;
+gint weight = active ? PANGO_WEIGHT_BOLD : PANGO_WEIGHT_NORMAL;
+GtkTreeIter iter;
+
+gtk_list_store_append(priv->list_store, &iter);
+gtk_list_store_set(priv->list_store, &iter,
+   ISO_IS_ACTIVE, active,
+   ISO_NAME, name,
+   FONT_WEIGHT, weight, -1);
+
+free(current_iso);
+}
+
+static void
+remote_viewer_iso_list_dialog_refresh_iso_list(RemoteViewerISOListDialog *self,
+   gboolean refreshing)
+{
+RemoteViewerISOListDialogPrivate *priv = self->priv;
+GList *iso_list;
+
+if (refreshing) {
+ovirt_foreign_menu_start(priv->foreign_menu);
+return;
+}
+
+iso_list = ovirt_foreign_menu_get_iso_names(priv->foreign_menu);
+
+gtk_list_store_clear(priv->list_store);
+g_list_foreach(iso_list, (GFunc) remote_viewer_iso_list_dialog_foreach, 
self);
+remote_viewer_iso_list_dialog_show_files(self);
+}
+
+static void
 remote_viewer_iso_list_dialog_response(GtkDialog *dialog,
gint response_id,
gpointer user_data G_GNUC_UNUSED)
@@ -78,6 +136,32 @@ remote_viewer_iso_list_dialog_response(GtkDialog *dialog,
 gtk_notebook_set_current_page(GTK_NOTEBOOK(priv->notebook), STATUS_PAGE);
 gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, 
FALSE);
 gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_CLOSE, 
FALSE);
+
+remote_viewer_iso_list_dialog_refresh_iso_list(self, TRUE);
+}
+
+void
+remote_viewer_iso_list_dialog_toggled(GtkCellRendererToggle *cell_renderer 
G_GNUC_UNUSED,
+  gchar *path G_GNUC_UNUSED,
+  gpointer user_data)
+{
+RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(user_data);
+RemoteViewerISOListDialogPrivate *priv = self->priv;
+GtkTreeModel *model = GTK_TREE_MODEL(priv->list_store);
+GtkTreeIter iter;
+gboolean active;
+gchar *name;
+
+gtk_tree_model_get(model, &iter,
+   ISO_IS_ACTIVE, &active,
+   ISO_NAME, &name, -1);
+
+gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, 
FALSE);
+gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_CLOSE, 
FALSE);
+gtk_widget_set_sensitive(priv->treeview, FALSE);
+
+ovirt_foreign_menu_set_current_iso_name(priv->foreign_menu, active ? NULL 
: name);
+g_free(name);
 }
 
 static void
@@ -85,6 +169,7 @@ remote_viewer_iso_list_dialog_init(RemoteViewerISOListDialog 
*self)
 {
 GtkWidget *content = gtk_dialog_get_content_area(GTK_DIALOG(self));
 RemoteViewerISOListDialogPrivate *priv = self->priv = DIALOG_PRIVATE(self);
+GtkCellRendererToggle *cell_renderer;
 
 gtk_widget_set_size_request(GTK_WIDGET(

Re: [virt-tools-list] [PATCH virt-viewer 03/11] ovirt-foreign-menu: Remove timer used to refresh iso list

2016-07-18 Thread Eduardo Lima (Etrunko)
On 07/18/2016 09:14 AM, Pavel Grunt wrote:
> On Sun, 2016-07-17 at 23:13 -0300, Eduardo Lima (Etrunko) wrote:
>> With the new ISO dialog, the user triggers the refresh manually.
> 
> This should come after the dialog is introduced> 

Well, I have no plans in merging any of these patches independently,
they are only split so that it makes more sense to understand the whole
series.

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


Re: [virt-tools-list] [PATCH virt-viewer 01/11] ovirt-foreign-menu: Rework states logic

2016-07-18 Thread Eduardo Lima (Etrunko)
On 07/18/2016 06:15 AM, Pavel Grunt wrote:
> Hi Eduardo,
> 
> On Sun, 2016-07-17 at 23:13 -0300, Eduardo Lima (Etrunko) wrote:
>> Use switch/case instead of lots of conditional blocks
> Yes, it is more readable
>>
>> Signed-off-by: Eduardo Lima (Etrunko) 
>> ---
>>  src/ovirt-foreign-menu.c | 76 +++--
>> ---
>>  1 file changed, 36 insertions(+), 40 deletions(-)
>>
>> diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c
>> index 33ff4f1..b0b8fec 100644
>> --- a/src/ovirt-foreign-menu.c
>> +++ b/src/ovirt-foreign-menu.c
>> @@ -312,51 +312,47 @@ ovirt_foreign_menu_next_async_step(OvirtForeignMenu
>> *menu,
>>  g_return_if_fail(current_state >= STATE_0);
>>  g_return_if_fail(current_state < STATE_ISOS);
>>  
>> -current_state++;
> my preference is to keep the increment outside the switch statement
> 
>> -
>> -if (current_state == STATE_API) {
>> -if (menu->priv->api == NULL) {
>> -ovirt_foreign_menu_fetch_api_async(menu);
>> -} else {
>> -current_state++;
>> +switch (++current_state) {
> Actually the increment is not needed at all thanks to your changes, imo
> switch(current_state + 1) would be more readable

Alright, I don't mind at all.

>> +case STATE_API: {
> 'case' should have the same indentation as its 'switch'
> 
> Remove extra {}, no need to have the null check in the extra block (applies to
> all cases)
> 

I don't think so, the if checks are necessary for the initialization
process, when we have everything initalized, it will fall straight to
the STATE_ISOS case. Or maybe you are talking about something else and I
misunderstood?


-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


[virt-tools-list] [PATCH virt-viewer v2 01/11] ovirt-foreign-menu: Rework states logic

2016-07-18 Thread Eduardo Lima (Etrunko)
Use switch/case instead of lots of conditional blocks

Signed-off-by: Eduardo Lima (Etrunko) 
---
 src/ovirt-foreign-menu.c | 41 +++--
 1 file changed, 15 insertions(+), 26 deletions(-)

diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c
index 33ff4f1..80e40a1 100644
--- a/src/ovirt-foreign-menu.c
+++ b/src/ovirt-foreign-menu.c
@@ -312,51 +312,40 @@ ovirt_foreign_menu_next_async_step(OvirtForeignMenu *menu,
 g_return_if_fail(current_state >= STATE_0);
 g_return_if_fail(current_state < STATE_ISOS);
 
-current_state++;
-
-if (current_state == STATE_API) {
+switch (++current_state) {
+case STATE_API:
 if (menu->priv->api == NULL) {
 ovirt_foreign_menu_fetch_api_async(menu);
-} else {
-current_state++;
+break;
 }
-}
-
-if (current_state == STATE_VM) {
+case STATE_VM:
 if (menu->priv->vm == NULL) {
 ovirt_foreign_menu_fetch_vm_async(menu);
-} else {
-current_state++;
+break;
 }
-}
-
-if (current_state == STATE_STORAGE_DOMAIN) {
+case STATE_STORAGE_DOMAIN:
 if (menu->priv->files == NULL) {
 ovirt_foreign_menu_fetch_storage_domain_async(menu);
-} else {
-current_state++;
+break;
 }
-}
-
-if (current_state == STATE_VM_CDROM) {
+case STATE_VM_CDROM:
 if (menu->priv->cdrom == NULL) {
 ovirt_foreign_menu_fetch_vm_cdrom_async(menu);
-} else {
-current_state++;
+break;
 }
-}
-
-if (current_state == STATE_CDROM_FILE) {
+case STATE_CDROM_FILE:
 ovirt_foreign_menu_refresh_cdrom_file_async(menu);
-}
-
-if (current_state == STATE_ISOS) {
+break;
+case STATE_ISOS:
 g_warn_if_fail(menu->priv->api != NULL);
 g_warn_if_fail(menu->priv->vm != NULL);
 g_warn_if_fail(menu->priv->files != NULL);
 g_warn_if_fail(menu->priv->cdrom != NULL);
 
 ovirt_foreign_menu_refresh_iso_list(menu);
+break;
+default:
+g_warn_if_reached();
 }
 }
 
-- 
2.7.4

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


Re: [virt-tools-list] [PATCH virt-viewer 01/11] ovirt-foreign-menu: Rework states logic

2016-07-18 Thread Eduardo Lima (Etrunko)
On 07/18/2016 11:58 AM, Pavel Grunt wrote:
> On Mon, 2016-07-18 at 10:22 -0300, Eduardo Lima (Etrunko) wrote:
>> On 07/18/2016 06:15 AM, Pavel Grunt wrote:
>>>
>>> Hi Eduardo,
>>>
>>> On Sun, 2016-07-17 at 23:13 -0300, Eduardo Lima (Etrunko) wrote:
>>>>
>>>> Use switch/case instead of lots of conditional blocks
>>> Yes, it is more readable
>>>>
>>>>
>>>> Signed-off-by: Eduardo Lima (Etrunko) 
>>>> ---
>>>>  src/ovirt-foreign-menu.c | 76 +++--
>>>> 
>>>> ---
>>>>  1 file changed, 36 insertions(+), 40 deletions(-)
>>>>
>>>> diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c
>>>> index 33ff4f1..b0b8fec 100644
>>>> --- a/src/ovirt-foreign-menu.c
>>>> +++ b/src/ovirt-foreign-menu.c
>>>> @@ -312,51 +312,47 @@ ovirt_foreign_menu_next_async_step(OvirtForeignMenu
>>>> *menu,
>>>>  g_return_if_fail(current_state >= STATE_0);
>>>>  g_return_if_fail(current_state < STATE_ISOS);
>>>>  
>>>> -current_state++;
>>> my preference is to keep the increment outside the switch statement
>>>
>>>>
>>>> -
>>>> -if (current_state == STATE_API) {
>>>> -if (menu->priv->api == NULL) {
>>>> -ovirt_foreign_menu_fetch_api_async(menu);
>>>> -} else {
>>>> -current_state++;
>>>> +switch (++current_state) {
>>> Actually the increment is not needed at all thanks to your changes, imo
>>> switch(current_state + 1) would be more readable
>>
>> Alright, I don't mind at all.
>>
>>>
>>>>
>>>> +case STATE_API: {
>>> 'case' should have the same indentation as its 'switch'
>>>
>>> Remove extra {}, no need to have the null check in the extra block (applies
>>> to
>>> all cases)
>>>
>>
>> I don't think so, the if checks are necessary for the initialization
>> process, when we have everything initalized, it will fall straight to
>> the STATE_ISOS case. Or maybe you are talking about something else and I
>> misunderstood?
>>
> 
> the {} are not needed, but it is a minor:

The break statements are put inside of the conditional block on purpose.
If the NULL check passes, the initialization is done asynchronously and
that function will be responsible for calling back with the next step.
If it fails, it means that it was already initialized, so we move to the
next initialization, one at a time, until everything is set up and it
will fall straight to the STATE_ISOS.

> ...
> case STATE_API:
> if (menu->priv->api == NULL) {
> ovirt_foreign_menu_fetch_api_async(menu);
> break;
> }
> case STATE_VM:
> if (menu->priv->vm == NULL) {
> ovirt_foreign_menu_fetch_vm_async(menu);
> break;
> }
> ...
> 
>>


-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


Re: [virt-tools-list] [PATCH virt-viewer 07/11] UI: Make 'Change CD' menu item a submenu under 'File' toplevel menu

2016-07-19 Thread Eduardo Lima (Etrunko)
On 07/19/2016 12:46 PM, Christophe Fergeau wrote:
> Your glade seems to have added unrelated changes to that commit. If
> newer versions of glade are always going to make these changes, we can
> commit them separately.
> 

Dunno why all those changes got in, most likely because of different
versions of glade. I will remove the unrelated changes and keep only
what the commit is meant to do.

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


Re: [virt-tools-list] [PATCH virt-viewer 03/11] ovirt-foreign-menu: Remove timer used to refresh iso list

2016-07-19 Thread Eduardo Lima (Etrunko)
On 07/19/2016 12:11 PM, Christophe Fergeau wrote:
> On Mon, Jul 18, 2016 at 10:18:14AM -0300, Eduardo Lima (Etrunko) wrote:
>> On 07/18/2016 09:14 AM, Pavel Grunt wrote:
>>> On Sun, 2016-07-17 at 23:13 -0300, Eduardo Lima (Etrunko) wrote:
>>>> With the new ISO dialog, the user triggers the refresh manually.
>>>
>>> This should come after the dialog is introduced> 
>>
>> Well, I have no plans in merging any of these patches independently,
>> they are only split so that it makes more sense to understand the whole
>> series.
> 
> I agree with Pavel, the series makes more sense if it's removed _after_
> the dialog is there. But if it's a conflict mess to move it later, let's
> keep it here.
> 

I will try reordering the commits this way. My initial idea was to merge
the series as a whole with --no-ff.

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com



signature.asc
Description: OpenPGP digital signature
___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list

Re: [virt-tools-list] [PATCH virt-viewer 05/11] ovirt-foreign-menu: Remove GtkMenu related functions

2016-07-19 Thread Eduardo Lima (Etrunko)
On 07/19/2016 12:43 PM, Christophe Fergeau wrote:
> On Sun, Jul 17, 2016 at 11:13:05PM -0300, Eduardo Lima (Etrunko) wrote:
>> With this commit, we finish cleaning up ovirt foreign menu code, which
>> only deals with data, leaving the UI bits to be handled properly in the
>> new ISO list dialog.
> 
> I'm not clear exactly what this is doing from the commit log. This
> removes the old toplevel "change cd" menu, replaces it with a
> File/Change CD menu entry, which is going to be not doing anything for
> now?
> 

Yes, that's it. It is strange, but I avoided removing old code and
replacing with lots of new code only to keep the functionality working.
Same happens with the dialog code, which is introduced with no
functionality at all, and only plugged into oVirt foreign menu later on.

>> -ovirt_foreign_menu_start(foreign_menu);
>> -}
>>  
>> +ovirt_foreign_menu_updated(self);
> 
> Why do we use _updated() instead of _start()?
> 

_updated() will only make the menu option visible, while I moved the
call to _start() when dialog is shown. This whole naming seems wrong
btw, because those functions are static to remote-viewer.c and not
present in ovirt-foreign-menu.c.

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


Re: [virt-tools-list] [PATCH virt-viewer 06/11] ovirt-foreign-menu: Notify of new files even if list did not change

2016-07-19 Thread Eduardo Lima (Etrunko)
On 07/19/2016 12:43 PM, Christophe Fergeau wrote:
> Why?
> 

Because with the dialog, every time the Refresh button is pressed, we
clear the ISO list and it will only be populated again when it receives
the notify::files signal.

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


Re: [virt-tools-list] [PATCH virt-viewer 08/11] Introduce ISO List dialog

2016-07-19 Thread Eduardo Lima (Etrunko)
On 07/19/2016 01:13 PM, Christophe Fergeau wrote:
> On Sun, Jul 17, 2016 at 11:13:08PM -0300, Eduardo Lima (Etrunko) wrote:
>> Signed-off-by: Eduardo Lima (Etrunko) 
>> ---
>>  src/Makefile.am|   3 +
>>  src/remote-viewer-iso-list-dialog.c| 115 +++
>>  src/remote-viewer-iso-list-dialog.h|  58 ++
>>  src/resources/ui/remote-viewer-iso-list.ui | 174 
>> +
>>  src/resources/virt-viewer.gresource.xml|   1 +
>>  5 files changed, 351 insertions(+)
>>  create mode 100644 src/remote-viewer-iso-list-dialog.c
>>  create mode 100644 src/remote-viewer-iso-list-dialog.h
>>  create mode 100644 src/resources/ui/remote-viewer-iso-list.ui
>>
>> diff --git a/src/Makefile.am b/src/Makefile.am
>> index 0c48e40..66a73f8 100644
>> --- a/src/Makefile.am
>> +++ b/src/Makefile.am
>> @@ -13,6 +13,7 @@ noinst_DATA = \
>>  resources/ui/virt-viewer-vm-connection.ui \
>>  resources/ui/virt-viewer-preferences.ui \
>>  resources/ui/remote-viewer-connect.ui \
>> +resources/ui/remote-viewer-iso-list.ui \
>>  $(NULL)
>>  
>>  EXTRA_DIST =\
>> @@ -96,6 +97,8 @@ if HAVE_OVIRT
>>  libvirt_viewer_la_SOURCES += \
>>  ovirt-foreign-menu.h \
>>  ovirt-foreign-menu.c \
>> +remote-viewer-iso-list-dialog.c \
>> +remote-viewer-iso-list-dialog.h \
>>  $(NULL)
>>  endif
>>  
>> diff --git a/src/remote-viewer-iso-list-dialog.c 
>> b/src/remote-viewer-iso-list-dialog.c
>> new file mode 100644
>> index 000..b3972ac
>> --- /dev/null
>> +++ b/src/remote-viewer-iso-list-dialog.c
>> @@ -0,0 +1,115 @@
>> +/*
>> + * Virt Viewer: A virtual machine console viewer
>> + *
>> + * Copyright (C) 2016 Red Hat, Inc.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
>> + */
>> +
>> +#include 
>> +
>> +#include 
>> +
>> +#include "remote-viewer-iso-list-dialog.h"
>> +#include "virt-viewer-util.h"
>> +
>> +G_DEFINE_TYPE(RemoteViewerISOListDialog, remote_viewer_iso_list_dialog, 
>> GTK_TYPE_DIALOG)
>> +
>> +#define DIALOG_PRIVATE(o) \
>> +(G_TYPE_INSTANCE_GET_PRIVATE((o), 
>> REMOTE_VIEWER_TYPE_ISO_LIST_DIALOG, RemoteViewerISOListDialogPrivate))
>> +
>> +struct _RemoteViewerISOListDialogPrivate
>> +{
>> +GtkBuilder *builder;
>> +GtkWidget *notebook;
>> +};
>> +
>> +enum RemoteViewerIsoListDialogPages
>> +{
>> +STATUS_PAGE = 0,
>> +ISO_LIST_PAGE,
>> +};
>> +
>> +static void
>> +remote_viewer_iso_list_dialog_dispose(GObject *object)
>> +{
>> +RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(object);
>> +RemoteViewerISOListDialogPrivate *priv = self->priv;
>> +
>> +g_clear_object(&priv->builder);
>> +
>> +
>> G_OBJECT_CLASS(remote_viewer_iso_list_dialog_parent_class)->dispose(object);
>> +}
>> +
>> +static void
>> +remote_viewer_iso_list_dialog_class_init(RemoteViewerISOListDialogClass 
>> *klass)
>> +{
>> +GObjectClass *object_class = G_OBJECT_CLASS(klass);
>> +
>> +g_type_class_add_private(klass, 
>> sizeof(RemoteViewerISOListDialogPrivate));
>> +
>> +object_class->dispose = remote_viewer_iso_list_dialog_dispose;
>> +}
>> +
>> +static void
>> +remote_viewer_iso_list_dialog_response(GtkDialog *dialog,
>> +   gint response_id,
>> +   gpointer user_data G_GNUC_UNUSED)
>> +{
>> +RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(dialog);
>> +RemoteViewerISOListDialogPrivate *priv = self->priv;
&

Re: [virt-tools-list] [PATCH virt-viewer 10/11] remote-viewer: Make ovirt-foreign-menu a property

2016-07-19 Thread Eduardo Lima (Etrunko)
On 07/19/2016 12:57 PM, Christophe Fergeau wrote:
>> +
>> +self = REMOTE_VIEWER_ISO_LIST_DIALOG(dialog);
>> +self->priv->foreign_menu = foreign_menu;
> 
> I'd g_object_ref it if you need to have it around (together with 
> g_clear_object
> in dispose/finalize).

Okay, fixed.

>> +
>> +#ifdef HAVE_OVIRT
>> +g_object_class_install_property(object_class,
>> +PROP_OVIRT_FOREIGN_MENU,
>> +
>> g_param_spec_pointer("ovirt-foreign-menu",
> 
> This can be a g_param_spec_object, OvirtForeignMenu is a GObject.

Also fixed.

> 
>> + "oVirt Foreign 
>> Menu",
>> + "Object which is 
>> used as interface to oVirt",
>> + G_PARAM_READABLE | 
>> G_PARAM_STATIC_STRINGS));
>> +#endif
>>  }
>>  
>>  static void
>> diff --git a/src/virt-viewer-window.c b/src/virt-viewer-window.c
>> index 867fb86..76fe80f 100644
>> --- a/src/virt-viewer-window.c
>> +++ b/src/virt-viewer-window.c
>> @@ -1072,11 +1072,25 @@ virt_viewer_window_menu_change_cd_activate(GtkWidget 
>> *menu G_GNUC_UNUSED,
>>  {
>>  VirtViewerWindowPrivate *priv = self->priv;
>>  static GtkWidget *dialog = NULL;
>> +GValue foreign_menu = G_VALUE_INIT;
>>  
>>  if (dialog)
>>  return;
>>  
>> -dialog = remote_viewer_iso_list_dialog_new(GTK_WINDOW(priv->window));
>> +g_value_init(&foreign_menu, G_TYPE_POINTER);
>> +g_object_get_property(G_OBJECT(priv->app), "ovirt-foreign-menu", 
>> &foreign_menu);
> 
> You can use g_object_get(G_OBJECT(priv->app), "ovirt-foreign_menu", 
> &foreign_menu, NULL);
> rather than a GValue.
> 

Thanks, fixed too.

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


Re: [virt-tools-list] [PATCH virt-viewer 10/11] remote-viewer: Make ovirt-foreign-menu a property

2016-07-19 Thread Eduardo Lima (Etrunko)
On 07/19/2016 02:54 PM, Eduardo Lima (Etrunko) wrote:
> On 07/19/2016 12:57 PM, Christophe Fergeau wrote:
>>> +
>>> +self = REMOTE_VIEWER_ISO_LIST_DIALOG(dialog);
>>> +self->priv->foreign_menu = foreign_menu;
>>
>> I'd g_object_ref it if you need to have it around (together with 
>> g_clear_object
>> in dispose/finalize).
> 
> Okay, fixed.
> 
>>> +
>>> +#ifdef HAVE_OVIRT
>>> +g_object_class_install_property(object_class,
>>> +PROP_OVIRT_FOREIGN_MENU,
>>> +
>>> g_param_spec_pointer("ovirt-foreign-menu",
>>
>> This can be a g_param_spec_object, OvirtForeignMenu is a GObject.
> 
> Also fixed.
> 
>>
>>> + "oVirt Foreign 
>>> Menu",
>>> + "Object which is 
>>> used as interface to oVirt",
>>> + G_PARAM_READABLE 
>>> | G_PARAM_STATIC_STRINGS));
>>> +#endif
>>>  }
>>>  
>>>  static void
>>> diff --git a/src/virt-viewer-window.c b/src/virt-viewer-window.c
>>> index 867fb86..76fe80f 100644
>>> --- a/src/virt-viewer-window.c
>>> +++ b/src/virt-viewer-window.c
>>> @@ -1072,11 +1072,25 @@ 
>>> virt_viewer_window_menu_change_cd_activate(GtkWidget *menu G_GNUC_UNUSED,
>>>  {
>>>  VirtViewerWindowPrivate *priv = self->priv;
>>>  static GtkWidget *dialog = NULL;
>>> +GValue foreign_menu = G_VALUE_INIT;
>>>  
>>>  if (dialog)
>>>  return;
>>>  
>>> -dialog = remote_viewer_iso_list_dialog_new(GTK_WINDOW(priv->window));
>>> +g_value_init(&foreign_menu, G_TYPE_POINTER);
>>> +g_object_get_property(G_OBJECT(priv->app), "ovirt-foreign-menu", 
>>> &foreign_menu);
>>
>> You can use g_object_get(G_OBJECT(priv->app), "ovirt-foreign_menu", 
>> &foreign_menu, NULL);
>> rather than a GValue.
>>
> 
> Thanks, fixed too.
> 

Actually I did make it a pointer on purpose, because I did not want to
have OvirtForeignMenu type in this file. Maybe I should change it to
GObject then?

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


[virt-tools-list] [PATCH virt-viewer v2 10/11] remote-viewer: Make ovirt-foreign-menu a property

2016-07-19 Thread Eduardo Lima (Etrunko)
The OvirtForeignMenu pointer is needed by the new ISO list dialog, and
we make it acessible via property to avoid interdependency between
objects.

Signed-off-by: Eduardo Lima (Etrunko) 
---
 src/remote-viewer-iso-list-dialog.c | 25 -
 src/remote-viewer-iso-list-dialog.h |  2 +-
 src/remote-viewer.c | 37 +
 src/virt-viewer-window.c| 14 +-
 4 files changed, 71 insertions(+), 7 deletions(-)

diff --git a/src/remote-viewer-iso-list-dialog.c 
b/src/remote-viewer-iso-list-dialog.c
index 3ede716..045d601 100644
--- a/src/remote-viewer-iso-list-dialog.c
+++ b/src/remote-viewer-iso-list-dialog.c
@@ -24,6 +24,7 @@
 
 #include "remote-viewer-iso-list-dialog.h"
 #include "virt-viewer-util.h"
+#include "ovirt-foreign-menu.h"
 
 G_DEFINE_TYPE(RemoteViewerISOListDialog, remote_viewer_iso_list_dialog, 
GTK_TYPE_DIALOG)
 
@@ -33,6 +34,7 @@ G_DEFINE_TYPE(RemoteViewerISOListDialog, 
remote_viewer_iso_list_dialog, GTK_TYPE
 struct _RemoteViewerISOListDialogPrivate
 {
 GtkWidget *notebook;
+OvirtForeignMenu *foreign_menu;
 };
 
 enum RemoteViewerIsoListDialogPages
@@ -44,6 +46,10 @@ enum RemoteViewerIsoListDialogPages
 static void
 remote_viewer_iso_list_dialog_dispose(GObject *object)
 {
+RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(object);
+RemoteViewerISOListDialogPrivate *priv = self->priv = DIALOG_PRIVATE(self);
+
+g_clear_object(&priv->foreign_menu);
 
G_OBJECT_CLASS(remote_viewer_iso_list_dialog_parent_class)->dispose(object);
 }
 
@@ -102,10 +108,19 @@ 
remote_viewer_iso_list_dialog_init(RemoteViewerISOListDialog *self)
 }
 
 GtkWidget *
-remote_viewer_iso_list_dialog_new(GtkWindow *parent)
+remote_viewer_iso_list_dialog_new(GtkWindow *parent, GObject *foreign_menu)
 {
-return g_object_new(REMOTE_VIEWER_TYPE_ISO_LIST_DIALOG,
-"title", _("Change CD"),
-"transient-for", parent,
-NULL);
+GtkWidget *dialog;
+RemoteViewerISOListDialog *self;
+
+g_return_val_if_fail(foreign_menu != NULL, NULL);
+
+dialog = g_object_new(REMOTE_VIEWER_TYPE_ISO_LIST_DIALOG,
+ "title", _("Change CD"),
+ "transient-for", parent,
+ NULL);
+
+self = REMOTE_VIEWER_ISO_LIST_DIALOG(dialog);
+self->priv->foreign_menu = OVIRT_FOREIGN_MENU(g_object_ref(foreign_menu));
+return dialog;
 }
diff --git a/src/remote-viewer-iso-list-dialog.h 
b/src/remote-viewer-iso-list-dialog.h
index def841b..8b936f5 100644
--- a/src/remote-viewer-iso-list-dialog.h
+++ b/src/remote-viewer-iso-list-dialog.h
@@ -51,7 +51,7 @@ struct _RemoteViewerISOListDialogClass
 
 GType remote_viewer_iso_list_dialog_get_type(void) G_GNUC_CONST;
 
-GtkWidget *remote_viewer_iso_list_dialog_new(GtkWindow *parent);
+GtkWidget *remote_viewer_iso_list_dialog_new(GtkWindow *parent, GObject 
*foreign_menu);
 
 G_END_DECLS
 
diff --git a/src/remote-viewer.c b/src/remote-viewer.c
index 95130dc..e0cd139 100644
--- a/src/remote-viewer.c
+++ b/src/remote-viewer.c
@@ -67,6 +67,13 @@ G_DEFINE_TYPE (RemoteViewer, remote_viewer, 
VIRT_VIEWER_TYPE_APP)
 #define GET_PRIVATE(o)\
 (G_TYPE_INSTANCE_GET_PRIVATE ((o), REMOTE_VIEWER_TYPE, 
RemoteViewerPrivate))
 
+enum RemoteViewerProperties {
+PROP_0,
+#ifdef HAVE_OVIRT
+PROP_OVIRT_FOREIGN_MENU,
+#endif
+};
+
 #ifdef HAVE_OVIRT
 static OvirtVm * choose_vm(GtkWindow *main_window,
char **vm_name,
@@ -213,6 +220,25 @@ end:
 }
 
 static void
+remote_viewer_get_property(GObject *object, guint property_id,
+   GValue *value, GParamSpec *pspec)
+{
+RemoteViewer *self = REMOTE_VIEWER(object);
+RemoteViewerPrivate *priv = self->priv;
+
+switch (property_id) {
+#ifdef HAVE_OVIRT
+case PROP_OVIRT_FOREIGN_MENU:
+g_value_set_object(value, priv->ovirt_foreign_menu);
+break;
+#endif
+
+default:
+G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec);
+}
+}
+
+static void
 remote_viewer_class_init (RemoteViewerClass *klass)
 {
 GObjectClass *object_class = G_OBJECT_CLASS (klass);
@@ -222,6 +248,7 @@ remote_viewer_class_init (RemoteViewerClass *klass)
 
 g_type_class_add_private (klass, sizeof (RemoteViewerPrivate));
 
+object_class->get_property = remote_viewer_get_property;
 object_class->dispose = remote_viewer_dispose;
 
 g_app_class->local_command_line = remote_viewer_local_command_line;
@@ -235,6 +262,16 @@ remote_viewer_class_init (RemoteViewerClass *klass)
 #else
 (void) gtk_app_class;
 #endif
+
+#ifdef HAVE_OVIRT
+g_object_class_install_property(object_class,
+ 

[virt-tools-list] [PATCH virt-viewer v2 07/11] UI: Make 'Change CD' menu item a submenu under 'File' toplevel menu

2016-07-19 Thread Eduardo Lima (Etrunko)
Signed-off-by: Eduardo Lima (Etrunko) 
---
 src/resources/ui/virt-viewer.ui | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/resources/ui/virt-viewer.ui b/src/resources/ui/virt-viewer.ui
index 6e3c5ad..af3ae46 100644
--- a/src/resources/ui/virt-viewer.ui
+++ b/src/resources/ui/virt-viewer.ui
@@ -1,6 +1,7 @@
 
+
 
-  
+  
   
   
 False
@@ -73,6 +74,13 @@
   
 
 
+  
+False
+_Change 
CD
+True
+  
+
+
   
 True
 False
@@ -247,14 +255,6 @@
 
   
 
-
-  
-False
-False
-_Change 
CD
-True
-  
-
   
   
 False
-- 
2.7.4

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


Re: [virt-tools-list] [PATCH virt-viewer 04/11] ovirt-foreign-menu: Add accessors for current iso and iso list

2016-07-19 Thread Eduardo Lima (Etrunko)
On 07/19/2016 12:32 PM, Christophe Fergeau wrote:
> On Sun, Jul 17, 2016 at 11:13:04PM -0300, Eduardo Lima (Etrunko) wrote:
>> Signed-off-by: Eduardo Lima (Etrunko) 
>> ---
>>  src/ovirt-foreign-menu.c | 49 
>> ++--
>>  src/ovirt-foreign-menu.h |  5 -
>>  2 files changed, 39 insertions(+), 15 deletions(-)
>>
>> diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c
>> index b071e27..2446239 100644
>> --- a/src/ovirt-foreign-menu.c
>> +++ b/src/ovirt-foreign-menu.c
>> @@ -47,6 +47,7 @@ static void 
>> ovirt_foreign_menu_fetch_storage_domain_async(OvirtForeignMenu *menu
>>  static void ovirt_foreign_menu_fetch_vm_cdrom_async(OvirtForeignMenu *menu);
>>  static void ovirt_foreign_menu_refresh_cdrom_file_async(OvirtForeignMenu 
>> *menu);
>>  static void ovirt_foreign_menu_fetch_iso_list_async(OvirtForeignMenu *menu);
>> +static void updated_cdrom_cb(GObject *source_object, GAsyncResult *result, 
>> gpointer user_data);
>>  
>>  G_DEFINE_TYPE (OvirtForeignMenu, ovirt_foreign_menu, G_TYPE_OBJECT)
>>  
>> @@ -85,7 +86,7 @@ enum {
>>  };
>>  
>>  
>> -static char *
>> +char *
>>  ovirt_foreign_menu_get_current_iso_name(OvirtForeignMenu *foreign_menu)
>>  {
>>  char *name;
>> @@ -100,6 +101,36 @@ 
>> ovirt_foreign_menu_get_current_iso_name(OvirtForeignMenu *foreign_menu)
>>  }
>>  
>>  
>> +void
>> +ovirt_foreign_menu_set_current_iso_name(OvirtForeignMenu *foreign_menu, 
>> char *name)
>> +{
> 
> For what it's worth, this is a bit misleading as this going to trigger
> an async update of the ISO name, and this sets "next_iso_name" more than
> "current_iso_name". I think you need to expose this an async method
> anyway, so that you can catch failures to change the ISO (ie the REST
> API call failed).
> 

Okay, I will work on it. I really did it the laziest way possible,
without any error treatment indeed.

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com



signature.asc
Description: OpenPGP digital signature
___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list

[virt-tools-list] [PATCH virt-viewer v2 12/12] iso-dialog: Use header bar for buttons

2016-07-22 Thread Eduardo Lima (Etrunko)
It seems to give more modern look to the dialog, but it requires Gtk+
3.12, thus I will am keeping this commit separated.

Signed-off-by: Eduardo Lima (Etrunko) 
---
 configure.ac|  4 ++--
 src/remote-viewer-iso-list-dialog.c | 33 -
 2 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/configure.ac b/configure.ac
index 7c437a3..7682f42 100644
--- a/configure.ac
+++ b/configure.ac
@@ -17,8 +17,8 @@ GLIB2_REQUIRED="2.38"
 GLIB2_ENCODED_VERSION="GLIB_VERSION_2_38"
 
 # Keep these two definitions in agreement.
-GTK_REQUIRED="3.10"
-GTK_ENCODED_VERSION="GDK_VERSION_3_10"
+GTK_REQUIRED="3.12"
+GTK_ENCODED_VERSION="GDK_VERSION_3_12"
 
 LIBXML2_REQUIRED="2.6.0"
 LIBVIRT_REQUIRED="0.10.0"
diff --git a/src/remote-viewer-iso-list-dialog.c 
b/src/remote-viewer-iso-list-dialog.c
index 7fe032b..c6fb5d8 100644
--- a/src/remote-viewer-iso-list-dialog.c
+++ b/src/remote-viewer-iso-list-dialog.c
@@ -34,6 +34,7 @@ G_DEFINE_TYPE(RemoteViewerISOListDialog, 
remote_viewer_iso_list_dialog, GTK_TYPE
 
 struct _RemoteViewerISOListDialogPrivate
 {
+GtkHeaderBar *header_bar;
 GtkListStore *list_store;
 GtkWidget *stack;
 GtkWidget *treeview;
@@ -88,6 +89,19 @@ 
remote_viewer_iso_list_dialog_show_files(RemoteViewerISOListDialog *self)
 }
 
 static void
+remote_viewer_iso_list_dialog_set_subtitle(RemoteViewerISOListDialog *self, 
const char *iso_name)
+{
+RemoteViewerISOListDialogPrivate *priv = self->priv;
+gchar *subtitle = NULL;
+
+if (iso_name && strlen(iso_name) != 0)
+subtitle = g_strdup_printf(_("Current: %s"), iso_name);
+
+gtk_header_bar_set_subtitle(priv->header_bar, subtitle);
+g_free(subtitle);
+}
+
+static void
 remote_viewer_iso_list_dialog_foreach(char *name, RemoteViewerISOListDialog 
*self)
 {
 RemoteViewerISOListDialogPrivate *priv = self->priv;
@@ -102,6 +116,9 @@ remote_viewer_iso_list_dialog_foreach(char *name, 
RemoteViewerISOListDialog *sel
ISO_NAME, name,
FONT_WEIGHT, weight, -1);
 
+if (active)
+remote_viewer_iso_list_dialog_set_subtitle(self, current_iso);
+
 free(current_iso);
 }
 
@@ -134,6 +151,7 @@ remote_viewer_iso_list_dialog_response(GtkDialog *dialog,
 if (response_id != GTK_RESPONSE_NONE)
 return;
 
+remote_viewer_iso_list_dialog_set_subtitle(self, NULL);
 gtk_stack_set_visible_child_full(GTK_STACK(priv->stack), "status",
  GTK_STACK_TRANSITION_TYPE_SLIDE_LEFT);
 gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, 
FALSE);
@@ -171,9 +189,13 @@ 
remote_viewer_iso_list_dialog_init(RemoteViewerISOListDialog *self)
 RemoteViewerISOListDialogPrivate *priv = self->priv = DIALOG_PRIVATE(self);
 GtkBuilder *builder = 
virt_viewer_util_load_ui("remote-viewer-iso-list.ui");
 GtkCellRendererToggle *cell_renderer;
+GtkWidget *button, *image;
 
 gtk_builder_connect_signals(builder, self);
 
+priv->header_bar = 
GTK_HEADER_BAR(gtk_dialog_get_header_bar(GTK_DIALOG(self)));
+gtk_header_bar_set_has_subtitle(priv->header_bar, TRUE);
+
 priv->stack = GTK_WIDGET(gtk_builder_get_object(builder, "stack"));
 gtk_box_pack_start(GTK_BOX(content), priv->stack, TRUE, TRUE, 0);
 
@@ -184,12 +206,11 @@ 
remote_viewer_iso_list_dialog_init(RemoteViewerISOListDialog *self)
 
 g_object_unref(builder);
 
-gtk_dialog_add_buttons(GTK_DIALOG(self),
-   _("Refresh"), GTK_RESPONSE_NONE,
-   _("Close"), GTK_RESPONSE_CLOSE,
-   NULL);
+button = gtk_dialog_add_button(GTK_DIALOG(self), "", GTK_RESPONSE_NONE);
+image = gtk_image_new_from_icon_name("view-refresh-symbolic", 
GTK_ICON_SIZE_BUTTON);
+gtk_button_set_image(GTK_BUTTON(button), image);
+gtk_button_set_always_show_image(GTK_BUTTON(button), TRUE);
 
-gtk_dialog_set_default_response(GTK_DIALOG(self), GTK_RESPONSE_CLOSE);
 gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, 
FALSE);
 g_signal_connect(self, "response", 
G_CALLBACK(remote_viewer_iso_list_dialog_response), NULL);
 }
@@ -229,6 +250,7 @@ ovirt_foreign_menu_notify_file(OvirtForeignMenu 
*foreign_menu,
 g_free(name);
 } while (gtk_tree_model_iter_next(model, &iter));
 
+remote_viewer_iso_list_dialog_set_subtitle(self, current_iso);
 gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, 
TRUE);
 gtk_widget_set_sensitive(priv->treeview, TRUE);
 
@@ -259,6 +281,7 @@ remote_viewer_iso_list_dialog_new(GtkWindow *parent, 
GObject *foreign_menu)
  "border-width", 18,
  "default-widt

[virt-tools-list] [PATCH virt-viewer v2 03/12] ovirt-foreign-menu: Remove timer used to refresh iso list

2016-07-22 Thread Eduardo Lima (Etrunko)
With the new ISO dialog, the user triggers the refresh manually.

Signed-off-by: Eduardo Lima (Etrunko) 
---
 src/ovirt-foreign-menu.c | 23 +++
 1 file changed, 3 insertions(+), 20 deletions(-)

diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c
index a51f2c9..565ef22 100644
--- a/src/ovirt-foreign-menu.c
+++ b/src/ovirt-foreign-menu.c
@@ -46,7 +46,7 @@ static void 
ovirt_foreign_menu_fetch_vm_async(OvirtForeignMenu *menu);
 static void ovirt_foreign_menu_fetch_storage_domain_async(OvirtForeignMenu 
*menu);
 static void ovirt_foreign_menu_fetch_vm_cdrom_async(OvirtForeignMenu *menu);
 static void ovirt_foreign_menu_refresh_cdrom_file_async(OvirtForeignMenu 
*menu);
-static gboolean ovirt_foreign_menu_refresh_iso_list(gpointer user_data);
+static void ovirt_foreign_menu_fetch_iso_list_async(OvirtForeignMenu *menu);
 
 G_DEFINE_TYPE (OvirtForeignMenu, ovirt_foreign_menu, G_TYPE_OBJECT)
 
@@ -313,7 +313,7 @@ ovirt_foreign_menu_next_async_step(OvirtForeignMenu *menu,
 g_warn_if_fail(menu->priv->files != NULL);
 g_warn_if_fail(menu->priv->cdrom != NULL);
 
-ovirt_foreign_menu_refresh_iso_list(menu);
+ovirt_foreign_menu_fetch_iso_list_async(menu);
 break;
 default:
 g_warn_if_reached();
@@ -754,8 +754,6 @@ static void iso_list_fetched_cb(GObject *source_object,
 files = 
g_hash_table_get_values(ovirt_collection_get_resources(collection));
 ovirt_foreign_menu_set_files(OVIRT_FOREIGN_MENU(user_data), files);
 g_list_free(files);
-
-g_timeout_add_seconds(300, ovirt_foreign_menu_refresh_iso_list, user_data);
 }
 
 
@@ -765,27 +763,12 @@ static void 
ovirt_foreign_menu_fetch_iso_list_async(OvirtForeignMenu *menu)
 return;
 }
 
+g_debug("Refreshing foreign menu iso list");
 ovirt_collection_fetch_async(menu->priv->files, menu->priv->proxy,
  NULL, iso_list_fetched_cb, menu);
 }
 
 
-static gboolean ovirt_foreign_menu_refresh_iso_list(gpointer user_data)
-{
-OvirtForeignMenu *menu;
-
-g_debug("Refreshing foreign menu iso list");
-menu = OVIRT_FOREIGN_MENU(user_data);
-ovirt_foreign_menu_fetch_iso_list_async(menu);
-
-/* ovirt_foreign_menu_fetch_iso_list_async() will schedule a new call to
- * that function through iso_list_fetched_cb() when it has finished
- * fetching the iso list
- */
-return G_SOURCE_REMOVE;
-}
-
-
 OvirtForeignMenu *ovirt_foreign_menu_new_from_file(VirtViewerFile *file)
 {
 OvirtProxy *proxy = NULL;
-- 
2.7.4

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


[virt-tools-list] [PATCH virt-viewer v2 07/12] UI: Make 'Change CD' menu item a submenu under 'File' toplevel menu

2016-07-22 Thread Eduardo Lima (Etrunko)
Signed-off-by: Eduardo Lima (Etrunko) 
---
 src/resources/ui/virt-viewer.ui | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/resources/ui/virt-viewer.ui b/src/resources/ui/virt-viewer.ui
index 6e3c5ad..af3ae46 100644
--- a/src/resources/ui/virt-viewer.ui
+++ b/src/resources/ui/virt-viewer.ui
@@ -1,6 +1,7 @@
 
+
 
-  
+  
   
   
 False
@@ -73,6 +74,13 @@
   
 
 
+  
+False
+_Change 
CD
+True
+  
+
+
   
 True
 False
@@ -247,14 +255,6 @@
 
   
 
-
-  
-False
-False
-_Change 
CD
-True
-  
-
   
   
 False
-- 
2.7.4

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


[virt-tools-list] [PATCH virt-viewer v2 11/12] iso-dialog: Implement functionality provided by oVirt foreign menu

2016-07-22 Thread Eduardo Lima (Etrunko)
Signed-off-by: Eduardo Lima (Etrunko) 
---
 src/remote-viewer-iso-list-dialog.c| 157 -
 src/resources/ui/remote-viewer-iso-list.ui |   5 +-
 2 files changed, 158 insertions(+), 4 deletions(-)

diff --git a/src/remote-viewer-iso-list-dialog.c 
b/src/remote-viewer-iso-list-dialog.c
index 7a84d0c..7fe032b 100644
--- a/src/remote-viewer-iso-list-dialog.c
+++ b/src/remote-viewer-iso-list-dialog.c
@@ -20,6 +20,7 @@
 
 #include 
 
+#include 
 #include 
 
 #include "remote-viewer-iso-list-dialog.h"
@@ -33,7 +34,9 @@ G_DEFINE_TYPE(RemoteViewerISOListDialog, 
remote_viewer_iso_list_dialog, GTK_TYPE
 
 struct _RemoteViewerISOListDialogPrivate
 {
+GtkListStore *list_store;
 GtkWidget *stack;
+GtkWidget *treeview;
 OvirtForeignMenu *foreign_menu;
 };
 
@@ -43,13 +46,25 @@ enum RemoteViewerIsoListDialogPages
 ISO_LIST_PAGE,
 };
 
+enum RemoteViewerIsoListDialogModel
+{
+ISO_IS_ACTIVE = 0,
+ISO_NAME,
+FONT_WEIGHT,
+};
+
+void remote_viewer_iso_list_dialog_toggled(GtkCellRendererToggle 
*cell_renderer, gchar *path, gpointer user_data);
+
 static void
 remote_viewer_iso_list_dialog_dispose(GObject *object)
 {
 RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(object);
 RemoteViewerISOListDialogPrivate *priv = self->priv;
 
-g_clear_object(&priv->foreign_menu);
+if (priv->foreign_menu) {
+g_signal_handlers_disconnect_by_data(priv->foreign_menu, object);
+g_clear_object(&priv->foreign_menu);
+}
 
G_OBJECT_CLASS(remote_viewer_iso_list_dialog_parent_class)->dispose(object);
 }
 
@@ -64,6 +79,51 @@ 
remote_viewer_iso_list_dialog_class_init(RemoteViewerISOListDialogClass *klass)
 }
 
 static void
+remote_viewer_iso_list_dialog_show_files(RemoteViewerISOListDialog *self)
+{
+RemoteViewerISOListDialogPrivate *priv = self->priv = DIALOG_PRIVATE(self);
+gtk_stack_set_visible_child_full(GTK_STACK(priv->stack), "iso-list",
+ GTK_STACK_TRANSITION_TYPE_SLIDE_LEFT);
+gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, 
TRUE);
+}
+
+static void
+remote_viewer_iso_list_dialog_foreach(char *name, RemoteViewerISOListDialog 
*self)
+{
+RemoteViewerISOListDialogPrivate *priv = self->priv;
+char * current_iso = 
ovirt_foreign_menu_get_current_iso_name(self->priv->foreign_menu);
+gboolean active = g_strcmp0(current_iso, name) == 0;
+gint weight = active ? PANGO_WEIGHT_BOLD : PANGO_WEIGHT_NORMAL;
+GtkTreeIter iter;
+
+gtk_list_store_append(priv->list_store, &iter);
+gtk_list_store_set(priv->list_store, &iter,
+   ISO_IS_ACTIVE, active,
+   ISO_NAME, name,
+   FONT_WEIGHT, weight, -1);
+
+free(current_iso);
+}
+
+static void
+remote_viewer_iso_list_dialog_refresh_iso_list(RemoteViewerISOListDialog *self,
+   gboolean refreshing)
+{
+RemoteViewerISOListDialogPrivate *priv = self->priv;
+GList *iso_list;
+
+if (refreshing) {
+gtk_list_store_clear(priv->list_store);
+ovirt_foreign_menu_start(priv->foreign_menu);
+return;
+}
+
+iso_list = ovirt_foreign_menu_get_iso_names(priv->foreign_menu);
+g_list_foreach(iso_list, (GFunc) remote_viewer_iso_list_dialog_foreach, 
self);
+remote_viewer_iso_list_dialog_show_files(self);
+}
+
+static void
 remote_viewer_iso_list_dialog_response(GtkDialog *dialog,
gint response_id,
gpointer user_data G_GNUC_UNUSED)
@@ -77,7 +137,31 @@ remote_viewer_iso_list_dialog_response(GtkDialog *dialog,
 gtk_stack_set_visible_child_full(GTK_STACK(priv->stack), "status",
  GTK_STACK_TRANSITION_TYPE_SLIDE_LEFT);
 gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, 
FALSE);
-gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_CLOSE, 
FALSE);
+remote_viewer_iso_list_dialog_refresh_iso_list(self, TRUE);
+}
+
+void
+remote_viewer_iso_list_dialog_toggled(GtkCellRendererToggle *cell_renderer 
G_GNUC_UNUSED,
+  gchar *path,
+  gpointer user_data)
+{
+RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(user_data);
+RemoteViewerISOListDialogPrivate *priv = self->priv;
+GtkTreeModel *model = GTK_TREE_MODEL(priv->list_store);
+GtkTreeIter iter;
+gboolean active;
+gchar *name;
+
+gtk_tree_model_get_iter_from_string(model, &iter, path);
+gtk_tree_model_get(model, &iter,
+   ISO_IS_ACTIVE, &active,
+   ISO_NAME, &name, -1);
+
+gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, 
FALSE);
+gtk_widget_set_sensitive(priv

[virt-tools-list] [PATCH virt-viewer v2 02/12] ovirt-foreign-menu: Use g_clear_pointer/g_clear_object

2016-07-22 Thread Eduardo Lima (Etrunko)
Signed-off-by: Eduardo Lima (Etrunko) 
Acked-by: Christophe Fergeau 
---
 src/ovirt-foreign-menu.c | 68 
 1 file changed, 16 insertions(+), 52 deletions(-)

diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c
index 03dfbe7..a51f2c9 100644
--- a/src/ovirt-foreign-menu.c
+++ b/src/ovirt-foreign-menu.c
@@ -143,33 +143,23 @@ ovirt_foreign_menu_set_property(GObject *object, guint 
property_id,
 
 switch (property_id) {
 case PROP_PROXY:
-if (priv->proxy != NULL) {
-g_object_unref(priv->proxy);
-}
+g_clear_object(&priv->proxy);
 priv->proxy = g_value_dup_object(value);
 break;
 case PROP_API:
-if (priv->api != NULL) {
-g_object_unref(priv->api);
-}
+g_clear_object(&priv->api);
 priv->api = g_value_dup_object(value);
 break;
 case PROP_VM:
-if (priv->vm != NULL) {
-g_object_unref(priv->vm);
-}
+g_clear_object(&priv->vm);
 priv->vm = g_value_dup_object(value);
-g_free(priv->vm_guid);
-priv->vm_guid = NULL;
+g_clear_pointer(&priv->vm_guid, g_free);
 if (priv->vm != NULL) {
 g_object_get(G_OBJECT(priv->vm), "guid", &priv->vm_guid, NULL);
 }
 break;
 case PROP_VM_GUID:
-if (priv->vm != NULL) {
-g_object_unref(priv->vm);
-priv->vm = NULL;
-}
+g_clear_object(&priv->vm);
 g_free(priv->vm_guid);
 priv->vm_guid = g_value_dup_string(value);
 break;
@@ -184,44 +174,20 @@ ovirt_foreign_menu_dispose(GObject *obj)
 {
 OvirtForeignMenu *self = OVIRT_FOREIGN_MENU(obj);
 
-if (self->priv->proxy) {
-g_object_unref(self->priv->proxy);
-self->priv->proxy = NULL;
-}
-
-if (self->priv->api != NULL) {
-g_object_unref(self->priv->api);
-self->priv->api = NULL;
-}
-
-if (self->priv->vm) {
-g_object_unref(self->priv->vm);
-self->priv->vm = NULL;
-}
-
-g_free(self->priv->vm_guid);
-self->priv->vm_guid = NULL;
-
-if (self->priv->files) {
-g_object_unref(self->priv->files);
-self->priv->files = NULL;
-}
-
-if (self->priv->cdrom) {
-g_object_unref(self->priv->cdrom);
-self->priv->cdrom = NULL;
-}
+g_clear_object(&self->priv->proxy);
+g_clear_object(&self->priv->api);
+g_clear_object(&self->priv->vm);
+g_clear_pointer(&self->priv->vm_guid, g_free);
+g_clear_object(&self->priv->files);
+g_clear_object(&self->priv->cdrom);
 
 if (self->priv->iso_names) {
 g_list_free_full(self->priv->iso_names, (GDestroyNotify)g_free);
 self->priv->iso_names = NULL;
 }
 
-g_free(self->priv->current_iso_name);
-self->priv->current_iso_name = NULL;
-
-g_free(self->priv->next_iso_name);
-self->priv->next_iso_name = NULL;
+g_clear_pointer(&self->priv->current_iso_name, g_free);
+g_clear_pointer(&self->priv->next_iso_name, g_free);
 
 G_OBJECT_CLASS(ovirt_foreign_menu_parent_class)->dispose(obj);
 }
@@ -408,8 +374,8 @@ static void updated_cdrom_cb(GObject *source_object,
 current_file?current_file:NULL);
 g_object_set(foreign_menu->priv->cdrom, "file", current_file, NULL);
 }
-g_free(foreign_menu->priv->next_iso_name);
-foreign_menu->priv->next_iso_name = NULL;
+
+g_clear_pointer(&foreign_menu->priv->next_iso_name, g_free);
 }
 
 
@@ -544,13 +510,11 @@ static void cdrom_file_refreshed_cb(GObject 
*source_object,
 }
 
 /* Content of OvirtCdrom is now current */
-g_free(menu->priv->current_iso_name);
+g_clear_pointer(&menu->priv->current_iso_name, g_free);
 if (menu->priv->cdrom != NULL) {
 g_object_get(G_OBJECT(menu->priv->cdrom),
  "file", &menu->priv->current_iso_name,
  NULL);
-} else {
-menu->priv->current_iso_name = NULL;
 }
 g_object_notify(G_OBJECT(menu), "file");
 if (menu->priv->cdrom != NULL) {
-- 
2.7.4

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


[virt-tools-list] [PATCH virt-viewer v2 01/12] ovirt-foreign-menu: Rework states logic

2016-07-22 Thread Eduardo Lima (Etrunko)
Use switch/case instead of lots of conditional blocks

Signed-off-by: Eduardo Lima (Etrunko) 
Acked-by: Pavel Grunt 
---
 src/ovirt-foreign-menu.c | 46 --
 1 file changed, 20 insertions(+), 26 deletions(-)

diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c
index 33ff4f1..03dfbe7 100644
--- a/src/ovirt-foreign-menu.c
+++ b/src/ovirt-foreign-menu.c
@@ -312,51 +312,45 @@ ovirt_foreign_menu_next_async_step(OvirtForeignMenu *menu,
 g_return_if_fail(current_state >= STATE_0);
 g_return_if_fail(current_state < STATE_ISOS);
 
-current_state++;
-
-if (current_state == STATE_API) {
+/* Each state will check if the member is initialized, falling directly to
+ * the next one if so. If not, the callback for the asynchronous call will
+ * be responsible for calling is function again with the next state as
+ * argument.
+ */
+switch (current_state + 1) {
+case STATE_API:
 if (menu->priv->api == NULL) {
 ovirt_foreign_menu_fetch_api_async(menu);
-} else {
-current_state++;
+break;
 }
-}
-
-if (current_state == STATE_VM) {
+case STATE_VM:
 if (menu->priv->vm == NULL) {
 ovirt_foreign_menu_fetch_vm_async(menu);
-} else {
-current_state++;
+break;
 }
-}
-
-if (current_state == STATE_STORAGE_DOMAIN) {
+case STATE_STORAGE_DOMAIN:
 if (menu->priv->files == NULL) {
 ovirt_foreign_menu_fetch_storage_domain_async(menu);
-} else {
-current_state++;
+break;
 }
-}
-
-if (current_state == STATE_VM_CDROM) {
+case STATE_VM_CDROM:
 if (menu->priv->cdrom == NULL) {
 ovirt_foreign_menu_fetch_vm_cdrom_async(menu);
-} else {
-current_state++;
+break;
 }
-}
-
-if (current_state == STATE_CDROM_FILE) {
+case STATE_CDROM_FILE:
 ovirt_foreign_menu_refresh_cdrom_file_async(menu);
-}
-
-if (current_state == STATE_ISOS) {
+break;
+case STATE_ISOS:
 g_warn_if_fail(menu->priv->api != NULL);
 g_warn_if_fail(menu->priv->vm != NULL);
 g_warn_if_fail(menu->priv->files != NULL);
 g_warn_if_fail(menu->priv->cdrom != NULL);
 
 ovirt_foreign_menu_refresh_iso_list(menu);
+break;
+default:
+g_warn_if_reached();
 }
 }
 
-- 
2.7.4

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


[virt-tools-list] [PATCH virt-viewer v2 00/12] Replace oVirt foreign menu with dedicated dialog

2016-07-22 Thread Eduardo Lima (Etrunko)
New version of the ISO list dialog, with changes suggested in first
round of reviews and lots of bug fixes. Before sending v1 I had
refactored a few bits and it introduced some serious crashes.

On the UI side, we now use GtkStack instead of GtkNotebook and the last
commit of the series also makes use of more modern looking GtkHeaderBar.
It has been kept as a separate commit because it requires newer version
of Gtk+ than the one which is currently required.

In progress:
 - Use GtkListBox instead of GtkTreeView.
 - Rework OvirtForeignMenu/ISOListDialog so that the async API is
   exposed, making it more robust in case of errors.

Eduardo Lima (Etrunko) (12):
  ovirt-foreign-menu: Rework states logic
  ovirt-foreign-menu: Use g_clear_pointer/g_clear_object
  ovirt-foreign-menu: Remove timer used to refresh iso list
  ovirt-foreign-menu: Add accessors for current iso and iso list
  ovirt-foreign-menu: Remove GtkMenu related functions
  ovirt-foreign-menu: Notify of new files even if nothing changed
  UI: Make 'Change CD' menu item a submenu under 'File' toplevel menu
  Introduce ISO List dialog
  Run iso-dialog when 'Change CD' menu is activated
  remote-viewer: Make ovirt-foreign-menu a property
  iso-dialog: Implement functionality provided by oVirt foreign menu
  iso-dialog: Use header bar for buttons

 configure.ac   |   4 +-
 po/POTFILES.in |   2 +
 src/Makefile.am|   3 +
 src/ovirt-foreign-menu.c   | 292 
 src/ovirt-foreign-menu.h   |   5 +-
 src/remote-viewer-iso-list-dialog.c| 301 +
 src/remote-viewer-iso-list-dialog.h|  58 ++
 src/remote-viewer.c|  89 -
 src/resources/ui/remote-viewer-iso-list.ui | 158 +++
 src/resources/ui/virt-viewer.ui|  19 +-
 src/resources/virt-viewer.gresource.xml|   1 +
 src/virt-viewer-window.c   |  37 
 12 files changed, 702 insertions(+), 267 deletions(-)
 create mode 100644 src/remote-viewer-iso-list-dialog.c
 create mode 100644 src/remote-viewer-iso-list-dialog.h
 create mode 100644 src/resources/ui/remote-viewer-iso-list.ui

-- 
2.7.4

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


[virt-tools-list] [PATCH virt-viewer v2 06/12] ovirt-foreign-menu: Notify of new files even if nothing changed

2016-07-22 Thread Eduardo Lima (Etrunko)
When user presses the Refresh button in ISO dialog, the list is cleared, and
currently, the only way it is informed of the new list is by the notify signal.
The same applies when an error occurs while trying to change the current ISO.

Signed-off-by: Eduardo Lima (Etrunko) 
---
 src/ovirt-foreign-menu.c | 41 ++---
 1 file changed, 14 insertions(+), 27 deletions(-)

diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c
index 493ca47..d8c0553 100644
--- a/src/ovirt-foreign-menu.c
+++ b/src/ovirt-foreign-menu.c
@@ -375,22 +375,24 @@ static void updated_cdrom_cb(GObject *source_object,
 g_free(foreign_menu->priv->current_iso_name);
 foreign_menu->priv->current_iso_name = 
foreign_menu->priv->next_iso_name;
 foreign_menu->priv->next_iso_name = NULL;
-g_object_notify(G_OBJECT(foreign_menu), "file");
-} else {
-/* Reset old state back as we were not successful in switching to
- * the new ISO */
-const char *current_file = foreign_menu->priv->current_iso_name;
+goto end;
+}
 
-if (error != NULL) {
-g_warning("failed to update cdrom resource: %s", error->message);
-g_clear_error(&error);
-}
-g_debug("setting OvirtCdrom:file back to '%s'",
-current_file?current_file:NULL);
-g_object_set(foreign_menu->priv->cdrom, "file", current_file, NULL);
+/* Reset old state back as we were not successful in switching to
+ * the new ISO */
+if (error != NULL) {
+g_warning("failed to update cdrom resource: %s", error->message);
+g_clear_error(&error);
 }
+g_debug("setting OvirtCdrom:file back to '%s'",
+foreign_menu->priv->current_iso_name);
+g_object_set(foreign_menu->priv->cdrom,
+ "file", foreign_menu->priv->current_iso_name,
+ NULL);
 
+end:
 g_clear_pointer(&foreign_menu->priv->next_iso_name, g_free);
+g_object_notify(G_OBJECT(foreign_menu), "file");
 }
 
 
@@ -399,7 +401,6 @@ static void ovirt_foreign_menu_set_files(OvirtForeignMenu 
*menu,
 {
 GList *sorted_files = NULL;
 const GList *it;
-GList *it2;
 
 for (it = files; it != NULL; it = it->next) {
 char *name;
@@ -416,20 +417,6 @@ static void ovirt_foreign_menu_set_files(OvirtForeignMenu 
*menu,
 (GCompareFunc)g_strcmp0);
 }
 
-for (it = sorted_files, it2 = menu->priv->iso_names;
- (it != NULL) && (it2 != NULL);
- it = it->next, it2 = it2->next) {
-if (g_strcmp0(it->data, it2->data) != 0) {
-break;
-}
-}
-
-if ((it == NULL) && (it2 == NULL)) {
-/* sorted_files and menu->priv->files content was the same */
-g_list_free_full(sorted_files, (GDestroyNotify)g_free);
-return;
-}
-
 g_list_free_full(menu->priv->iso_names, (GDestroyNotify)g_free);
 menu->priv->iso_names = sorted_files;
 g_object_notify(G_OBJECT(menu), "files");
-- 
2.7.4

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


[virt-tools-list] [PATCH virt-viewer v2 05/12] ovirt-foreign-menu: Remove GtkMenu related functions

2016-07-22 Thread Eduardo Lima (Etrunko)
With this commit, we finish cleaning up ovirt foreign menu code, which
only deals with data, leaving the UI bits to be handled properly in the
new ISO list dialog.

This patch also updates remote-viewer to reflect the change.

Signed-off-by: Eduardo Lima (Etrunko) 
---
 src/ovirt-foreign-menu.c | 79 
 src/remote-viewer.c  | 52 +--
 2 files changed, 8 insertions(+), 123 deletions(-)

diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c
index eec6bc6..493ca47 100644
--- a/src/ovirt-foreign-menu.c
+++ b/src/ovirt-foreign-menu.c
@@ -359,22 +359,6 @@ ovirt_foreign_menu_start(OvirtForeignMenu *menu)
 }
 
 
-static void
-ovirt_foreign_menu_activate_item_cb(GtkMenuItem *menuitem, gpointer user_data);
-
-
-static void
-menu_item_set_active_no_signal(GtkMenuItem *menuitem,
-   gboolean active,
-   GCallback callback,
-   gpointer user_data)
-{
-g_signal_handlers_block_by_func(menuitem, callback, user_data);
-gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(menuitem), active);
-g_signal_handlers_unblock_by_func(menuitem, callback, user_data);
-}
-
-
 static void updated_cdrom_cb(GObject *source_object,
  GAsyncResult *result,
  gpointer user_data)
@@ -410,69 +394,6 @@ static void updated_cdrom_cb(GObject *source_object,
 }
 
 
-static void
-ovirt_foreign_menu_activate_item_cb(GtkMenuItem *menuitem, gpointer user_data)
-{
-OvirtForeignMenu *foreign_menu;
-const char *iso_name = NULL;
-gboolean checked;
-
-checked = gtk_check_menu_item_get_active(GTK_CHECK_MENU_ITEM(menuitem));
-foreign_menu = OVIRT_FOREIGN_MENU(user_data);
-g_return_if_fail(foreign_menu->priv->cdrom != NULL);
-g_return_if_fail(foreign_menu->priv->next_iso_name == NULL);
-
-g_debug("'%s' clicked", gtk_menu_item_get_label(menuitem));
-
-/* We only want to move the check mark for the currently selected ISO
- * when ovirt_cdrom_update_async() is successful, so for now we move
- * the check mark back to where it was before
- */
-menu_item_set_active_no_signal(menuitem, !checked,
-   
(GCallback)ovirt_foreign_menu_activate_item_cb,
-   foreign_menu);
-
-if (checked) {
-iso_name = gtk_menu_item_get_label(menuitem);
-}
-
-ovirt_foreign_menu_set_current_iso_name(foreign_menu, iso_name);
-}
-
-
-GtkWidget *ovirt_foreign_menu_get_gtk_menu(OvirtForeignMenu *foreign_menu)
-{
-GtkWidget *gtk_menu;
-GList *it;
-char *current_iso;
-
-if (foreign_menu->priv->iso_names == NULL) {
-g_debug("ISO list is empty, no menu to show");
-return NULL;
-}
-g_debug("Creating GtkMenu for foreign menu");
-current_iso = ovirt_foreign_menu_get_current_iso_name(foreign_menu);
-gtk_menu = gtk_menu_new();
-for (it = foreign_menu->priv->iso_names; it != NULL; it = it->next) {
-GtkWidget *menuitem;
-
-menuitem = gtk_check_menu_item_new_with_label((char *)it->data);
-if (g_strcmp0((char *)it->data, current_iso) == 0) {
-g_warn_if_fail(g_strcmp0(current_iso, 
foreign_menu->priv->current_iso_name) == 0);
-gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(menuitem),
-   TRUE);
-}
-g_signal_connect(menuitem, "activate",
- G_CALLBACK(ovirt_foreign_menu_activate_item_cb),
- foreign_menu);
-gtk_menu_shell_append(GTK_MENU_SHELL(gtk_menu), menuitem);
-}
-g_free(current_iso);
-
-return gtk_menu;
-}
-
-
 static void ovirt_foreign_menu_set_files(OvirtForeignMenu *menu,
  const GList *files)
 {
diff --git a/src/remote-viewer.c b/src/remote-viewer.c
index 6d29bf2..95130dc 100644
--- a/src/remote-viewer.c
+++ b/src/remote-viewer.c
@@ -734,33 +734,11 @@ authenticate_cb(RestProxy *proxy, G_GNUC_UNUSED 
RestProxyAuth *auth,
 static void
 ovirt_foreign_menu_update(GtkApplication *gtkapp, GtkWindow *gtkwin, 
G_GNUC_UNUSED gpointer data)
 {
-RemoteViewer *app = REMOTE_VIEWER(gtkapp);
+RemoteViewer *self = REMOTE_VIEWER(gtkapp);
 VirtViewerWindow *win = g_object_get_data(G_OBJECT(gtkwin), 
"virt-viewer-window");
-GtkWidget *menu = g_object_get_data(G_OBJECT(win), "foreign-menu");
-GtkWidget *submenu;
-
-if (app->priv->ovirt_foreign_menu == NULL) {
-/* nothing to do */
-return;
-}
-
-submenu = ovirt_foreign_menu_get_gtk_menu(app->priv->ovirt_foreign_menu);
-if (submenu == NULL) {
-/* No items to show, no point in showing the menu */
-if (menu != NULL)
-   gtk_widget_set_visible(menu,

[virt-tools-list] [PATCH virt-viewer v2 08/12] Introduce ISO List dialog

2016-07-22 Thread Eduardo Lima (Etrunko)
Signed-off-by: Eduardo Lima (Etrunko) 
---
 po/POTFILES.in |   2 +
 src/Makefile.am|   3 +
 src/remote-viewer-iso-list-dialog.c| 112 +
 src/remote-viewer-iso-list-dialog.h|  58 +++
 src/resources/ui/remote-viewer-iso-list.ui | 155 +
 src/resources/virt-viewer.gresource.xml|   1 +
 6 files changed, 331 insertions(+)
 create mode 100644 src/remote-viewer-iso-list-dialog.c
 create mode 100644 src/remote-viewer-iso-list-dialog.h
 create mode 100644 src/resources/ui/remote-viewer-iso-list.ui

diff --git a/po/POTFILES.in b/po/POTFILES.in
index 6775f53..54de445 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -1,9 +1,11 @@
 data/remote-viewer.appdata.xml.in
 data/remote-viewer.desktop.in
 data/virt-viewer-mime.xml.in
+src/remote-viewer-iso-list-dialog.c
 src/remote-viewer-main.c
 src/remote-viewer.c
 [type: gettext/glade] src/resources/ui/remote-viewer-connect.ui
+[type: gettext/glade] src/resources/ui/remote-viewer-iso-list.ui
 [type: gettext/glade] src/resources/ui/virt-viewer-about.ui
 src/virt-viewer-app.c
 src/virt-viewer-auth.c
diff --git a/src/Makefile.am b/src/Makefile.am
index 0c48e40..66a73f8 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -13,6 +13,7 @@ noinst_DATA = \
resources/ui/virt-viewer-vm-connection.ui \
resources/ui/virt-viewer-preferences.ui \
resources/ui/remote-viewer-connect.ui \
+   resources/ui/remote-viewer-iso-list.ui \
$(NULL)
 
 EXTRA_DIST =   \
@@ -96,6 +97,8 @@ if HAVE_OVIRT
 libvirt_viewer_la_SOURCES += \
ovirt-foreign-menu.h \
ovirt-foreign-menu.c \
+   remote-viewer-iso-list-dialog.c \
+   remote-viewer-iso-list-dialog.h \
$(NULL)
 endif
 
diff --git a/src/remote-viewer-iso-list-dialog.c 
b/src/remote-viewer-iso-list-dialog.c
new file mode 100644
index 000..a319adb
--- /dev/null
+++ b/src/remote-viewer-iso-list-dialog.c
@@ -0,0 +1,112 @@
+/*
+ * Virt Viewer: A virtual machine console viewer
+ *
+ * Copyright (C) 2016 Red Hat, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#include 
+
+#include 
+
+#include "remote-viewer-iso-list-dialog.h"
+#include "virt-viewer-util.h"
+
+G_DEFINE_TYPE(RemoteViewerISOListDialog, remote_viewer_iso_list_dialog, 
GTK_TYPE_DIALOG)
+
+#define DIALOG_PRIVATE(o) \
+(G_TYPE_INSTANCE_GET_PRIVATE((o), REMOTE_VIEWER_TYPE_ISO_LIST_DIALOG, 
RemoteViewerISOListDialogPrivate))
+
+struct _RemoteViewerISOListDialogPrivate
+{
+GtkWidget *stack;
+};
+
+enum RemoteViewerIsoListDialogPages
+{
+STATUS_PAGE = 0,
+ISO_LIST_PAGE,
+};
+
+static void
+remote_viewer_iso_list_dialog_dispose(GObject *object)
+{
+
G_OBJECT_CLASS(remote_viewer_iso_list_dialog_parent_class)->dispose(object);
+}
+
+static void
+remote_viewer_iso_list_dialog_class_init(RemoteViewerISOListDialogClass *klass)
+{
+GObjectClass *object_class = G_OBJECT_CLASS(klass);
+
+g_type_class_add_private(klass, sizeof(RemoteViewerISOListDialogPrivate));
+
+object_class->dispose = remote_viewer_iso_list_dialog_dispose;
+}
+
+static void
+remote_viewer_iso_list_dialog_response(GtkDialog *dialog,
+   gint response_id,
+   gpointer user_data G_GNUC_UNUSED)
+{
+RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(dialog);
+RemoteViewerISOListDialogPrivate *priv = self->priv;
+
+if (response_id != GTK_RESPONSE_NONE)
+return;
+
+gtk_stack_set_visible_child_full(GTK_STACK(priv->stack), "status",
+ GTK_STACK_TRANSITION_TYPE_SLIDE_LEFT);
+gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, 
FALSE);
+gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_CLOSE, 
FALSE);
+}
+
+static void
+remote_viewer_iso_list_dialog_init(RemoteViewerISOListDialog *self)
+{
+GtkWidget *content = gtk_dialog_get_content_area(GTK_DIALOG(self));
+RemoteViewerISOListDialogPrivate *priv = self->priv = DIALOG_PRIVATE(self);
+GtkBuilder *builder = 
virt_viewer_util_load_ui("remote-viewer-iso-list.ui");
+
+gtk_builder_connect_

[virt-tools-list] [PATCH virt-viewer v2 04/12] ovirt-foreign-menu: Add accessors for current iso and iso list

2016-07-22 Thread Eduardo Lima (Etrunko)
Signed-off-by: Eduardo Lima (Etrunko) 
---
 src/ovirt-foreign-menu.c | 49 ++--
 src/ovirt-foreign-menu.h |  5 -
 2 files changed, 39 insertions(+), 15 deletions(-)

diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c
index 565ef22..eec6bc6 100644
--- a/src/ovirt-foreign-menu.c
+++ b/src/ovirt-foreign-menu.c
@@ -47,6 +47,7 @@ static void 
ovirt_foreign_menu_fetch_storage_domain_async(OvirtForeignMenu *menu
 static void ovirt_foreign_menu_fetch_vm_cdrom_async(OvirtForeignMenu *menu);
 static void ovirt_foreign_menu_refresh_cdrom_file_async(OvirtForeignMenu 
*menu);
 static void ovirt_foreign_menu_fetch_iso_list_async(OvirtForeignMenu *menu);
+static void updated_cdrom_cb(GObject *source_object, GAsyncResult *result, 
gpointer user_data);
 
 G_DEFINE_TYPE (OvirtForeignMenu, ovirt_foreign_menu, G_TYPE_OBJECT)
 
@@ -85,7 +86,7 @@ enum {
 };
 
 
-static char *
+char *
 ovirt_foreign_menu_get_current_iso_name(OvirtForeignMenu *foreign_menu)
 {
 char *name;
@@ -100,6 +101,36 @@ ovirt_foreign_menu_get_current_iso_name(OvirtForeignMenu 
*foreign_menu)
 }
 
 
+void
+ovirt_foreign_menu_set_current_iso_name(OvirtForeignMenu *foreign_menu, char 
*name)
+{
+g_return_if_fail(foreign_menu->priv->cdrom != NULL);
+g_return_if_fail(foreign_menu->priv->next_iso_name == NULL);
+
+if (name) {
+g_debug("Updating VM cdrom image to '%s'", name);
+foreign_menu->priv->next_iso_name = g_strdup(name);
+} else {
+g_debug("Removing current cdrom image");
+foreign_menu->priv->next_iso_name = NULL;
+}
+
+g_object_set(foreign_menu->priv->cdrom,
+ "file", name,
+ NULL);
+ovirt_cdrom_update_async(foreign_menu->priv->cdrom, TRUE,
+ foreign_menu->priv->proxy, NULL,
+ updated_cdrom_cb, foreign_menu);
+}
+
+
+GList*
+ovirt_foreign_menu_get_iso_names(OvirtForeignMenu *foreign_menu)
+{
+return foreign_menu->priv->iso_names;
+}
+
+
 static void
 ovirt_foreign_menu_get_property(GObject *object, guint property_id,
GValue *value, GParamSpec *pspec)
@@ -383,7 +414,7 @@ static void
 ovirt_foreign_menu_activate_item_cb(GtkMenuItem *menuitem, gpointer user_data)
 {
 OvirtForeignMenu *foreign_menu;
-const char *iso_name;
+const char *iso_name = NULL;
 gboolean checked;
 
 checked = gtk_check_menu_item_get_active(GTK_CHECK_MENU_ITEM(menuitem));
@@ -403,19 +434,9 @@ ovirt_foreign_menu_activate_item_cb(GtkMenuItem *menuitem, 
gpointer user_data)
 
 if (checked) {
 iso_name = gtk_menu_item_get_label(menuitem);
-g_debug("Updating VM cdrom image to '%s'", iso_name);
-foreign_menu->priv->next_iso_name = g_strdup(iso_name);
-} else {
-g_debug("Removing current cdrom image");
-iso_name = NULL;
-foreign_menu->priv->next_iso_name = NULL;
 }
-g_object_set(foreign_menu->priv->cdrom,
- "file", iso_name,
- NULL);
-ovirt_cdrom_update_async(foreign_menu->priv->cdrom, TRUE,
- foreign_menu->priv->proxy, NULL,
- updated_cdrom_cb, foreign_menu);
+
+ovirt_foreign_menu_set_current_iso_name(foreign_menu, iso_name);
 }
 
 
diff --git a/src/ovirt-foreign-menu.h b/src/ovirt-foreign-menu.h
index cf18b52..f1a1ddb 100644
--- a/src/ovirt-foreign-menu.h
+++ b/src/ovirt-foreign-menu.h
@@ -70,7 +70,10 @@ OvirtForeignMenu* ovirt_foreign_menu_new(OvirtProxy *proxy);
 OvirtForeignMenu *ovirt_foreign_menu_new_from_file(VirtViewerFile *self);
 void ovirt_foreign_menu_start(OvirtForeignMenu *menu);
 
-GtkWidget *ovirt_foreign_menu_get_gtk_menu(OvirtForeignMenu *foreign_menu);
+char *ovirt_foreign_menu_get_current_iso_name(OvirtForeignMenu *menu);
+void ovirt_foreign_menu_set_current_iso_name(OvirtForeignMenu *menu, char 
*name);
+
+GList *ovirt_foreign_menu_get_iso_names(OvirtForeignMenu *menu);
 
 G_END_DECLS
 
-- 
2.7.4

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


[virt-tools-list] [PATCH virt-viewer v2 09/12] Run iso-dialog when 'Change CD' menu is activated

2016-07-22 Thread Eduardo Lima (Etrunko)
Signed-off-by: Eduardo Lima (Etrunko) 
---
 src/resources/ui/virt-viewer.ui |  1 +
 src/virt-viewer-window.c| 24 
 2 files changed, 25 insertions(+)

diff --git a/src/resources/ui/virt-viewer.ui b/src/resources/ui/virt-viewer.ui
index af3ae46..e9609ec 100644
--- a/src/resources/ui/virt-viewer.ui
+++ b/src/resources/ui/virt-viewer.ui
@@ -78,6 +78,7 @@
 False
 _Change 
CD
 True
+
   
 
 
diff --git a/src/virt-viewer-window.c b/src/virt-viewer-window.c
index 7e6b93f..b82035e 100644
--- a/src/virt-viewer-window.c
+++ b/src/virt-viewer-window.c
@@ -43,6 +43,8 @@
 #include "virt-viewer-util.h"
 #include "virt-viewer-timed-revealer.h"
 
+#include "remote-viewer-iso-list-dialog.h"
+
 #define ZOOM_STEP 10
 
 /* Signal handlers for main window (move in a VirtViewerMainWindow?) */
@@ -62,6 +64,7 @@ void virt_viewer_window_menu_file_smartcard_insert(GtkWidget 
*menu, VirtViewerWi
 void virt_viewer_window_menu_file_smartcard_remove(GtkWidget *menu, 
VirtViewerWindow *self);
 void virt_viewer_window_menu_view_release_cursor(GtkWidget *menu, 
VirtViewerWindow *self);
 void virt_viewer_window_menu_preferences_cb(GtkWidget *menu, VirtViewerWindow 
*self);
+void virt_viewer_window_menu_change_cd_activate(GtkWidget *menu, 
VirtViewerWindow *self);
 
 
 /* Internal methods */
@@ -1052,6 +1055,27 @@ virt_viewer_window_menu_help_about(GtkWidget *menu 
G_GNUC_UNUSED,
 g_object_unref(G_OBJECT(about));
 }
 
+static void
+iso_dialog_response(GtkDialog *dialog,
+gint response_id,
+gpointer user_data G_GNUC_UNUSED)
+{
+if (response_id == GTK_RESPONSE_NONE)
+return;
+
+gtk_widget_destroy(GTK_WIDGET(dialog));
+}
+
+void
+virt_viewer_window_menu_change_cd_activate(GtkWidget *menu G_GNUC_UNUSED,
+   VirtViewerWindow *self)
+{
+VirtViewerWindowPrivate *priv = self->priv;
+GtkWidget *dialog = 
remote_viewer_iso_list_dialog_new(GTK_WINDOW(priv->window));
+g_signal_connect(dialog, "response", G_CALLBACK(iso_dialog_response), 
&dialog);
+gtk_widget_show_all(dialog);
+gtk_dialog_run(GTK_DIALOG(dialog));
+}
 
 static void
 virt_viewer_window_toolbar_setup(VirtViewerWindow *self)
-- 
2.7.4

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


[virt-tools-list] [PATCH virt-viewer v2 10/12] remote-viewer: Make ovirt-foreign-menu a property

2016-07-22 Thread Eduardo Lima (Etrunko)
The OvirtForeignMenu pointer is needed by the new ISO list dialog, and
we make it acessible via property to avoid interdependency between
objects.

Signed-off-by: Eduardo Lima (Etrunko) 
---
 src/remote-viewer-iso-list-dialog.c | 31 +++
 src/remote-viewer-iso-list-dialog.h |  2 +-
 src/remote-viewer.c | 37 +
 src/virt-viewer-window.c| 17 +++--
 4 files changed, 76 insertions(+), 11 deletions(-)

diff --git a/src/remote-viewer-iso-list-dialog.c 
b/src/remote-viewer-iso-list-dialog.c
index a319adb..7a84d0c 100644
--- a/src/remote-viewer-iso-list-dialog.c
+++ b/src/remote-viewer-iso-list-dialog.c
@@ -24,6 +24,7 @@
 
 #include "remote-viewer-iso-list-dialog.h"
 #include "virt-viewer-util.h"
+#include "ovirt-foreign-menu.h"
 
 G_DEFINE_TYPE(RemoteViewerISOListDialog, remote_viewer_iso_list_dialog, 
GTK_TYPE_DIALOG)
 
@@ -33,6 +34,7 @@ G_DEFINE_TYPE(RemoteViewerISOListDialog, 
remote_viewer_iso_list_dialog, GTK_TYPE
 struct _RemoteViewerISOListDialogPrivate
 {
 GtkWidget *stack;
+OvirtForeignMenu *foreign_menu;
 };
 
 enum RemoteViewerIsoListDialogPages
@@ -44,6 +46,10 @@ enum RemoteViewerIsoListDialogPages
 static void
 remote_viewer_iso_list_dialog_dispose(GObject *object)
 {
+RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(object);
+RemoteViewerISOListDialogPrivate *priv = self->priv;
+
+g_clear_object(&priv->foreign_menu);
 
G_OBJECT_CLASS(remote_viewer_iso_list_dialog_parent_class)->dispose(object);
 }
 
@@ -100,13 +106,22 @@ 
remote_viewer_iso_list_dialog_init(RemoteViewerISOListDialog *self)
 }
 
 GtkWidget *
-remote_viewer_iso_list_dialog_new(GtkWindow *parent)
+remote_viewer_iso_list_dialog_new(GtkWindow *parent, GObject *foreign_menu)
 {
-return g_object_new(REMOTE_VIEWER_TYPE_ISO_LIST_DIALOG,
-"title", _("Change CD"),
-"transient-for", parent,
-"border-width", 18,
-"default-width", 400,
-"default-height", 300,
-NULL);
+GtkWidget *dialog;
+RemoteViewerISOListDialog *self;
+
+g_return_val_if_fail(foreign_menu != NULL, NULL);
+
+dialog = g_object_new(REMOTE_VIEWER_TYPE_ISO_LIST_DIALOG,
+ "title", _("Change CD"),
+ "transient-for", parent,
+ "border-width", 18,
+ "default-width", 400,
+ "default-height", 300,
+ NULL);
+
+self = REMOTE_VIEWER_ISO_LIST_DIALOG(dialog);
+self->priv->foreign_menu = OVIRT_FOREIGN_MENU(g_object_ref(foreign_menu));
+return dialog;
 }
diff --git a/src/remote-viewer-iso-list-dialog.h 
b/src/remote-viewer-iso-list-dialog.h
index def841b..8b936f5 100644
--- a/src/remote-viewer-iso-list-dialog.h
+++ b/src/remote-viewer-iso-list-dialog.h
@@ -51,7 +51,7 @@ struct _RemoteViewerISOListDialogClass
 
 GType remote_viewer_iso_list_dialog_get_type(void) G_GNUC_CONST;
 
-GtkWidget *remote_viewer_iso_list_dialog_new(GtkWindow *parent);
+GtkWidget *remote_viewer_iso_list_dialog_new(GtkWindow *parent, GObject 
*foreign_menu);
 
 G_END_DECLS
 
diff --git a/src/remote-viewer.c b/src/remote-viewer.c
index 95130dc..e0cd139 100644
--- a/src/remote-viewer.c
+++ b/src/remote-viewer.c
@@ -67,6 +67,13 @@ G_DEFINE_TYPE (RemoteViewer, remote_viewer, 
VIRT_VIEWER_TYPE_APP)
 #define GET_PRIVATE(o)\
 (G_TYPE_INSTANCE_GET_PRIVATE ((o), REMOTE_VIEWER_TYPE, 
RemoteViewerPrivate))
 
+enum RemoteViewerProperties {
+PROP_0,
+#ifdef HAVE_OVIRT
+PROP_OVIRT_FOREIGN_MENU,
+#endif
+};
+
 #ifdef HAVE_OVIRT
 static OvirtVm * choose_vm(GtkWindow *main_window,
char **vm_name,
@@ -213,6 +220,25 @@ end:
 }
 
 static void
+remote_viewer_get_property(GObject *object, guint property_id,
+   GValue *value, GParamSpec *pspec)
+{
+RemoteViewer *self = REMOTE_VIEWER(object);
+RemoteViewerPrivate *priv = self->priv;
+
+switch (property_id) {
+#ifdef HAVE_OVIRT
+case PROP_OVIRT_FOREIGN_MENU:
+g_value_set_object(value, priv->ovirt_foreign_menu);
+break;
+#endif
+
+default:
+G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec);
+}
+}
+
+static void
 remote_viewer_class_init (RemoteViewerClass *klass)
 {
 GObjectClass *object_class = G_OBJECT_CLASS (klass);
@@ -222,6 +248,7 @@ remote_viewer_class_init (RemoteViewerClass *klass)
 
 g_type_class_add_private (klass, sizeof (RemoteViewerPrivate));
 
+object_class->get_property = remote_viewer_get_prop

[virt-tools-list] [PATCH virt-viewer v3 02/10] ovirt-foreign-menu: Add accessors for current iso and iso list

2016-07-29 Thread Eduardo Lima (Etrunko)
Signed-off-by: Eduardo Lima (Etrunko) 
---
 src/ovirt-foreign-menu.c | 49 ++--
 src/ovirt-foreign-menu.h |  5 -
 2 files changed, 39 insertions(+), 15 deletions(-)

diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c
index 565ef22..eec6bc6 100644
--- a/src/ovirt-foreign-menu.c
+++ b/src/ovirt-foreign-menu.c
@@ -47,6 +47,7 @@ static void 
ovirt_foreign_menu_fetch_storage_domain_async(OvirtForeignMenu *menu
 static void ovirt_foreign_menu_fetch_vm_cdrom_async(OvirtForeignMenu *menu);
 static void ovirt_foreign_menu_refresh_cdrom_file_async(OvirtForeignMenu 
*menu);
 static void ovirt_foreign_menu_fetch_iso_list_async(OvirtForeignMenu *menu);
+static void updated_cdrom_cb(GObject *source_object, GAsyncResult *result, 
gpointer user_data);
 
 G_DEFINE_TYPE (OvirtForeignMenu, ovirt_foreign_menu, G_TYPE_OBJECT)
 
@@ -85,7 +86,7 @@ enum {
 };
 
 
-static char *
+char *
 ovirt_foreign_menu_get_current_iso_name(OvirtForeignMenu *foreign_menu)
 {
 char *name;
@@ -100,6 +101,36 @@ ovirt_foreign_menu_get_current_iso_name(OvirtForeignMenu 
*foreign_menu)
 }
 
 
+void
+ovirt_foreign_menu_set_current_iso_name(OvirtForeignMenu *foreign_menu, char 
*name)
+{
+g_return_if_fail(foreign_menu->priv->cdrom != NULL);
+g_return_if_fail(foreign_menu->priv->next_iso_name == NULL);
+
+if (name) {
+g_debug("Updating VM cdrom image to '%s'", name);
+foreign_menu->priv->next_iso_name = g_strdup(name);
+} else {
+g_debug("Removing current cdrom image");
+foreign_menu->priv->next_iso_name = NULL;
+}
+
+g_object_set(foreign_menu->priv->cdrom,
+ "file", name,
+ NULL);
+ovirt_cdrom_update_async(foreign_menu->priv->cdrom, TRUE,
+ foreign_menu->priv->proxy, NULL,
+ updated_cdrom_cb, foreign_menu);
+}
+
+
+GList*
+ovirt_foreign_menu_get_iso_names(OvirtForeignMenu *foreign_menu)
+{
+return foreign_menu->priv->iso_names;
+}
+
+
 static void
 ovirt_foreign_menu_get_property(GObject *object, guint property_id,
GValue *value, GParamSpec *pspec)
@@ -383,7 +414,7 @@ static void
 ovirt_foreign_menu_activate_item_cb(GtkMenuItem *menuitem, gpointer user_data)
 {
 OvirtForeignMenu *foreign_menu;
-const char *iso_name;
+const char *iso_name = NULL;
 gboolean checked;
 
 checked = gtk_check_menu_item_get_active(GTK_CHECK_MENU_ITEM(menuitem));
@@ -403,19 +434,9 @@ ovirt_foreign_menu_activate_item_cb(GtkMenuItem *menuitem, 
gpointer user_data)
 
 if (checked) {
 iso_name = gtk_menu_item_get_label(menuitem);
-g_debug("Updating VM cdrom image to '%s'", iso_name);
-foreign_menu->priv->next_iso_name = g_strdup(iso_name);
-} else {
-g_debug("Removing current cdrom image");
-iso_name = NULL;
-foreign_menu->priv->next_iso_name = NULL;
 }
-g_object_set(foreign_menu->priv->cdrom,
- "file", iso_name,
- NULL);
-ovirt_cdrom_update_async(foreign_menu->priv->cdrom, TRUE,
- foreign_menu->priv->proxy, NULL,
- updated_cdrom_cb, foreign_menu);
+
+ovirt_foreign_menu_set_current_iso_name(foreign_menu, iso_name);
 }
 
 
diff --git a/src/ovirt-foreign-menu.h b/src/ovirt-foreign-menu.h
index cf18b52..f1a1ddb 100644
--- a/src/ovirt-foreign-menu.h
+++ b/src/ovirt-foreign-menu.h
@@ -70,7 +70,10 @@ OvirtForeignMenu* ovirt_foreign_menu_new(OvirtProxy *proxy);
 OvirtForeignMenu *ovirt_foreign_menu_new_from_file(VirtViewerFile *self);
 void ovirt_foreign_menu_start(OvirtForeignMenu *menu);
 
-GtkWidget *ovirt_foreign_menu_get_gtk_menu(OvirtForeignMenu *foreign_menu);
+char *ovirt_foreign_menu_get_current_iso_name(OvirtForeignMenu *menu);
+void ovirt_foreign_menu_set_current_iso_name(OvirtForeignMenu *menu, char 
*name);
+
+GList *ovirt_foreign_menu_get_iso_names(OvirtForeignMenu *menu);
 
 G_END_DECLS
 
-- 
2.7.4

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


[virt-tools-list] [PATCH virt-viewer v3 07/10] Run iso-dialog when 'Change CD' menu is activated

2016-07-29 Thread Eduardo Lima (Etrunko)
Signed-off-by: Eduardo Lima (Etrunko) 
---
 src/resources/ui/virt-viewer.ui |  1 +
 src/virt-viewer-window.c| 24 
 2 files changed, 25 insertions(+)

diff --git a/src/resources/ui/virt-viewer.ui b/src/resources/ui/virt-viewer.ui
index af3ae46..e9609ec 100644
--- a/src/resources/ui/virt-viewer.ui
+++ b/src/resources/ui/virt-viewer.ui
@@ -78,6 +78,7 @@
 False
 _Change 
CD
 True
+
   
 
 
diff --git a/src/virt-viewer-window.c b/src/virt-viewer-window.c
index 99fd102..d172af6 100644
--- a/src/virt-viewer-window.c
+++ b/src/virt-viewer-window.c
@@ -43,6 +43,8 @@
 #include "virt-viewer-util.h"
 #include "virt-viewer-timed-revealer.h"
 
+#include "remote-viewer-iso-list-dialog.h"
+
 #define ZOOM_STEP 10
 
 /* Signal handlers for main window (move in a VirtViewerMainWindow?) */
@@ -62,6 +64,7 @@ void virt_viewer_window_menu_file_smartcard_insert(GtkWidget 
*menu, VirtViewerWi
 void virt_viewer_window_menu_file_smartcard_remove(GtkWidget *menu, 
VirtViewerWindow *self);
 void virt_viewer_window_menu_view_release_cursor(GtkWidget *menu, 
VirtViewerWindow *self);
 void virt_viewer_window_menu_preferences_cb(GtkWidget *menu, VirtViewerWindow 
*self);
+void virt_viewer_window_menu_change_cd_activate(GtkWidget *menu, 
VirtViewerWindow *self);
 
 
 /* Internal methods */
@@ -1056,6 +1059,27 @@ virt_viewer_window_menu_help_about(GtkWidget *menu 
G_GNUC_UNUSED,
 g_object_unref(G_OBJECT(about));
 }
 
+static void
+iso_dialog_response(GtkDialog *dialog,
+gint response_id,
+gpointer user_data G_GNUC_UNUSED)
+{
+if (response_id == GTK_RESPONSE_NONE)
+return;
+
+gtk_widget_destroy(GTK_WIDGET(dialog));
+}
+
+void
+virt_viewer_window_menu_change_cd_activate(GtkWidget *menu G_GNUC_UNUSED,
+   VirtViewerWindow *self)
+{
+VirtViewerWindowPrivate *priv = self->priv;
+GtkWidget *dialog = 
remote_viewer_iso_list_dialog_new(GTK_WINDOW(priv->window));
+g_signal_connect(dialog, "response", G_CALLBACK(iso_dialog_response), 
NULL);
+gtk_widget_show_all(dialog);
+gtk_dialog_run(GTK_DIALOG(dialog));
+}
 
 static void
 virt_viewer_window_toolbar_setup(VirtViewerWindow *self)
-- 
2.7.4

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


[virt-tools-list] [PATCH virt-viewer v3 03/10] ovirt-foreign-menu: Remove GtkMenu related functions

2016-07-29 Thread Eduardo Lima (Etrunko)
With this commit, we finish cleaning up ovirt foreign menu code, which
only deals with data, leaving the UI bits to be handled properly in the
new ISO list dialog.

This patch also updates remote-viewer to reflect the change.

Signed-off-by: Eduardo Lima (Etrunko) 
---
 src/ovirt-foreign-menu.c | 79 
 src/remote-viewer.c  | 52 +--
 2 files changed, 8 insertions(+), 123 deletions(-)

diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c
index eec6bc6..493ca47 100644
--- a/src/ovirt-foreign-menu.c
+++ b/src/ovirt-foreign-menu.c
@@ -359,22 +359,6 @@ ovirt_foreign_menu_start(OvirtForeignMenu *menu)
 }
 
 
-static void
-ovirt_foreign_menu_activate_item_cb(GtkMenuItem *menuitem, gpointer user_data);
-
-
-static void
-menu_item_set_active_no_signal(GtkMenuItem *menuitem,
-   gboolean active,
-   GCallback callback,
-   gpointer user_data)
-{
-g_signal_handlers_block_by_func(menuitem, callback, user_data);
-gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(menuitem), active);
-g_signal_handlers_unblock_by_func(menuitem, callback, user_data);
-}
-
-
 static void updated_cdrom_cb(GObject *source_object,
  GAsyncResult *result,
  gpointer user_data)
@@ -410,69 +394,6 @@ static void updated_cdrom_cb(GObject *source_object,
 }
 
 
-static void
-ovirt_foreign_menu_activate_item_cb(GtkMenuItem *menuitem, gpointer user_data)
-{
-OvirtForeignMenu *foreign_menu;
-const char *iso_name = NULL;
-gboolean checked;
-
-checked = gtk_check_menu_item_get_active(GTK_CHECK_MENU_ITEM(menuitem));
-foreign_menu = OVIRT_FOREIGN_MENU(user_data);
-g_return_if_fail(foreign_menu->priv->cdrom != NULL);
-g_return_if_fail(foreign_menu->priv->next_iso_name == NULL);
-
-g_debug("'%s' clicked", gtk_menu_item_get_label(menuitem));
-
-/* We only want to move the check mark for the currently selected ISO
- * when ovirt_cdrom_update_async() is successful, so for now we move
- * the check mark back to where it was before
- */
-menu_item_set_active_no_signal(menuitem, !checked,
-   
(GCallback)ovirt_foreign_menu_activate_item_cb,
-   foreign_menu);
-
-if (checked) {
-iso_name = gtk_menu_item_get_label(menuitem);
-}
-
-ovirt_foreign_menu_set_current_iso_name(foreign_menu, iso_name);
-}
-
-
-GtkWidget *ovirt_foreign_menu_get_gtk_menu(OvirtForeignMenu *foreign_menu)
-{
-GtkWidget *gtk_menu;
-GList *it;
-char *current_iso;
-
-if (foreign_menu->priv->iso_names == NULL) {
-g_debug("ISO list is empty, no menu to show");
-return NULL;
-}
-g_debug("Creating GtkMenu for foreign menu");
-current_iso = ovirt_foreign_menu_get_current_iso_name(foreign_menu);
-gtk_menu = gtk_menu_new();
-for (it = foreign_menu->priv->iso_names; it != NULL; it = it->next) {
-GtkWidget *menuitem;
-
-menuitem = gtk_check_menu_item_new_with_label((char *)it->data);
-if (g_strcmp0((char *)it->data, current_iso) == 0) {
-g_warn_if_fail(g_strcmp0(current_iso, 
foreign_menu->priv->current_iso_name) == 0);
-gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(menuitem),
-   TRUE);
-}
-g_signal_connect(menuitem, "activate",
- G_CALLBACK(ovirt_foreign_menu_activate_item_cb),
- foreign_menu);
-gtk_menu_shell_append(GTK_MENU_SHELL(gtk_menu), menuitem);
-}
-g_free(current_iso);
-
-return gtk_menu;
-}
-
-
 static void ovirt_foreign_menu_set_files(OvirtForeignMenu *menu,
  const GList *files)
 {
diff --git a/src/remote-viewer.c b/src/remote-viewer.c
index 6d29bf2..95130dc 100644
--- a/src/remote-viewer.c
+++ b/src/remote-viewer.c
@@ -734,33 +734,11 @@ authenticate_cb(RestProxy *proxy, G_GNUC_UNUSED 
RestProxyAuth *auth,
 static void
 ovirt_foreign_menu_update(GtkApplication *gtkapp, GtkWindow *gtkwin, 
G_GNUC_UNUSED gpointer data)
 {
-RemoteViewer *app = REMOTE_VIEWER(gtkapp);
+RemoteViewer *self = REMOTE_VIEWER(gtkapp);
 VirtViewerWindow *win = g_object_get_data(G_OBJECT(gtkwin), 
"virt-viewer-window");
-GtkWidget *menu = g_object_get_data(G_OBJECT(win), "foreign-menu");
-GtkWidget *submenu;
-
-if (app->priv->ovirt_foreign_menu == NULL) {
-/* nothing to do */
-return;
-}
-
-submenu = ovirt_foreign_menu_get_gtk_menu(app->priv->ovirt_foreign_menu);
-if (submenu == NULL) {
-/* No items to show, no point in showing the menu */
-if (menu != NULL)
-   gtk_widget_set_visible(menu,

[virt-tools-list] [PATCH virt-viewer v3 08/10] remote-viewer: Make ovirt-foreign-menu a property

2016-07-29 Thread Eduardo Lima (Etrunko)
The OvirtForeignMenu pointer is needed by the new ISO list dialog, and
we make it acessible via property to avoid interdependency between
objects.

Signed-off-by: Eduardo Lima (Etrunko) 
---
 src/remote-viewer-iso-list-dialog.c | 31 +++
 src/remote-viewer-iso-list-dialog.h |  2 +-
 src/remote-viewer.c | 37 +
 src/virt-viewer-window.c| 15 ++-
 4 files changed, 75 insertions(+), 10 deletions(-)

diff --git a/src/remote-viewer-iso-list-dialog.c 
b/src/remote-viewer-iso-list-dialog.c
index 0fbea28..858719c 100644
--- a/src/remote-viewer-iso-list-dialog.c
+++ b/src/remote-viewer-iso-list-dialog.c
@@ -24,6 +24,7 @@
 
 #include "remote-viewer-iso-list-dialog.h"
 #include "virt-viewer-util.h"
+#include "ovirt-foreign-menu.h"
 
 G_DEFINE_TYPE(RemoteViewerISOListDialog, remote_viewer_iso_list_dialog, 
GTK_TYPE_DIALOG)
 
@@ -33,11 +34,16 @@ G_DEFINE_TYPE(RemoteViewerISOListDialog, 
remote_viewer_iso_list_dialog, GTK_TYPE
 struct _RemoteViewerISOListDialogPrivate
 {
 GtkWidget *stack;
+OvirtForeignMenu *foreign_menu;
 };
 
 static void
 remote_viewer_iso_list_dialog_dispose(GObject *object)
 {
+RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(object);
+RemoteViewerISOListDialogPrivate *priv = self->priv;
+
+g_clear_object(&priv->foreign_menu);
 
G_OBJECT_CLASS(remote_viewer_iso_list_dialog_parent_class)->dispose(object);
 }
 
@@ -94,13 +100,22 @@ 
remote_viewer_iso_list_dialog_init(RemoteViewerISOListDialog *self)
 }
 
 GtkWidget *
-remote_viewer_iso_list_dialog_new(GtkWindow *parent)
+remote_viewer_iso_list_dialog_new(GtkWindow *parent, GObject *foreign_menu)
 {
-return g_object_new(REMOTE_VIEWER_TYPE_ISO_LIST_DIALOG,
-"title", _("Change CD"),
-"transient-for", parent,
-"border-width", 18,
-"default-width", 400,
-"default-height", 300,
-NULL);
+GtkWidget *dialog;
+RemoteViewerISOListDialog *self;
+
+g_return_val_if_fail(foreign_menu != NULL, NULL);
+
+dialog = g_object_new(REMOTE_VIEWER_TYPE_ISO_LIST_DIALOG,
+  "title", _("Change CD"),
+  "transient-for", parent,
+  "border-width", 18,
+  "default-width", 400,
+  "default-height", 300,
+  NULL);
+
+self = REMOTE_VIEWER_ISO_LIST_DIALOG(dialog);
+self->priv->foreign_menu = OVIRT_FOREIGN_MENU(g_object_ref(foreign_menu));
+return dialog;
 }
diff --git a/src/remote-viewer-iso-list-dialog.h 
b/src/remote-viewer-iso-list-dialog.h
index def841b..8b936f5 100644
--- a/src/remote-viewer-iso-list-dialog.h
+++ b/src/remote-viewer-iso-list-dialog.h
@@ -51,7 +51,7 @@ struct _RemoteViewerISOListDialogClass
 
 GType remote_viewer_iso_list_dialog_get_type(void) G_GNUC_CONST;
 
-GtkWidget *remote_viewer_iso_list_dialog_new(GtkWindow *parent);
+GtkWidget *remote_viewer_iso_list_dialog_new(GtkWindow *parent, GObject 
*foreign_menu);
 
 G_END_DECLS
 
diff --git a/src/remote-viewer.c b/src/remote-viewer.c
index 95130dc..e0cd139 100644
--- a/src/remote-viewer.c
+++ b/src/remote-viewer.c
@@ -67,6 +67,13 @@ G_DEFINE_TYPE (RemoteViewer, remote_viewer, 
VIRT_VIEWER_TYPE_APP)
 #define GET_PRIVATE(o)\
 (G_TYPE_INSTANCE_GET_PRIVATE ((o), REMOTE_VIEWER_TYPE, 
RemoteViewerPrivate))
 
+enum RemoteViewerProperties {
+PROP_0,
+#ifdef HAVE_OVIRT
+PROP_OVIRT_FOREIGN_MENU,
+#endif
+};
+
 #ifdef HAVE_OVIRT
 static OvirtVm * choose_vm(GtkWindow *main_window,
char **vm_name,
@@ -213,6 +220,25 @@ end:
 }
 
 static void
+remote_viewer_get_property(GObject *object, guint property_id,
+   GValue *value, GParamSpec *pspec)
+{
+RemoteViewer *self = REMOTE_VIEWER(object);
+RemoteViewerPrivate *priv = self->priv;
+
+switch (property_id) {
+#ifdef HAVE_OVIRT
+case PROP_OVIRT_FOREIGN_MENU:
+g_value_set_object(value, priv->ovirt_foreign_menu);
+break;
+#endif
+
+default:
+G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec);
+}
+}
+
+static void
 remote_viewer_class_init (RemoteViewerClass *klass)
 {
 GObjectClass *object_class = G_OBJECT_CLASS (klass);
@@ -222,6 +248,7 @@ remote_viewer_class_init (RemoteViewerClass *klass)
 
 g_type_class_add_private (klass, sizeof (RemoteViewerPrivate));
 
+object_class->get_property = remote_viewer_get_property;
 object_class->dispose = remote_viewer_dispose;
 
 g_app_class->local_command_line = remote_viewer_local_command_line;
@

[virt-tools-list] [PATCH virt-viewer v3 01/10] ovirt-foreign-menu: Remove timer used to refresh iso list

2016-07-29 Thread Eduardo Lima (Etrunko)
With the new ISO dialog, the user triggers the refresh manually.

Signed-off-by: Eduardo Lima (Etrunko) 
---
 src/ovirt-foreign-menu.c | 23 +++
 1 file changed, 3 insertions(+), 20 deletions(-)

diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c
index a51f2c9..565ef22 100644
--- a/src/ovirt-foreign-menu.c
+++ b/src/ovirt-foreign-menu.c
@@ -46,7 +46,7 @@ static void 
ovirt_foreign_menu_fetch_vm_async(OvirtForeignMenu *menu);
 static void ovirt_foreign_menu_fetch_storage_domain_async(OvirtForeignMenu 
*menu);
 static void ovirt_foreign_menu_fetch_vm_cdrom_async(OvirtForeignMenu *menu);
 static void ovirt_foreign_menu_refresh_cdrom_file_async(OvirtForeignMenu 
*menu);
-static gboolean ovirt_foreign_menu_refresh_iso_list(gpointer user_data);
+static void ovirt_foreign_menu_fetch_iso_list_async(OvirtForeignMenu *menu);
 
 G_DEFINE_TYPE (OvirtForeignMenu, ovirt_foreign_menu, G_TYPE_OBJECT)
 
@@ -313,7 +313,7 @@ ovirt_foreign_menu_next_async_step(OvirtForeignMenu *menu,
 g_warn_if_fail(menu->priv->files != NULL);
 g_warn_if_fail(menu->priv->cdrom != NULL);
 
-ovirt_foreign_menu_refresh_iso_list(menu);
+ovirt_foreign_menu_fetch_iso_list_async(menu);
 break;
 default:
 g_warn_if_reached();
@@ -754,8 +754,6 @@ static void iso_list_fetched_cb(GObject *source_object,
 files = 
g_hash_table_get_values(ovirt_collection_get_resources(collection));
 ovirt_foreign_menu_set_files(OVIRT_FOREIGN_MENU(user_data), files);
 g_list_free(files);
-
-g_timeout_add_seconds(300, ovirt_foreign_menu_refresh_iso_list, user_data);
 }
 
 
@@ -765,27 +763,12 @@ static void 
ovirt_foreign_menu_fetch_iso_list_async(OvirtForeignMenu *menu)
 return;
 }
 
+g_debug("Refreshing foreign menu iso list");
 ovirt_collection_fetch_async(menu->priv->files, menu->priv->proxy,
  NULL, iso_list_fetched_cb, menu);
 }
 
 
-static gboolean ovirt_foreign_menu_refresh_iso_list(gpointer user_data)
-{
-OvirtForeignMenu *menu;
-
-g_debug("Refreshing foreign menu iso list");
-menu = OVIRT_FOREIGN_MENU(user_data);
-ovirt_foreign_menu_fetch_iso_list_async(menu);
-
-/* ovirt_foreign_menu_fetch_iso_list_async() will schedule a new call to
- * that function through iso_list_fetched_cb() when it has finished
- * fetching the iso list
- */
-return G_SOURCE_REMOVE;
-}
-
-
 OvirtForeignMenu *ovirt_foreign_menu_new_from_file(VirtViewerFile *file)
 {
 OvirtProxy *proxy = NULL;
-- 
2.7.4

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


[virt-tools-list] [PATCH virt-viewer v3 05/10] UI: Make 'Change CD' menu item a submenu under 'File' toplevel menu

2016-07-29 Thread Eduardo Lima (Etrunko)
Signed-off-by: Eduardo Lima (Etrunko) 
---
 src/resources/ui/virt-viewer.ui | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/resources/ui/virt-viewer.ui b/src/resources/ui/virt-viewer.ui
index 6e3c5ad..af3ae46 100644
--- a/src/resources/ui/virt-viewer.ui
+++ b/src/resources/ui/virt-viewer.ui
@@ -1,6 +1,7 @@
 
+
 
-  
+  
   
   
 False
@@ -73,6 +74,13 @@
   
 
 
+  
+False
+_Change 
CD
+True
+  
+
+
   
 True
 False
@@ -247,14 +255,6 @@
 
   
 
-
-  
-False
-False
-_Change 
CD
-True
-  
-
   
   
 False
-- 
2.7.4

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


[virt-tools-list] [PATCH virt-viewer v3 04/10] ovirt-foreign-menu: Notify of new files even if nothing changed

2016-07-29 Thread Eduardo Lima (Etrunko)
When user presses the Refresh button in ISO dialog, the list is cleared, and
currently, the only way it is informed of the new list is by the notify signal.
The same applies when an error occurs while trying to change the current ISO.

Signed-off-by: Eduardo Lima (Etrunko) 
---
 src/ovirt-foreign-menu.c | 43 +++
 1 file changed, 15 insertions(+), 28 deletions(-)

diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c
index 493ca47..8320552 100644
--- a/src/ovirt-foreign-menu.c
+++ b/src/ovirt-foreign-menu.c
@@ -375,22 +375,24 @@ static void updated_cdrom_cb(GObject *source_object,
 g_free(foreign_menu->priv->current_iso_name);
 foreign_menu->priv->current_iso_name = 
foreign_menu->priv->next_iso_name;
 foreign_menu->priv->next_iso_name = NULL;
-g_object_notify(G_OBJECT(foreign_menu), "file");
-} else {
-/* Reset old state back as we were not successful in switching to
- * the new ISO */
-const char *current_file = foreign_menu->priv->current_iso_name;
-
-if (error != NULL) {
-g_warning("failed to update cdrom resource: %s", error->message);
-g_clear_error(&error);
-}
-g_debug("setting OvirtCdrom:file back to '%s'",
-current_file?current_file:NULL);
-g_object_set(foreign_menu->priv->cdrom, "file", current_file, NULL);
+goto end;
 }
 
+/* Reset old state back as we were not successful in switching to
+ * the new ISO */
+if (error != NULL) {
+g_warning("failed to update cdrom resource: %s", error->message);
+g_clear_error(&error);
+}
+g_debug("setting OvirtCdrom:file back to '%s'",
+foreign_menu->priv->current_iso_name);
+g_object_set(foreign_menu->priv->cdrom,
+ "file", foreign_menu->priv->current_iso_name,
+ NULL);
 g_clear_pointer(&foreign_menu->priv->next_iso_name, g_free);
+
+end:
+g_object_notify(G_OBJECT(foreign_menu), "file");
 }
 
 
@@ -399,7 +401,6 @@ static void ovirt_foreign_menu_set_files(OvirtForeignMenu 
*menu,
 {
 GList *sorted_files = NULL;
 const GList *it;
-GList *it2;
 
 for (it = files; it != NULL; it = it->next) {
 char *name;
@@ -416,20 +417,6 @@ static void ovirt_foreign_menu_set_files(OvirtForeignMenu 
*menu,
 (GCompareFunc)g_strcmp0);
 }
 
-for (it = sorted_files, it2 = menu->priv->iso_names;
- (it != NULL) && (it2 != NULL);
- it = it->next, it2 = it2->next) {
-if (g_strcmp0(it->data, it2->data) != 0) {
-break;
-}
-}
-
-if ((it == NULL) && (it2 == NULL)) {
-/* sorted_files and menu->priv->files content was the same */
-g_list_free_full(sorted_files, (GDestroyNotify)g_free);
-return;
-}
-
 g_list_free_full(menu->priv->iso_names, (GDestroyNotify)g_free);
 menu->priv->iso_names = sorted_files;
 g_object_notify(G_OBJECT(menu), "files");
-- 
2.7.4

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


[virt-tools-list] [PATCH virt-viewer v3 00/10] Replace oVirt foreign menu with dedicated dialog

2016-07-29 Thread Eduardo Lima (Etrunko)
I have pushed the first two patches of the series because they were
already acknowledged and were pretty much self-contained.

I tried to replace GtkTreeView in favor of new GtkListBox, but after some
painful work, I decided to drop it because it is not possible to
reproduce the exact same behavior of the former with the latter. For
instance, once we have a GtkRadioButton activated, it is not possible to
deactivate it. We also need to create a new GObject that would be used
as the model, while with a tree view, things can be automatically done
with Glade. Finally, it would be necessary to raise the requirements for
both gtk+ (3.16) and glib (2.44). If some one wants to take a look on
the work in progress, check out the 'dialog' branch in my github clone
(http://github.com/etrunko/virt-viewer).

In this version:

 * Removed leftover enum when changed from GtkNotebook to GtkStack
 * Some cosmetic fixes (indentation, renaming, etc)
 * UI Tweaks:
   - Added some padding between items of the list.
   - Set tree view selection to current ISO when the list is first loaded
 or refreshed.
   - Handle tree view "activate" signal the same way as radio button
 toggle
   - Removed "Select ISO" label and alignment in header bar patch.

Eduardo Lima (Etrunko) (10):
  ovirt-foreign-menu: Remove timer used to refresh iso list
  ovirt-foreign-menu: Add accessors for current iso and iso list
  ovirt-foreign-menu: Remove GtkMenu related functions
  ovirt-foreign-menu: Notify of new files even if nothing changed
  UI: Make 'Change CD' menu item a submenu under 'File' toplevel menu
  Introduce ISO List dialog
  Run iso-dialog when 'Change CD' menu is activated
  remote-viewer: Make ovirt-foreign-menu a property
  iso-dialog: Implement functionality provided by oVirt foreign menu
  iso-dialog: Use header bar for buttons

 configure.ac   |   4 +-
 po/POTFILES.in |   2 +
 src/Makefile.am|   3 +
 src/ovirt-foreign-menu.c   | 182 +
 src/ovirt-foreign-menu.h   |   5 +-
 src/remote-viewer-iso-list-dialog.c| 316 +
 src/remote-viewer-iso-list-dialog.h|  58 ++
 src/remote-viewer.c|  89 
 src/resources/ui/remote-viewer-iso-list.ui | 135 
 src/resources/ui/virt-viewer.ui|  19 +-
 src/resources/virt-viewer.gresource.xml|   1 +
 src/virt-viewer-window.c   |  37 
 12 files changed, 660 insertions(+), 191 deletions(-)
 create mode 100644 src/remote-viewer-iso-list-dialog.c
 create mode 100644 src/remote-viewer-iso-list-dialog.h
 create mode 100644 src/resources/ui/remote-viewer-iso-list.ui

-- 
2.7.4

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


[virt-tools-list] [PATCH virt-viewer v3 09/10] iso-dialog: Implement functionality provided by oVirt foreign menu

2016-07-29 Thread Eduardo Lima (Etrunko)
Signed-off-by: Eduardo Lima (Etrunko) 
---
 src/remote-viewer-iso-list-dialog.c| 180 -
 src/resources/ui/remote-viewer-iso-list.ui |   5 +-
 2 files changed, 181 insertions(+), 4 deletions(-)

diff --git a/src/remote-viewer-iso-list-dialog.c 
b/src/remote-viewer-iso-list-dialog.c
index 858719c..4a1cf44 100644
--- a/src/remote-viewer-iso-list-dialog.c
+++ b/src/remote-viewer-iso-list-dialog.c
@@ -20,6 +20,7 @@
 
 #include 
 
+#include 
 #include 
 
 #include "remote-viewer-iso-list-dialog.h"
@@ -33,17 +34,32 @@ G_DEFINE_TYPE(RemoteViewerISOListDialog, 
remote_viewer_iso_list_dialog, GTK_TYPE
 
 struct _RemoteViewerISOListDialogPrivate
 {
+GtkListStore *list_store;
 GtkWidget *stack;
+GtkWidget *tree_view;
 OvirtForeignMenu *foreign_menu;
 };
 
+enum RemoteViewerISOListDialogModel
+{
+ISO_IS_ACTIVE = 0,
+ISO_NAME,
+FONT_WEIGHT,
+};
+
+void remote_viewer_iso_list_dialog_toggled(GtkCellRendererToggle 
*cell_renderer, gchar *path, gpointer user_data);
+void remote_viewer_iso_list_dialog_row_activated(GtkTreeView *view, 
GtkTreePath *path, GtkTreeViewColumn *col, gpointer user_data);
+
 static void
 remote_viewer_iso_list_dialog_dispose(GObject *object)
 {
 RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(object);
 RemoteViewerISOListDialogPrivate *priv = self->priv;
 
-g_clear_object(&priv->foreign_menu);
+if (priv->foreign_menu) {
+g_signal_handlers_disconnect_by_data(priv->foreign_menu, object);
+g_clear_object(&priv->foreign_menu);
+}
 
G_OBJECT_CLASS(remote_viewer_iso_list_dialog_parent_class)->dispose(object);
 }
 
@@ -58,6 +74,58 @@ 
remote_viewer_iso_list_dialog_class_init(RemoteViewerISOListDialogClass *klass)
 }
 
 static void
+remote_viewer_iso_list_dialog_show_files(RemoteViewerISOListDialog *self)
+{
+RemoteViewerISOListDialogPrivate *priv = self->priv = DIALOG_PRIVATE(self);
+gtk_stack_set_visible_child_full(GTK_STACK(priv->stack), "iso-list",
+ GTK_STACK_TRANSITION_TYPE_SLIDE_LEFT);
+gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, 
TRUE);
+}
+
+static void
+remote_viewer_iso_list_dialog_foreach(char *name, RemoteViewerISOListDialog 
*self)
+{
+RemoteViewerISOListDialogPrivate *priv = self->priv;
+char * current_iso = 
ovirt_foreign_menu_get_current_iso_name(self->priv->foreign_menu);
+gboolean active = g_strcmp0(current_iso, name) == 0;
+gint weight = active ? PANGO_WEIGHT_BOLD : PANGO_WEIGHT_NORMAL;
+GtkTreeIter iter;
+
+gtk_list_store_append(priv->list_store, &iter);
+gtk_list_store_set(priv->list_store, &iter,
+   ISO_IS_ACTIVE, active,
+   ISO_NAME, name,
+   FONT_WEIGHT, weight, -1);
+
+if (active) {
+GtkTreePath *path = 
gtk_tree_model_get_path(GTK_TREE_MODEL(priv->list_store), &iter);
+gtk_tree_view_set_cursor(GTK_TREE_VIEW(priv->tree_view), path, NULL, 
FALSE);
+gtk_tree_view_scroll_to_cell(GTK_TREE_VIEW(priv->tree_view), path, 
NULL, TRUE, 0.5, 0.5);
+gtk_tree_path_free(path);
+}
+
+free(current_iso);
+}
+
+static void
+remote_viewer_iso_list_dialog_refresh_iso_list(RemoteViewerISOListDialog *self,
+   gboolean refreshing)
+{
+RemoteViewerISOListDialogPrivate *priv = self->priv;
+GList *iso_list;
+
+if (refreshing) {
+gtk_list_store_clear(priv->list_store);
+ovirt_foreign_menu_start(priv->foreign_menu);
+return;
+}
+
+iso_list = ovirt_foreign_menu_get_iso_names(priv->foreign_menu);
+g_list_foreach(iso_list, (GFunc) remote_viewer_iso_list_dialog_foreach, 
self);
+remote_viewer_iso_list_dialog_show_files(self);
+}
+
+static void
 remote_viewer_iso_list_dialog_response(GtkDialog *dialog,
gint response_id,
gpointer user_data G_GNUC_UNUSED)
@@ -71,7 +139,45 @@ remote_viewer_iso_list_dialog_response(GtkDialog *dialog,
 gtk_stack_set_visible_child_full(GTK_STACK(priv->stack), "status",
  GTK_STACK_TRANSITION_TYPE_SLIDE_LEFT);
 gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, 
FALSE);
-gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_CLOSE, 
FALSE);
+remote_viewer_iso_list_dialog_refresh_iso_list(self, TRUE);
+}
+
+void
+remote_viewer_iso_list_dialog_toggled(GtkCellRendererToggle *cell_renderer 
G_GNUC_UNUSED,
+  gchar *path,
+  gpointer user_data)
+{
+RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(user_data);
+RemoteViewerISOListDialogPrivate *priv = self->priv;
+GtkTreeModel *model = GTK_TR

[virt-tools-list] [PATCH virt-viewer v3 10/10] iso-dialog: Use header bar for buttons

2016-07-29 Thread Eduardo Lima (Etrunko)
It seems to give more modern look to the dialog, but it requires Gtk+
3.12, thus I will am keeping this commit separated.

On the glade UI file, we get rid of the GtkAlignment object that was
used to put some space between the tree view and dialog buttons. The
"Select ISO" label is gone too.

Signed-off-by: Eduardo Lima (Etrunko) 
---
 configure.ac   |  4 +-
 src/remote-viewer-iso-list-dialog.c| 31 +--
 src/resources/ui/remote-viewer-iso-list.ui | 87 +++---
 3 files changed, 60 insertions(+), 62 deletions(-)

diff --git a/configure.ac b/configure.ac
index 7c437a3..7682f42 100644
--- a/configure.ac
+++ b/configure.ac
@@ -17,8 +17,8 @@ GLIB2_REQUIRED="2.38"
 GLIB2_ENCODED_VERSION="GLIB_VERSION_2_38"
 
 # Keep these two definitions in agreement.
-GTK_REQUIRED="3.10"
-GTK_ENCODED_VERSION="GDK_VERSION_3_10"
+GTK_REQUIRED="3.12"
+GTK_ENCODED_VERSION="GDK_VERSION_3_12"
 
 LIBXML2_REQUIRED="2.6.0"
 LIBVIRT_REQUIRED="0.10.0"
diff --git a/src/remote-viewer-iso-list-dialog.c 
b/src/remote-viewer-iso-list-dialog.c
index 4a1cf44..dd652d7 100644
--- a/src/remote-viewer-iso-list-dialog.c
+++ b/src/remote-viewer-iso-list-dialog.c
@@ -34,6 +34,7 @@ G_DEFINE_TYPE(RemoteViewerISOListDialog, 
remote_viewer_iso_list_dialog, GTK_TYPE
 
 struct _RemoteViewerISOListDialogPrivate
 {
+GtkHeaderBar *header_bar;
 GtkListStore *list_store;
 GtkWidget *stack;
 GtkWidget *tree_view;
@@ -83,6 +84,19 @@ 
remote_viewer_iso_list_dialog_show_files(RemoteViewerISOListDialog *self)
 }
 
 static void
+remote_viewer_iso_list_dialog_set_subtitle(RemoteViewerISOListDialog *self, 
const char *iso_name)
+{
+RemoteViewerISOListDialogPrivate *priv = self->priv;
+gchar *subtitle = NULL;
+
+if (iso_name && strlen(iso_name) != 0)
+subtitle = g_strdup_printf(_("Current: %s"), iso_name);
+
+gtk_header_bar_set_subtitle(priv->header_bar, subtitle);
+g_free(subtitle);
+}
+
+static void
 remote_viewer_iso_list_dialog_foreach(char *name, RemoteViewerISOListDialog 
*self)
 {
 RemoteViewerISOListDialogPrivate *priv = self->priv;
@@ -102,6 +116,7 @@ remote_viewer_iso_list_dialog_foreach(char *name, 
RemoteViewerISOListDialog *sel
 gtk_tree_view_set_cursor(GTK_TREE_VIEW(priv->tree_view), path, NULL, 
FALSE);
 gtk_tree_view_scroll_to_cell(GTK_TREE_VIEW(priv->tree_view), path, 
NULL, TRUE, 0.5, 0.5);
 gtk_tree_path_free(path);
+remote_viewer_iso_list_dialog_set_subtitle(self, current_iso);
 }
 
 free(current_iso);
@@ -136,6 +151,7 @@ remote_viewer_iso_list_dialog_response(GtkDialog *dialog,
 if (response_id != GTK_RESPONSE_NONE)
 return;
 
+remote_viewer_iso_list_dialog_set_subtitle(self, NULL);
 gtk_stack_set_visible_child_full(GTK_STACK(priv->stack), "status",
  GTK_STACK_TRANSITION_TYPE_SLIDE_LEFT);
 gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, 
FALSE);
@@ -187,9 +203,13 @@ 
remote_viewer_iso_list_dialog_init(RemoteViewerISOListDialog *self)
 RemoteViewerISOListDialogPrivate *priv = self->priv = DIALOG_PRIVATE(self);
 GtkBuilder *builder = 
virt_viewer_util_load_ui("remote-viewer-iso-list.ui");
 GtkCellRendererToggle *cell_renderer;
+GtkWidget *button, *image;
 
 gtk_builder_connect_signals(builder, self);
 
+priv->header_bar = 
GTK_HEADER_BAR(gtk_dialog_get_header_bar(GTK_DIALOG(self)));
+gtk_header_bar_set_has_subtitle(priv->header_bar, TRUE);
+
 priv->stack = GTK_WIDGET(gtk_builder_get_object(builder, "stack"));
 gtk_box_pack_start(GTK_BOX(content), priv->stack, TRUE, TRUE, 0);
 
@@ -201,12 +221,11 @@ 
remote_viewer_iso_list_dialog_init(RemoteViewerISOListDialog *self)
 
 g_object_unref(builder);
 
-gtk_dialog_add_buttons(GTK_DIALOG(self),
-   _("Refresh"), GTK_RESPONSE_NONE,
-   _("Close"), GTK_RESPONSE_CLOSE,
-   NULL);
+button = gtk_dialog_add_button(GTK_DIALOG(self), "", GTK_RESPONSE_NONE);
+image = gtk_image_new_from_icon_name("view-refresh-symbolic", 
GTK_ICON_SIZE_BUTTON);
+gtk_button_set_image(GTK_BUTTON(button), image);
+gtk_button_set_always_show_image(GTK_BUTTON(button), TRUE);
 
-gtk_dialog_set_default_response(GTK_DIALOG(self), GTK_RESPONSE_CLOSE);
 gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, 
FALSE);
 g_signal_connect(self, "response", 
G_CALLBACK(remote_viewer_iso_list_dialog_response), NULL);
 }
@@ -246,6 +265,7 @@ ovirt_foreign_menu_notify_file(OvirtForeignMenu 
*foreign_menu,
 g_free(name);
 } while (gtk_tree_model_iter_next(model, &iter));
 
+remote_viewer_iso_list_dialog_set_subtitle(self, 

[virt-tools-list] [PATCH virt-viewer v3 06/10] Introduce ISO List dialog

2016-07-29 Thread Eduardo Lima (Etrunko)
Signed-off-by: Eduardo Lima (Etrunko) 
---
 po/POTFILES.in |   2 +
 src/Makefile.am|   3 +
 src/remote-viewer-iso-list-dialog.c| 106 
 src/remote-viewer-iso-list-dialog.h|  58 +++
 src/resources/ui/remote-viewer-iso-list.ui | 155 +
 src/resources/virt-viewer.gresource.xml|   1 +
 6 files changed, 325 insertions(+)
 create mode 100644 src/remote-viewer-iso-list-dialog.c
 create mode 100644 src/remote-viewer-iso-list-dialog.h
 create mode 100644 src/resources/ui/remote-viewer-iso-list.ui

diff --git a/po/POTFILES.in b/po/POTFILES.in
index 6775f53..54de445 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -1,9 +1,11 @@
 data/remote-viewer.appdata.xml.in
 data/remote-viewer.desktop.in
 data/virt-viewer-mime.xml.in
+src/remote-viewer-iso-list-dialog.c
 src/remote-viewer-main.c
 src/remote-viewer.c
 [type: gettext/glade] src/resources/ui/remote-viewer-connect.ui
+[type: gettext/glade] src/resources/ui/remote-viewer-iso-list.ui
 [type: gettext/glade] src/resources/ui/virt-viewer-about.ui
 src/virt-viewer-app.c
 src/virt-viewer-auth.c
diff --git a/src/Makefile.am b/src/Makefile.am
index 0c48e40..66a73f8 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -13,6 +13,7 @@ noinst_DATA = \
resources/ui/virt-viewer-vm-connection.ui \
resources/ui/virt-viewer-preferences.ui \
resources/ui/remote-viewer-connect.ui \
+   resources/ui/remote-viewer-iso-list.ui \
$(NULL)
 
 EXTRA_DIST =   \
@@ -96,6 +97,8 @@ if HAVE_OVIRT
 libvirt_viewer_la_SOURCES += \
ovirt-foreign-menu.h \
ovirt-foreign-menu.c \
+   remote-viewer-iso-list-dialog.c \
+   remote-viewer-iso-list-dialog.h \
$(NULL)
 endif
 
diff --git a/src/remote-viewer-iso-list-dialog.c 
b/src/remote-viewer-iso-list-dialog.c
new file mode 100644
index 000..0fbea28
--- /dev/null
+++ b/src/remote-viewer-iso-list-dialog.c
@@ -0,0 +1,106 @@
+/*
+ * Virt Viewer: A virtual machine console viewer
+ *
+ * Copyright (C) 2016 Red Hat, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#include 
+
+#include 
+
+#include "remote-viewer-iso-list-dialog.h"
+#include "virt-viewer-util.h"
+
+G_DEFINE_TYPE(RemoteViewerISOListDialog, remote_viewer_iso_list_dialog, 
GTK_TYPE_DIALOG)
+
+#define DIALOG_PRIVATE(o) \
+(G_TYPE_INSTANCE_GET_PRIVATE((o), REMOTE_VIEWER_TYPE_ISO_LIST_DIALOG, 
RemoteViewerISOListDialogPrivate))
+
+struct _RemoteViewerISOListDialogPrivate
+{
+GtkWidget *stack;
+};
+
+static void
+remote_viewer_iso_list_dialog_dispose(GObject *object)
+{
+
G_OBJECT_CLASS(remote_viewer_iso_list_dialog_parent_class)->dispose(object);
+}
+
+static void
+remote_viewer_iso_list_dialog_class_init(RemoteViewerISOListDialogClass *klass)
+{
+GObjectClass *object_class = G_OBJECT_CLASS(klass);
+
+g_type_class_add_private(klass, sizeof(RemoteViewerISOListDialogPrivate));
+
+object_class->dispose = remote_viewer_iso_list_dialog_dispose;
+}
+
+static void
+remote_viewer_iso_list_dialog_response(GtkDialog *dialog,
+   gint response_id,
+   gpointer user_data G_GNUC_UNUSED)
+{
+RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(dialog);
+RemoteViewerISOListDialogPrivate *priv = self->priv;
+
+if (response_id != GTK_RESPONSE_NONE)
+return;
+
+gtk_stack_set_visible_child_full(GTK_STACK(priv->stack), "status",
+ GTK_STACK_TRANSITION_TYPE_SLIDE_LEFT);
+gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_NONE, 
FALSE);
+gtk_dialog_set_response_sensitive(GTK_DIALOG(self), GTK_RESPONSE_CLOSE, 
FALSE);
+}
+
+static void
+remote_viewer_iso_list_dialog_init(RemoteViewerISOListDialog *self)
+{
+GtkWidget *content = gtk_dialog_get_content_area(GTK_DIALOG(self));
+RemoteViewerISOListDialogPrivate *priv = self->priv = DIALOG_PRIVATE(self);
+GtkBuilder *builder = 
virt_viewer_util_load_ui("remote-viewer-iso-list.ui");
+
+gtk_builder_connect_signals(builder, self);
+
+priv->stack = GTK_WIDGET(gtk_builder_get_object(builder, "stack&

Re: [virt-tools-list] [PATCH virt-viewer v3 00/10] Replace oVirt foreign menu with dedicated dialog

2016-08-03 Thread Eduardo Lima (Etrunko)
On 08/02/2016 01:17 PM, Christophe Fergeau wrote:
> On Fri, Jul 29, 2016 at 06:40:23PM -0300, Eduardo Lima (Etrunko) wrote:
>> I have pushed the first two patches of the series because they were
>> already acknowledged and were pretty much self-contained.
>>
>> I tried to replace GtkTreeView in favor of new GtkListBox, but after some
>> painful work, I decided to drop it because it is not possible to
>> reproduce the exact same behavior of the former with the latter. For
>> instance, once we have a GtkRadioButton activated, it is not possible to
>> deactivate it. 
> 
> For what it's worth, I'm not sure we should keep GtkRadioButtons at all
> cost in the list, imo they look not so nice. Dunno if it would be
> possibe to have either empty column or check mark instead of a
> GtkRadioButton.
> 

Well, I am no UI expert, but it seems to me that radio buttons are the
best choice to express the behavior of the list, exactly 1 or 0 item can
be active at any given time. With check buttons, user is given the
impression that more than once can be selected, which is not the case.

All in all it is a one line change to show check buttons instead of
radio buttons.

>>
>> In this version:
>>
>>  * Removed leftover enum when changed from GtkNotebook to GtkStack
>>  * Some cosmetic fixes (indentation, renaming, etc)
>>  * UI Tweaks:
>>- Added some padding between items of the list.
>>- Set tree view selection to current ISO when the list is first loaded
>>  or refreshed.
>>- Handle tree view "activate" signal the same way as radio button
>>  toggle
>>- Removed "Select ISO" label and alignment in header bar patch.
>>
>> Eduardo Lima (Etrunko) (10):
>>   ovirt-foreign-menu: Remove timer used to refresh iso list
>>   ovirt-foreign-menu: Add accessors for current iso and iso list
>>   ovirt-foreign-menu: Remove GtkMenu related functions
>>   ovirt-foreign-menu: Notify of new files even if nothing changed
>>   UI: Make 'Change CD' menu item a submenu under 'File' toplevel menu
>>   Introduce ISO List dialog
>>   Run iso-dialog when 'Change CD' menu is activated
>>   remote-viewer: Make ovirt-foreign-menu a property
>>   iso-dialog: Implement functionality provided by oVirt foreign menu
>>   iso-dialog: Use header bar for buttons
> 
> I'm not sure how to approach this series as I don't think
> 'ovirt-foreign-menu: Add accessors for current iso and iso list'
> 'ovirt-foreign-menu: Notify of new files even if nothing changed'
> are needed if you want to have proper error handling in the dialog.
> So I don't think I'd have much more to add compared to the first
> iteration of the review. Or are there specific changes you want me to
> look at?

I have been busy with other things at the moment, and I could not finish
the work here. The idea was to gather comments about UI and behavior in
general, as the new changes will not affect the appearance of the
dialog. So it seems it is looking good, apart from the Radio/Check
buttons decision.

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com



signature.asc
Description: OpenPGP digital signature
___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list

Re: [virt-tools-list] [PATCH virt-viewer] display: Do not override size-allocate handler

2016-08-10 Thread Eduardo Lima (Etrunko)
On 08/09/2016 12:44 PM, Christophe Fergeau wrote:
> On Tue, Aug 09, 2016 at 05:17:31PM +0200, Pavel Grunt wrote:
>> Just connect to the signal
> 
> I'm tempted to ask "why?". I think it's recommended (more efficient) to
> just override the vfunc directly when you can (ie when you derive a new
> widget) rather than connecting to a signal.
> 

This is a gray area to me, I don't remember exactly which cases, but
sometimes overriding the virtual function will produce weird behaviors,
even though the parent class function is called. Meanwhile connecting to
the signal will work fine.

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


Re: [virt-tools-list] [PATCH virt-viewer] display: Do not override size-allocate handler

2016-08-10 Thread Eduardo Lima (Etrunko)
On 08/10/2016 07:41 AM, Pavel Grunt wrote:
> On Tue, 2016-08-09 at 17:44 +0200, Christophe Fergeau wrote:
>> On Tue, Aug 09, 2016 at 05:17:31PM +0200, Pavel Grunt wrote:
>>>
>>> Just connect to the signal
>>
>> I'm tempted to ask "why?". I think it's recommended (more efficient) to
>> just override the vfunc directly when you can (ie when you derive a new
>> widget) rather than connecting to a signal.
> 
> no strong reason, just a personal preference coming from the fact that
> virt-viewer-display is not "adjusting" its allocation, rather it is only using
> it for setting the allocation for its child.
> 

Well, in this case I think it would make sense. Did you get any
collateral effects with this patch?

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list


Re: [virt-tools-list] [PATCH virt-viewer v3 00/10] Replace oVirt foreign menu with dedicated dialog

2016-09-06 Thread Eduardo Lima (Etrunko)
On 09/05/2016 09:15 AM, Christophe Fergeau wrote:
> On Tue, Aug 02, 2016 at 06:17:31PM +0200, Christophe Fergeau wrote:
>>> Eduardo Lima (Etrunko) (10):
>>>   ovirt-foreign-menu: Remove timer used to refresh iso list
>>>   ovirt-foreign-menu: Add accessors for current iso and iso list
>>>   ovirt-foreign-menu: Remove GtkMenu related functions
>>>   ovirt-foreign-menu: Notify of new files even if nothing changed
>>>   UI: Make 'Change CD' menu item a submenu under 'File' toplevel menu
>>>   Introduce ISO List dialog
>>>   Run iso-dialog when 'Change CD' menu is activated
>>>   remote-viewer: Make ovirt-foreign-menu a property
>>>   iso-dialog: Implement functionality provided by oVirt foreign menu
>>>   iso-dialog: Use header bar for buttons
>>
>> I'm not sure how to approach this series as I don't think
>> 'ovirt-foreign-menu: Add accessors for current iso and iso list'
> 
> See the attached patch for some variation around this which avoids
> relying on notify::file and add an async method to do it.
> 
> Christophe
> 

Hey Christophe,

Thanks for the patch, it seems really straightforward. I will include it
in the series, rebase and post again.

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com



signature.asc
Description: OpenPGP digital signature
___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list

Re: [virt-tools-list] [PATCH virt-viewer v3 00/10] Replace oVirt foreign menu with dedicated dialog

2016-09-06 Thread Eduardo Lima (Etrunko)
On 08/04/2016 12:09 AM, Xiaodai Wang wrote:
> - Original Message -
>> From: "Fabiano Fidêncio" 
>> To: "Eduardo Lima (Etrunko)" 
>> Cc: "Christophe Fergeau" , "virt" 
>> 
>> Sent: Wednesday, August 3, 2016 10:36:07 PM
>> Subject: Re: [virt-tools-list] [PATCH virt-viewer v3 00/10] Replace oVirt 
>> foreign menu with dedicated dialog
>>
>> On Wed, Aug 3, 2016 at 3:04 PM, Eduardo Lima (Etrunko)
>>  wrote:
>>> On 08/02/2016 01:17 PM, Christophe Fergeau wrote:
>>>> On Fri, Jul 29, 2016 at 06:40:23PM -0300, Eduardo Lima (Etrunko) wrote:
>>>>> I have pushed the first two patches of the series because they were
>>>>> already acknowledged and were pretty much self-contained.
>>>>>
>>>>> I tried to replace GtkTreeView in favor of new GtkListBox, but after some
>>>>> painful work, I decided to drop it because it is not possible to
>>>>> reproduce the exact same behavior of the former with the latter. For
>>>>> instance, once we have a GtkRadioButton activated, it is not possible to
>>>>> deactivate it.
>>>>
>>>> For what it's worth, I'm not sure we should keep GtkRadioButtons at all
>>>> cost in the list, imo they look not so nice. Dunno if it would be
>>>> possibe to have either empty column or check mark instead of a
>>>> GtkRadioButton.
>>>>
>>>
>>> Well, I am no UI expert, but it seems to me that radio buttons are the
>>> best choice to express the behavior of the list, exactly 1 or 0 item can
>>> be active at any given time. With check buttons, user is given the
>>> impression that more than once can be selected, which is not the case.
>>
>> I do agree with Eduardo here.
>> I have the feeling that radio buttons are the most suitable option here.
>>
> 
> How about the UI for "Attach CD" option in ovirt portal.
> 
> You can find it in "Edit Virtual Machine" dialog, then select "Boot Options".
> 
> Or How about the "Change CD" dialog? 
> 


Hi Xiaodai,

I'm sorry I didn't reply to your message earlier, I somehow missed it.

Well, I looked at the option you mentioned, and in Gtk+ the equivalent
widget would be a ComboBox, but it seems to have the same problems as
the old menu item we are trying to replace. When there are lots of items
to show, it will expand to the limits of the screen.

Regards, Eduardo


-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list

Re: [virt-tools-list] RFC: time for a 5.0 release ?

2016-10-10 Thread Eduardo Lima (Etrunko)
On 10/10/16 12:05, Daniel P. Berrange wrote:
> On Mon, Oct 10, 2016 at 04:26:29PM +0200, Fabiano Fidêncio wrote:
>> On Mon, Oct 10, 2016 at 2:38 PM, Daniel P. Berrange  
>> wrote:
>>> The 4.0 release was in June, and there have not been very many changes
>>> since then, but there are a couple of significant ones
>>>
>>>  - Fix resizing of windows when using CSDs (relevant for Wayland)
>>>  - Fix virt-viewer not exiting on broken connection to guest
>>>
>>> If we do a new release we can also pull in the nasty Win32 redraw
>>> bug that plagued 4.0. Of course we could do a new MSI build v4.1
>>> as an alternative.
>>
>> I'm completely in favor of a new release.
>>
>> Pavel was looking forward for a libusb* release at some point, so we
>> could enable USB Redirection support in the Windows builds.
>>
>> *: libusb released the -rc6 on October 1st. Would worth to talk to
>> them and see what are their plan before running our release.
> 
> I want to do the Windows build against Fedora 25, so given the current
> GTK bug pointed out, we'll have to wait a little anyways. So lets look
> again in a week or two and hopefully we'll have fixed gtk + libusb.

It would be good to have the ISO dialog merged. I need to get back to
work on it though, for I have been sidetracked a bit in other matters.
The remaining work necessary should not be too much and a couple of
weeks should be enough.

Regards, Eduardo.

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list

[virt-tools-list] [PATCH virt-viewer] Add ovirt_foreign_menu_set_current_iso_name_async

2016-10-31 Thread Eduardo Lima (Etrunko)
From: Christophe Fergeau 

Signed-off-by: Eduardo Lima (Etrunko) 
---

I tweaked your patch a bit with a proper dialog for showing error if it
happens, this is going to be reused for the case of fetching the ISO
list as well.

---
 src/ovirt-foreign-menu.c| 41 -
 src/ovirt-foreign-menu.h|  9 +++-
 src/remote-viewer-iso-list-dialog.c | 40 
 3 files changed, 71 insertions(+), 19 deletions(-)

diff --git a/src/ovirt-foreign-menu.c b/src/ovirt-foreign-menu.c
index 8320552..c349d93 100644
--- a/src/ovirt-foreign-menu.c
+++ b/src/ovirt-foreign-menu.c
@@ -47,7 +47,7 @@ static void 
ovirt_foreign_menu_fetch_storage_domain_async(OvirtForeignMenu *menu
 static void ovirt_foreign_menu_fetch_vm_cdrom_async(OvirtForeignMenu *menu);
 static void ovirt_foreign_menu_refresh_cdrom_file_async(OvirtForeignMenu 
*menu);
 static void ovirt_foreign_menu_fetch_iso_list_async(OvirtForeignMenu *menu);
-static void updated_cdrom_cb(GObject *source_object, GAsyncResult *result, 
gpointer user_data);
+static void iso_name_set_cb(GObject *source_object, GAsyncResult *result, 
gpointer user_data);
 
 G_DEFINE_TYPE (OvirtForeignMenu, ovirt_foreign_menu, G_TYPE_OBJECT)
 
@@ -102,8 +102,14 @@ ovirt_foreign_menu_get_current_iso_name(OvirtForeignMenu 
*foreign_menu)
 
 
 void
-ovirt_foreign_menu_set_current_iso_name(OvirtForeignMenu *foreign_menu, char 
*name)
+ovirt_foreign_menu_set_current_iso_name_async(OvirtForeignMenu *foreign_menu,
+  char *name,
+  GCancellable *cancellable,
+  GAsyncReadyCallback callback,
+  gpointer user_data)
 {
+GTask *task;
+
 g_return_if_fail(foreign_menu->priv->cdrom != NULL);
 g_return_if_fail(foreign_menu->priv->next_iso_name == NULL);
 
@@ -118,11 +124,21 @@ ovirt_foreign_menu_set_current_iso_name(OvirtForeignMenu 
*foreign_menu, char *na
 g_object_set(foreign_menu->priv->cdrom,
  "file", name,
  NULL);
+
+task = g_task_new(foreign_menu, cancellable, callback, user_data);
 ovirt_cdrom_update_async(foreign_menu->priv->cdrom, TRUE,
  foreign_menu->priv->proxy, NULL,
- updated_cdrom_cb, foreign_menu);
+ iso_name_set_cb, task);
 }
 
+gboolean ovirt_foreign_menu_set_current_iso_name_finish(OvirtForeignMenu 
*foreign_menu,
+GAsyncResult *result,
+GError **error)
+{
+g_return_val_if_fail(OVIRT_IS_FOREIGN_MENU(foreign_menu), FALSE);
+
+return g_task_propagate_boolean(G_TASK(result), error);
+}
 
 GList*
 ovirt_foreign_menu_get_iso_names(OvirtForeignMenu *foreign_menu)
@@ -359,15 +375,16 @@ ovirt_foreign_menu_start(OvirtForeignMenu *menu)
 }
 
 
-static void updated_cdrom_cb(GObject *source_object,
- GAsyncResult *result,
- gpointer user_data)
+static void iso_name_set_cb(GObject *source_object,
+GAsyncResult *result,
+gpointer user_data)
 {
 GError *error = NULL;
 OvirtForeignMenu *foreign_menu;
 gboolean updated;
+GTask *task = G_TASK(user_data);
 
-foreign_menu = OVIRT_FOREIGN_MENU(user_data);
+foreign_menu = OVIRT_FOREIGN_MENU(g_task_get_source_object(task));
 updated = ovirt_cdrom_update_finish(OVIRT_CDROM(source_object),
 result, &error);
 g_debug("Finished updating cdrom content");
@@ -375,6 +392,7 @@ static void updated_cdrom_cb(GObject *source_object,
 g_free(foreign_menu->priv->current_iso_name);
 foreign_menu->priv->current_iso_name = 
foreign_menu->priv->next_iso_name;
 foreign_menu->priv->next_iso_name = NULL;
+g_task_return_boolean(task, TRUE);
 goto end;
 }
 
@@ -382,7 +400,11 @@ static void updated_cdrom_cb(GObject *source_object,
  * the new ISO */
 if (error != NULL) {
 g_warning("failed to update cdrom resource: %s", error->message);
-g_clear_error(&error);
+g_task_return_error(task, error);
+} else {
+g_warn_if_reached();
+g_task_return_error(task, g_error_new_literal(OVIRT_ERROR, 
OVIRT_ERROR_FAILED,
+  "failed to update cdrom 
resource"));
 }
 g_debug("setting OvirtCdrom:file back to '%s'",
 foreign_menu->priv->current_iso_name);
@@ -392,10 +414,9 @@ static void updated_cdrom_cb(GObject *source_object,
 g_clear_pointer(&foreign_menu->priv->next_iso_name, g_free);
 
 end:
-

[virt-tools-list] [PATCH virt-viewer v4 08/12] remote-viewer: Make ovirt-foreign-menu a property

2016-11-01 Thread Eduardo Lima (Etrunko)
The OvirtForeignMenu pointer is needed by the new ISO list dialog, and
we make it acessible via property to avoid interdependency between
objects.

Signed-off-by: Eduardo Lima (Etrunko) 
---
 src/remote-viewer-iso-list-dialog.c | 31 +++
 src/remote-viewer-iso-list-dialog.h |  2 +-
 src/remote-viewer.c | 37 +
 src/virt-viewer-window.c| 15 ++-
 4 files changed, 75 insertions(+), 10 deletions(-)

diff --git a/src/remote-viewer-iso-list-dialog.c 
b/src/remote-viewer-iso-list-dialog.c
index 0fbea28..858719c 100644
--- a/src/remote-viewer-iso-list-dialog.c
+++ b/src/remote-viewer-iso-list-dialog.c
@@ -24,6 +24,7 @@
 
 #include "remote-viewer-iso-list-dialog.h"
 #include "virt-viewer-util.h"
+#include "ovirt-foreign-menu.h"
 
 G_DEFINE_TYPE(RemoteViewerISOListDialog, remote_viewer_iso_list_dialog, 
GTK_TYPE_DIALOG)
 
@@ -33,11 +34,16 @@ G_DEFINE_TYPE(RemoteViewerISOListDialog, 
remote_viewer_iso_list_dialog, GTK_TYPE
 struct _RemoteViewerISOListDialogPrivate
 {
 GtkWidget *stack;
+OvirtForeignMenu *foreign_menu;
 };
 
 static void
 remote_viewer_iso_list_dialog_dispose(GObject *object)
 {
+RemoteViewerISOListDialog *self = REMOTE_VIEWER_ISO_LIST_DIALOG(object);
+RemoteViewerISOListDialogPrivate *priv = self->priv;
+
+g_clear_object(&priv->foreign_menu);
 
G_OBJECT_CLASS(remote_viewer_iso_list_dialog_parent_class)->dispose(object);
 }
 
@@ -94,13 +100,22 @@ 
remote_viewer_iso_list_dialog_init(RemoteViewerISOListDialog *self)
 }
 
 GtkWidget *
-remote_viewer_iso_list_dialog_new(GtkWindow *parent)
+remote_viewer_iso_list_dialog_new(GtkWindow *parent, GObject *foreign_menu)
 {
-return g_object_new(REMOTE_VIEWER_TYPE_ISO_LIST_DIALOG,
-"title", _("Change CD"),
-"transient-for", parent,
-"border-width", 18,
-"default-width", 400,
-"default-height", 300,
-NULL);
+GtkWidget *dialog;
+RemoteViewerISOListDialog *self;
+
+g_return_val_if_fail(foreign_menu != NULL, NULL);
+
+dialog = g_object_new(REMOTE_VIEWER_TYPE_ISO_LIST_DIALOG,
+  "title", _("Change CD"),
+  "transient-for", parent,
+  "border-width", 18,
+  "default-width", 400,
+  "default-height", 300,
+  NULL);
+
+self = REMOTE_VIEWER_ISO_LIST_DIALOG(dialog);
+self->priv->foreign_menu = OVIRT_FOREIGN_MENU(g_object_ref(foreign_menu));
+return dialog;
 }
diff --git a/src/remote-viewer-iso-list-dialog.h 
b/src/remote-viewer-iso-list-dialog.h
index def841b..8b936f5 100644
--- a/src/remote-viewer-iso-list-dialog.h
+++ b/src/remote-viewer-iso-list-dialog.h
@@ -51,7 +51,7 @@ struct _RemoteViewerISOListDialogClass
 
 GType remote_viewer_iso_list_dialog_get_type(void) G_GNUC_CONST;
 
-GtkWidget *remote_viewer_iso_list_dialog_new(GtkWindow *parent);
+GtkWidget *remote_viewer_iso_list_dialog_new(GtkWindow *parent, GObject 
*foreign_menu);
 
 G_END_DECLS
 
diff --git a/src/remote-viewer.c b/src/remote-viewer.c
index 95130dc..e0cd139 100644
--- a/src/remote-viewer.c
+++ b/src/remote-viewer.c
@@ -67,6 +67,13 @@ G_DEFINE_TYPE (RemoteViewer, remote_viewer, 
VIRT_VIEWER_TYPE_APP)
 #define GET_PRIVATE(o)\
 (G_TYPE_INSTANCE_GET_PRIVATE ((o), REMOTE_VIEWER_TYPE, 
RemoteViewerPrivate))
 
+enum RemoteViewerProperties {
+PROP_0,
+#ifdef HAVE_OVIRT
+PROP_OVIRT_FOREIGN_MENU,
+#endif
+};
+
 #ifdef HAVE_OVIRT
 static OvirtVm * choose_vm(GtkWindow *main_window,
char **vm_name,
@@ -213,6 +220,25 @@ end:
 }
 
 static void
+remote_viewer_get_property(GObject *object, guint property_id,
+   GValue *value, GParamSpec *pspec)
+{
+RemoteViewer *self = REMOTE_VIEWER(object);
+RemoteViewerPrivate *priv = self->priv;
+
+switch (property_id) {
+#ifdef HAVE_OVIRT
+case PROP_OVIRT_FOREIGN_MENU:
+g_value_set_object(value, priv->ovirt_foreign_menu);
+break;
+#endif
+
+default:
+G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec);
+}
+}
+
+static void
 remote_viewer_class_init (RemoteViewerClass *klass)
 {
 GObjectClass *object_class = G_OBJECT_CLASS (klass);
@@ -222,6 +248,7 @@ remote_viewer_class_init (RemoteViewerClass *klass)
 
 g_type_class_add_private (klass, sizeof (RemoteViewerPrivate));
 
+object_class->get_property = remote_viewer_get_property;
 object_class->dispose = remote_viewer_dispose;
 
 g_app_class->local_command_line = remote_viewer_local_command_line;
@

<    1   2   3   4   5   >