9
\$\begingroup\$

I wanted to make a GUI-ish (a TUI) thing in the terminal as I have no experience with PyGame or the like. It works, but how can I improve this?

import os, colorama
from msvcrt import getch
from colorama import Fore, Back, Style

colorama.init()

def newGame():
    print("Sorry, currently not a feature")

def cont():
    print("We can't do that")

def highscores():
    print("Yep, again, no can do")

class comm:
    def __init__(self, comm):
        self.comm = comm

def getKeyInput():
    key = ord(getch())
    if key == 224:
        key = ord(getch())
    if key == 72: return 1 # Up arrow
    elif key == 80: return -1 # Down arrow
    elif key == 13: return 0 # Enter

class Menu:
    def __init__(self,
                 title,
                 options, # GUI Options (a list)
                 comms = []): # A list of actions corresponding to options
        self.options = options
        self.title = title
        self.comms = comms
        self.pointer = 0

    def show(self):
        os.system("cls")
        buffer = "-"*len(self.title)
        print("{}\n{}\n{}".format(buffer, self.title, buffer))

        for i in self.options:
            colour = Fore.WHITE + Back.BLACK
            if self.pointer == self.options.index(i): # This is why self.options cannot be a dictionary
                colour = Back.WHITE + Fore.BLACK
            print("{}{}".format(colour, i))
            colour = Fore.WHITE + Back.BLACK
            print(colour, end = '')

        key = getKeyInput()

        if key == 0:
            os.system("cls")
            if self.comms:
                self.comms[self.pointer].comm()
                return 0
            print('Picking {}'.format(self.options[self.pointer]))
            return 0

        self.pointer -= key
        self.pointer = max(min(self.pointer, len(self.options)-1), 0)
        self.show()

if __name__ == "__main__":
    m = Menu("Main Menu",

             ['Start New Game',
              'Continue',
              'Highscores',
              'Quit'],

             [comm(lambda : newGame()),
              comm(lambda : cont()),
              comm(lambda : highscores()),
              comm(lambda : quit())])
    m.show()
\$\endgroup\$
3
  • \$\begingroup\$ In what sense is this a Graphical user interface? It looks entirely text-based to me. \$\endgroup\$ Commented Apr 26, 2018 at 20:36
  • \$\begingroup\$ @200_success I agree, but I meant as in there is not just a text box which is used to pick an option, but an actual selection process. It is a bit of a stretch, since the classic definition is a point & click, but this is less GUI than a GUI but more GUI than a simple text-based menu. If there is a term for it, I'm not aware of it. \$\endgroup\$
    – LForchini
    Commented Apr 26, 2018 at 20:40
  • 1
    \$\begingroup\$ According to Wikipedia, such an interface has been called a TUI: Text-based user interface, textual user interface or terminal user interface en.wikipedia.org/wiki/Text-based_user_interface Not much in the way of evidence on that page though. \$\endgroup\$
    – Josiah
    Commented Apr 26, 2018 at 21:36

1 Answer 1

11
\$\begingroup\$

General

  1. Try to comply with PEP-8. PEP-8 is the name of the official Python style guide. Complying with it makes your code easier to read. Ultimately, developers can more easily collaborate on a project.

    • Every import-statement should be on a separate line. The only exception is importing 'from' a module / package. Furthermore, imports should be grouped:

      1. standard library imports
      2. third-party imports
      3. local project imports

      ... Where the groups are separated by a single empty line.1

      Putting this together:

      from msvcrt import getch
      import os
      # Sort alphabetically
      
      import colorama
      from colorama import Fore, Back, Style
      
    • Functions should follow the snake_case naming convention.2 Classes should follow the PascalCase naming convention.3

    • Top-level function and class definitions should be preceded by two empty lines.4

    • Keyword arguments should separate their name and default value only by an equals-sign ('='), not by any spaces.5

    • Don't mix single and double quotes. They're functionally the same, but when you start mixing them, this can become confusing to people who are used to other languages, like C, where there is a difference.

  2. In getKeyInput(), you use 7 magic numbers.

    Instead of adding comments to explain what these numbers represent, you can give them a name. For example, by declaring these constants at the top of the file:

    NEXT_KEY_IS_ARROW_KEY = 224
    UP_ARROW_KEY = 72
    DOWN_ARROW_KEY = 80
    ENTER_KEY = 13
    
    POINTER_UP = 1
    POINTER_DOWN = -1
    PRESS_BUTTON = 0
    

    ... no additional explanation is needed in getKeyInput():

    def getKeyInput():
        key = ord(getch())
        if key == NEXT_KEY_IS_ARROW_KEY:
            key = ord(getch())
        if key == UP_ARROW_KEY:
            return POINTER_UP
        elif key == DOWN_ARROW_KEY:
            return POINTER_DOWN
        elif key == ENTER_KEY:
            return PRESS_BUTTON
    

    (Even better would be to use an enum).

Bugs

  1. You forgot to define quit().

  2. I'm not sure if this is a quirk of my terminal emulator / MacOS Sierra, or if it's present regardless of setup, but the bounds checking in Menu.show() doesn't do its job: I can keep moving the pointer down forever, which causes interesting screen glitches.6, 7

  3. This isn't strictly a bug, but if a user presses anything other than , , or return, the program crashes.

Public API

  1. I know you used the -tag, but you might reach a broader audience by making this platform-independent.

    msvcrt and os.system("cls") only work on Windows platforms. If I run the program on MacOS, I get a ModuleNotFoundError. Even if I could patch that, the os.system("cls") call would print out an error message instead of clearing the screen. There's three partially satisfying solutions:

    • msvcrt.getch() returns a single keypress. By default, the terminal is in 'cooked' mode. (I'm using UNIX terminology, not sure how much of this applies to Windows) so sys.stdin.read(1) won't work, as the user must press enter first. By putting the terminal in 'cbreak' mode, we can read one keypress at a time. The standard library tty.setcbreak() function does exactly that. One small issue is that there is no standard library function for resetting the terminal mode. Luckily, it's as simple as saving the terminal options, putting the terminal in cbreak mode, and restoring the saved options when done.

    • On Windows, the command for clearing the screen is '$ cls'. On most other systems, the command is '$ clear'. Keeping that in mind (pseudocode):

      if platform is Windows then
          $ cls
      else
          $ clear
      
    • If your terminal supports ANSI escape codes, you don't even need to make a system call to clear the screen. Instead, you print the 'Erase in Display' code (source, modified for formatting):

      CSI n J

      ED – Erase in Display

      Clears part of the screen. If n is 0 (or missing), clear from cursor to end of screen. If n is 1, clear from cursor to beginning of the screen. If n is 2, clear entire screen (and moves cursor to upper left on DOS ANSI.SYS). If n is 3, clear entire screen and delete all lines saved in the scrollback buffer (this feature was added for xterm and is supported by other terminal applications).

      A major drawback of this approach is that there is no straightforward way of testing whether or not a terminal supports these codes. Most modern terminals do, but there are some exceptions.8

  2. Unless you plan on extending the class with methods, comm is superfluous. A lambda is enough.

  3. Move colorama.init() to the if __name__ == "__main__" body.

  4. Try abbreviating names less. While abbreviating is customary in other languages, in Python, developers should be as verbose as possible (within reason).

  5. Make functions and attributes private when it makes sense. Especially when you're designing a public API, you want to hide away all the unimportant / difficult to comprehend stuff. For example, in Menu.__init__(), all the arguments should be private attributes, because developers should not be allowed to modify them after initialization.

  6. Add docstrings to public functions and classes.

Names

  1. getKeyInput() is a misleading name. It doesn't return user input, it returns an action (move the cursor up, move the cursor down, press a button) based on the input. I suggest get_action_from_user_input().

  2. In this context, the variable name buffer suggests it has to do with I/O buffering, which is not true. I suggest decorative_line (?).

  3. Menu. Well, sure. But what kind of menu? It's a menu where you make a selection → SelectionMenu.

  4. Menu.show() tells only part of the story. It doesn't just output stuff, it also waits for user input: it is waiting for the user to press return. How about Menu.await_selection()?

My take on it

import enum
import os
import sys

import colorama as cla

try:
    import msvcrt
except ImportError:
    # POSIX
    import termios
    import tty


class Terminal:
    """A `Terminal` instance represents a terminal (emulator)."""

    _ANSI_CODE_CLEAR_SCREEN = "\033[2J"
    _ANSI_CODE_MOVE_CURSOR_TOP_LEFT = "\033[1;1H"

    def __init__(self, ansi_escape_codes_supported=False):
        """Initialize the terminal.
        If this method is called on a POSIX system, set the terminal
        mode to 'cbreak'.

        Arguments:
            - ansi_escape_codes_supported [bool]: Whether or not to
            assume ANSI escape codes are supported. Defaults to
            `False`. When in doubt, don't set to `True`.
        Returns:
            - `None`.
        """
        if os.name == "posix":
            self.posix = True
        elif os.name == "nt":
            self.posix = False
        else:
            raise RuntimeError("Only POSIX and NT systems are supported.")
        self._ansi_escape_codes_supported = ansi_escape_codes_supported
        self._prepare()

    def _prepare(self):
        """Prepare the terminal.
        On NT systems, this is a no-op. On POSIX systems, save the
        terminal attributes, then set the terminal mode to 'cbreak'
        using `tty.setcbreak()`.

        Returns:
            - `None`.
        """
        if not self.posix:
            return
        self._saved_terminal_options = termios.tcgetattr(sys.stdin)
        tty.setcbreak(sys.stdin)

    def clear_screen(self):
        """Clear the screen.
        If ANSI escape codes are supported, print ED (CSI 2 J), then
        print CUP (CSI 1 ; 1 H). Otherwise: on POSIX systems, call
        '$ clear'. On NT systems, call '$ cls'.

        Returns:
            - `None`.
        """
        if self._ansi_escape_codes_supported:
            print(self._ANSI_CODE_CLEAR_SCREEN)
            print(self._ANSI_CODE_MOVE_CURSOR_TOP_LEFT, end="")
        elif self.posix:
            os.system("clear")
        else:
            os.system("cls")

    def get_character(self):
        """Get a single character from stdin.

        Returns:
            - [str] A single character.
        """
        if self.posix:
            return sys.stdin.read(1)
        return msvcrt.getch()

    def finalize(self):
        """Finalize the terminal.
        On NT systems, this is a no-op. On POSIX systems, reset the
        saved terminal attributes.

        Returns:
            - `None`.
        """
        if not self.posix:
            return
        termios.tcsetattr(
            sys.stdin,
            termios.TCSANOW,
            self._saved_terminal_options
        )


class POSIXCodePointValues:
    """
    MacOS Sierra code point values.
    NOTE: Don't rely on this!
    There are no standardized code points for arrow keys, so these are
    the values for my machine. They are most likely *not* the same for
    you.
    """
    NEXT_KEY_IS_ARROW_KEY_SEQ = (27, 91)
    UP_ARROW_KEY = 65
    DOWN_ARROW_KEY = 66
    ENTER_KEY_SEQ = (10, 13)
    # Either 10 or 13


class NTCodePointValues:
    """
    NT code point values.
    NOTE: These are most likely not very portable either.
    """
    NEXT_KEY_IS_ARROW_KEY = 224
    UP_ARROW_KEY = 72
    DOWN_ARROW_KEY = 80
    ENTER_KEY = 13  


class UIAction(enum.Enum):
    """Values:
        - POINTER_UP (1): Move the cursor up one row.
        - POINTER_DOWN (2): Move the pointer down one row.
        - PRESS_BUTTON (3): Press the currently selected button.
        - NONE (4): Do nothing.
    """

    POINTER_UP = 1
    POINTER_DOWN = 2
    PRESS_BUTTON = 3
    NONE = 4


def _get_action_from_user_input_nt(terminal):
    code_point = ord(terminal.get_character())
    if code_point == NTCodePointValues.ENTER_KEY:
        return UIAction.PRESS_BUTTON
    elif code_point != NTCodePointValues.NEXT_KEY_IS_ARROW_KEY:
        return UIAction.NONE

    # Definitely an arrow key
    code_point = ord(terminal.get_character())
    if code_point == NTCodePointValues.UP_ARROW_KEY:
        return UIAction.POINTER_UP
    elif code_point == NTCodePointValues.DOWN_ARROW_KEY:
        return UIAction.POINTER_DOWN
    # Left or right arrow key
    return UIAction.NONE


def _get_action_from_user_input_posix(terminal):
    code_point = ord(terminal.get_character())
    if code_point in POSIXCodePointValues.ENTER_KEY_SEQ:
        return UIAction.PRESS_BUTTON
    elif code_point != POSIXCodePointValues.NEXT_KEY_IS_ARROW_KEY_SEQ[0]:
        return UIAction.NONE

    code_point = ord(terminal.get_character())
    if code_point != POSIXCodePointValues.NEXT_KEY_IS_ARROW_KEY_SEQ[1]:
        return UIAction.NONE

    code_point = ord(terminal.get_character())
    # Definitely an arrow key
    if code_point == POSIXCodePointValues.UP_ARROW_KEY:
        return UIAction.POINTER_UP
    elif code_point == POSIXCodePointValues.DOWN_ARROW_KEY:
        return UIAction.POINTER_DOWN
    # Left or right arrow key
    return UIAction.NONE


def get_action_from_user_input(terminal):
    """Get a UI action from user input.
    Read characters from stdin until a relevant UI action can be
    constructed. See `UIAction` for the possible actions.

    Arguments:
        - terminal [Terminal]: A terminal instance, used for reading
        single characters.

    Returns:
        - [UIAction] A UI action.
    """
    if terminal.posix:
        return _get_action_from_user_input_posix(terminal=terminal)
    return _get_action_from_user_input_nt(terminal=terminal)


class SelectionMenu:
    """A `SelectionMenu` represents a group of options,
    from which the user can pick only one.
    """

    _RESET_COLOUR = cla.Fore.WHITE + cla.Back.BLACK

    def __init__(
        self,
        title,
        options,
        callbacks,
        terminal,
    ):
        """Initialize the menu.

        Arguments:
            - title [str]: The text to display as a title.
            - options [list]: A list of options the user can pick from.
              The elements must be `str` instances.
            - callbacks [list]: A list of functions to call when the user
              makes a selection. The length of this list must be equal to
              the amount of options provided.
            - terminal [Terminal]: A terminal instance to use for I/O.

        Returns:
            - `None`.
        """
        self._title = title
        self._options = options
        self._callbacks = callbacks
        self._terminal = terminal
        self._pointer = 0

    def await_selection(self):
        """Wait for the user to make a selection.
        Handle user input, draw the menu, and wait until the user
        presses enter.

        Returns:
            - `None`.
        """
        self._terminal.clear_screen()
        decorative_line = "-" * len(self._title)
        print("{}\n{}\n{}".format(
            decorative_line,
            self._title,
            decorative_line
            )
        )

        for option in self._options:
            colour = cla.Fore.WHITE + cla.Back.BLACK
            if self._pointer == self._options.index(option):
                # i.e. if this is the currently selected button
                colour = cla.Fore.BLACK + cla.Back.WHITE
            print("{}{}\n{}".format(colour, option, self._RESET_COLOUR), end="")

        action = get_action_from_user_input(self._terminal)

        if action == UIAction.PRESS_BUTTON:
            self._terminal.clear_screen()
            if self._callbacks:
                self._callbacks[self._pointer]()
            else:
                print("Picking {}".format(self._options[self._pointer]))
            self._terminal.finalize()
            return None
        elif action == UIAction.POINTER_DOWN:
            self._pointer += 1
        elif action == UIAction.POINTER_UP:
            self._pointer -= 1

        self._pointer = max(min(self._pointer, len(self._options) - 1), 0)
        # Assure the pointer stays within bounds
        self.await_selection()


def margherita():
    print("Margherita it is!")


def pepperoni():
    print("Pepperoni it is!")


def four_cheese():
    print("Four Cheese it is!")


def no_pizza():
    print("Curious.")


if __name__ == "__main__":
    cla.init()

    TITLE = "Pizza"
    BUTTONS = [
        "Margherita",
        "Pepperoni",
        "Four Cheese",
        "Ew! No pizza for me..."
    ]
    ACTIONS = [
        margherita,
        pepperoni,
        four_cheese,
        no_pizza
    ]

    terminal = Terminal(True)

    select_a_pizza = SelectionMenu(TITLE, BUTTONS, ACTIONS, terminal=terminal)
    select_a_pizza.await_selection()

References

1 PEP-8: Imports

2 PEP-8: Function and variable names

3 PEP-8: Class Names

4 PEP-8: Blank Lines

5 PEP-8: Other Recommendations

6 Here's what I mean.

7 Using ANSI escape codes fixes the problem for me, which suggests the bug is related to '$ clear'.

8 Wikipedia: ANSI escape code: Platform support

Appendix: My reasoning

With most of the points I wanted to make out of the way, I want to explain some of the changes I made.

  • I started out by rewriting getKeyInput() to handle both POSIX and NT systems. After having found out I was also going to need prepare_terminal(), reset_terminal(), clear_screen() and get_character() functions, I decided to extract all terminal-related stuff into a class of its own.

  • I originally let the Menu constructor initialize a new Terminal instance. That's a pretty bad design choice: it means every time a UI widget is created, a new Terminal instance is created as well; and developers can't interact with the terminal directly. Therefore, I made it a parameter.

  • Since code point values associated with arrow keys differ per system, I created a class to hold the values for my laptop. From the little research I've done, it seems the values are the same for most recent Windows releases.

  • Instead of crashing, the UI should ignore meaningless input. That's why I added UIAction.NONE to the UIAction enum.

Appendix: Design

API design

Since you plan on matching PyGame's functionality, you should take the time to think about how you want to design your API. You've taken an event-driven (callback functions) approach, which is probably the most sensible thing to do for user interfaces in general. As it stands, though, the caller can't retrieve the return value of the callback functions, which means in more complex cases, you'd have to modify a shared variable, which can get really messy, really fast. And I haven't even covered threading yet.

Interrupt handling

If I press Ctrl-C, I get a bunch of traceback lines, but more importantly, my terminal is still in cbreak mode. Once your codebase starts growing, it becomes tiresome to wrap every terminal.get_character() call in a try / except-block. You could put the block in terminal.get_character(), but that would mean a forceful exit any time the user presses Ctrl-C. Unfortunately, there's no trivial solution.

Backend

I have my doubts about the UI backend:

  • Using recursion in Menu.show(). There's really no need to, you might as well use a while-loop.

The following two points only apply if you want your UIs to be dynamic, and you need to create reasonably complex applications.

  • There's no parent-child relationship. You can create two instances of Menu, but they won't be related to one another. The second instance simply waits for the first one to complete, and then, completely independently, does its thing. If you want to add other types of UI widgets (frames, text labels, input fields, multiline text boxes, sliders, ...), you need a widget hierarchy.

  • Text is left-aligned, and there's no way of placing it dynamically. Once you've implemented a widget hierarchy, you need a way to map widgets anywhere on-screen. This is where ANSI escape sequences come in handy, because they allow you to move the cursor anywhere you want, and even save the cursor position and restore it later.

Getting this right is possible, but you must be willing to spend time and effort to do research, lay out an API, and document it.

\$\endgroup\$
1
  • \$\begingroup\$ I think you have two typos in your code. in the SelectionMenu class self._RESET_COLOUR = cla.Fore.WHITE + cla.Back.BLACK (l.200) should be _RESET_COLOUR = cla.Fore.WHITE + cla.Back.BLACK and colour = clas.Fore.BLACK + cla.Back.WHITE (l.250) should be colour = cla.Fore.BLACK + cla.Back.WHITE \$\endgroup\$
    – ovs
    Commented May 12, 2018 at 12:30

Not the answer you're looking for? Browse other questions tagged or ask your own question.