Hi Takashi, On Mar 31 15:20, Takashi Yano wrote: > Hi, > > I would like to propose 3 patches attached to improve console code.
Not a hard requirement, but it would be nice if you could send your patches as patchset with cover letter via `git send-email' from the git-email package or in the same style. See, e. g. https://sourceware.org/ml/cygwin-patches/2019-q1/msg00036.html This really simplifies review, discussion and applying patches. With attached patches, replying to a patch does not quote the patch so inline commenting is pretty tricky. Thanks for considering. Having said that, this is a snippet from patch 1: + /* Check if 24bit color is available */ + DWORD dwVersion = GetVersion (); + dwVersion = (LOBYTE (LOWORD (dwVersion)) << 24) + | (HIBYTE (LOWORD (dwVersion)) << 16) | HIWORD (dwVersion); + if (dwVersion >= ((10 << 24) | (0 << 16) | 14931)) + { + con.cap24bit_color = true; OS features or bug tests should be performed via wincap, see wincap.cc and wincap.h. We do not care for Windows test release, so this should be enabled for W10 1703 and later. Just add an "has_con_24bit_colors"(*) flag to the wincaps struct and set it to false on older Windows versions, true otherwise. So the above code snippet can go away and in subsequent code just check for wincap.has_con_24bit_colors (). (*) exact name of the flag is your choice + /* If system has 24bit color capability, + use xterm compatible mode. */ + setenv ("TERM", "xterm-256color", 1); Having the 24bit color capability check in wincap also means, setting TERM could be moved into environ.cc, function win32env_to_cygenv() where we already set TERM=cygwin. This could then be done conditionally based on the wincap.has_con_24bit_colors check. Patch 2: Looks good, just a question: + if (mouse_event.dwEventFlags == MOUSE_MOVED) + { + b = con.last_button_code; + } + else if (mouse_event.dwButtonState < con.dwLastButtonState && !con.ext_mouse_mode6) + { + b = 3; + strcpy (sz, "btn up"); + } + else if ((mouse_event.dwButtonState & 1) != (con.dwLastButtonState & 1)) + { + b = 0; + strcpy (sz, "btn1 down"); + } + else if ((mouse_event.dwButtonState & 2) != (con.dwLastButtonState & 2)) + { + b = 2; + strcpy (sz, "btn2 down"); + } + else if ((mouse_event.dwButtonState & 4) != (con.dwLastButtonState & 4)) + { + b = 1; + strcpy (sz, "btn3 down"); + } + This is not your code but while you're at it, would you mind to reformat to 80 chars line length max? Thanks! Patch 3: Looks good. Thanks, Corinna -- Corinna Vinschen Cygwin Maintainer
signature.asc
Description: PGP signature
