[virt-tools-list] [virt-viewer][PATCH 1/4 v3] virt-viewer: Moved preferences from app to window

Fabiano Fidêncio fabiano at fidencio.org
Thu May 21 23:37:13 UTC 2015


On Wed, May 20, 2015 at 5:28 PM, Jonathon Jongsma <jjongsma at redhat.com> wrote:
> On Wed, 2015-05-20 at 08:41 +0200, Pavel Grunt wrote:
>> If we agree on this,On Tue, 2015-05-19 at 11:40 -0500, Jonathon Jongsma wrote:
>> > General comment on the approach: it seems to me that it would be
>> > cleaner
>> > to implement this using virtual functions. For example, in patch #3,
>> > you
>> > add a "can-reconnect" property to VirtViewerApp. This means that it's
>> > technically possible to construct a RemoteViewer object with
>> > can-reconnect=TRUE, even though RemoteViewer cannot reconnect.
>> >
>> > There are two basic ways to achieve this with virtual functions:
>> >
>> > 1) make virt_viewer_app_get_preferences() a virtual function. Each
>> > class
>> > (RemoteViewer and VirtViewer) would implement this virtual function
>> > and
>> > return the preferences that it supports. There may be a fair amount
>> > of
>> > code duplication in this approach, but it might be possible to
>> > overcome
>> > that.
>> >
>> > 2) make virt_viewer_app_can_reconnect() a virtual function.
>> > RemoteViewer
>> > would implement this function by returning FALSE, and VirtViewer
>> > would
>> > return TRUE (look at virt_viewer_session_can_share_folder() for
>> > inspiration)
>> >
>> > In addition, "preferences" to me indicates a persistent setting, but
>> > it
>> > doesn't appear that these settings are stored anywhere. Should we
>> > hook
>> > them up to the existing config infrastructure in VirtViewerApp so
>> > they
>> > end up stored in e.g. ~/.local/virt-viewer/settings? Furthermore,
>> > should
>> > we add the existing "ask-quit" setting to this preferences dialog?
>> > Opinions?
>> >
>>
>> I think it would be beneficial for the user to have "ask-quit" in the
>> "preferences" - changing the default configuration by editing a file
>> is not user-friendly.
>>
>> About making "reconnect" the persistent setting - I am ok with it, it
>> makes sense to let the user define some default behavior for it. But I
>> think that with this change the synchronization between the
>> commandline option and the gui should be removed (to avoid making the
>> permanent setting just because it was once used with '--reconnect').
>> Maybe we should add the '--no-reconnect' commandline option to
>> override the default setting?
>>
>> Pavel
>
>
> Hmm, indeed it would be weird to change a persistent setting when the
> user started the application with the --reconnect switch. But I think
> it's also weird to have a non-persistent setting in the preferences
> dialog. I personally still think that this option probably doesn't need
> to be exposed in the UI.
>

Hmm. I agree that persistent options (the ones present in the settings
file) should be exposed in the UI.
What I do not agree is that these options are the only ones to be
exposed. OTOH, I do not have a good idea about where/how to expose
them to the users in a good way.

An input from a UX designer would be more than appreciated about all
those things.

> Jonathon
>
> _______________________________________________
> virt-tools-list mailing list
> virt-tools-list at redhat.com
> https://www.redhat.com/mailman/listinfo/virt-tools-list


Best Regards,
-- 
Fabiano Fidêncio




More information about the virt-tools-list mailing list