inmbolmie / 5250_usb_converter

Converter to plug an IBM 5251 terminal to a Linux PC via USB emulating a VT52 terminal
GNU General Public License v3.0
34 stars 6 forks source link

VT52_to_5250.BS() leaves cursor in wrong position, subsequent input appears in wrong position too #18

Open dcoshea opened 7 months ago

dcoshea commented 7 months ago

I tried using VT52_to_5250.BS() directly from some Python code and found that it didn't behave as I expected. It seems that I can reproduce it this way:

  1. Start 5250_terminal.py
  2. In the terminal, type a few random characters
  3. Hit the backspace key to clear the random characters, it behaves normally
  4. Run the command export TERM=dumb via the terminal
  5. Type say asdf, see asdf█
  6. Hit the backspace key, see asd █ - note that the cursor hasn't moved
  7. Type x, see asd x█
  8. Hit Ctrl-L to redraw the line, now see asdx█ instead
  9. Hit backspace a few more times, it keeps deleting characters as expected, but always leaves the cursor one position to the right

Note that it seems that with the standard TERM=vt52, bash (at least version 4.2 on CentOS 7) doesn't output a backspace character in response to backspace being pressed.

There seem to be two problems here if I understand correctly:

  1. BS() sends a WRITE_DATA_LOAD_CURSOR command to overwrite the character being deleted, which results in the terminal moving the cursor back to where it started.
  2. BS() doesn't move the cursor back to the just-deleted character, but it also doesn't update self.cursorX and self.cursorY to reflect that WRITE_DATA_LOAD_CURSOR moved the cursor.

I presume that there should be an additional LOAD_CURSOR_REGISTER command sent to move the cursor again after WRITE_DATA_LOAD_CURSOR.

I'm happy to file a pull request including this fix myself if you think that what I propose is correct, but I'm not actually sure what the correct handling for receiving ASCII character 0x08 is - it doesn't seem to be a VT52 control character based on a quick read of the Wikipedia article.

inmbolmie commented 7 months ago

Hi, I think you may be right. When you press backspace in term=vt52 both a ESC-D and ESC-K (0x1B 0x44 + 0x1B 0x4B) are generated. ESC-D moves the cursor one position to the left and ESC-K deletes the line from the current position to the end of the line. The way to simulate that is with the escD + escK commands. But on TERM="dumb" instead the 0x0B character is sent back to the terminal. Reading the maintenance manual on bitsavers as you said 0x08 isn't a valid control character for a vt52 so most likely it would ignore it (I don't have any real vt52 to properly test that) That is consistent with the need to emulate it with the ESC-D + ESC-K combo.

But that said it makes no hurt to pretend that the BS character is valid, in fact xterm in vt52 mode does just that, it accepts the 0x08 character and behaves like ESC-D + ESC-K. So go ahead if you want to fill a pull request. Maybe the quickest way to do it is just invoking ESC_D() and ESC_K() from BS()

That said... idk if there was any particular pathologic case where the BS() needed to be handled like it is. If we change it I would test it a lot to see that doesn't break anything.

The testing would go like running the script with the -i parameter, then launching a xterm in vt52 mode with:

xterm -ti vt52 -tn vt52

Then inside the xterm do:

export TERM="vt52"
tail -f read.log

An testing as many apps as possible the output on the xterm should match what is seen on the emulated terminal

Thanks and best regards

inmbolmie commented 7 months ago

Well now that I think it maybe it is better just like you said and not with ESC_D() and ESC_K() because ESC_K() will delete till the end of the line and idk if it is what it should do. Will need some testing anyways.

Thanks and best regards