[PATCH] Cygwin: pty: Disable pseudo console if TERM is dumb or not set.

[Takashi Yano]

Hi Corinna,

On Sun, 30 Aug 2020 14:49:25 +0200
Corinna Vinschen wrote:
> Hi Takashi,
> 
> On Aug 29 20:12, Takashi Yano via Cygwin-patches wrote:
> > Hi Corinna,
> > 
> > On Sat, 29 Aug 2020 04:25:54 +0900
> > Takashi Yano via Cygwin-patches <cygwin-patches@cygwin.com> wrote:
> > > Hi Corinna,
> > >
> > > On Fri, 28 Aug 2020 15:45:03 +0200
> > > Corinna Vinschen wrote:
> > > > Hi Takashi,
> > > > 
> > > > On Aug 26 21:00, Takashi Yano via Cygwin-patches wrote:
> > > > > Pseudo console generates escape sequences on execution of non-cygwin
> > > > > apps.  If the terminal does not support escape sequence, output will
> > > > > be garbled. This patch prevents garbled output in dumb terminal by
> > > > > disabling pseudo console.
> > [...]
> > > > 
> > > > Would you mind to encapsulate the TERM checks into a fhandler_pty_slave
> > > > method so the TERM specific stuff is done in the fhandler code, not
> > > > in spawn.cc?
> > > 
> > > Thansk for the suggestion. I will submit v2 patch.
> > 
> > What do you think of v3 patch attached? With this patch,
> > terminal capability is checked by looking into terminfo
> > database rather than just checking terminal name. This
> > solution is more essential for the issue to be solved,
> > I think.
> > 
> > One downside of this solution, I noticed, is that tmux
> > sets TERM to "screen", which does not have CSI6n, by
> > default. As a result, pseudo console is disbled in tmux
> > by default. Setting TERM, such as screen.xterm-256color,
> > will solve the issue.
> 
> I like the idea in general, but isn't there a noticable perfomance hit?

I have measured the startup time of non-cygwin process
with v2 and v8 patch.

mintty with v2    : 92.7ms
emacs-dumb with v2: 22.8ms

mintty with v8    : 94.6ms
emacs-dumb with v8: 22.7ms

There is not noticeable difference more than measurement
error.

By the way, I have implemented timeout strategy for CSI6n, you
mentioned in the previous comment, for a test. This check is
done only on the first execution of non-cygwin apps in the pty.
With this patch, first checks if the terminfo has cursor_home
(ESC [H). If terminfo has cursor_home, ANSI escape sequence is
supposed to be supported. In this case, I expect not to display
garbage "^[[6n" even if CSI6n is sent because the parser ignores
unsupported CSI sequences.

With this implementation, pseudo console works in tmux as well
even if TERM=screen.

Please have a look v9 patch attached.

The performance of v9 is also checked.

mintty with v9    : 93.9ms
emacs-dumb with v9: 22.8ms
ANSI without CSI6n: 22.8ms

[the first time in v9]
mintty            : 94.8ms
emacs-dumb        : 22.5ms
ANSI without CSI6n: 63.5ms

Most of the results are the same as v2 and v8 except for the
first execution of non-cygwin apps in ansi terminal without
CSI6n. It takes about 40ms (timeout) longer than dumb terminal
in ANSI terminal without CSI6n support.

However, this causes only on the first execution of non-cygwin
apps in pty.

I think this is the most reasonable one I have ever proposed.

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: v9-0001-Cygwin-pty-Disable-pseudo-console-if-TERM-does-no.patch
Type: application/octet-stream
Size: 9419 bytes
Desc: not available
URL: <https://cygwin.com/pipermail/cygwin-patches/attachments/20200831/b7830455/attachment-0001.obj>