Optimisation and Better functions

[USLTD]

Fixed few performance and functional issues

[USLTD]

I currently only have Termux (Linux terminal environment for Android) so I can't test Windows functions therefore they are experimental

[USLTD]

Also for some reason click and release functions are not registered on my Termux during testing, although scrolling does. I don't know what is reason so I will check code again and test each parts. If it is working on other OSes than it is okay 👍

[USLTD]

Checked original code and it registers click/releases so it means something is wrong in my code. But I don't see any problems.

[bczsalba]

Hey!

May I ask what's up with all the formatting changes you've also included? I appreciate the sentiment of the PR, but there is hundreds of odd changes to files you didn't touch otherwise.

There is no contribution guide as of yet, but if there was it would mention the use of both pylint and black. I haven't actually run your code yet as I haven't had time, but running it through pylint gave a good couple of results.

[USLTD]

I used black (formatting) and isort (sorting imports). I didn't used pylint but used mypy which yielded errors but code itself was working. I only manually refactored ansi_interface.py. I left translate_mouse and report_mouse functions currently as it is but will refactor them as soon as I will have time. I also modified few functions that used tput to use ANSI sequences instead so they would be cross-platform. Also I changed set_echo and unset_echo so they would be cross-platform too (including Windows) but since I tested it on Linux it is currently experimental and only in theory. I also added TerminalModes class which has raw mode function which you can use like this:

with terminal_modes.NoEcho():
    # Starts `no_echo` mode
    # Do something
# Exits `no_echo` mode

I also refactored Color class and hex translation is now better

[USLTD]

Also as I see your code is not optimized and thus decreasing performance. Please don't use _ in places where you aren't intending function, variable or class to be private.

[USLTD]

Please follow PEP 8 rules

[bczsalba]

In what places do I not follow the _ internal notation? Pylint yells at you if you access any underscore name from outside it's intended scope, and I make very certain it doesn't happen. And even if I did misuse this notation, that does not mean the code is unoptimized.

Also, black and pylint are made to follow PEP8, and my code fully compiles with it.

[bczsalba]

So I looked at the code some more, and I do enjoy a lot of the changes. My problem is that this PR is massive, and there is quite a few things I do not enjoy about it, and without having them in granular commits it is very hard to pick apart what to add and what not to.

I particularly enjoy the context manager functionalities added to the terminal, but I feel like it can be merged into the _Terminal class, as it doesn't really do all the much at the moment. I also like the idea for the _WinSIGWINCH implementation, though I'm not quite sure on the use of async for it (as all other parts of the code are threaded instead).

[USLTD]

signals are asynchronous, not threaded (It says in documentation that they are asynchronous) so I decided to use asynchronous implementation. About TerminalModes class being merged with Terminal (not _Terminal. Please remove _ during imports and in variables/classes which are intended to be public) class is possible but I prefer them being separated since I am also planning to add few other modes context managers and it would be more readable for them to be in separate class. Also I tested code and it's working. (Don't know hover and drag events since I don't have physical mouse, only touchscreen)

[USLTD]

Also I am researching ANSI mouse reports and I will update code as soon as I have working functions for both UTF-8 (1005 mode) and Urxvt (rxvt-unicode) (1015 mode). I know formats but don't know what are event codes so I will research them. Also it would be better to change translator to work with regex capture groups instead of manual parsing (which is inefficient and decreases speed). But for know it will be better to merge this PR.

[bczsalba]

Terminal (not _Terminal. Please remove _ during imports and in variables/classes which are intended to be public)

As I explain in my review, _Terminal is NOT intended to be public, terminal is. I don't really appreciate the tone of voice here either.

Also it would be better to change translator to work with regex capture groups instead of manual parsing (which is inefficient and decreases speed)

What do you mean here? It already uses regex, the only part that doesn't is the M suffix which is only present in one subtype.

But for know it will be better to merge this PR.

As you can probably tell from my review above I don't really think so. I'm not really sure why you use such declarative language in these comments, saying "it might be better" sounds a lot less demanding.

[USLTD]

Terminal (not _Terminal. Please remove _ during imports and in variables/classes which are intended to be public)

As I explain in my review, _Terminal is NOT intended to be public, terminal is. I don't really appreciate the tone of voice here either.

Also it would be better to change translator to work with regex capture groups instead of manual parsing (which is inefficient and decreases speed)

What do you mean here? It already uses regex, the only part that doesn't is the M suffix which is only present in one subtype.

But for know it will be better to merge this PR.

As you can probably tell from my review above I don't really think so. I'm not really sure why you use such declarative language in these comments, saying "it might be better" sounds a lot less demanding.

Sorry if I sounded declarative but I didn't meant to. My English knowledge is not that high so I use translator occasionally which doesn't always yield how I want to (but what choice do I have 😅) say. Also this part here

But for know it will be better to merge this PR.

This is mistake of translator. I wanted to say following (I don't know if this is correct translation)

I think this PR after merging will better it.

Yes it sounds so much different but that's how bad translator is. Again, sorry if I sounded declarative 😅 but please know that I didn't meant to

[bczsalba]

That's alright! I still stand by my statements on the code in general, however.

[USLTD]

Alright I will close this PR and open new but in parts. First will be WinSIGWINCH implementation (but it will stay asynchronous) Second: TerminalModes which will be merged with _Terminal. Third: Better Color Then optimisation of others

[bczsalba]

That would be perfect! I'd prefer if you didn't run isort on the code, as it creates a lot of changes that I will not merge, so it's easier for everyone.

[USLTD]

Alright

[USLTD]

What about black? Also I will use pylint this time

[bczsalba]

You should use black, thank you!