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

scroll-out in secondary console buffer pollutes main scrollback buffer #17874

Closed
Tracked by #17643
avih opened this issue Sep 7, 2024 · 21 comments
Closed
Tracked by #17643

scroll-out in secondary console buffer pollutes main scrollback buffer #17874

avih opened this issue Sep 7, 2024 · 21 comments
Labels
Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting

Comments

@avih
Copy link

avih commented Sep 7, 2024

Windows Terminal version

1.22.240823002-preview

Windows build number

10.0.19045.4780

Other Software

No response

Steps to reproduce

  • Ensure there's some content at the scrollback of the (main) terminal buffer.
  • Run an application which creates a new buffer using CreateConsoleScreenBuffer and then activates it using SetConsoleActiveScreenBuffer.
  • The application writes some lines to the new buffer until it starts scrolling (more lines than the window height can keep visible).
  • The application switches back to the main buffer and exits.

Test program which: prints 100 lines to the main buffer, then creates and activates a new screen buffer, then writes 40 lines to the new buffer, then activates the original buffer and exits:

scrollbuf.c
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <windows.h>


#define writecon(h, s) WriteConsole(h, (s), strlen(s), 0, 0)
#define ERR_EXIT(...) (fprintf(stderr, __VA_ARGS__), exit(1), 0)

void write_n_lines(HANDLE h, const char* prefix, int n)
{
	char buf[64];

	for (int i = 1; i <= n; ++i) {
		writecon(h, prefix);
		sprintf(buf, "%d\n", i);
		writecon(h, buf);
	}
}

int main(int argc, char **argv)
{
	int mode = argc > 1 ? atoi(argv[1]) : 0;

	HANDLE hcon = GetStdHandle(STD_OUTPUT_HANDLE);
	if (hcon == INVALID_HANDLE_VALUE)
		ERR_EXIT("can't get stdout handle\n");

	if (!writecon(hcon, "orig screen buffer\n"))
		ERR_EXIT("can't write to stdout console\n");

	write_n_lines(hcon, "origbuf ", 100);


	HANDLE hnewbuf = CreateConsoleScreenBuffer(
		GENERIC_WRITE, FILE_SHARE_WRITE,
		0, CONSOLE_TEXTMODE_BUFFER, 0);

	if (hnewbuf == INVALID_HANDLE_VALUE)
		ERR_EXIT("can't create new screen buf\n");

	if (!SetConsoleActiveScreenBuffer(hnewbuf))
		ERR_EXIT("can't activete new screen buf\n");

	if (!writecon(hnewbuf, "new screen buffer\n"))
		ERR_EXIT("can't write to new screen buf\n");

	write_n_lines(hnewbuf, "newbuf ", 40);

	Sleep(1000);

	if (!SetConsoleActiveScreenBuffer(hcon))
		ERR_EXIT("can't restore orig screen buf\n");
	CloseHandle(hnewbuf);

	return 0;
}

Expected Behavior

The original scrollback buffer is restored without traces of content which was printed to the new/secondary console buffer.

Actual Behavior

Lines which were printed to the new (secondary) buffer buf were scrolled out of view become part of the scrollback of the main buffer.

With the test program above (which prints 40 lines to the secondary buffer), if the terminal height is less than 40, for instance 30 lines, then the first 10 lines printed to the secondary buffer will be added to the scrollback of the main buffer.

After the program exits, one needs to scroll up (e.g. using the mouse wheel) to ovserve the lines from the secondary buffer.

Additional notes:

  • This does not seem to be a recent regression. I reproduced it also with Windows Terminal 1.15 (the oldest version I have locally), 1.22-preview, and git master artifacts build from a day or two ago (with Original console buffer is not restored when closing a secondary one #17817 already fixed).
  • There is no issue in OpenConsole.exe and also no issue in the native win10 conhost (i.e. the main scrollback buffer is unaffected by overflow at the secondary buffer).

EDIT:
The issue manifests when using less.exe for windows in Windows terminal, e.g. less some-file.c and then scrolling down using the down-arrow, then after less exits then the scrollback buffer has content which was scrolled out-of-view in less (but no issue in OpenConsole or conhost).

@avih avih added Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Sep 7, 2024
Copy link

We've found some similar issues:

If any of the above are duplicates, please consider closing this issue out and adding additional context in the original issue.

Note: You can give me feedback by 👍 or 👎 this comment.

@avih
Copy link
Author

avih commented Sep 7, 2024

The bot suggested #17669 might be similar, and it is similar, but not the same: #17669 is about the main buffer polluting the secondary buffer, while this is the other way around.

Also, this issue still exists, while #17669 is presumably already fixed.

@lhecker
Copy link
Member

lhecker commented Sep 8, 2024

No matter whether or how we fix it (pages perhaps? but that only works for WT), less should use the alternative screen buffer on Windows on versions that support it.

@avih
Copy link
Author

avih commented Sep 8, 2024

how we fix it (pages perhaps? but that only works for WT)

Now sure what "only works for WT" means. Do you mean it would be fixed for the windows-terminal but not for OpenConsole/conhost?

If yes, then as I noted above, OpenConsole/conhost don't have this issue. It only happens in windows-terminal.

less should use the alternative screen buffer on Windows on versions that support it.

Maybe, but that's work that someone will have to do, and with more code paths which have to be maintained and tested, and while code which uses CreateConsoleScreenBuffer already exists and not supposed to be broken.

(similar *nix code which uses alt-screen does exist in less, but can't be shared easily with windows code because it uses terminfo db for the sequences etc, and it's also hacky because using alt-screen is not bullet-proof like SGR sequences)

I believe the micro text editor does the same without switching to the alt screen buffer using VT escape sequences, and I'm sure there are many existing application which might not be very actively maintained and which do use CreateConsoleScreenBuffer in order to keep the main buffer (and its scrollback) clean of temporary full-screen TUI content.

@lhecker
Copy link
Member

lhecker commented Sep 8, 2024

FWIW the less shipped by msys2 (e.g. git-for-windows) works correctly, because it uses cygwin and so it uses the alternate screen buffer.


Now sure what "only works for WT" means.

I wrote it as a comment targeted to the other maintainers. We received an implementation for VT pages a while ago (#16615). It allows you to have multiple "pages" (= similar to console buffers, but more limited) at the same time, instead of just the two usual ones (main and alt buffer). That would allow us to improve our CreateConsoleScreenBuffer translation. However, most terminals don't support these sequences AFAIK and so we cannot always use them. That's what I meant with "only works for WT".

Maybe, but that's work that someone will have to do, and with more code paths which have to be maintained and tested

To be fair, that's also true for us. But I understand that we should bear the burden here.
However, unlike less, etc., we cannot make the VT translation perfect. That's something I'll get into more detail below.

I believe the micro text editor does the same without switching to the alt screen buffer using VT escape sequences

Similarly, it would be nice if micro could switch over to using the alt buffer. Windows had support for it for about as long as micro has existed.

and I'm sure there are many existing application which might not be very actively maintained and which do use CreateConsoleScreenBuffer in order to keep the main buffer (and its scrollback) clean of temporary full-screen TUI content.

Such existing applications should be translated to VT to the absolute best of our abilities of course. It's not their fault that Windows changed its focus from the old console APIs over to VT sequences over the last decade. However...


...our abilities to do that translation are limited. There are multiple problems:

  • Pages are not necessarily supported by your favorite terminal (e.g. wezterm).
  • The number of pages is finite, while the number of console screen buffers is infinite.
  • Each console buffer can specify a buffer size (dwSize) on top of a window size (srWindow). I.e. the application can specify your scrollback size, not you. That's a concept that (mostly?) doesn't exist in the VT world (certainly not outside of VT pages).
  • Each console buffer may have different size. We can translate that to CSI t (resize window sequence) but that may be ignored by the hosting terminal. That's definitely true for Windows Terminal: we don't want to allow hidden tabs to randomly resize your window. If the resize is ignored, console applications often break, because they don't check if the size actually changed. CSI t also doesn't address the problem that console buffers may have a custom scrollback size.

But most importantly:

SetConsoleActiveScreenBuffer of a console buffer with existing contents is a lossy operation and some of your screen contents may be permanently lost, because we cannot know what custom VT sequences your favorite terminal supports so we cannot save them either. Since SetConsoleActiveScreenBuffer is used by less (and micro?) to switch back to the main buffer, this issue will always affect you when using these applications.
We're expected to support complex VT, complex Unicode, and complex console APIs simultaneously. But the only thing Windows historically supported was the latter, while the former two are obviously incompatible with that: CHAR_INFO does not support complex VT attributes nor Unicode, buffer read-back doesn't exist in the VT world, and SetConsoleActiveScreenBuffer cannot be cleanly translated to any VT. There's a conflict here and the saying "You can't have your cake and eat it too" comes to my mind.
The solution appears to introduce a VT sequence that represents CreateConsoleScreenBuffer and SetConsoleActiveScreenBuffer perfectly, but then we need to get that new sequence supported by other terminals, including those on Linux (so you can SSH into Windows). At that point the question is: Why should all terminals support the new sequence as opposed to terminal applications not using ReadConsoleOutput and SetConsoleActiveScreenBuffer anymore?

In any case, we'll always try to do our absolute best, and perhaps we can improve the symptoms of your issue. But I don't believe it's fair to expect us to make two completely conflicting technologies fit perfectly.

@avih
Copy link
Author

avih commented Sep 8, 2024

Yeah, I totally understand the main issue and that's console buffer has many features which don't have an equivalent in the unix terminal world, or not enough terminals support enough features to implement many of those in the VT world, so that it can be expected to work fully inside some 3rd part terminal emulator.

That being said, I'd guess one goal of the windows terminal is to provide an environment in which windows console application can behave as if they run in conhost, so I'd think that console screen buffer would need to work somehow, including many/most/all of its features which don't have an equivalent in the VT world.

That may or may not extend to arbitrary conpty clients, but obviously prefably it does extend.

Specifically though, "less" for windows uses the console buffer like alt screen. I.e. it doesn't use a custom size (it does set size, but copies it from the main buffer), and it doesn't do read-back (that I know of, but I think it doesn't), and VT alt-screen also doesn't spill scrolled-out content into the main buffer scrollback, so at least at this specific use case, I don't think it requires features which "normal" alt-screen doesn't have. I.e. I'd think that it could be exposed as normal alt-screen to conpty clients and it would work as expected.

Admittedly I don't know most of the specifics about implementation details with the windows terminal, but I'm now getting the impression that it's a pure VT (conpty) app, probably like other 3rd party windows VT terminals (wezterm, alacritty, etc) and which operates mainly or exclussively using VT sequences?

I understand the issues this approach poses, but also think there's a non-negligible need to support existing console apps which might not get updated to use the VT interface.

Maybe conhost/OpenConsole can identify that a console app uses some features which translate to VT sequences which are not commonly supported by 3rd party terminals, and issue a warning (once-ish per app) that 3rd party terminals might not be compatible with this application (while the windows terminal would support it via extended sequences which emulate the console buffer fully, etc).

It's not my place to suggest solutions here and I'm unaware of the constraints you work with, but I do think that at least the windows terminal should be a good environment for legacy console apps, even if they use features which translate to VT sequences which are not universally supported.

Isn't that right?

@avih
Copy link
Author

avih commented Sep 15, 2024

Does this await resolution?

@lhecker
Copy link
Member

lhecker commented Sep 16, 2024

That being said, I'd guess one goal of the windows terminal is to provide an environment in which windows console application can behave as if they run in conhost [...]

I fully agree with this, which is why I made the following plan: https://github.com/microsoft/terminal/blob/main/doc/specs/%2313000%20-%20In-process%20ConPTY.md
That will need at least another year or so to be done though.

[...] but I'm now getting the impression that it's a pure VT (conpty) app, [...] which operates mainly or exclussively using VT sequences?

Yep, currently Windows Terminal operates exclusively via VT sequences. The above plan would change this: Windows Terminal would then be a native console server (like good old conhost), while we would still only use pure VT for windows applications run under WSL, as well as SSH and some other tools.

I think that approach has the best trade-off of being relatively easy to implement (by us and others), it's performant and robust. The only downside is that it'll take a while to get there.

@avih
Copy link
Author

avih commented Sep 16, 2024

I fully agree with this, which is why I made the following plan: https://github.com/microsoft/terminal/blob/main/doc/specs/%2313000%20-%20In-process%20ConPTY.md
That will need at least another year or so to be done though.

Can't say I entirely or even mostly grok it, but out of curiosity, would that mean that the windows terminal would be a good env for "legacy" console apps, but those same apps would run less good in 3rd party terminal emulators, because those work exclussively via VT sequences, and then some APIs can't be translated well enough (like is the case now with the windows terminal)?

And specifically about this issue, does that mean that it won't be fixed until this transition is complete?

Because:

Specifically though, "less" for windows uses the console buffer like alt screen. I.e. it doesn't use a custom size (it does set size, but copies it from the main buffer), and it doesn't do read-back (that I know of, but I think it doesn't), and VT alt-screen also doesn't spill scrolled-out content into the main buffer scrollback, so at least at this specific use case, I don't think it requires features which "normal" alt-screen doesn't have. I.e. I'd think that it could be exposed as normal alt-screen to conpty clients and it would work as expected.

I'd imagine that console buffer would be implemented/exposed using alt-screen, and as such, the issue shouldn't exist, because content which scrolls out of the alt-screen doesn't normally pollute the main scrollback buffer in any *nix terminal, right?

IOW, I'd think that this specific issue, depending on implementation details, does not actually need this planned architechture change?

@lhecker
Copy link
Member

lhecker commented Sep 16, 2024

Can't say I entirely or even mostly grok it, but out of curiosity, would that mean that the windows terminal would be a good env for "legacy" console apps, but those same apps would run less good in 3rd party terminal emulators, because those work exclussively via VT sequences, and then some APIs can't be translated well enough (like is the case now with the windows terminal)?

No, because over SSH these apps will be broken in Windows Terminal just as before, just like they're broken in conhost right now. Further "no", because the linked proposal can be implemented by any other terminal - it's not a proposal specific to Windows Terminal. And lastly, if we were to introduce a new sequence for translating console screen buffers instead, it would still take years for any other terminal adopt it. Given other precedent, what's most likely is that no other terminal will ever adopt it.

Just to clarify: Implementing the linked spec document above doesn't take a year. It takes maybe 1 month or so. It's just that I also need to work on a lot of other issues as well.

And specifically about this issue, does that mean that it won't be fixed until this transition is complete?

Unfortunately, yes. As I outlined above, "pages" VT sequences aren't supported by other terminals and also don't perfectly map console buffers, so even if we were to use them, someone would likely soon find another bug with them. Custom VT sequences would similarly not be supported by other terminals. Lastly, implementing a solution with VT sequences now will take just as long to implement as implementing the linked proposal above, because Windows Terminal currently doesn't support more than 2 text buffers (outside of VT pages), etc.

I'd imagine that console buffer would be implemented/exposed using alt-screen, and as such, the issue shouldn't exist, because content which scrolls out of the alt-screen doesn't normally pollute the main scrollback buffer in any *nix terminal, right?

I don't want to do any such translations at the moment because they're "risky". Any edge cases around console buffer switching would need to be fixed by me. Since Windows has had supported for the xterm alt buffer for almost a decade now, I think it's instead on these still fully maintained applications to switch over to using the alt buffer.

I would understand it, if this issue was a major one (like a crash), but it's really more of a really annoying edge case, wouldn't you agree? Basically, I just don't want to invest 50% of the time now to get a quick hack in that solves 10% of the problem, when I can also just wait 100% of the time and fix the problem 100%.
Personally, I'm just kind of tired of using software that's a hundred quick hacks glued together under a single trench coat (I'm sure you can imagine a lot of examples, particularly also from Microsoft, perhaps including this very application). For now, I just want to write some quality software. I hope you understand. 🙁

@avih
Copy link
Author

avih commented Sep 16, 2024

if we were to introduce a new sequence for translating console screen buffers instead, it would still take years for any other terminal adopt it. Given other precedent, what's most likely is that no other terminal will ever adopt it.

We're guessing, but my guess would be the same as yours.

Unfortunately, yes. As I outlined above, "pages" VT sequences aren't supported by other terminals and also don't perfectly map console buffers, so even if we were to use them, someone would likely soon find another bug with them. Custom VT sequences would similarly not be supported by other terminals.

I agree. I don't think solving it with custom or uncommon sequences is great.

I'd imagine that console buffer would be implemented/exposed using alt-screen, and as such, the issue shouldn't exist,

I don't want to do any such translations at the moment because they're "risky". Any edge cases around console buffer switching would need to be fixed by me. Since Windows has had supported for the xterm alt buffer for almost a decade now, I think it's instead on these still fully maintained applications to switch over to using the alt buffer.

Well, I was guessing that console buffers are already getting translated to alt-screen, and if that were the case then I thought the issue shouldn't exist already currently, but judging by this statement, that's not the case.

May I ask what happens currently, implementation-wise (but high level), which causes text which scrolls out of a (non-main) screen buffer to reach the terminal's scrollback?

I would understand it, if this issue was a major one (like a crash), but it's really more of a really annoying edge case, wouldn't you agree?

I do agree, but for me it's annoying enough to get back to conhost or openconsole, because I do use "less" a lot, and I do value my scrollback history, but if everytime I use "less" it adds pages of junk to my scrollback, then it will be too annoying.

(or I might implement alt-screen in "less" when VT is supported, because I already contribute to "less", but that's orthogonal to the issue here IMO)

In fact, I already did just that for a long while (using OpenConsole rather than windows terminal), and tried to get back to the terminal recently, and noticed some console buffer issues.

Believe it or not, my main issue with OpenConsole is that shift-mouse-select is unsupported (when quick-edit is disabled, e.g. because the application wants to capture mouse events). Other than that OpenConsole is quite great for me (though I do value the nice features of Windows Terminal too, but for me they're more nice to have than essential, maybe except scrollback search which is very useful).

Shall I open a feature request to support shift-mouse-select in OpenConsole to override disabled-quickedit?

Personally, I'm just kind of tired of using software that's a hundred quick hacks glued together

Completely agreed, though many times unavoidable in practice, because such is life. But if you do get the chance to avoid yet another hack, without great cost, I agree you should.

I just want to write some quality software

Not doing too bad so far, if I may say so.

@tig
Copy link

tig commented Sep 22, 2024

I suspect #17949 is a dupe of this.?

@lhecker
Copy link
Member

lhecker commented Sep 23, 2024

Well, I was guessing that console buffers are already getting translated to alt-screen, and if that were the case then I thought the issue shouldn't exist already currently, but judging by this statement, that's not the case.

Yeah exactly. As I said, I objectively don't think we should try to map it to the alt-screen as it's somewhat risky to subtly break things. I'd much prefer working on the in-process ConPTY spec linked above (= make Windows Terminal a proper console server), as I consider that a perfect solution.

May I ask what happens currently, implementation-wise (but high level), which causes text which scrolls out of a (non-main) screen buffer to reach the terminal's scrollback?

Since Windows Terminal is currently a pure VT terminal (like on UNIX), it doesn't have "console screen buffers". As a consequence, if you use SetConsoleActiveScreenBuffer, you never leave Windows Terminal's main screen either. Instead, ConPTY (the Console API --> VT translation layer) writes the new console screen buffer contents straight to the main buffer of Windows Terminal. When you switch back to your original console screen buffer it again writes the original's contents to Windows Terminal's main buffer. This gives the impression as if you had multiple console buffers in Windows Terminal, but in reality, it's ConPTY which turns the contents to pure VT.

When you scroll in less it assumes that it's in the alternate screen buffer without scrollback. It then writes more text than fits on the screen when scroll down. But Windows Terminal is not in the alternate screen buffer and so the scrolled off text ends up in the scrollback accidentally.
Similarly, when you change the font size, etc., you zoom the entire contents of Windows Terminal's main buffer including scrollback and that also breaks the illusion.

Shall I open a feature request to support shift-mouse-select in OpenConsole to override disabled-quickedit?

Please do!
The console API supports reporting shift when mouse input is enabled: https://learn.microsoft.com/en-us/windows/console/mouse-event-record-str
So, to not conflict with that, I think we'll have to check whether ENABLE_MOUSE_INPUT is set. Otherwise, I think we should be able to support that. 🙂


I suspect #17949 is a dupe of this.?

It's not strictly related. 🙂 Let's continue talking about your issue over there.

@avih
Copy link
Author

avih commented Sep 23, 2024

if you use SetConsoleActiveScreenBuffer, you never leave Windows Terminal's main screen either. Instead, ConPTY (the Console API --> VT translation layer) writes the new console screen buffer contents straight to the main buffer of Windows Terminal. When you switch back to your original console screen buffer it again writes the original's contents to Windows Terminal's main buffer. This gives the impression as if you had multiple console buffers in Windows Terminal, but in reality, it's ConPTY which turns the contents to pure VT.

Thanks. This indeed explains why this issue happens.

Shall I open a feature request to support shift-mouse-select in OpenConsole to override disabled-quickedit?

Please do! The console API supports reporting shift when mouse input is enabled: https://learn.microsoft.com/en-us/windows/console/mouse-event-record-str So, to not conflict with that, I think we'll have to check whether ENABLE_MOUSE_INPUT is set. Otherwise, I think we should be able to support that. 🙂

Unfortunately, the case where shift-mouse-select is required for override is exactly when ENABLE_MOUSE_INPUT is set (where the app also disables quick-edit, because otherwise some mouse events don't reach the app), but in windows terminal shift-mouse-select seem to work also in this specific case - which is what I hoped would be possible to do with OpenConsole as well. But this is off topic for this issue. I'll open a feature request (not sure when) and let's see there.

Thanks for the info.

@avih
Copy link
Author

avih commented Sep 29, 2024

Other than maybe performance issues, and assuming this is not too hard to do, and that I roughly get the kinds of translations which happen under the hood, would these translations of "scroll in non-main console buffer due to newline at the bottom line, or print when cursor is in bottom-right" work?

(or maybe simply "scroll is about to happen" or "scroll just happened", which are maybe simpler to identify and possibly cover more use cases. those would require minor trivial modifications of the suggested procedures below)

Few suggesitons when such scroll is identified. Do this Instead of printing the char which causes scroll to the VT terminal:

  1. cursor to top-left, print the content of 2nd line, 3rd line... last line (so it looks like it scrolled, but it shouldn't enter the scrollback), clear the bottom line, cursor to bottom-left. This probably requires more bandwidth than we want, but maybe it's OK, and it's simple.
  2. after the console buffer scrolled, instead of printing the char which caused scroll, copy the console buffer view to the VT terminal (similar/identical to how the main console buffer is restored). Technically this might be identical to 1 above, but hopefully using existing functionality as a whole.
  3. cursor to top-left, print the content of 2nd line (so the first two lines just got "scrolled", but without affecting the scrollback), set scrolling region from line 2 (CSI 2 r), scroll one line (CSI S) - (scrolls 3rd...last lines, and the bottom line should be cleared by the scroll), restore default scrolling region, cursor to bottom left. The average bandwidth of this is roughly twice the normal bandwidth, which I think is a fair price for correctness (the translation prints each line twice - once normally, and once at line 1 as part of the simulated scroll).

(some of these "print line N" might require clearing the line before print, to remove traces of line N-1 which was in its place previously, or some other solution)

Am I roughly in the ballpark of how these VT translations work and what they're capable of?

Should that work if someone implemented it?

@lhecker
Copy link
Member

lhecker commented Sep 30, 2024

Am I roughly in the ballpark of how these VT translations work and what they're capable of?

Doesn't less enable ENABLE_VIRTUAL_TERMINAL_PROCESSING? So, it'd be more like a VT -> VT translation, if anything. We can influence that, but certainly not accurately. For regular console APIs (without VT processing being enabled), we can do almost arbitrary translations, like those you mentioned.

@avih
Copy link
Author

avih commented Sep 30, 2024

Doesn't less enable ENABLE_VIRTUAL_TERMINAL_PROCESSING?

It does, but output which utilizes VT features is not used normally.

By default the console API is used (SetConsoleTextAttribute, SetConsoleCursorPosition, etc). VT would get used if the internal "VT to Console API translation" layer doesn't identify the escape sequence (and the console supports VT mode), like currently with 256/true colors sequences, or if the user uses a flag to bypass the internal translation and passthrough everything to the console - -Da (and it supports VT). But it's still not used for positioning - only for SGR sequences and maybe OSC 8. Other console control don't go through this internal traslation layer. This mixed VT/Console API does work with conhost/openconsole.

However, while I initially observed the issue with "less", and it would be nice to get conhost behavior of "less" work correctly also at the Terminal, the goal of this issue was to improve the behavior of the terminal with application which use conhost, and specifically to not pollute the main scrollback buffer when a non-main console screen buffer scrolls content out of view.

Initially you said that it might require missing/uncommon features which most terminals don't support, and we agree it won't be a great solution, but now I suggested a translation which I think should work in all terminals, and which is hopefully not too hard to implement (identify the trigger, translate it accordingly).

So, it'd be more like a VT -> VT translation, if anything. We can influence that, but certainly not accurately

Implementation wise, I clearly don't know the details, but I would have guessed that it wouldn't matter (much), because if the non-main screen buffer is about to get scrolled, does it matter whether the trigger was WriteConsole or some normal print in VT mode? (of normal text, or whatever). Both end up scrolling the console buffer, and that's the event which should be identified ideally - not the source which caused it. Am I wrong? (at least theoretically, but practical considerations do have a way to make it different than theory...)

I get that apps which use mixed Console/VT API may be harder to translate, but let's start with Console API, and hopefully VT input would still mostly work like it does with conhost?

@avih
Copy link
Author

avih commented Sep 30, 2024

Implementation wise, I clearly don't know the details

But you do! mind pointing me to the file[s] where this translation to VT happens? I don't mind giving it a look, maybe I can grasp something (for curiosity).

@lhecker
Copy link
Member

lhecker commented Oct 8, 2024

Initially you said that it might require missing/uncommon features which most terminals don't support, and we agree it won't be a great solution, but now I suggested a translation which I think should work in all terminals, and which is hopefully not too hard to implement (identify the trigger, translate it accordingly).

I also mentioned before that the reason for needing the missing features is because such a translation is excessively difficult. I'll not implement such a translation within the current ConPTY architecture for the aforementioned reasons.

Implementation wise, I clearly don't know the details, but I would have guessed that it wouldn't matter (much), because if the non-main screen buffer is about to get scrolled, does it matter whether the trigger was WriteConsole or some normal print in VT mode?

WriteConsole can be used with VT mode, but I know what you meant. We currently don't do VT -> VT translation at all. Since less uses both console APIs and VT simultaneously, emulating it correctly would require us to parse and translate not just all console APIs but in addition also all VT sequences. This would not just be similar but identical to implementing tmux within ConPTY - a very complex project. There are also no shortcuts when correctness is the goal. Just imagine for instance how sixels would need to be implemented in such a setting. It would take years upon years of my life to sand away on the edge cases that this would cause. I have to outright tell you that I'd not spend time on this myself, even if I was asked to do so at the threat of being fired. That's how much I think this would be a mistake, in the most objective way.

The correct approach is the previously mentioned in-process ConPTY. It'll be easier to implement, easier to maintain, and be more correct.

The relevant VT translation functions are here:

void BackupCursor() const;
void WriteUTF8(std::string_view str) const;
void WriteUTF16(std::wstring_view str) const;
void WriteUTF16TranslateCRLF(std::wstring_view str) const;
void WriteUTF16StripControlChars(std::wstring_view str) const;
void WriteUCS2(wchar_t ch) const;
void WriteCUP(til::point position) const;
void WriteDECTCEM(bool enabled) const;
void WriteSGR1006(bool enabled) const;
void WriteDECAWM(bool enabled) const;
void WriteASB(bool enabled) const;
void WriteWindowVisibility(bool visible) const;
void WriteWindowTitle(std::wstring_view title) const;
void WriteAttributes(const TextAttribute& attributes) const;
void WriteInfos(til::point target, std::span<const CHAR_INFO> infos) const;
void WriteScreenInfo(SCREEN_INFORMATION& newContext, til::size oldSize) const;

You can find the implementation in VtIo.cpp and their usage throughout the project.

@DHowett
Copy link
Member

DHowett commented Oct 9, 2024

Thanks for the lively discussion here! We're not going to be investing in fixing this with VT, but with the new ConPTY architecture. Feel free to continue discussing, but I'm going to close this issue as it's not currently actionable for us.

@DHowett DHowett closed this as not planned Won't fix, can't repro, duplicate, stale Oct 9, 2024
@lhecker
Copy link
Member

lhecker commented Oct 9, 2024

If it makes it any better, I'm planning to work on the new architecture early next year. This will fix the issue for Windows Terminal under most circumstances. I'll also work on bringing this to other terminals.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting
Projects
None yet
Development

No branches or pull requests

4 participants