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

Anne Mulhern amulhern at redhat.com
Tue Jun 23 17:45:17 UTC 2015





----- Original Message -----
> From: "Anne Mulhern" <amulhern at redhat.com>
> To: "anaconda patch review" <anaconda-patches at lists.fedorahosted.org>
> Sent: Tuesday, June 23, 2015 1:31:46 PM
> Subject: Re: [rhel7-branch][PATCH] Improve how we set the default screen	height in	text mode (#1184378)
> 
> 
> 
> 
> 
> ----- 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:

except that the above "else" should be

if pos != num_lines:

because you could have finished the loop and run out of lines at the same time.


>                 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.
> 
> - mulhern
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> > --
> > 2.1.0
> > 
> > _______________________________________________
> > 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
> 


More information about the anaconda-patches mailing list