Re: [PATCH] Reworks for console code

2019-03-31 Thread Corinna Vinschen
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


[PATCH] Reworks for console code

2019-03-30 Thread Takashi Yano
Hi,

I would like to propose 3 patches attached to improve console code.

Patch 0001:
This revises console code for better color handling. This provides
24 bit color support for Windows 10 build 14931 or later. For legacy
console, fake 24 bit color support is implemented, which use the
nearest color from 16 system colors.

To check the color handling, use:
https://gist.github.com/hSATAC/1095100

Results are at:
http://tyan0.dip.jp/cgi-bin/FSWiki/wiki.cgi?page=24+bit+color+support+for+cygwin+console

Patch 0002:
Previously, select() would return by typing only one key even in
canonical mode. With this patch, it returns after one line is
completed.

To check this behaviour, use attached STC (confirm-select.c) and
type something and [Enter]. Type ^D to exit from the STC.

Expected result:
This is one line.
select() returns 1
read() returns 18
54, 68, 69, 73, 20, 69, 73, 20, 6f, 6e, 65, 20, 6c, 69, 6e, 65, 2e, 0a,
select() returns 1
read() returns 0

Cygwin current version:
select() returns 1
This is one line.
read() returns 18
54, 68, 69, 73, 20, 69, 73, 20, 6f, 6e, 65, 20, 6c, 69, 6e, 65, 2e, 0a,
select() returns 1
read() returns 0

Patch 0003:
POSIX states I/O functions shall be thread-safe, however, cygwin
console I/O functions were not. This patch makes console I/O
functions thread-safe.

To check this, use attached STC (confirm-thread-safe.c).
Expected resut is red 'A's at the first 12 lines and green 'B's
at the second 12 lines. However, the result is completely broken
in current cygwin.

-- 
Takashi Yano 


0001-Cygwin-console-support-24-bit-color.patch
Description: Binary data


0002-Cygwin-console-fix-select-behaviour.patch
Description: Binary data


0003-Cygwin-console-Make-I-O-functions-thread-safe.patch
Description: Binary data
#include 
#include 
#include 
#include 
#include 

int main()
{
	fd_set fds;
	int ret;

	while (1) {
		FD_ZERO(&fds);
		FD_SET(0, &fds);
		errno = 0;
		ret = select(1, &fds, NULL, NULL, NULL);
		printf("select() returns %d\n", ret);
		if (FD_ISSET(0, &fds)) {
			char buf[1024];
			size_t n = read(0, buf, sizeof(buf));
			printf("read() returns %d\n", n);
			if (n <= 0) break;
			size_t i;
			for (i=0; i#include 
#include 
#include 
#include 

void *write_thread(void *arg)
{
	int n = (int)arg;
	const int rows = 12;
	const int cols = 79;
	char buf[64];
	for (int i=0; i<1; i++) {
		for (int r=1; r<=rows; r++) {
			for (int c=1; c<=cols; c++) {
sprintf(buf, "\033[%d;%dH\033[%dm%c", r + n*rows, c, 31+n, 'A'+n);
write(1, buf, strlen(buf));
			}
		}
	}
}

int main()
{
	pthread_t th1, th2;

	printf("\033[H\033[J");
	fflush(stdout);
	if (pthread_create(&th1, NULL, write_thread, (void*)0)) {
		perror ("1: pthred_create");
		return -1;
	}
	if (pthread_create(&th2, NULL, write_thread, (void*)1)) {
		perror ("2: pthred_create");
		return -1;
	}
	pthread_join(th1, NULL);
	pthread_join(th2, NULL);
	usleep(300);
	printf("\033[H\033[J\033[m");
	fflush(stdout);
	return 0;
}