From: "Martin Kolman" <mkolman(a)redhat.com>
To: "anaconda patch review" <anaconda-patches(a)lists.fedorahosted.org>
Sent: Tuesday, November 18, 2014 9:19:57 AM
Subject: Re: [PATCH] Provide useful hints on TTY1 during the installation
On Tue, 2014-11-18 at 08:16 -0500, Anne Mulhern wrote:
>
>
>
> ----- Original Message -----
> > From: "Vratislav Podzimek" <vpodzime(a)redhat.com>
> > To: "anaconda patch review"
<anaconda-patches(a)lists.fedorahosted.org>
> > Sent: Tuesday, November 18, 2014 2:53:11 AM
> > Subject: Re: [PATCH] Provide useful hints on TTY1 during the installation
> >
> > On Fri, 2014-11-14 at 21:30 +0100, Martin Kolman wrote:
> > > Until now TTY1 was not really used for anything other than printing
> > > current Anaconda version. This has been changed and the Anaconda
> > > version is now followed by useful hints for Anaconda users with
> > > special emphasis of Anaconda debugging (where are the log files stored,
> > > how can I get a shell, etc.).
> > >
> > > The hints are also parametrized to be displayed only when appropriate
> > > (no need to tell users that Anaconda has a text mode if they already
> > > are in textmode) and are translatable.
> > >
> > > Signed-off-by: Martin Kolman <mkolman(a)redhat.com>
> > > ---
> > > anaconda | 28 ++++++++++++++++++++++++++++
> > > 1 file changed, 28 insertions(+)
> > >
> > > diff --git a/anaconda b/anaconda
> > > index a3f527c..c61dc71 100755
> > > --- a/anaconda
> > > +++ b/anaconda
> > > @@ -988,11 +988,39 @@ if __name__ == "__main__":
> > >
> > > verdesc = "%s for %s %s" % (getAnacondaVersionString(),
> > > product.productName,
> > > product.productVersion)
> > > +
> > > + logs_note = _(" * installation log files are stored in /tmp
during
> > > the
> > > installation")
> > > + shell_note = _(" * shell is available on TTY2 and in second
TMUX
> > > pane
> > > (ctrl+b, then press 2)")
> > > + tmux_only_note = _(" * shell is available in second TMUX pane
> > > (ctrl+b,
> > > then press 2)")
> > > + text_mode_note = _(" * if the graphical installation interface
> > > fails
> > > to start, try again with the\n\
> > > + inst.text bootoption to start text installation")
> > You can put two strings next to each other and they get concatenated, so
> > something like
> >
> > ("first part "
> > "second part")
> >
> > which would be nicer here than the backslash at the end of the line.
> >
> > > + separate_attachements_note = _(" * when reporting a bug attach
> > > logs as
> > > separate text/plain attachments")
> > > +
> > > if product.isFinal:
> > > print("anaconda %s started." % verdesc)
> > > else:
> > > print("anaconda %s (pre-release) started." % verdesc)
> > >
> > > + # we are past the --version and --help shortcut so we can import
> > > Blivet
> > > + # now without slowing down anything critical
> > > +
> > > + # pylint: disable=import-error
> > > + from blivet import arch
> > > +
> > > + if not opts.images and not opts.dirinstall:
> > > + print(logs_note)
> > > + # no fancy stuff like TTYs on a s390...
> > > + if not arch.isS390():
> > > + print(shell_note)
> > > + # ...but there is apparently TMUX during the manual
> > > installation!
> > > + elif not opts.ksfile:
> > > + print(tmux_only_note)
> > > + # no need to tell users how to switch to text mode
> > > + # if already in text mode
> > > + if opts.display_mode == 'g':
> > > + print(text_mode_note)
> > > + print(separate_attachements_note)
> > > +
> > > from pyanaconda.anaconda import Anaconda
> > > anaconda = Anaconda()
> > > iutil.setup_translations(gettext)
> > As mulhern pointed out, you need to put all these prints after this line
> > so that they actually appear translated. However, it seems that user has
> > no chance to change the language before the execution comes to this
> > place so it may be better to remove the _() calls in the end.
> >
> > --
> > Vratislav Podzimek
> >
> > Anaconda Rider | RHCE | Red Hat, Inc. | Brno - Czech Republic
> >
> >
> > _______________________________________________
> > anaconda-patches mailing list
> > anaconda-patches(a)lists.fedorahosted.org
> >
https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
> >
>
> If not translating then situation becomes even more like that
> for the command line option help text and offloading the text to a data
> file
> in much the same way as with those seems like an even better fit for this
> problem.
I would like to make the hint code as simple as possible to maximize the
chance these hints are actually displayed if something goes wrong and I
think there is quite a lot of stuff that can go wrong even if just
loading them from a file. Also there are multiple notes that are
displayed based on different conditions, so the file would need to be
loaded & parsed to the corresponding notes, which introduces additional
complexity (and room for failure) for a little actual benefit.
So I would like to keep it simple and just use normal variables.
>
> - mulhern
> _______________________________________________
> anaconda-patches mailing list
> anaconda-patches(a)lists.fedorahosted.org
>
https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
_______________________________________________
anaconda-patches mailing list
anaconda-patches(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
I think the benefit would be enough to justify it and have
every confidence in your ability to manage the additional
complexity.
But I do agree that doing explicit file IO introduces a new way to fail
that is beyond the control of the programmer and these are rescue messages
that really should be displayed even in difficult circumstances.
Why not offload the file IO problem to the Python interpreter by
offloading these *_note constants into pyanaconda.constants
which looks like it is already imported and used by the time this
code would be hit?
Minor: The *_note names should all be capitalized, being constants.
- mulhern