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

[Jonathon Jongsma]

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. 

Jonathon