[virt-tools-list] [virt-viewer PATCH] Show errors generated by connection dialog

[Pavel Grunt]

On Thu, 2017-02-09 at 12:10 +0100, Christophe de Dinechin wrote:
> Thanks for the comments.
> 
> > On 9 Feb 2017, at 11:23, Pavel Grunt <pgrunt at redhat.com> wrote:
> > 
> > Hello Christophe,
> > 
> > On Wed, 2017-02-08 at 17:48 +0100, Christophe de Dinechin wrote:
> > > When running 'remote-viewer' without arguments,
> > > and selecting a non-supported protocol, e.g. ssh://foo,
> > > the generated error was silently swallowed by the retry loop.
> > > This was introduced in 06b2c382468876a19600374bd59475e66d488af8.
> > > ---
> > > src/remote-viewer.c | 3 +++
> > > 1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/src/remote-viewer.c b/src/remote-viewer.c
> > > index 13c6e7c..c9ef4bb 100644
> > > --- a/src/remote-viewer.c
> > > +++ b/src/remote-viewer.c
> > > @@ -1196,6 +1196,9 @@ cleanup:
> > >    type = NULL;
> > > 
> > >    if (!ret && priv->open_recent_dialog) {
> > > +        if (error) {
> > 
> > in case of pointers we prefer an explicit comparison to NULL (just
> > a
> > style - which is not followed…)
> 
> OK
> 
> > 
> > so it can be error != NULL && error->message != NULL to make
> > everyone
> > happy.
> 
> If the message is NULL, we should still display something like
> “unknown error”. If we think it’s worth testing for error->message
> != NULL, then I suggest another patch just for that, that fixes all
> places and not just this one.
I agree
> 
> 
> > Although as you said the error->message is never checked...
> > otoh if the message is NULL then an empty dialog would appear.
> > 
> > > +            virt_viewer_app_simple_message_dialog(&self-
> > > >parent, 
> > 
> > the first param can be app
> 
> OK.
> 
> > 
> > > "%s", error->message);
> > 
> > the string will be displayed to the user, so it should be
> > translated -
> > in case of the empty message, it should say something.

> The incoming string is already translated, I think.
> 
> BTW, I did not find where the localization strings for the project
> were stored. How does localization happen ? If I add a new message,
> who should I warn to have the message translated?
It is handled in zanata https://fedora.zanata.org/iteration/view/virt-
viewer/master?dswid=3794

> 
> > 
> > maybe "Unable to connect" error->message 
> 
> Today, the message is for example
> 	Unsupported graphic type ‘tap’
> 
> Is it really an improvement to have:
> 	Unable to connect: Unsupported graphic type ‘tap’

I think "Unable to connect unknown error"
is better than just "unknown error"

> 
> ?
> 
> I personally don’t mind either way.
> 
>