[rhel7-branch][PATCH] Improve how we set the default screen height in text mode (#1184378)

Vratislav Podzimek vpodzime at redhat.com
Fri Jun 26 06:48:23 UTC 2015


On Tue, 2015-06-23 at 13:31 -0400, Anne Mulhern wrote:
> 
> 
> 
> ----- Original Message -----
> > From: "Vratislav Podzimek" <vpodzime at redhat.com>
> > To: anaconda-patches at lists.fedorahosted.org
> > Sent: Tuesday, June 23, 2015 8:59:21 AM
> > Subject: [rhel7-branch][PATCH] Improve how we set the default screen height in	text mode (#1184378)
> > 
> > We can either use given value or try to use the $LINES environment variable
> > or
> > use the default which should be 24 lines because that's what serial consoles
> > use
> > (see the bug).
> > 
> > Also fix the line counting when printing long widgets so that there are no
> > gaps
> > (missing lines).
> > 
> > Signed-off-by: Vratislav Podzimek <vpodzime at redhat.com>
> > ---
> >  pyanaconda/constants_text.py         |  3 +++
> >  pyanaconda/ui/tui/simpleline/base.py | 17 +++++++++++++----
> >  2 files changed, 16 insertions(+), 4 deletions(-)
> > 
> > diff --git a/pyanaconda/constants_text.py b/pyanaconda/constants_text.py
> > index 750a984..9fb5e03 100644
> > --- a/pyanaconda/constants_text.py
> > +++ b/pyanaconda/constants_text.py
> > @@ -56,3 +56,6 @@ TEXT_NO_BUTTON = Translator(TEXT_NO_STR, TEXT_NO_CHECK)
> >  # Make the return calls from the UIScreen input() function more clear
> >  INPUT_PROCESSED = None
> >  INPUT_DISCARDED = False
> > +
> > +# default screen height in number of lines (24 lines is the default for
> > serial consoles)
> > +DEFAULT_SCREEN_HEIGHT = 24
> > diff --git a/pyanaconda/ui/tui/simpleline/base.py
> > b/pyanaconda/ui/tui/simpleline/base.py
> > index 6371b93..c4fe736 100644
> > --- a/pyanaconda/ui/tui/simpleline/base.py
> > +++ b/pyanaconda/ui/tui/simpleline/base.py
> > @@ -22,6 +22,7 @@
> >  __all__ = ["App", "UIScreen", "Widget"]
> >  
> >  import sys
> > +import os
> >  import Queue
> >  import getpass
> >  import threading
> > @@ -30,6 +31,7 @@ from pyanaconda.threads import threadMgr, AnacondaThread
> >  from pyanaconda.ui.communication import hubQ
> >  from pyanaconda import constants, iutil
> >  from pyanaconda.i18n import _, N_, C_
> > +from pyanaconda.constants_text import DEFAULT_SCREEN_HEIGHT
> >  
> >  RAW_INPUT_LOCK = threading.Lock()
> >  
> > @@ -491,17 +493,24 @@ class UIScreen(object):
> >      # title line of the screen
> >      title = u"Screen.."
> >  
> > -    def __init__(self, app, screen_height = 25):
> > +    def __init__(self, app, screen_height=0):
> >          """
> >          :param app: reference to application main class
> >          :type app: instance of class App
> >  
> >          :param screen_height: height of the screen (useful for printing long
> >          widgets)
> > +                              or 0 to use the default
> >          :type screen_height: int
> >          """
> >  
> >          self._app = app
> > -        self._screen_height = screen_height
> > +
> > +        if screen_height > 0:
> > +            self._screen_height = screen_height
> > +        elif "LINES" in os.environ:
> > +            self._screen_height = os.environ["LINES"]
> > +        else:
> > +            self._screen_height = DEFAULT_SCREEN_HEIGHT
> 
> Everything above here looks fine to me.
> 
> >  
> >          # list that holds the content to be printed out
> >          self._window = []
> > @@ -563,10 +572,10 @@ class UIScreen(object):
> >                  # prompt (2 lines)
> >                  for line in lines[pos:]:
> >                      print(line)
> > -                pos += self._screen_height - 1
> > +                pos += self._screen_height - 2
> >              else:
> >                  # print part with a prompt to continue
> > -                for line in lines[pos:(pos + self._screen_height - 2)]:
> > +                for line in lines[pos:(pos + self._screen_height - 1)]:
> >                      print(line)
> >                  self._app.raw_input(_("Press ENTER to continue"))
> >                  pos += self._screen_height - 1
> 
> Please make the above changes into a separate patch.
> 
> I don't think that it is all that helpful to print an an extra line of widget
> if you have not run out of lines yet. That extra line could be the last line, and
> so you would be prompting the user to press ENTER and then get nothing, not
> even a single additional line for their pains. So, I think that filling that
> missing line with an empty line is a bit better.
> 
> And I think _print_long_widget() could be made a little less computational
> and complex.
> 
> Something like:
> 
>     def _print_long_widget(self, widget):
>         """Prints a long widget (possibly longer than the screen height) with
>            user interaction (when needed).
> 
>            :param widget: possibly long widget to print
>            :type widget: Widget instance
> 
>         """
>         lines = widget.get_lines()
>         num_lines = len(lines)
>         text_len = self._screen_height - 2
> 
>         pos = 0
>         while pos != num_lines:
>             for _ in range(text_len):
>                 if pos == num_lines:
>                     break
>                 print(lines[pos])
>                 pos += 1
>             else:
>                 print("") # Make sure prompt appears at very bottom
>                 self._app.raw_input(_("Press ENTER to continue"))
> 
> should, I think, do everything that is necessary. It should be equivalent
> to that proposed method, modulo the one change I suggested.
There are two things you might be missing:
1) The normal prompt is on two lines only the continuation prompt above
is one-line (and we need to prompt user to continue to the normal prompt
if it doesn't fit).

2) There is a check right above the changed block that checks if the
rest of the widget plus two lines of the normal prompt fit into the
screen and prints it all if yes.

And in general I'd really rather change two numbers and keep the
algorithm the same on rhel7-branch than rewrite it completely and
potentially introduce new bugs.

-- 
Vratislav Podzimek

Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic



More information about the anaconda-patches mailing list