PySide2 version + integration with QProcess. about termqt HOT 19 CLOSED

Thanks for your interest in this little project!

1. I used PyQt5 because it was the choice of the greater project where I was supposed to deploy this module. I don't see any problems using PySide2. I know that a lot of PyQt projects do some sort of conditional import at the beginning of the script (they search for both PyQt5 and PySide2 and import whatever is installed).

2. Yes, column is an important concept in implementing a "console". A lot of console programs actually count how many columns there are and move the cursor between different columns.

3. I'm not a fan of Python 2.7 and I think the intentional usage of those out-of-date versions should never be encouraged. You are free to start a new branch and backport it to Python 2.7, but I think there is little support I can provide since I don't plan to set up a Python 2.7 development environment.

from termqt.

from termqt.

Variable column width is a very common situation for modern terminal emulators. Console programs that need to know the number of columns will emit a escape sequence to the terminal and the terminal is responsible for returning this information to the programs.

In your case, the console has no way to tell the program anything. This could be a potential problem. However, most of the simple programs don't require this. And a lot of programs have some fallback settings so that even if the terminal emulator doesn't support querying, it can still behave in a way that is not too wrong.

I would say, you can try some of the programs you intend to use with the example startup script but comment out the stdin callback and see how things behave. If the program itself doesn't behave well, that would be a roadblock and we need to see if there is any workaround.

from termqt.

Hello @TerryGeng
I finally got some time today to try this. I've had to do a few minor changes to get it working at all, which are in this commit master...herronelou:termqt:master (although I noticed after committing that my IDE has done some formatting changes, I'll revert those).

For the most it seems to work, however, I might be doing something wrong, as ANSI symbols do not seem to get handled when I feed them to Terminal.write_at_cursor:

I'm also wondering if there is a particular reason why you decided to require connecting an external scrollbar to your widget rather than having the scrollbar as part of the widget?

from termqt.

So, I have found that the reason it wa snot processing ANSI was because I was connecting my signal sending already UTF8 decoded strings to write_at_cursor instead of stdout.

Connecting to stdout made it process properly, and it also made the terminal repaint properly. I had to add a repaint call in an earlier commit because the terminal would not repaint the canvas when writing. I have reverted that change for the time being, however, you might want to consider adding it if the terminal is expected to display things when write is called.

Switching to the stdout() method made ANSI work as expected, however, it broke UTF-8 decoding, as you insert the bytes one by one, but UTF-8 could have a variable number of bytes per char. I have fixed that for my use case, see diff in previous post.

If you would like to reproduce the issue, here is the bytes object I'm feeding to Terminal.stdout(): b'\rTotal progress: 100%|\xe2\x96\x88\xe2\x96\x88\xe2\x96\x88\xe2\x96\x88\xe2\x96\x88\xe2\x96\x88\xe2\x96\x88\xe2\x96\x88\xe2\x96\x88\xe2\x96\x88| 20/20 [00:04<00:00, 6.47it/s]\x1b[A\rTotal progress: 100%|\xe2\x96\x88\xe2\x96\x88\xe2\x96\x88\xe2\x96\x88\xe2\x96\x88\xe2\x96\x88\xe2\x96\x88\xe2\x96\x88\xe2\x96\x88\xe2\x96\x88| 20/20 [00:04<00:00, 4.81it/s]\n' (This is generated by another process that is using https://github.com/tqdm/tqdm to generate progress bars).

from termqt.

I don't recommend sending actually string instead of bytestring since the escape processor work with bytes. I would recommend you use bytestring instead and stick to the stdout function. It also handles repainting smarter, speeding things up quite a bit. If you directly call write_at_cursor, then indeed you need to handle the repaint by yourself.

from termqt.

from termqt.

Hi Erwan,

I had a look at your branch. Here are some feedbacks:

• I maintain the view that you shouldn't change anything in _stdout_string. I don't have the intention to expose this function to users. I understand that those changes might have been done when you were still struggling with bytes and strings. I recommend sticking to bytes and using stdout instead. You might let me know more about your specific use case so I might offer more help. The very reason I built this widget is to use it in a remote setting where the terminal widget is connected to some process running on a server. I guess it is very close to your case.

• Not sure why you changed the logger part. If you don't have a logger, just omit this argument when initializing and it would be fine.

from termqt.

Hi Terry.
Those 2 were bug fixes.

For the logger:
If initializing a Terminal without providing the logger argument in your code, the following happens:
https://github.com/TerryGeng/termqt/blob/master/termqt/terminal_widget.py#L54
self.logger is assigned a default logger, but the logger variable is not updated (still None).
https://github.com/TerryGeng/termqt/blob/master/termqt/terminal_widget.py#L57
Here, the parent class TerminalBuffer is initialized, but using the unmodified logger variable which is still None.
https://github.com/TerryGeng/termqt/blob/master/termqt/terminal_buffer.py#L561
Here, self.logger is set again, but this time it's assigned None.

This results in any time self.logger.info() is called it raises an AttributeError.

Now for the changes in _stdout_string, your code did not in fact support utf8 encoding, it failed to decode any character that doesn't fit into a single byte. UTF8 may encode the characters with 1 to 4 bytes. What you had, in super simplified pseudo-code:

for byte in bytes:
    string = ''
    if byte_is_special:
        do_special_action()
    else:
        string += char(byte)  # This can't be properly decoded
write(string)

Which I have changed to:

for byte in bytes:
    new_bytes = bytearray()
    if byte_is_special:
        do_special_action()
    else:
        new_bytes += byte
write(new_bytes.decode('utf-8'))  # Decoding is done here now, so we can make use of proper uft8 (or other encodings)

This is still not perfect, for example, I'm unsure if any character composed of multiple bytes in utf8 might have one of the bytes match with one of the specially defined bytes, and mess up the decoding.
You can reproduce the error by sending the following bytes to your implementation: b'\rTotal progress: 100%|\xe2\x96\x88\xe2\x96\x88\xe2\x96\x88\xe2\x96\x88\xe2\x96\x88\xe2\x96\x88\xe2\x96\x88\xe2\x96\x88\xe2\x96\x88\xe2\x96\x88| 20/20 [00:04<00:00, 6.47it/s]\x1b[A\rTotal progress: 100%|\xe2\x96\x88\xe2\x96\x88\xe2\x96\x88\xe2\x96\x88\xe2\x96\x88\xe2\x96\x88\xe2\x96\x88\xe2\x96\x88\xe2\x96\x88\xe2\x96\x88| 20/20 [00:04<00:00, 4.81it/s]\n'
It should look like my screenshot above, but without my changes, it uses some different characters.

from termqt.

Nice catch. I will post a quick fix to the logger issues.

Regarding wide characters in UTF-8 encoding, I think this is the same problem described in #12. The main challenge here is to make auto-wrapping and resizing support handling wide characters. I consider this a higher-priority item than #13 and will have a look as soon as I get a chance.

from termqt.

I'm fairly confident my patch improves from what was there, I've seen #12 though it sounds more font related than decoding?
Let me share a screenshot to make the problem more obvious.

The expected result, using my changes:

The result when using your original code:

from termqt.

Yes. UTF-8 characters can be wide both in the bytes taken to store and in the actually displayed size. I think I should tackle them both at the same time.

I understand what you mean and confirm your code worked better, but I need to make more fundamental changes to fully support UTF-8, i.e. not just in _stdout_string but in other crucial places like paint, write, resize, etc.

Don't worry. I will take a look at it once I get a chance to think about it and understand what I actually need to do.

from termqt.

sounds good. I also noticed TerminalBuffer.stdout still uses _stdout_char, while Terminal uses _stdout_string, it feels like _stdout_char should be deprecated in favor of _stdout_string?

from termqt.

I agree. _stdout_char will be removed very soon.

from termqt.

I posted a PR that implements the wide-character support. Would you mind having a look at it? Thanks!

from termqt.

That works nicely!
I've applied those changes onto my fork as a patch and ran the tests that failed with the previous version, and it seems like things are resolved. I've noticed a minor issue that was already present in the older version and will make a separate issue for it, other than that all good.

from termqt.

Closing this, most questions have been answered, I'll keep my fork for the Qt.py imports, and make separate issues if new things crop up. Thanks!

from termqt.

Closing this, most questions have been answered, I'll keep my fork for the Qt.py imports, and make separate issues if new things crop up. Thanks!

I actually think Qt.py might be a better idea. I would be happy to test and see how well it works if you open a PR.

Cheers!

from termqt.

Sure, I'll do that when I find a bit of time

from termqt.