[PATCH v2 1/3] conhost: Add the ENABLE_VIRTUAL_TERMINAL_PROCESSING mode flag by default.
Newer versions of Windows set it by default.
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- programs/conhost/conhost.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/programs/conhost/conhost.c b/programs/conhost/conhost.c index 39544fb..54ead48 100644 --- a/programs/conhost/conhost.c +++ b/programs/conhost/conhost.c @@ -80,7 +80,7 @@ static struct screen_buffer *create_screen_buffer( struct console *console, int if (!(screen_buffer = calloc( 1, sizeof(*screen_buffer) ))) return NULL; screen_buffer->console = console; screen_buffer->id = id; - screen_buffer->mode = ENABLE_PROCESSED_OUTPUT | ENABLE_WRAP_AT_EOL_OUTPUT; + screen_buffer->mode = ENABLE_PROCESSED_OUTPUT | ENABLE_WRAP_AT_EOL_OUTPUT | ENABLE_VIRTUAL_TERMINAL_PROCESSING; screen_buffer->cursor_size = 25; screen_buffer->cursor_visible = 1; screen_buffer->width = width;
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- programs/conhost/conhost.c | 12 +++++- programs/conhost/tests/tty.c | 80 ++++++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 1 deletion(-)
diff --git a/programs/conhost/conhost.c b/programs/conhost/conhost.c index 54ead48..7377945 100644 --- a/programs/conhost/conhost.c +++ b/programs/conhost/conhost.c @@ -1948,7 +1948,17 @@ static NTSTATUS write_console( struct screen_buffer *screen_buffer, const WCHAR
if (screen_buffer->cursor_x == screen_buffer->width) { - if (screen_buffer->mode & ENABLE_WRAP_AT_EOL_OUTPUT) screen_buffer->cursor_x--; + if (screen_buffer->mode & ENABLE_WRAP_AT_EOL_OUTPUT) + { + if (screen_buffer->mode & ENABLE_VIRTUAL_TERMINAL_PROCESSING) + screen_buffer->cursor_x--; + else + { + screen_buffer->cursor_x = 0; + if (++screen_buffer->cursor_y == screen_buffer->height) + new_line( screen_buffer, &update_rect ); + } + } else screen_buffer->cursor_x = update_rect.left; }
diff --git a/programs/conhost/tests/tty.c b/programs/conhost/tests/tty.c index 7cc37fa..122e8a9 100644 --- a/programs/conhost/tests/tty.c +++ b/programs/conhost/tests/tty.c @@ -149,6 +149,7 @@ enum req_type REQ_CREATE_SCREEN_BUFFER, REQ_FILL_CHAR, REQ_GET_INPUT, + REQ_GET_SB_INFO, REQ_READ_CONSOLE, REQ_READ_CONSOLE_A, REQ_READ_CONSOLE_FILE, @@ -551,6 +552,26 @@ static void expect_char_key_(unsigned int line, WCHAR ch) expect_key_pressed_(line, ch, ch, vk, ctrl); }
+#define test_cursor_pos(a,b) _test_cursor_pos(__LINE__,a,b) +static void _test_cursor_pos(unsigned line, int expect_x, int expect_y) +{ + struct pseudoconsole_req req = { REQ_GET_SB_INFO }; + CONSOLE_SCREEN_BUFFER_INFO info; + DWORD read; + BOOL ret; + + ret = WriteFile(child_pipe, &req, sizeof(req), &read, NULL); + ok(ret, "WriteFile failed: %u\n", GetLastError()); + + ret = ReadFile(child_pipe, &info, sizeof(info), &read, NULL); + ok(ret, "ReadFile failed: %u\n", GetLastError()); + + ok_(__FILE__,line)(info.dwCursorPosition.X == expect_x, "dwCursorPosition.X = %u, expected %u\n", + info.dwCursorPosition.X, expect_x); + ok_(__FILE__,line)(info.dwCursorPosition.Y == expect_y, "dwCursorPosition.Y = %u, expected %u\n", + info.dwCursorPosition.Y, expect_y); +} + static void test_write_console(void) { child_string_request(REQ_WRITE_CONSOLE, L"abc"); @@ -761,6 +782,55 @@ static void test_write_console(void) expect_empty_output();
child_set_output_mode(ENABLE_PROCESSED_OUTPUT | ENABLE_WRAP_AT_EOL_OUTPUT); + + child_set_cursor(28, 20); + skip_hide_cursor(); + expect_output_sequence("\x1b[21;29H"); /* set cursor */ + skip_sequence("\x1b[?25h"); /* show cursor */ + expect_empty_output(); + + child_string_request(REQ_WRITE_CONSOLE, L"ab"); + skip_hide_cursor(); + expect_output_sequence("ab"); + expect_output_sequence("\r\n"); + skip_sequence("\x1b[?25h"); /* show cursor */ + expect_empty_output(); + test_cursor_pos(0, 21); + + child_string_request(REQ_WRITE_CONSOLE, L"c"); + skip_hide_cursor(); + expect_output_sequence("c"); + skip_sequence("\x1b[?25h"); /* show cursor */ + expect_empty_output(); + test_cursor_pos(1, 21); + + child_set_cursor(28, 22); + skip_hide_cursor(); + expect_output_sequence("\x1b[23;29H"); /* set cursor */ + skip_sequence("\x1b[?25h"); /* show cursor */ + expect_empty_output(); + + child_string_request(REQ_WRITE_CONSOLE, L"x"); + skip_hide_cursor(); + expect_output_sequence("x"); + skip_sequence("\x1b[?25h"); /* show cursor */ + expect_empty_output(); + test_cursor_pos(29, 22); + + child_string_request(REQ_WRITE_CONSOLE, L"y"); + skip_hide_cursor(); + expect_output_sequence("y"); + expect_output_sequence("\r\n"); + skip_sequence("\x1b[?25h"); /* show cursor */ + expect_empty_output(); + test_cursor_pos(0, 23); + + child_string_request(REQ_WRITE_CONSOLE, L"z"); + skip_hide_cursor(); + expect_output_sequence("z"); + skip_sequence("\x1b[?25h"); /* show cursor */ + expect_empty_output(); + test_cursor_pos(1, 23); }
static void test_tty_output(void) @@ -1300,6 +1370,16 @@ static void child_process(HANDLE pipe) break; }
+ case REQ_GET_SB_INFO: + { + CONSOLE_SCREEN_BUFFER_INFO info; + ret = GetConsoleScreenBufferInfo(output, &info); + ok(ret, "GetConsoleScreenBufferInfo failed: %u\n", GetLastError()); + ret = WriteFile(pipe, &info, sizeof(info), &count, NULL); + ok(ret, "WriteFile failed: %u\n", GetLastError()); + break; + } + case REQ_READ_CONSOLE: ret = ReadConsoleW(input, buf, req->u.size, &count, NULL ); ok(ret, "ReadConsoleW failed: %u\n", GetLastError());
Remember the fact we completed the entire line without altering the cursor. A newline is delayed so it wraps when writing a new character.
This can happen in practice when writing one character at a time to the console; right now it will keep overwriting the last character on the line.
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com ---
Note: I don't know why Windows sends a \b if we write two characters at once (i.e. "ab") to complete the line, but not if we write them one at a time ("a" then "b"). Doesn't the latter mess up the tty cursor? It seems like a Windows bug to me. For now I just skipped it in the tests, since Wine outputs it (which is proper when it syncs the tty cursor).
programs/conhost/conhost.c | 27 +++++++++++-- programs/conhost/conhost.h | 1 + programs/conhost/tests/tty.c | 75 ++++++++++++++++++++++++++++++++++++ 3 files changed, 100 insertions(+), 3 deletions(-)
diff --git a/programs/conhost/conhost.c b/programs/conhost/conhost.c index 7377945..7970eb1 100644 --- a/programs/conhost/conhost.c +++ b/programs/conhost/conhost.c @@ -1136,6 +1136,7 @@ static void update_read_output( struct console *console ) RECT update_rect;
empty_update_rect( screen_buffer, &update_rect ); + screen_buffer->delayed_newline = 0;
if (ctx->update_end >= ctx->update_begin) { @@ -1359,6 +1360,11 @@ static NTSTATUS read_console( struct console *console, unsigned int ioctl, size_ console->edit_line.home_x = console->active->cursor_x; console->edit_line.home_y = console->active->cursor_y; console->edit_line.status = STATUS_PENDING; + if (console->active->delayed_newline) + { + console->edit_line.home_x = 0; + console->edit_line.home_y++; + } if (edit_line_grow( console, 1 )) console->edit_line.buf[0] = 0;
console->pending_read = out_size; @@ -1827,6 +1833,7 @@ static NTSTATUS set_output_info( struct screen_buffer *screen_buffer, return STATUS_INVALID_PARAMETER; }
+ screen_buffer->delayed_newline = 0; if (screen_buffer->cursor_x != info->cursor_x || screen_buffer->cursor_y != info->cursor_y) { screen_buffer->cursor_x = info->cursor_x; @@ -1856,8 +1863,13 @@ static NTSTATUS set_output_info( struct screen_buffer *screen_buffer, screen_buffer->win.bottom -= screen_buffer->win.top; screen_buffer->win.top = 0; } - if (screen_buffer->cursor_x >= info->width) screen_buffer->cursor_x = info->width - 1; - if (screen_buffer->cursor_y >= info->height) screen_buffer->cursor_y = info->height - 1; + if (screen_buffer->cursor_x >= info->width) + { + screen_buffer->cursor_x = info->width - 1; + screen_buffer->delayed_newline = 0; + } + if (screen_buffer->cursor_y >= info->height) + screen_buffer->cursor_y = info->height - 1;
notify_screen_buffer_size( screen_buffer ); } @@ -1910,6 +1922,10 @@ static NTSTATUS write_console( struct screen_buffer *screen_buffer, const WCHAR
empty_update_rect( screen_buffer, &update_rect );
+ if (screen_buffer->delayed_newline && screen_buffer->cursor_x == screen_buffer->width - 1) + screen_buffer->cursor_x = screen_buffer->width; + screen_buffer->delayed_newline = 0; + for (i = 0; i < len; i++) { if (screen_buffer->mode & ENABLE_PROCESSED_OUTPUT) @@ -1917,10 +1933,12 @@ static NTSTATUS write_console( struct screen_buffer *screen_buffer, const WCHAR switch (buffer[i]) { case '\b': + if (screen_buffer->cursor_x == screen_buffer->width) screen_buffer->cursor_x--; if (screen_buffer->cursor_x) screen_buffer->cursor_x--; continue; case '\t': - j = min( screen_buffer->width - screen_buffer->cursor_x, 8 - (screen_buffer->cursor_x % 8) ); + j = screen_buffer->cursor_x < screen_buffer->width ? screen_buffer->cursor_x : 0; + j = min( screen_buffer->width - j, 8 - (j % 8) ); while (j--) write_char( screen_buffer, ' ', &update_rect, NULL ); continue; case '\n': @@ -1951,7 +1969,10 @@ static NTSTATUS write_console( struct screen_buffer *screen_buffer, const WCHAR if (screen_buffer->mode & ENABLE_WRAP_AT_EOL_OUTPUT) { if (screen_buffer->mode & ENABLE_VIRTUAL_TERMINAL_PROCESSING) + { + screen_buffer->delayed_newline = 1; screen_buffer->cursor_x--; + } else { screen_buffer->cursor_x = 0; diff --git a/programs/conhost/conhost.h b/programs/conhost/conhost.h index 6446cd1..fddd771 100644 --- a/programs/conhost/conhost.h +++ b/programs/conhost/conhost.h @@ -117,6 +117,7 @@ struct screen_buffer unsigned int height; unsigned int cursor_size; /* size of cursor (percentage filled) */ unsigned int cursor_visible; /* cursor visibility flag */ + unsigned int delayed_newline; /* whether to output a newline on the next character written */ unsigned int cursor_x; /* position of cursor */ unsigned int cursor_y; /* position of cursor */ unsigned short attr; /* default fill attributes (screen colors) */ diff --git a/programs/conhost/tests/tty.c b/programs/conhost/tests/tty.c index 122e8a9..cbcaf76 100644 --- a/programs/conhost/tests/tty.c +++ b/programs/conhost/tests/tty.c @@ -781,6 +781,81 @@ static void test_write_console(void) skip_sequence("\x1b[?25h"); /* show cursor */ expect_empty_output();
+ child_set_output_mode(ENABLE_PROCESSED_OUTPUT | ENABLE_WRAP_AT_EOL_OUTPUT | ENABLE_VIRTUAL_TERMINAL_PROCESSING); + + child_set_cursor(28, 12); + skip_hide_cursor(); + expect_output_sequence("\x1b[28C"); /* move cursor to the end of the line */ + skip_sequence("\x1b[?25h"); /* show cursor */ + expect_empty_output(); + + child_string_request(REQ_WRITE_CONSOLE, L"ab"); + skip_hide_cursor(); + expect_output_sequence("ab"); + expect_output_sequence("\b"); + skip_sequence("\x1b[?25h"); /* show cursor */ + expect_empty_output(); + test_cursor_pos(29, 12); + + child_string_request(REQ_WRITE_CONSOLE, L"c"); + skip_hide_cursor(); + expect_output_sequence("\r\n"); + expect_output_sequence("c"); + skip_sequence("\x1b[?25h"); /* show cursor */ + expect_empty_output(); + test_cursor_pos(1, 13); + + child_set_cursor(28, 14); + skip_hide_cursor(); + expect_output_sequence("\x1b[15;29H"); /* set cursor */ + skip_sequence("\x1b[?25h"); /* show cursor */ + expect_empty_output(); + + child_string_request(REQ_WRITE_CONSOLE, L"x"); + skip_hide_cursor(); + expect_output_sequence("x"); + skip_sequence("\x1b[?25h"); /* show cursor */ + expect_empty_output(); + test_cursor_pos(29, 14); + + child_string_request(REQ_WRITE_CONSOLE, L"y"); + skip_hide_cursor(); + expect_output_sequence("y"); + skip_sequence("\b"); /* Windows doesn't send this here for some reason */ + skip_sequence("\x1b[?25h"); /* show cursor */ + expect_empty_output(); + test_cursor_pos(29, 14); + + child_string_request(REQ_WRITE_CONSOLE, L"\b"); + skip_hide_cursor(); + expect_output_sequence("\b"); + skip_sequence("\x1b[?25h"); /* show cursor */ + expect_empty_output(); + test_cursor_pos(28, 14); + + child_string_request(REQ_WRITE_CONSOLE, L"z"); + skip_hide_cursor(); + expect_output_sequence("z"); + skip_sequence("\x1b[?25h"); /* show cursor */ + expect_empty_output(); + test_cursor_pos(29, 14); + + child_string_request(REQ_WRITE_CONSOLE, L"w"); + skip_hide_cursor(); + expect_output_sequence("w"); + skip_sequence("\b"); /* Windows doesn't send this here for some reason */ + skip_sequence("\x1b[?25h"); /* show cursor */ + expect_empty_output(); + test_cursor_pos(29, 14); + + child_string_request(REQ_WRITE_CONSOLE, L"X"); + skip_hide_cursor(); + expect_output_sequence("\r\n"); + expect_output_sequence("X"); + skip_sequence("\x1b[?25h"); /* show cursor */ + expect_empty_output(); + test_cursor_pos(1, 15); + child_set_output_mode(ENABLE_PROCESSED_OUTPUT | ENABLE_WRAP_AT_EOL_OUTPUT);
child_set_cursor(28, 20);
On 4/1/21 6:43 PM, Gabriel Ivăncescu wrote:
Remember the fact we completed the entire line without altering the cursor. A newline is delayed so it wraps when writing a new character.
That sounds fine, but I don't think that you need a new variable for that. cursor_x == width means something like that already inside write_console call. Right now we make sure that it's normalize that before leaving, but we may just make other parts of code aware of that (somewhat like your previous patch did).
This can happen in practice when writing one character at a time to the console; right now it will keep overwriting the last character on the line.
Signed-off-by: Gabriel Ivăncescugabrielopcode@gmail.com
Note: I don't know why Windows sends a \b if we write two characters at once (i.e. "ab") to complete the line, but not if we write them one at a time ("a" then "b"). Doesn't the latter mess up the tty cursor? It seems like a Windows bug to me. For now I just skipped it in the tests, since Wine outputs it (which is proper when it syncs the tty cursor).
We should be able to just never emit those '\b's in those cases. Host should already handle that well, we just need to report it properly to console client.
Thanks, Jacek
On 02/04/2021 15:38, Jacek Caban wrote:
On 4/1/21 6:43 PM, Gabriel Ivăncescu wrote:
Remember the fact we completed the entire line without altering the cursor. A newline is delayed so it wraps when writing a new character.
That sounds fine, but I don't think that you need a new variable for that. cursor_x == width means something like that already inside write_console call. Right now we make sure that it's normalize that before leaving, but we may just make other parts of code aware of that (somewhat like your previous patch did).
Oh okay, I was worried about new code being added forgetting to normalize cursor_x when read, but sure.
This can happen in practice when writing one character at a time to the console; right now it will keep overwriting the last character on the line.
Signed-off-by: Gabriel Ivăncescugabrielopcode@gmail.com
Note: I don't know why Windows sends a \b if we write two characters at once (i.e. "ab") to complete the line, but not if we write them one at a time ("a" then "b"). Doesn't the latter mess up the tty cursor? It seems like a Windows bug to me. For now I just skipped it in the tests, since Wine outputs it (which is proper when it syncs the tty cursor).
We should be able to just never emit those '\b's in those cases. Host should already handle that well, we just need to report it properly to console client.
So I should follow Windows' behavior and it won't break anything? My concern is also how to do it? The current code rightfully syncs the tty cursor by putting the \b (which is correct if more than one char is written, even on Windows).
I feel like I'll have to add a hack to do this and I don't like it. For example, check for delayed newline and len == 1 and then decrement tty_cursor_x *before* tty_sync is called, so it doesn't put the \b. (I don't know if len == 1 is a good condition, if I have to add this hack I'll probably test it some more)
Is that acceptable or do you have something else in mind?
On 4/2/21 3:06 PM, Gabriel Ivăncescu wrote:
I feel like I'll have to add a hack to do this and I don't like it. For example, check for delayed newline and len == 1 and then decrement tty_cursor_x *before* tty_sync is called, so it doesn't put the \b. (I don't know if len == 1 is a good condition, if I have to add this hack I'll probably test it some more)
Unless I'm missing something, if you leave cursor_x == width, then tty cursor will be already in sync so there will be no reason to add '\b's.
Jacek
On 02/04/2021 16:11, Jacek Caban wrote:
On 4/2/21 3:06 PM, Gabriel Ivăncescu wrote:
I feel like I'll have to add a hack to do this and I don't like it. For example, check for delayed newline and len == 1 and then decrement tty_cursor_x *before* tty_sync is called, so it doesn't put the \b. (I don't know if len == 1 is a good condition, if I have to add this hack I'll probably test it some more)
Unless I'm missing something, if you leave cursor_x == width, then tty cursor will be already in sync so there will be no reason to add '\b's.
Jacek
Yeah, but what about when we write "ab" (two chars at once) and Windows does insert a \b in that case? I'm not sure yet, I'll see what happens, but I can't promise it won't have some semi-hacks if it's really needed.
-
Gabriel Ivăncescu -
Jacek Caban