[PATCH] Provide useful hints on TTY1 during the installation

Martin Kolman mkolman at redhat.com
Tue Nov 18 15:34:24 UTC 2014


On Tue, 2014-11-18 at 09:52 -0500, Anne Mulhern wrote:
> 
> 
> 
> ----- Original Message -----
> > From: "Martin Kolman" <mkolman at redhat.com>
> > To: "anaconda patch review" <anaconda-patches at 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 at redhat.com>
> > > > To: "anaconda patch review" <anaconda-patches at 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 at 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 at 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 at lists.fedorahosted.org
> > > https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
> > 
> > 
> > _______________________________________________
> > anaconda-patches mailing list
> > anaconda-patches at 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?
I think that the current solution improves the code readability by
making it easily apparent which text will be visible in which
situation (GUI, TUI, s390, etc.).

If it was moved to constants, this would be less apparent. It also feels
a bit like polluting the constants namespace by adding items that are
only used together and in a single place anyway.

>  
> 
> Minor: The *_note names should all be capitalized, being constants.
Well, I think of the more like local variables in this case. There also
other similar cases nearby (uname, verdesc, pidfile, etc.) and I
generally try to keep the coding style similar to the one already
present in the file.
> 
> - mulhern
> 
> _______________________________________________
> anaconda-patches mailing list
> anaconda-patches at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches




More information about the anaconda-patches mailing list