Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support disabling option "wrap text output on resize" #546

Closed
ahauan4 opened this issue Jan 15, 2024 · 12 comments
Closed

Support disabling option "wrap text output on resize" #546

ahauan4 opened this issue Jan 15, 2024 · 12 comments
Labels
external The issue is due to external causes outside of Clink

Comments

@ahauan4
Copy link

ahauan4 commented Jan 15, 2024

I'm used to have the "wrap text output on resize" option disabled for cmd.exe in Windows 10/11.
clink_wrap_text_output_on_resize

If the command line text is shorter than the window width, everything works fine:
clink_1_1_ok

But if the window gets resized shorter than the command line and then POS1 and END is pressed, the cursor does not jump to the end of the line (and the scrollbar not to the right) but instead the cursor jumps to the end of the visible line:
clink_1_2_pos1_end

If I then try to add some new text to the end, the resulting command line text gets messed up.
Here how it looks like after making the window wide enough and pressing END:
clink_1_3_newtext

With "wrap text output on resize" enabled (default), everything works fine.
image

Am I doing something wrong or is disabling "wrap text output on resize" not supported by clink?

@chrisant996
Copy link
Owner

chrisant996 commented Jan 15, 2024

In recent years, Windows added built-in support for ANSI escape codes. The implementation of ANSI escape code support considers the "terminal display" to be only the part that is visible. I.e. if there's a horizontal scrollbar, then the ANSI escape codes for positioning the cursor are unable to position the cursor past (any of) the edges of the visible area of the terminal display.

The OS API ReadConsole() is what Clink replaces. The OS implementation of ReadConsole() works with "Wrap text output on resize" because it doesn't use any ANSI escape codes (it doesn't have any color or fanciness or etc).

Workaround: If you run clink set terminal.emulation emulate, then Clink doesn't rely on ANSI escape codes for cursor positioning. Or, rather, Clink uses its own custom implementation of ANSI escape codes instead of the built-in OS implementation, and Clink's implementation handles cursor positioning escape codes by calling certain deprecated console APIs.

I'll take a look at detecting when the Window width is different from the Screen Buffer width, and maybe in that case always use only the deprecated console APIs for cursor positioning, instead of ANSI escape codes.



P.S. The issue isn't exactly about "Wrap text output on resize". The issue occurs when "Width:" is set differently for "Screen Buffer Size" versus "Window Size". Turning off "Wrap text output on resize" is the easiest way to cause that. But there are other ways as well. For example, the following program code demonstrates another way for the widths to become different from each other.

#include <stdio.h>
#include <windows.h>
int _cdecl main(int argc, char const** argv)
{
    HANDLE hOut = GetStdHandle(STD_OUTPUT_HANDLE);
    CONSOLE_SCREEN_BUFFER_INFO csbi;
    GetConsoleScreenBufferInfo(hOut, &csbi);
    csbi.srWindow.Right -= 4;
    SetConsoleWindowInfo(hOut, true, &csbi.srWindow);
}

@chrisant996 chrisant996 added the enhancement New feature or request label Jan 15, 2024
@chrisant996
Copy link
Owner

chrisant996 commented Jan 15, 2024

I'm sorry. It's a limitation of how Windows has implemented its terminal support.

Workaround:

At this time, my best recommendation is that to be able to have a horizontal scrollbar in the terminal, it's necessary to run clink set terminal.emulation emulate to prevent Clink from using the OS terminal support for ANSI escape codes.

However, the visible window area is going to jitter around (i.e. the cursor position should always be visible, but position of the horizontal scrollbar will jitter around and may position itself in undesirable ways).

Notes:

  1. The bash shell running in a native Windows console will experience similar problems.
  2. The bash shell works ok in a mintty terminal (e.g. Git-Bash) because mintty is an independent implementation of a terminal, and it isn't possible to have a horizontal scrollbar in mintty.
  3. The problem doesn't occur in Windows Terminal, because it isn't possible to have a horizontal scrollbar in Windows Terminal.
  4. The built-in OS implementation of ReadConsoleW doesn't suffer from those problems because the API is implemented by the console itself, so the API is able to just directly manipulate the internals of the console (which apps cannot do).

More Details:

The current implementation of ANSI escape code support in Windows makes it essentially impossible for apps to use ANSI escape codes safely/properly if the Window Width is different from the Screen Buffer Width (i.e. when there's a horizontal scrollbar).

I can avoid using escape codes for cursor positioning (and shifting characters right/left). But regardless the OS forcibly adjusts the horizontal scrollbar position. And for example, the escape codes to "clear to the end of the line" only clear the part that's visible -- everything to the left or right of the current horizontal scrollbar position does not get cleared (so, scrolling horizontally reveals that there's leftover text in the display; the display is effectively garbled). So it ends up being necessary to avoid any ANSI escape codes other than for colors. And that forces the display to jitter around.

In my opinion, the best way to solve this would be for the OS implementation of ANSI escape code support to apply to the combination of the Window Height and the Screen Buffer Width. But they currently apply to the Window Width instead, which ruins the ability for an app to accommodate a horizontal scrollbar in the terminal. I think part of the reason for this oversight is likely that the new Windows Terminal doesn't support a horizontal scrollbar (just like mintty doesn't support a horizontal scrollbar).

In the meantime, there's nothing an app can do about it, except to build their own custom terminal emulation layer. But even then the OS automatically adjusts the horizontal scrollbar and there will be a lot of flicker while an app fights with the OS about the Window area and the cursor position. Even when Clink's terminal emulation is enabled, Clink currently does not fight with the OS about the Window area position. At some point in the future, I may look into the possibility of making Clink fight with the OS, but that would create even more jitter.

@chrisant996 chrisant996 added external The issue is due to external causes outside of Clink and removed enhancement New feature or request labels Jan 15, 2024
@chrisant996
Copy link
Owner

(Marked as "external" because there's no way for Clink to properly overcome the OS limitation.)

@chrisant996
Copy link
Owner

chrisant996 commented Jan 15, 2024

I've made changes to hack around the OS limitation. It's jittery, though, as expected.

Also, the clear-screen and clear-display Readline commands won't work as expected, because they rely on escape codes.

I'm pretty sure the problems are an oversight in the escape code implementations in the OS, as there's no way to be compatible with Linux programs the way it currently stands, and the main purpose of adding escape code support was to enable Linux compatibility. There's only so much I can do to work around the various problems, but at least it will sort of work in the meantime while waiting for OS fixes.

chrisant996 added a commit that referenced this issue Jan 16, 2024
See #546.  When "Wrap text output on resize" is unchecked (or when
Window Width != Screen Buffer Width) then escape codes for cursor
positioning and insert/delete characters and clearing text and etc only
affect the part of the terminal display that's within the visible area
(the CONSOLE_SCREEN_BUFFER_INFO::srWindow area).

That results in jitter and some text getting displayed inaccurately.

There's only so much Clink can do to compensate for the problem, but
this change tries to mitigate it as much as possible.  However, the
visible area jitters around.  There's nothing Clink can do about that;
the behavior is built into the OS console APIs.
@chrisant996
Copy link
Owner

Unfortunately, the changes I made only work around issues when printing the prompt and printing the input line. But there are many other places in Readline and Clink that also run into problems because escape codes are restricted to only affect the visible window (per horizontal scrollbar position).

I'm not interested in investing further on rewriting Readline and Clink display code to work around OS limitations on escape code support (i.e. stop using escape codes). It's a big and costly exercise, with too much risk for breakages, the console APIs are being actively deprecated in favor of using escape codes, and no matter what Clink does it can never solve the jitter problems. And Windows Terminal doesn't support horizontal scrollbars, and is working on deprecating the legacy conhost terminal anyway.

I'm sorry. I agree that the situation is unfortunate, but it's an external issue and Clink can't by itself solve the problems.

So, the only options are either don't use a horizontal scrollbar, or run clink set terminal.emulation emulate (and lose support for 8-bit and 24-bit colors). The changes I made will at least improve the window area positioning (at the cost of introducing even more jitter). That's about all that can be done by Clink; only the OS console subsystem itself (part of Windows Terminal) can provide thorough fixes for the issues.

@ahauan4
Copy link
Author

ahauan4 commented Jan 17, 2024

Thanks for having a look at this so quickly.
I tried it with terminal.emulation emulate as you suggested, but unfortunately it doesn't seem to behave better.

If I press END, the cursor is positioned at the end of the visible line too, instead of the real end
clink_2_2_pos1_end

When I try to add some new text to the (supposed) end of the line, it looks like this:
clink_2_3_newtext

After making the Window wide enough, the line looks messed up again:
clink_2_3_newtext_wide_end

@chrisant996
Copy link
Owner

chrisant996 commented Jan 17, 2024

When v1.6.2 is published it will include the commit that should work around the visible window positioning issues; i.e. it should work, but there will be jitter (and there might be edge cases that turn up and need further tweaks).

The good news is, I pm'd with some of the WT team members and they agree that the escape code support should use Screen Buffer Width not Window Width. I'll open an issue in the WT repo (it contains the source code for the escape code support in Windows, even for the legacy conhost terminal window). It looks to me like it probably won't be a simple/quick fix, and I don't know what the priority level will be, so it might take a while but a fix should come.

@ahauan4
Copy link
Author

ahauan4 commented Jan 17, 2024

I just tested on a different computer with clink v1.6.1 on Windows 11 (10.0.22621.3007). There it seems to work with terminal.emulation set to native, but not with emulate.

@ahauan4
Copy link
Author

ahauan4 commented Jan 17, 2024

The previous tests with the screenshots was on Windows 10 (10.0.19045.3930)

@chrisant996
Copy link
Owner

chrisant996 commented Jan 22, 2024

@ahauan4 that's interesting follow-up info about Win10 vs Win11, and about native vs emulate on Win11.

Over the weekend I had a chance to test the change I made on Win10 with native and emulate, and on Win11 with native and emulate:

On Win11 it seems to work properly in both native and emulate, with my changes (which aren't published yet).
On WIn10 it also seems to work properly in both native and emulate, with my changes.

Except that on Win10 there's noticeable jitter of the visible area.

Once the change is published, please let me know if there are still problems. I may not be able to compensate further, but it will be good to have a record of what is/isn't working.

Thanks!

@chrisant996
Copy link
Owner

v1.6.2 has been published with the changes to compensate for the escape code limitations in Win10.

@ahauan4
Copy link
Author

ahauan4 commented Jan 26, 2024

Very big Thanks to you!
I tested it on Win10 + Win11.
It works perfectly in both modes (native and emulate) 👍
With a light flickering on Windows 10, because of the text buffer redrawing, like you said.
But that is absolutely ok for me.
The most important thing is, that with v1.6.2, the command line gets not corrupted anymore 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external The issue is due to external causes outside of Clink
Projects
None yet
Development

No branches or pull requests

2 participants