Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use ansi escape and readline so output doesnt mix with input in cli #638

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

gospodin55
Copy link

It works on linux, tell me if I should handle Windows console differently

@Et0h
Copy link
Contributor

Et0h commented Oct 15, 2023

In Windows I get the following error:
Traceback (most recent call last): File "syncplayClient.py", line 14, in <module> File "<frozen zipimport>", line 259, in load_module File "syncplay\ep_client.pyc", line 3, in <module> File "<frozen zipimport>", line 259, in load_module File "syncplay\clientManager.pyc", line 2, in <module> File "<frozen zipimport>", line 259, in load_module File "syncplay\ui\__init__.pyc", line 14, in <module> File "<frozen zipimport>", line 259, in load_module File "syncplay\ui\consoleUI.pyc", line 7, in <module> ModuleNotFoundError: No module named 'readline'

I assume that this is because readline has not been added to the buildPy2exe include list, i.e.:

'includes': 'twisted, sys, encodings, datetime, os, time, math, urllib, ast, unicodedata, _ssl, win32pipe, win32file, sqlite3',

@Et0h
Copy link
Contributor

Et0h commented Oct 15, 2023

I could be wrong, but I think the reason it errors on your reverted Windows tweak is because readline needs to be added to requirements.txt.

@gospodin55
Copy link
Author

It seems there's no native readline implementation for windows, I think it's too much to handle, I will just check for the current system and disable this feature if readline is unavailable

@Et0h
Copy link
Contributor

Et0h commented Oct 15, 2023

Ah, okay.

@Et0h
Copy link
Contributor

Et0h commented Oct 15, 2023

Just checked, and it looks like if there was demand for that functionality in the future it might be possible to do so with pyreadline3, but nobody has raised any concerns about the current behaviour of SynplayConsole.exe so for now it doesn't seem worth the complexity.

@daniel-123
Copy link
Contributor

daniel-123 commented Nov 13, 2023

I've tried to test this on Debian 13 - with this change closing the application (either by ctrl+c or by closing the media player) results in basically non-functional prompt. As in typing commands further doesn't cause them to show up for example, ctrl+c adds two new lines etc.

EDIT: just to specify - I have tested this with the CLI mode (--no-gui). Which is actually used by at least several people who made comments in various issues over the yers.

Since I'm not quite sure what exactly in detail is wrong with current behavior of Syncplay and what's the expected result of this pull request would actually do - only feedback I can currently provide is that this doesn't seem good. Could you explain what the "output doesnt mix with input in cli" means in an example?

@gospodin55
Copy link
Author

image
If a user receives a message while they're typing, it appears on the same line as their input and interferes with it.
I don't have the same issue as you. Maybe that depends on the shell/terminal, I should've tested it more

@daniel-123
Copy link
Contributor

I'm using largely standard Debian, so this is just bash.

What distro/terminal did you use?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants