lxsang / PTerm

MIT License
35 stars 8 forks source link

Add support to function keys F1-F12, change TERM=xterm-256color #64

Closed lxsang closed 4 months ago

lxsang commented 6 months ago

Add basic function keys F1-F12 support #60 .

Rinzwind commented 6 months ago

It would seem better to use ‘xterm’ for ‘TERM’ rather than ‘xterm-256color’ as there’s no support yet for using 256 colors. When using ‘xterm-256color’, the function key row in GNU midnight commander doesn’t show the function key numbers: it shows instead of . It does show the key numbers when ‘xterm’ is used.

A problem that occurs with both ‘xterm’ and ‘xterm-256color’ is that GNU ‘screen’ no longer shows its startup message (with the version number and license) like it shows with ‘TERM’ set to ‘linux’.

For #processFunctionKey: on TerminalEmulatorMorph, it might make sense to use a class variable with a dictionary to map the function keys to the byte arrays for the downcallAll: message. For the keys of the dictionary, KMSingleKeyCombination instances could be used, which can be gotten from a KeyboardKey through #asKeyCombination, which can also be sent to a KeyboardEvent.

lxsang commented 6 months ago

It would seem better to use ‘xterm’ for ‘TERM’

Yes, it is better use the term type string provided by the TerminalEmulator... class used, fixed in the last commit.

A problem that occurs with both ‘xterm’ and ‘xterm-256color’ is that GNU ‘screen’ no longer shows its startup message (with the version number and license) like it shows with ‘TERM’ set to ‘linux’.

The Mac terminal don't have this problem with TERM set to xterm-256color, i don't know what causes this...

For #processFunctionKey: on TerminalEmulatorMorph, it might make sense to use a class variable with a dictionary to map..

Agree on a dedicated class method that defines a function key map, however, the use of KMSingleKeyCombination as key is a bit overkill as that makes the code more complicate. I prefer the use of Symbol (e.g. #F1) which is easier to understand.

Rinzwind commented 5 months ago

I tried to debug the GNU ‘screen’ problem. It is related to the send of #setWidth: in #decrmSingle: on TerminalEmulatorVT102. When the send is replaced by self, the problem no longer occurs. It might be due to the send causing two sends of #setWinsize: with different arguments. Try the following: open a Terminal window, put a breakpoint at the start of #setWinsize: on PTerm, then execute printf '\33[?3l' in the Terminal window. Two debuggers are opened, in one the argument is 81@21 and in the other it is 79@21.

Rinzwind commented 5 months ago

Note that the ’xterm’ manual says the DECCOLM escape sequence is ignored unless the following option is given:

-132 Normally, the VT102 DECCOLM escape sequence that switches between 80 and 132 column mode is ignored. This option causes the DECCOLM escape sequence to be recognized, and the xterm window will resize appropriately.

Rinzwind commented 5 months ago

While the two sends of #setWinsize: with different arguments is something that should be fixed, it does not seem to be the cause of the GNU ‘screen’ problem. A similar problem occurs in xterm if the option -132 is used: after using printf '\33[8;25;100;t' to resize the window to 25 lines and 100 columns, when ‘screen’ is started it shows a message line “Aborted because of window size change” instead of the startup message. There’s a reference to this problem in Debian’s system-wide ‘screenrc’ file which has a ‘termcapinfo’ command that changes ‘is’ for ‘xterm’:

# Change the xterm initialization string from is2=\E[!p\E[?3;4l\E[4l\E>
# (This fixes the "Aborted because of window size change" konsole symptoms found
#  in bug #134198)
termcapinfo xterm 'is=\E[r\E[m\E[2J\E[H\E[?7h\E[?1;4;6l'

The example file ‘etcscreen’ in the ‘screen’ repository has similar commands to avoid the use of DECCOLM:

# we do not want the width to change to 80 characters on startup:
# on suns, /etc/termcap has :is=\E[r\E[m\E[2J\E[H\E[?7h\E[?1;3;4;6l:
termcap xterm 'is=\E[r\E[m\E[2J\E[H\E[?7h\E[?1;4;6l'
terminfo xterm 'is=\E[r\E[m\E[2J\E[H\E[?7h\E[?1;4;6l'

There doesn’t seem to be a system-wide ‘screenrc’ on macOS (the file /usr/local/etc/screenrc referred to in the man page on macOS v13.6.7 doesn’t exist) hence ‘screen’ uses the standard value for ‘is’ given by the following:

$ printf 'is=%q\n' "$(tput is)"
is=$'\E[!p\E[?3;4l\E[4l\E>'

I think this can be dealt with separately from this pull request. The setting of the window size should be fixed so that the number of columns is correctly set to 80 instead of 81 or 79. The default value for allow132 in #initializeTeletype: on TerminalEmulatorMorph should maybe be changed to false.

lxsang commented 4 months ago

Yes, my bad. Fixed now