ast
ast copied to clipboard
att
Use the curses API rather than depend on the `tput` command
Issue #1459 caused me to notice that this block of code is, at best, confusing:
https://github.com/att/ast/blob/1fd82bddfd36c8d381d4eb75bc1e6b2d4ccb6750/src/cmd/ksh93/edit/edit.c#L454-L464
The problem is that it is assigning to shell var .sh.subscript a value having absolutely nothing to do with the documented meaning of that var. The code could have used literally any arbitrary, otherwise ostensibly private, var to temporarily hold the value returned by tput cuu1. Abusing .sh.subscript is gross and unnecessary. It's also dangerous since, theoretically, it could have been set prior to that block of code being run. Which, again, illustrates that the old AST team relied far too much on knowledge of the existing implementation rather than first principals when writing such code.
We should modify ksh to not use the tput command in this fashion. Ksh should be using the curses API to get this info rather than running an external command. Note that there are only two places in the code that use an external tput command in this fashion. The other being in emacs_escape() to run tput clear when [ctrl-L] is seen. Which means that if a tput command wasn't found when the build was configured you can't use [ctrl-L] to clear the screen. Which seems suboptimal and needlessly expensive. Not to mention that even if tput was found when the build was configured there is no guarantee it will be in $PATH when it is needed. Which explains the 2>/dev/null redirection in the ed_setup() function. But not why the same redirection isn't present in the emacs_escape() function.
The problem is that it is assigning to shell var .sh.subscript a value having absolutely nothing to do with the documented meaning of that var. The code could have used literally any arbitrary, otherwise ostensibly private, var to temporarily hold the value returned by tput cuu1. Abusing .sh.subscript is gross and unnecessary. It's also dangerous since, theoretically, it could have been set prior to that block of code being run. Which, again, illustrates that the old AST team relied far too much on knowledge of the existing implementation rather than first principals when writing such code.
and you believe Korn (or whoever) used that var /sh.subscript not intentionally? or that he did it intentionally, but had no reason for doing so except being too lazy to define another variable? go figure...
if something is "gross" around here, it is not the ksh93 code base but your careless/clueless approach. looking forward to the next regression introduced by this.
Notice also that the code only runs tput to determine the correct value for the CURSOR_UP string but not the KILL_LINE string. You can't have it both ways. Either the code is terminal agnostic or it assumes VT100/ANSI X3.64 semantics. The current implementation does both which is inconsistent. This is okay in practice because most people are using ANSI X3.64 compatible terminals. However, even on ostensible ANSI X3.64 compatible terminals there are differences in the respective strings. For example,
$ tput -Txterm cuu1 | od -tx1z
0000000 1b 5b 41 >.[A<
0000003
$ tput -Ttmux-256color cuu1 | od -tx1z
0000000 1b 4d >.M<
0000002
Which is probably why they shelled out to run tput cuu1. But it's still wrong to otherwise use hardcoded terminal control sequences for other operations. Not to mention relying on the tput command being in the current $PATH. If the system supports a tput command it presumably supports the curses API. There isn't much point, other than convenience, in running tput rather than directly calling the relevant curses API functions.
The reason to abuse .sh.subscript might be that it is probably not used in the context of editing (only within disciple functions). I don't know why it is needed here - a kind of global variable? I don't know also why it is difficult to introduce something under a different name to fulfill this role.
Regarding the use of the tput command: it is easier to do than to try to figure which curses library is the right one to use. For example linking to a third-party ncurses library may cause the terminal definitions to be read from the terminfo database supplied by it instead of the system provided one (termcap or terminfo).
A slight problem here is that tput cuu1 will return nothing on termcap-based BSDs; it should be tput up for these.
This code is #ifdef'd around _cmd_tput and ksh will use hardcoded value for CURSOR_UP otherwise.
ESC J seems to be the common code for the "clear to the end of screen" command for many, but not all terminals. This can be read as a "ed" terminfo and "cd" termcap property.
"cuu1" terminfo and "up" termcap properties seem to be either ESC [ A or ESC M for the most currently used terminals. An ancient vt52 has ESC K for cursor up but still uses ESC J for "clear end of screen". It seems to me that the "clear end of screen" value got standardized way earlier; it is probably much older idea than the use of the cursor keys.
We could probably add the tput ed/tput cd somewhere there for completeness, once we decide which shell variable shall we abuse next :)
