prompt-toolkit / python-prompt-toolkit

Library for building powerful interactive command line applications in Python
https://python-prompt-toolkit.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
9.1k stars 717 forks source link

Refactoring of StdoutProxy to take app_session from the caller #1854

Closed jooncheol closed 4 months ago

jooncheol commented 4 months ago

This changes makes StdoutProxy to take the exact app_session from by taking it as optional argument in the constructor instead finding it by calling get_app_session() globally.

It can avoid confliction of printing output within the patch_stdout context in the ssh session when the multiple ssh connections are performing concurrently.

The changed StdoutProxy now passes the correct app instance from the given app_session in the constructor to the run_in_terminal() in it instead of calling get_app_or_none() globally that can give wrong app instance from the prompt session in the last ssh connection.

The key usage has been added into the example ssh/asyncssh-server.py.


How to test

% ssh -p 8222 localhost
We will be running a few prompt_toolkit applications through this 
SSH connection.

 100.0% [================================================>]  50/ 50 eta [00:00]
(normal prompt) Type something:
You typed 
(autocompletion) Type an animal:
You typed 
(HTML syntax highlighting) Type something:
You typed 
Showing yes/no dialog... [ENTER]
Showing input dialog... [ENTER]
Counter: 0
Counter: 1
Counter: 2
Counter: 3
Type something with background task: test
You typed: test
Connection to localhost closed.
jooncheol@Jooncheols-MBP ~ % 

Result


Background & motivation I found the issue when I used patch_stdout in the asyncssh-server.py based custom ssh server. Basically the print within the patch_stdout block was messed because of the multiple the patch_stdout context interfere the other one from the other ssh session.

I used "with StdoutProxy(...) as output" in the example code asyncssh-server.py to test this patch. It can more explicitly use the right output object than using "print" within "patch_stdout" block in case of the ssh server example. I wanted to touch "patch_stdout" to take the app_session too to achieve the goal, but It seems to be dangerous for the existing echo system of this project. so, I tried to modify only the StdoutProxy class to take the additional optional parameter 'app_session'.

Thanks in advance for the feedback / review. and thanks for the good library!