General
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:
- standard library imports
- third-party imports
- 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.
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
You forgot to define quit()
.
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
This isn't strictly a bug, but if a user presses anything other than △, ▽, or return, the program crashes.
Public API
I know you used the windows-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
Unless you plan on extending the class with methods, comm
is superfluous. A lambda
is enough.
Move colorama.init()
to the if __name__ == "__main__"
body.
Try abbreviating names less. While abbreviating is customary in other languages, in Python, developers should be as verbose as possible (within reason).
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.
Add docstrings to public functions and classes.
Names
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()
.
In this context, the variable name buffer
suggests it has to do with I/O buffering, which is not true. I suggest decorative_line
(?).
Menu
. Well, sure. But what kind of menu? It's a menu where you make a selection → SelectionMenu
.
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.