termux-app icon indicating copy to clipboard operation
termux-app copied to clipboard

[Bug]: Incorrect handling of "Erase in Line" on wrapped line

Open crbednarz opened this issue 2 years ago • 3 comments

Problem description

I've been observing an issue for some time where text will occasionally be incorrectly offset to the left by one character.

I typically use Termux to SSH into another machine, attach to a tmux session, and open neovim (with a theme and other plugins). With this particular combination I'd frequently see the following: (Ignore the incorrect characters- I reset my font as part of trying to isolate the issue) Screenshot_20230816_195015_Termux (Note lines 15 and below) And even without tmux in play, the same issue could occur- albeit less frequently: Screenshot_20230816_193638_Termux (Note lines 13, 14, 19, and 26 are all incorrectly offset)

What made things more interesting is that if I attached to the same tmux session from another terminal, I would not see the misalignment.

To help isolate the problem, I ran script to capture the raw output and found the issue appeared to be with how lines were cleared. Whenever the entire display was resent, each line would begin with ^[[K to clear the line ahead. However, it appears that whenever a full row's worth of characters is sent, followed by ^[[K, instead of clearing the new line it clears the last character of the previous line and resets the cursor there.

In other words, if my terminal was 5 characters wide

^[[K12345^[[K12345

would display as

12341
2345

instead of

12345
12345

I believe this is especially frequent with tmux as it appears to always send a full line, even if it's just empty space. So the alignment is only corrected when an individual line is resent.

Now, I'm not remotely knowledgeable about terminal emulation, so hopefully my assessment hasn't been too far off base. But I'm thinking the intended behavior is for ^[[K to clear the line the cursor would have wrapped to.

Steps to reproduce the behavior.

To help reproduce the issue, I've created a Python script to showcase the behavior.

import os
from itertools import islice, cycle

def main():
    size = os.get_terminal_size()
    message = ''.join(islice(cycle("0123456789"), size.columns))
    print(("\x1b[K" + message) * 5)

if __name__ == '__main__':
    main()

This will print 5 lines beginning with ^[[K followed by 0-9 repeating until the full width of the terminal is reached.

On my phone I see the following: Screenshot_20230816_201033_Termux

While other terminals (such as Windows Terminal) I see: bug

What is the expected behavior?

When ^[[K would land on a new line due to wrapping, it should clear that new line and leave the cursor in place.

System information

  • Termux application version: Tested with both the latest f-droid version (v0.118) and the latest commit (0.118.0+7c262b8)
  • Android OS version: 13 (I believe this applied to 12 as well)
  • Device model: Galaxy Fold 4

crbednarz avatar Aug 17 '23 03:08 crbednarz

I did do some poking around in the source earlier, and it seems this is the code responsible for handling it: https://github.com/termux/termux-app/blob/master/terminal-emulator/src/main/java/com/termux/terminal/TerminalEmulator.java#L1023 I haven't had a chance to verify whether similar behavior is observed with other options (or DECSED), but they seem closely related.

If I can I'll try to put together a fix/PR for this, but it's been quite awhile since I've done any Android development, so I'm not sure how well that'll go.

crbednarz avatar Aug 17 '23 03:08 crbednarz

After debugging a bit, it seems the issue actually relates to here: https://github.com/termux/termux-app/blob/55cdef01e7b2c0c1c7946a95e716098bf441256c/terminal-emulator/src/main/java/com/termux/terminal/TerminalEmulator.java#L1524 (It was EL not DECSEL, although perhaps it applies there as well?)

It looks like when the last character of a line is written mAboutToAutoWrap is set to true, and the mCursorCol is not incremented. As the processing for EL does not take this into account, it simply clears the last column along with the mAboutToAutoWrap flag. This means the next character written will be on the last column, which previously had another character on it.

Currently I'm trying to work through why many of these cases under doCsi set mAboutToAutoWrap to false. I suspect there's a good reason. But either way it seems that one or more of those doCsi cases should be mAboutToAutoWrap-aware.

crbednarz avatar Aug 19 '23 03:08 crbednarz

Related https://github.com/termux/termux-app/commit/7256b04317b0aff7d08650a5b106063ef00869d0

agnostic-apollo avatar Aug 23 '23 22:08 agnostic-apollo