[PATCH] Cygwin: console, pty: Prevent error in legacy console mode.

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] Cygwin: console, pty: Prevent error in legacy console mode.

Takashi Yano
---
 winsup/cygwin/fhandler.h          |  1 +
 winsup/cygwin/fhandler_console.cc | 45 ++++++++++++++++++++-----------
 winsup/cygwin/fhandler_tty.cc     | 13 +++++++++
 3 files changed, 43 insertions(+), 16 deletions(-)

diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index d5aa57300..313172ec5 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -1831,6 +1831,7 @@ enum cltype
 class dev_console
 {
   pid_t owner;
+  bool is_legacy;
 
   WORD default_color, underline_color, dim_color;
 
diff --git a/winsup/cygwin/fhandler_console.cc b/winsup/cygwin/fhandler_console.cc
index 241759203..79fa00bdb 100644
--- a/winsup/cygwin/fhandler_console.cc
+++ b/winsup/cygwin/fhandler_console.cc
@@ -309,7 +309,7 @@ fhandler_console::set_cursor_maybe ()
 {
   con.fillin (get_output_handle ());
   /* Nothing to do for xterm compatible mode. */
-  if (wincap.has_con_24bit_colors ())
+  if (wincap.has_con_24bit_colors () && !con.is_legacy)
     return;
   if (con.dwLastCursorPosition.X != con.b.dwCursorPosition.X ||
       con.dwLastCursorPosition.Y != con.b.dwCursorPosition.Y)
@@ -349,7 +349,7 @@ fhandler_console::send_winch_maybe ()
     {
       con.scroll_region.Top = 0;
       con.scroll_region.Bottom = -1;
-      if (wincap.has_con_24bit_colors ())
+      if (wincap.has_con_24bit_colors () && !con.is_legacy)
  fix_tab_position (get_output_handle (), con.dwWinSize.X);
       get_ttyp ()->kill_pgrp (SIGWINCH);
       return true;
@@ -483,7 +483,7 @@ sig_exit:
 fhandler_console::input_states
 fhandler_console::process_input_message (void)
 {
-  if (wincap.has_con_24bit_colors ())
+  if (wincap.has_con_24bit_colors () && !con.is_legacy)
     {
       DWORD dwMode;
       /* Enable xterm compatible mode in input */
@@ -589,7 +589,8 @@ fhandler_console::process_input_message (void)
     }
   /* Allow Ctrl-Space to emit ^@ */
   else if (input_rec[i].Event.KeyEvent.wVirtualKeyCode
-   == (wincap.has_con_24bit_colors () ? '2' : VK_SPACE)
+   == ((wincap.has_con_24bit_colors () && !con.is_legacy) ?
+       '2' : VK_SPACE)
    && (ctrl_key_state & CTRL_PRESSED)
    && !(ctrl_key_state & ALT_PRESSED))
     toadd = "";
@@ -1023,17 +1024,27 @@ fhandler_console::open (int flags, mode_t)
       /* Enable xterm compatible mode in output */
       GetConsoleMode (get_output_handle (), &dwMode);
       dwMode |= ENABLE_VIRTUAL_TERMINAL_PROCESSING;
-      SetConsoleMode (get_output_handle (), dwMode);
+      if (!SetConsoleMode (get_output_handle (), dwMode))
+ con.is_legacy = true;
+      else
+ con.is_legacy = false;
       /* Enable xterm compatible mode in input */
-      GetConsoleMode (get_handle (), &dwMode);
-      dwMode |= ENABLE_VIRTUAL_TERMINAL_INPUT;
-      SetConsoleMode (get_handle (), dwMode);
+      if (!con.is_legacy)
+ {
+  GetConsoleMode (get_handle (), &dwMode);
+  dwMode |= ENABLE_VIRTUAL_TERMINAL_INPUT;
+  if (!SetConsoleMode (get_handle (), dwMode))
+    con.is_legacy = true;
+ }
+      if (con.is_legacy)
+ setenv ("TERM", "cygwin", 1);
     }
 
   DWORD cflags;
   if (GetConsoleMode (get_handle (), &cflags))
     SetConsoleMode (get_handle (), ENABLE_WINDOW_INPUT
-    | (wincap.has_con_24bit_colors () ? 0 : ENABLE_MOUSE_INPUT)
+    | ((wincap.has_con_24bit_colors () && !con.is_legacy) ?
+       0 : ENABLE_MOUSE_INPUT)
     | cflags);
 
   debug_printf ("opened conin$ %p, conout$ %p", get_handle (),
@@ -1062,7 +1073,7 @@ fhandler_console::close ()
   output_mutex = NULL;
 
   if (shared_console_info && getpid () == con.owner &&
-      wincap.has_con_24bit_colors ())
+      wincap.has_con_24bit_colors () && !con.is_legacy)
     {
       DWORD dwMode;
       /* Disable xterm compatible mode in input */
@@ -1209,7 +1220,7 @@ fhandler_console::output_tcsetattr (int, struct termios const *t)
   acquire_output_mutex (INFINITE);
   DWORD flags = ENABLE_PROCESSED_OUTPUT | ENABLE_WRAP_AT_EOL_OUTPUT;
   /* If system has 24 bit color capability, use xterm compatible mode. */
-  if (wincap.has_con_24bit_colors ())
+  if (wincap.has_con_24bit_colors () && !con.is_legacy)
     {
       flags |= ENABLE_VIRTUAL_TERMINAL_PROCESSING;
       if (!(t->c_oflag & OPOST) || !(t->c_oflag & ONLCR))
@@ -1274,9 +1285,10 @@ fhandler_console::input_tcsetattr (int, struct termios const *t)
     }
 
   flags |= ENABLE_WINDOW_INPUT |
-    (wincap.has_con_24bit_colors () ? 0 : ENABLE_MOUSE_INPUT);
+    ((wincap.has_con_24bit_colors () && !con.is_legacy) ?
+     0 : ENABLE_MOUSE_INPUT);
   /* if system has 24 bit color capability, use xterm compatible mode. */
-  if (wincap.has_con_24bit_colors ())
+  if (wincap.has_con_24bit_colors () && !con.is_legacy)
     flags |= ENABLE_VIRTUAL_TERMINAL_INPUT;
 
   int res;
@@ -1650,7 +1662,7 @@ bool fhandler_console::write_console (PWCHAR buf, DWORD len, DWORD& done)
 {
   bool need_fix_tab_position = false;
   /* Check if screen will be alternated. */
-  if (wincap.has_con_24bit_colors ()
+  if (wincap.has_con_24bit_colors () && !con.is_legacy
       && memmem (buf, len*sizeof (WCHAR), L"\033[?1049", 7*sizeof (WCHAR)))
     need_fix_tab_position = true;
 
@@ -2498,7 +2510,8 @@ fhandler_console::write_normal (const unsigned char *src,
   memset (&ps, 0, sizeof ps);
   while (found < end
  && found - src < CONVERT_LIMIT
- && (wincap.has_con_24bit_colors () || base_chars[*found] == NOR) )
+ && ((wincap.has_con_24bit_colors () && !con.is_legacy)
+     || base_chars[*found] == NOR) )
     {
       switch (ret = f_mbtowc (_REENT, NULL, (const char *) found,
        end - found, &ps))
@@ -2958,7 +2971,7 @@ fhandler_console::fixup_after_fork_exec (bool execing)
 {
   set_unit ();
   setup_io_mutex ();
-  if (wincap.has_con_24bit_colors ())
+  if (wincap.has_con_24bit_colors () && !con.is_legacy)
     {
       DWORD dwMode;
       /* Disable xterm compatible mode in input */
diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index da6119dfb..f87ac73f2 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -26,6 +26,7 @@ details. */
 #include <asm/socket.h>
 #include "cygwait.h"
 #include "tls_pbuf.h"
+#include "registry.h"
 
 #define ALWAYS_USE_PCON false
 #define USE_API_HOOK true
@@ -3121,6 +3122,8 @@ fhandler_pty_master::setup_pseudoconsole ()
      process in a pseudo console and get them from the helper.
      Slave process will attach to the pseudo console in the
      helper process using AttachConsole(). */
+  bool error = false;
+
   COORD size = {80, 25};
   CreatePipe (&from_master, &to_slave, &sec_none, 0);
   SetLastError (ERROR_SUCCESS);
@@ -3131,6 +3134,16 @@ fhandler_pty_master::setup_pseudoconsole ()
       if (res != S_OK)
  system_printf ("CreatePseudoConsole() failed. %08x\n",
        GetLastError ());
+      error = true;
+    }
+
+  reg_key reg (HKEY_CURRENT_USER, KEY_READ, L"Console", NULL);
+  if (reg.error ())
+    error = true;
+  if (reg.get_dword (L"ForceV2", 1) == 0)
+    error = true;
+  if (error)
+    {
       CloseHandle (from_master);
       CloseHandle (to_slave);
       from_master = from_master_cyg;
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Cygwin: console, pty: Prevent error in legacy console mode.

Corinna Vinschen-2
Hi Takashi,

the patch is fine in general.  Still, what I really like to see is a
descriptive log message, as well as a matching comment...

On Nov  6 20:59, Takashi Yano wrote:
> @@ -3131,6 +3134,16 @@ fhandler_pty_master::setup_pseudoconsole ()
>        if (res != S_OK)
>   system_printf ("CreatePseudoConsole() failed. %08x\n",
>         GetLastError ());
> +      error = true;
> +    }
> +

...here, to explain briefly why this check is done.

> +  reg_key reg (HKEY_CURRENT_USER, KEY_READ, L"Console", NULL);
> +  if (reg.error ())
> +    error = true;
> +  if (reg.get_dword (L"ForceV2", 1) == 0)
> +    error = true;
> +  if (error)
> +    {
>        CloseHandle (from_master);
>        CloseHandle (to_slave);
>        from_master = from_master_cyg;
> --
> 2.21.0
Thanks,
Corinna

--
Corinna Vinschen
Cygwin Maintainer

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Cygwin: console, pty: Prevent error in legacy console mode.

Brian Inglis
In reply to this post by Takashi Yano
On 2019-11-06 04:59, Takashi Yano wrote:
> ---
>  winsup/cygwin/fhandler.h          |  1 +
>  winsup/cygwin/fhandler_console.cc | 45 ++++++++++++++++++++-----------
>  winsup/cygwin/fhandler_tty.cc     | 13 +++++++++
>  3 files changed, 43 insertions(+), 16 deletions(-)
>
> diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
> index d5aa57300..313172ec5 100644

> diff --git a/winsup/cygwin/fhandler_console.cc b/winsup/cygwin/fhandler_console.cc
> index 241759203..79fa00bdb 100644

>        /* Enable xterm compatible mode in output */
>        GetConsoleMode (get_output_handle (), &dwMode);
>        dwMode |= ENABLE_VIRTUAL_TERMINAL_PROCESSING;
> -      SetConsoleMode (get_output_handle (), dwMode);

Nit:

> +      if (!SetConsoleMode (get_output_handle (), dwMode))
> + con.is_legacy = true;
> +      else
> + con.is_legacy = false;

prefer direct result:

> +      con.is_legacy = !SetConsoleMode (get_output_handle (), dwMode);

Beef:

> +      if (con.is_legacy)
> + setenv ("TERM", "cygwin", 1);
>      }

handlers should not be changing user's env vars: that is the user's selection to
get their preferred operation in their apps.

If you need to set TERM, shouldn't you also set it appropriately for non-legacy
console?

Perhaps it may be set as a default, only if it is not already set by the user.

There are a lot of **un**settings in cygwin compared to my TERM:

$ infocmp -Id $TERM cygwin
comparing xterm-256color to cygwin.
    comparing booleans.
        bce: T:F.
        OTbs: T:F.
        ccc: T:F.
        xenl: T:F.
        km: T:F.
        hs: F:T.
        npc: T:F.
        mc5i: T:F.
        xon: F:T.
    comparing numbers.
        cols: 80, NULL.
        lines: 24, NULL.
        colors: 256, 8.
        pairs: 65536, 64.
    comparing strings.
        acsc: '``aaffggiijjkkllmmnnooppqqrrssttuuvvwwxxyyzz{{||}}~~',
'+\020\,\021-\030.^Y0\333`\004a\261f\370g\361h\260j\331k\277l\332m\300n\305o~p\304q\304r\304s_t\303u\264v\301w\302x\263y\363z\362{\343|\330}\234~\376'.
        cbt: '\E[Z', NULL.
        csr: '\E[%i%p1%d;%p2%dr', NULL.
        tbc: '\E[3g', NULL.
        mgc: '\E[?69l', NULL.
        clear: '\E[H\E[2J', '\E[H\E[J'.
        cud1: '\n', '\E[B'.
        civis: '\E[?25l', NULL.
        cnorm: '\E[?12l\E[?25h', NULL.
        cvvis: '\E[?12;25h', NULL.
        smacs: '\E(0', '\E[11m'.
        smam: '\E[?7h', NULL.
        blink: '\E[5m', NULL.
        smcup: '\E[?1049h\E[22;0;0t', '\E7\E[?47h'.
        dim: '\E[2m', NULL.
        sitm: '\E[3m', NULL.
        smpch: NULL, '\E[11m'.
        ech: '\E[%p1%dX', NULL.
        rmacs: '\E(B', '\E[10m'.
        rmam: '\E[?7l', NULL.
        sgr0: '\E(B\E[m', '\E[0;10m'.
        rmcup: '\E[?1049l\E[23;0;0t', '\E[2J\E[?47l\E8'.
        ritm: '\E[23m', NULL.
        rmpch: NULL, '\E[10m'.
        flash: '\E[?5h$<100/>\E[?5l', NULL.
        fsl: NULL, '^G'.
        is2: '\E[!p\E[?3;4l\E[4l\E>', NULL.
        initc:
'\E]4;%p1%d;rgb\:%p2%{255}%*%{1000}%/%2.2X/%p3%{255}%*%{1000}%/%2.2X/%p4%{255}%*%{1000}%/%2.2X\E\\',
NULL.
        ich1: NULL, '\E[@'.
        ka1: '\EOw', NULL.
        ka3: '\EOy', NULL.
        kb2: '\EOu', '\E[G'.
        kcbt: '\E[Z', NULL.
        kc1: '\EOq', NULL.
        kc3: '\EOs', NULL.
        kcud1: '\EOB', '\E[B'.
        kend: '\EOF', '\E[4~'.
        kent: '\EOM', NULL.
        kf1: '\EOP', '\E[[A'.
        kf13: '\E[1;2P', '\E[25~'.
        kf14: '\E[1;2Q', '\E[26~'.
        kf15: '\E[1;2R', '\E[28~'.
        kf16: '\E[1;2S', '\E[29~'.
        kf17: '\E[15;2~', '\E[31~'.
        kf18: '\E[17;2~', '\E[32~'.
        kf19: '\E[18;2~', '\E[33~'.
        kf2: '\EOQ', '\E[[B'.
        kf20: '\E[19;2~', '\E[34~'.
        kf21: '\E[20;2~', NULL.
        kf22: '\E[21;2~', NULL.
        kf23: '\E[23;2~', NULL.
        kf24: '\E[24;2~', NULL.
        kf25: '\E[1;5P', NULL.
        kf26: '\E[1;5Q', NULL.
        kf27: '\E[1;5R', NULL.
        kf28: '\E[1;5S', NULL.
        kf29: '\E[15;5~', NULL.
        kf3: '\EOR', '\E[[C'.
        kf30: '\E[17;5~', NULL.
        kf31: '\E[18;5~', NULL.
        kf32: '\E[19;5~', NULL.
        kf33: '\E[20;5~', NULL.
        kf34: '\E[21;5~', NULL.
        kf35: '\E[23;5~', NULL.
        kf36: '\E[24;5~', NULL.
        kf37: '\E[1;6P', NULL.
        kf38: '\E[1;6Q', NULL.
        kf39: '\E[1;6R', NULL.
        kf4: '\EOS', '\E[[D'.
        kf40: '\E[1;6S', NULL.
        kf41: '\E[15;6~', NULL.
        kf42: '\E[17;6~', NULL.
        kf43: '\E[18;6~', NULL.
        kf44: '\E[19;6~', NULL.
        kf45: '\E[20;6~', NULL.
        kf46: '\E[21;6~', NULL.
        kf47: '\E[23;6~', NULL.
        kf48: '\E[24;6~', NULL.
        kf49: '\E[1;3P', NULL.
        kf5: '\E[15~', '\E[[E'.
        kf50: '\E[1;3Q', NULL.
        kf51: '\E[1;3R', NULL.
        kf52: '\E[1;3S', NULL.
        kf53: '\E[15;3~', NULL.
        kf54: '\E[17;3~', NULL.
        kf55: '\E[18;3~', NULL.
        kf56: '\E[19;3~', NULL.
        kf57: '\E[20;3~', NULL.
        kf58: '\E[21;3~', NULL.
        kf59: '\E[23;3~', NULL.
        kf60: '\E[24;3~', NULL.
        kf61: '\E[1;4P', NULL.
        kf62: '\E[1;4Q', NULL.
        kf63: '\E[1;4R', NULL.
        khome: '\EOH', '\E[1~'.
        kcub1: '\EOD', '\E[D'.
        kmous: '\E[<', NULL.
        kcuf1: '\EOC', '\E[C'.
        kDC: '\E[3;2~', NULL.
        kEND: '\E[1;2F', NULL.
        kind: '\E[1;2B', NULL.
        kHOM: '\E[1;2H', NULL.
        kIC: '\E[2;2~', NULL.
        kLFT: '\E[1;2D', NULL.
        kNXT: '\E[6;2~', NULL.
        kPRV: '\E[5;2~', NULL.
        kri: '\E[1;2A', NULL.
        kRIT: '\E[1;2C', NULL.
        kspd: NULL, '^Z'.
        kcuu1: '\EOA', '\E[A'.
        rmkx: '\E[?1l\E>', NULL.
        smkx: '\E[?1h\E=', NULL.
        rmm: '\E[?1034l', NULL.
        smm: '\E[?1034h', NULL.
        nel: NULL, '\r\n'.
        oc: '\E]104\007', NULL.
        indn: '\E[%p1%dS', NULL.
        rin: '\E[%p1%dT', NULL.
        mc0: '\E[i', NULL.
        mc4: '\E[4i', NULL.
        mc5: '\E[5i', NULL.
        rep: '%p1%c\E[%p2%{1}%-%db', NULL.
        rs1: '\Ec\E]104\007', '\Ec\E]R'.
        rs2: '\E[!p\E[?3;4l\E[4l\E>', NULL.
        setab:
'\E[%?%p1%{8}%<%t4%p1%d%e%p1%{16}%<%t10%p1%{8}%-%d%e48;5;%p1%d%;m', '\E[4%p1%dm'.
        setaf:
'\E[%?%p1%{8}%<%t3%p1%d%e%p1%{16}%<%t9%p1%{8}%-%d%e38;5;%p1%d%;m', '\E[3%p1%dm'.
        sgr:
'%?%p9%t\E(0%e\E(B%;\E[0%?%p6%t;1%;%?%p5%t;2%;%?%p2%t;4%;%?%p1%p3%|%t;7%;%?%p4%t;5%;%?%p7%t;8%;m',
'\E[0;10%?%p1%t;7%;%?%p2%t;4%;%?%p3%t;7%;%?%p6%t;1%;%?%p7%t;8%;%?%p9%t;11%;m'.
        smglr: '\E[?69h\E[%i%p1%d;%p2%ds', NULL.
        hts: '\EH', NULL.
        tsl: NULL, '\E];'.
        u8: '\E[?%[;0123456789]c', '\E[?6c'.

--
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

This email may be disturbing to some readers as it contains
too much technical detail. Reader discretion is advised.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Cygwin: console, pty: Prevent error in legacy console mode.

Takashi Yano
In reply to this post by Corinna Vinschen-2
Hi Corinna,

On Wed, 6 Nov 2019 15:05:47 +0100
Corinna Vinschen wrote:

> the patch is fine in general.  Still, what I really like to see is a
> descriptive log message, as well as a matching comment...
>
> On Nov  6 20:59, Takashi Yano wrote:
> > @@ -3131,6 +3134,16 @@ fhandler_pty_master::setup_pseudoconsole ()
> >        if (res != S_OK)
> >   system_printf ("CreatePseudoConsole() failed. %08x\n",
> >         GetLastError ());
> > +      error = true;
> > +    }
> > +
>
> ...here, to explain briefly why this check is done.
>
> > +  reg_key reg (HKEY_CURRENT_USER, KEY_READ, L"Console", NULL);
> > +  if (reg.error ())
> > +    error = true;
> > +  if (reg.get_dword (L"ForceV2", 1) == 0)
> > +    error = true;
> > +  if (error)
> > +    {
> >        CloseHandle (from_master);
> >        CloseHandle (to_slave);
> >        from_master = from_master_cyg;

I will submit revised version as v2 patch.

--
Takashi Yano <[hidden email]>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Cygwin: console, pty: Prevent error in legacy console mode.

Takashi Yano
In reply to this post by Brian Inglis
On Wed, 6 Nov 2019 08:06:55 -0700
Brian Inglis wrote:
> > +      if (con.is_legacy)
> > + setenv ("TERM", "cygwin", 1);
> >      }
>
> handlers should not be changing user's env vars: that is the user's selection to
> get their preferred operation in their apps.
>
> If you need to set TERM, shouldn't you also set it appropriately for non-legacy
> console?

The environment TERM is set to cygwin or xterm-256color in environ.cc
based on wincap.has_con_24bit_colors().

However, if legacy console mode is enabled, new terminal capability
compatible with xterm is disabled. So TERM is override to cygwin by
the code above.

This is done only in the first initialization stage, so TERM value
set by user in .login, .bashrc, .tcshrc and etc, ... will be kept.

Only the case in which TERM is overrid is:
1) Enable console legacy mode.
2) Open command prompt.
3) set TERM
4) start cygwin

What situation do you assume this causes problem?

--
Takashi Yano <[hidden email]>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Cygwin: console, pty: Prevent error in legacy console mode.

Brian Inglis
On 2019-11-06 08:44, Takashi Yano wrote:

> On Wed, 6 Nov 2019 08:06:55 -0700
> Brian Inglis wrote:
>>> +      if (con.is_legacy)
>>> + setenv ("TERM", "cygwin", 1);
>>>      }
>>
>> handlers should not be changing user's env vars: that is the user's selection to
>> get their preferred operation in their apps.
>>
>> If you need to set TERM, shouldn't you also set it appropriately for non-legacy
>> console?
>
> The environment TERM is set to cygwin or xterm-256color in environ.cc
> based on wincap.has_con_24bit_colors().
>
> However, if legacy console mode is enabled, new terminal capability
> compatible with xterm is disabled. So TERM is override to cygwin by
> the code above.
>
> This is done only in the first initialization stage, so TERM value
> set by user in .login, .bashrc, .tcshrc and etc, ... will be kept.
>
> Only the case in which TERM is overrid is:
> 1) Enable console legacy mode.
> 2) Open command prompt.
> 3) set TERMq
> 4) start cygwin
>
> What situation do you assume this causes problem?

Is this not executed on every object creation and on every fork?
If that is not the case, then legacy_console/() should be a singleton
object/method, constructed when accessed, or in wincap, like
has_con_24bit_colors() - is_con_legacy().

When user explicitly sets TERM before starting Cygwin, or after forking, Cygwin
does not touch it, so you should not, and perhaps the legacy console check
should be added there:

newlib-cygwin/winsup/doc/setup-env.xml:
<para>
The <envar>TERM</envar> environment variable specifies your terminal
type.  It is automatically set to <literal>cygwin</literal> if you have
not set it to something else.
</para>

newlib-cygwin/winsup/cygwin/environ.cc:
   /* If console has 24 bit color capability, TERM=xterm-256color,
      otherwise, TERM=cygwin */
   if (!sawTERM)
-    envp[i++] = strdup (wincap.has_con_24bit_colors () ? xterm : cygterm);
+    envp[i++] = strdup (wincap.has_con_24bit_colors () &&
!wincap.is_con_legacy() ? xterm : cygterm);

--
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

This email may be disturbing to some readers as it contains
too much technical detail. Reader discretion is advised.
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Cygwin: console, pty: Prevent error in legacy console mode.

Takashi Yano
On Wed, 6 Nov 2019 09:37:11 -0700
Brian Inglis wrote:
> Is this not executed on every object creation and on every fork?

No. It is executed only in the first console object.

> If that is not the case, then legacy_console/() should be a singleton
> object/method, constructed when accessed, or in wincap, like
> has_con_24bit_colors() - is_con_legacy().

con.is_legacy is in shared_console_info which is shared among
console instances for identical console.

> When user explicitly sets TERM before starting Cygwin, or after forking, Cygwin
> does not touch it, so you should not, and perhaps the legacy console check
> should be added there:
>
> newlib-cygwin/winsup/doc/setup-env.xml:
> <para>
> The <envar>TERM</envar> environment variable specifies your terminal
> type.  It is automatically set to <literal>cygwin</literal> if you have
> not set it to something else.
> </para>
>
> newlib-cygwin/winsup/cygwin/environ.cc:
>    /* If console has 24 bit color capability, TERM=xterm-256color,
>       otherwise, TERM=cygwin */
>    if (!sawTERM)
> -    envp[i++] = strdup (wincap.has_con_24bit_colors () ? xterm : cygterm);
> +    envp[i++] = strdup (wincap.has_con_24bit_colors () &&
> !wincap.is_con_legacy() ? xterm : cygterm);

In command prompt, the new feature is disabled if legacy
console mode is enabled. However, it is still enabled in
windows terminal (preview) even if legacy console mode is
enabled.

So the code I posted checks the availability by return
value of SetConsoleMode(). This needs handle to console.
Therefore, the check can not be done in wincap.

I revised the code so that TERM is set only if it was
not set when cygwin was started. Please look at v3 patch.

--
Takashi Yano <[hidden email]>