[PATCH v3 0/3] Reworks for console code
Hi Corinna, On Sun, 31 Mar 2019 16:36:51 +0200 Corinna Vinschen rote: > This hunk is ok, but I wonder if the time hasn't come to simplify the > original code. The `static char NO_COPY' only makes marginal sense > since it's strdup'ed anyway. > > What if we just define two const char's like this > > const char cygterm[] = "TERM=cygwin"; > const char xterm[] = "TERM=xterm-256color"; > > and then just strdup them conditionally: > > if (!sawTERM) > envp[i++] = strdup (wincap.has_con_24bit_colors () ? xterm : > cygterm); > > What do you think? > Sorry, didn't notice this before: Please prepend this block with > a comment along the lines of "/* Not yet defined in Mingw-w64 */" Adopted. > Doesn't this belong into the select patch? Actually, no. This makes select() recognize Ctrl-space, but is just tentative. Patch 0002 overwrites this fix. This is corresponding to: > @@ -435,7 +451,8 @@ fhandler_console::read (void *pv, size_t& buflen) > toadd = tmp; > } > /* Allow Ctrl-Space to emit ^@ */ > - else if (input_rec.Event.KeyEvent.wVirtualKeyCode == VK_SPACE > + else if (input_rec.Event.KeyEvent.wVirtualKeyCode > +== (wincap.has_con_24bit_colors () ? '2' : VK_SPACE) > && (ctrl_key_state & CTRL_PRESSED) > && !(ctrl_key_state & ALT_PRESSED)) > toadd = ""; Takashi Yano (3): Cygwin: console: support 24 bit color Cygwin: console: fix select() behaviour Cygwin: console: Make I/O functions thread-safe winsup/cygwin/environ.cc |7 +- winsup/cygwin/fhandler.h | 34 +- winsup/cygwin/fhandler_console.cc | 1154 +++-- winsup/cygwin/select.cc | 90 +-- winsup/cygwin/wincap.cc | 10 + winsup/cygwin/wincap.h|2 + 6 files changed, 840 insertions(+), 457 deletions(-) -- 2.17.0
Re: [PATCH v3 0/3] Reworks for console code
On Apr 1 00:47, Takashi Yano wrote: > Hi Corinna, > > On Sun, 31 Mar 2019 16:36:51 +0200 Corinna Vinschen rote: > > This hunk is ok, but I wonder if the time hasn't come to simplify the > > original code. The `static char NO_COPY' only makes marginal sense > > since it's strdup'ed anyway. > > > > What if we just define two const char's like this > > > > const char cygterm[] = "TERM=cygwin"; > > const char xterm[] = "TERM=xterm-256color"; > > > > and then just strdup them conditionally: > > > > if (!sawTERM) > > envp[i++] = strdup (wincap.has_con_24bit_colors () ? xterm : > > cygterm); > > > > What do you think? > > > Sorry, didn't notice this before: Please prepend this block with > > a comment along the lines of "/* Not yet defined in Mingw-w64 */" > > Adopted. > > > Doesn't this belong into the select patch? > > Actually, no. This makes select() recognize Ctrl-space, but > is just tentative. Patch 0002 overwrites this fix. > > This is corresponding to: > > @@ -435,7 +451,8 @@ fhandler_console::read (void *pv, size_t& buflen) > > toadd = tmp; > > } > > /* Allow Ctrl-Space to emit ^@ */ > > - else if (input_rec.Event.KeyEvent.wVirtualKeyCode == VK_SPACE > > + else if (input_rec.Event.KeyEvent.wVirtualKeyCode > > + == (wincap.has_con_24bit_colors () ? '2' : VK_SPACE) > >&& (ctrl_key_state & CTRL_PRESSED) > >&& !(ctrl_key_state & ALT_PRESSED)) > > toadd = ""; Ah, right, that makes sense. Pushed. I added release info accordingly. I'm also just building new developer snapshots with this patchset included. Thanks, Corinna -- Corinna Vinschen Cygwin Maintainer signature.asc Description: PGP signature
