IngoMeyer441 / simple-term-menu

A Python package which creates simple interactive menus on the command line.
MIT License
492 stars 43 forks source link

On unsupported environments, show() leaves the terminal in an unusable state #66

Closed Tina-otoge closed 1 year ago

Tina-otoge commented 1 year ago

First off, thank you for this library.

I'm writing a script that can be run on many different platforms. I first attempt to run TerminalMenu.show, if it fails, I revert to a simpler prompt using print and input.

The way I'm achieving this is by doing something like so

choices = ["1", "2", "3"]
menu = TerminalMenu(choices)
try:
    index = menu.show()
except Exception:
    print('\n'.join(choices))
    text = input("Your choice? > ")
    index = int(text)

However, before throwing an exception, TerminalMenu, runs some modifications on the terminal that leaves it in an unusable state, we can not grab any input from the user anymore. The call to input stay hanging forever with no way to input any text.

    # hangs forever, terminal cursor does not appear, we can not input text or use ctrl+c etc
    text = input("Your choice? > ")
    index = int(text) # will never be reached

Currently, my approach is to run TerminalMenu._reset_term() before the call to input, which fixes my issue, but 1) feels ugly because I'm calling a private and 2) would be best if the terminal was left untouched if TerminalMenu isn't going to work

It would be great if TerminalMenu could determine ahead of time that it is not going to be able to work, and do not modify the terminal at all by aborting early if so.

Example traceback on an unsupported environment:

>>> from simple_term_menu import TerminalMenu
>>> TerminalMenu(["test"]).show()
Traceback (most recent call last):
  File "/data/data/com.termux/files/usr/lib/python3.10/site-packages/simple_term_menu.py", line 1467, in show
    self._paint_menu()
  File "/data/data/com.termux/files/usr/lib/python3.10/site-packages/simple_term_menu.py", line 1370, in _paint_menu
    displayed_menu_height += print_menu_entries()
  File "/data/data/com.termux/files/usr/lib/python3.10/site-packages/simple_term_menu.py", line 992, in print_menu_entries
    all_cursors_width = wcswidth(self._menu_cursor) + (
  File "/data/data/com.termux/files/usr/lib/python3.10/site-packages/simple_term_menu.py", line 115, in wcswidth
    wcswidth.libc = ctypes.cdll.LoadLibrary("libc.so.6")  # type: ignore
  File "/data/data/com.termux/files/usr/lib/python3.10/ctypes/__init__.py", line 452, in LoadLibrary
    return self._dlltype(name)
  File "/data/data/com.termux/files/usr/lib/python3.10/ctypes/__init__.py", line 374, in __init__
    self._handle = _dlopen(self._name, mode)                      OSError: dlopen failed: library "libc.so.6" not found

During handling of the above exception, another exception occurred:                                                                 
Traceback (most recent call last):                                  File "<stdin>", line 1, in <module>
  File "/data/data/com.termux/files/usr/lib/python3.10/site-packages/simple_term_menu.py", line 1531, in show
    self._clear_menu()
  File "/data/data/com.termux/files/usr/lib/python3.10/site-packages/simple_term_menu.py", line 1388, in _clear_menu
    assert self._previous_displayed_menu_height is not None
AssertionError

Here the issue is libc.so.6 not being available, a similar error occurs if tput is not available.

Usage in an actual project: https://github.com/Tina-otoge/nyaadl/blob/45dc11d09bd0a8d71b7ec0f34ce511c8b3c2039a/src/nyaadl/cli.py#L51-L54

IngoMeyer441 commented 1 year ago

Thanks for your bug report. Yes, simple-term-menu should not alter the terminal environment on unsupported platforms or at least should restore the initial state on errors automatically. I am curious: On which system happens the libc load error?

Tina-otoge commented 1 year ago

On which system happens the libc load error?

On Termux (Android)

Quite the edge case but it was an important environment target for my project.

IngoMeyer441 commented 1 year ago

I added a patch for the libc and wcwidths loading issues to the latest develop branch. The wcwidths function now has a default fall back to the Python len function which is not perfect but is better than nothing and does not crash the menu any more.

I could not reproduce the problems with tput. In my tests, an exception is raised, but the terminal state is left untouched.

Other errors which could occur during the show method should not lead to terminal state issues since there is a call to _reset_term in a finally block.

Could you please test the latest patch? You can obtain the latest development version directly from the GitHub repository:

pip install git+https://github.com/IngoMeyer441/simple-term-menu@develop
Tina-otoge commented 1 year ago

It indeed does not fail anymore now 🙏

https://user-images.githubusercontent.com/9400466/191036584-fe887041-493d-4973-ae9f-9d9523af9a20.mp4

IngoMeyer441 commented 1 year ago

Great, I can create a bug fix release later 👍

IngoMeyer441 commented 1 year ago

Sorry for the delay, the bug fix release is finally released on PyPI. Feel free to reopen if necessary.