aisk / pick

create curses based interactive selection list in the terminal
MIT License
721 stars 60 forks source link

Using pick.pick within curses.wrapper() breaks input #91

Closed snail-coupe closed 1 year ago

snail-coupe commented 1 year ago

test_pick.txt

I'm trying to use pick to add a menu to an existing application, however after doing so the application fails to detect input correctly.

In the attached example I get and print a key press, run a pick menu, then attempt to get and print another key press. However rather than the second keypress be read it ends up being echoed to the terminal.

https://user-images.githubusercontent.com/53581981/202872656-21cd55a7-4fc6-4e5a-bd55-041eb38a1ade.mp4

This using Python 3.9 running in a WSL Debian Bullseye

$ pip list | grep pick
pick              2.1.0
$ python -V
Python 3.9.2
$ uname -a
Linux Potoroo 5.15.68.1-microsoft-standard-WSL2 #1 SMP Mon Sep 19 19:14:52 UTC 2022 x86_64 GNU/Linux
$ head -1 /etc/os-release
PRETTY_NAME="Debian GNU/Linux 11 (bullseye)"
snail-coupe commented 1 year ago

I don't know if this relates to / is same as #83

snail-coupe commented 1 year ago

For completeness, I also tested on my raspberry pi and it was reproducible:

$ pip list | grep pick
pick        2.1.0
$ python -V
Python 3.10.7
$ uname -a
Linux possum 5.15.61-v7l+ #1579 SMP Fri Aug 26 11:13:03 BST 2022 armv7l GNU/Linux
$ cat /etc/debian_version
11.5
wong2 commented 1 year ago

Thanks for your report, I believe the problem is you're creating nested curses screens: pick itself is also running inside a curses.wrapper call.

wong2 commented 1 year ago

Perhaps pick can accept an optional screen parameter, if provided, use it instead of creating a new curses screen.

snail-coupe commented 1 year ago

That would make sense as it'd mean pick had done all the teardown type operations.

(and I've seen similar with a different menu module too)

I'd be happy to create a fork and give that a try (and make a PR if it works) if you'd like

wong2 commented 1 year ago

Sure! Go ahead

snail-coupe commented 1 year ago

That seems to fix it, at least in my limited use case (and shouldn't break your existing code path)