[PATCH v2 3/3] conhost: Fix delayed wrapping around when ENABLE_VIRTUAL_TERMINAL_PROCESSING is set.

[Gabriel Ivăncescu]

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ăncescu<gabrielopcode at 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?