Implement snap-on-input for conhost

[lhecker]

Once COOKED_READ_DATA uses VT sequences to position the cursor,
nothing will snap the viewport back to the prompt line anymore.
The same problem already exists with WSL. Unfortunately, the inbox
conhost in build ~26100 has the same issue.

This PR implements a "snap on input" functionality similar to the one
in Windows Terminal by ignoring control keys and otherwise snapping
the viewport to the current cursor position. This is slightly different
to Windows Terminal however, because it should actually snap to the
virtual viewport position instead. The benefit of this difference is
that the diff is way small which hopefully makes backporting easier.
Additionally, it only snaps vertically on input, as to not break
interactive applications that use WriteConsoleOutput and have
come to rely on navigation keys not scrolling the viewport.

Additionally, this implements horizontal scrolling similar to vim:
When the cursor goes outside of the viewport it'll snap to the nearest
multiple of half the viewport width. That means if you have a 120-
column window, you're at the right edge, and you type 1 more character,
it'll scroll by 60 columns to the right. The cursor will then be right
in the center of the viewport.

Closes #18073
Closes MSFT:49027268

[DHowett]

Does this fix match the behavior we agreed on? I forget

[j4james]

I don't mean to throw a spanner in the works here, but it occurred to me that this approach could be problematic for applications that control the viewport themselves. As one example, imagine something like a spreadsheet that uses a wide buffer to allow for all of the columns in a sheet, and which pans the viewport horizontally as the focus is moved across those columns.

For an app like that, I would expect the focus to be implemented with a change of cell color, which would likely be achieved with something like FillConsoleOutputAttribute. In that case the focus is completely unrelated to the cursor position (which may not even be visible). So snapping to the cursor position every time a key is pressed could be extremely annoying.

I understand the need to address this snapping issue in some way, but would it not be possible to go with an approach that's closer to the original conhost behavior, and thus less likely to be a breaking change for anyone?

Feel free to ignore me if you don't think this is a realistic concern though.

[lhecker]

The original implementation was entirely based around WC_KEEP_CURSOR_VISIBLE (I don't believe anything else was involved). It was set whenever we were printing text, including VT text. That way WSL input snapping worked indirectly, simply because we would snap whenever there was any output.

The internal issue MSFT:49027268 is about another related behavior: When you previously pressed Enter/Ctrl-C in cmd.exe, it would scroll horizontally all the way to the left, simply because cmd's output happened to start writing from column 0. This is why I added vim-style snapping, because this causes the viewport to scroll all the way to the left if the prompt is less wide than the viewport.

We can restore the WC_KEEP_CURSOR_VISIBLE behavior for WriteCharsLegacy of course and this would fix the latter issue. But how would we go about solving the snapping for WSL? Is there any alternative mechanism we can have that is both progressive for modern VT expectations (snap on input) and also works for old applications (snap on output)?

One option I can imagine is that we check the current OutputMode and if it contains ENABLE_VIRTUAL_TERMINAL_PROCESSING we snap on input, otherwise on output. This makes it perfect for an application author (I think), but what about the user? For a user it may seem random and unpredictable, because sometimes you couldn't scroll up while text is printing (it would always scroll back down) while other times you can. What toggles this behavior would be invisible to the user, because it's a hidden flag.

Perhaps no application relies on the scenario you mentioned, so would it make sense to risk breaking apps, in favor of predictable behavior for users?

Edit: I should mention that I think this is a very realistic concern. I actually hadn't thought of that so far, so thank you so much for mentioning it! I'll mention this risk in the PR message.

[j4james]

I think it might be worth considering vertical and horizontal scrolling separately, because they're really somewhat different concepts. Just some brainstorming thoughts...

Horizontal scrolling:

• Modern VT terminals: Few if any Linux terminals support this, so I don't think there's any expectation from that camp about how this should work.
• DEC terminals: The horizontal scroll offset can be manually controlled, or made to track the cursor position automatically (essentially snap-on-output), dependent on a mode setting.

I'm not sure what the exact rules are for the conhost horizontal snapping, but I think there's a good chance we could implement that as a form of snap-on-output that is DEC compatible, without having to concern ourselves with modern VT compatibility. I doubt anyone expects horizontal scrolling to be snap-on-input. Being able to turn it off completely would be nice (like the DEC mode), but that can come later.

Vertical scrolling:

• Modern VT terminals: I haven't done much research on this, but just based on a quick test, there seemed to be a fair amount of variation in how this was handled. Xterm in particular seemed to be snap-on-output rather than snap-on-input (at least by default). So while I suspect snap-on-input may be more popular, we probably have a bit of leeway to do our own thing here.
• DEC terminals: As with horizontal scrolling they have a snap-on-output mode, but this has to do with panning the viewport over a larger page, rather than exiting the scrollback (which is what we're really concerned with here). So I think it's safe to ignore DEC compatibility with regard to vertical movement.

So maybe we could get away with a vertical snap-on-input that also includes some additional restrictions, like the input must also have triggered some output, and/or the cursor must be visible (i.e. not turned off). I don't know if that's enough to prevent unwanted snapping. I'm also not sure there won't be cases where snapping is expected that we'll be missing.

One option I can imagine is that we check the current OutputMode and if it contains ENABLE_VIRTUAL_TERMINAL_PROCESSING we snap on input, otherwise on output.

This is possibly the best option in terms of backwards compatibility, but I agree it's not great in terms of a consistent user experience. It's also something that I suspect we can't really emulate over conpty - I don't know how much that matters.

[j4james]

I was looking to see what options Xterm had for this, and I've just discovered that it actually has two modes (originally from rxvt) which look like they control the snapping behaviour:

• DECSET 1010 - Scroll to bottom on tty output
• DECSET 1011 - Scroll to bottom on key press

So maybe we could implement those as well, and enable 1011 when the ENABLE_VIRTUAL_TERMINAL_PROCESSING mode is set, and disable it when the mode is reset (possibly also then enabling mode 1010).

And it also occurred to me that the inconsistency in behavior may not be that big a deal, because I suspect the main time users are likely to care about this is when they're in the shell, and most shells now would be using VT mode wouldn't they?

But note that this would just be for the vertical scrolling. As mentioned above, I think we should treat horizontal scrolling separately, and either ignore it for now, or maybe hardcode it to snap on output if that matches the existing conhost behavior.

Edit: Another thought - maybe just default to DECSET 1011 for Windows Terminal and DECSET 1010 for conhost, and if users or apps want something different they can always override those settings themselves.

[lhecker]

I tried implementing it the way you suggested. I'm not particularly happy with how _makeLocationVisible looks like, but... eh. It now scrolls vertically on input and snaps horizontally by half-viewport-widths on output. Selection still uses cell-wise scrolling.

[j4james]

I've been playing around with this branch for a bit now, and I've got to say I'm not a big fan of the half-viewport movement. It doesn't match the original conhost behavior nor the expected DEC behavior (at least as far I know). And I think that's going to make it difficult if not impossible for a VT app to position the viewport programmatically.

Also there seems to be a bug in the positioning at the far edge of the buffer. Like if I set my buffer width to 132 and the viewport width to 80, it scrolls when I first reach the right edge of the viewport, but if I keep moving past the new right boundary it doesn't scroll again - the cursor just moves offscreen.

[lhecker]

I've been playing around with this branch for a bit now, and I've got to say I'm not a big fan of the half-viewport movement.

If you're in cmd, type a really long prompt until you can't see the C:\path> anymore, and press Esc to clear it. In that situation it'll not show any part of C:\path>, which is a behavior I've always disliked. Most TUI editors I've used show at least parts of the line to the left. That's why I made it work like vim.

Is there a situation where you disliked this behavior in particular, or did you dislike it overall?

I've just pushed a commit that should behave more traditional like conhost. Does that work better for you?

[lhecker]

I've rebased this PR so that we have both variants back-to-back to play around with. I've also fixed the bug with the 132 column buffer you mentioned. 🙂

[j4james]

If you're in cmd, type a really long prompt until you can't see the C:\path> anymore, and press Esc to clear it. In that situation it'll not show any part of C:\path>, which is a behavior I've always disliked.

I see your point, but that seems like something the shell itself should be controlling. If the terminal takes ownership of that behavior, it's no longer something that apps can decide for themselves.

Is there a situation where you disliked this behavior in particular, or did you dislike it overall?

Personally this kind of jump scrolling feels primitive to me - like it's the sort of thing that an app would do because it doesn't have the ability to scroll smoothly. And when typing in a buffer that scrolls, I find it more difficult to follow what I'm doing if the cursor keeps jumping backwards every time I reach the edge of the window.

I can accept that's a personal preference, though, and I wouldn't care that much if this was something that was specific to the shell, or maybe the cooked read implementation, but it doesn't seem right to force that behavior on every application.

I need to double check my facts, but my understanding was that the legacy console APIs enabled applications to write anywhere in the buffer without it scrolling, and then they could position the viewport themselves exactly where they needed it to be.

I'm also fairly sure there are standard DEC VT operations we can use that would provide equivalent functionality for VT apps. But that isn't going to work if the terminal is making its own decisions about where it thinks the viewport should be positioned.

In my ideal world, the console APIs would work exactly the same way they've always worked, at least in terms of horizontal scrolling. And at some point in the future we'll convert those APIs into standard VT sequences that can be passed over conpty, so Windows Terminal can support that functionality as well.

I'm obviously not expecting that to happen now, but I'd rather not go down a path that guarantees we'll never be able to achieve that in the future.

Again, though, feel free to ignore my ranting. It's not that big a deal to me.

[j4james]

Been doing some more testing with various console APIs and comparing the results with the V1 console. There are two differences I've noticed:

1. We now pan the viewport down to bring the cursor line into view when a key is pressed which wasn't the case in the V1 console. It's possible that may be a problem for someone, but we can always add Xterm's 1011 mode to let users or apps toggle the behaviour themselves if it turns out to be an issue.

2. We now pan the viewport both horizontally and vertically to bring the output position into view when the WriteConsole API is called, but it seems that the original V1 console only panned vertically. I don't think this is a big deal, though, because if you didn't want the viewport to move when outputting something you would likely have been using one of the other APIs anyway, and they all still work as expected.

As for the choice of vim mode vs conhost mode, I'm still very much in favor of the latter, but I wouldn't be terribly upset if I lost that argument. From a backwards compatibility point of view it only affects WriteConsole, and that's not strictly compatible in conhost mode either (as noted above). It's more concerning for me in VT mode, but that's not going to be a problem until we try and support the windowing extension, which may be never.

[lhecker]

We now pan the viewport both horizontally and vertically to bring the output position into view when the WriteConsole API is called

Hmm weird. That's definitely not my intention. SnapOnOutput() should only pan horizontally. Perhaps some other part of conhost calls MakeCursorVisible()? I may have to investigate this. But as you said, it's probably not that big of a deal either.

[j4james]

We now pan the viewport both horizontally and vertically to bring the output position into view when the WriteConsole API is called

That's definitely not my intention. SnapOnOutput() should only pan horizontally. Perhaps some other part of conhost calls MakeCursorVisible()?

I believe it's the code here:

terminal/src/host/_stream.cpp

Lines 102 to 109 in 746cf1f

	// if at right or bottom edge of window, scroll right or down one char.
	if (cursorMovedPastViewport)
	{
	til::point WindowOrigin;
	WindowOrigin.x = 0;
	WindowOrigin.y = coordCursor.y - screenInfo.GetViewport().BottomInclusive();
	LOG_IF_FAILED(screenInfo.SetViewportOrigin(false, WindowOrigin, true));
	}

Despite what the comment says, that only pans vertically. That's the way it's always worked, AFAIK, even in the old V1 console. It's the horizontal panning added by SnapOnOutput that's a deviation from the original behavior.

And if your intention is to make it only pan horizontally, that would be extremely weird. Imagine you're scrolled back in the buffer while an application is slowly producing output. If the viewport were to jump down to the output line, as it has always done, that would be understandable (although I know some people find that annoying). But if it just started bouncing the horizontal offset backwards and forwards for no obvious reason, that's guaranteed to confuse people.