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

DEC private 47/1047 not supported (alt buffer) #3082

Open
egmontkob opened this issue Oct 5, 2019 · 9 comments
Open

DEC private 47/1047 not supported (alt buffer) #3082

egmontkob opened this issue Oct 5, 2019 · 9 comments
Labels
Area-VT Virtual Terminal sequence support Help Wanted We encourage anyone to jump in on these. Issue-Task It's a feature request, but it doesn't really need a major design. Product-Conhost For issues in the Console codebase
Milestone

Comments

@egmontkob
Copy link

Environment

Windows build number: Win32NT 10.0.18362.0
Windows Terminal version (if applicable): 0.5.2762.0

Steps to reproduce

printf '\e[?47h'

or the same with 1047, and l.

Expected behavior

DEC private mode controls 47 and 1047 should switch to/from the alternate screen, just as 1049 already does in WT. Except that these shouldn't save and restore the cursor and clear the alternate screen.

Current terminfo uses 1049 for TERM=xterm-256color and friends, but older systems or apps not relying on terminfo might still emit the older 47 or 1047.

(While at it, please check 1048 too, I haven't checked that one.)

Actual behavior

Nothing happens on these.

@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Oct 5, 2019
@DHowett-MSFT DHowett-MSFT added Product-Conhost For issues in the Console codebase Area-VT Virtual Terminal sequence support Issue-Task It's a feature request, but it doesn't really need a major design. labels Oct 6, 2019
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Oct 6, 2019
@j4james
Copy link
Collaborator

j4james commented Oct 9, 2019

Do you know of any applications that actually depend on these commands though? It's just that XTerm has a boatload of proprietary sequences, several of which are duplicates, or backwards compatibility hacks of questionable value. If it were up to me, I wouldn't want to bloat the code with cruft like this unless it was genuinely needed.

And in case anyone does want to implement this, it's also worth mentioning that the differences between 1049, 1047 and 47 are a little more subtle than what you've described. 1049 clears the screen on the way in, 1047 clears on the way out, and 47 doesn't clear the screen at all.

@egmontkob
Copy link
Author

egmontkob commented Oct 10, 2019

Thanks a lot for this precious feedback!

Do you know of any applications that actually depend on these commands though?

I don't have any concrete example.

I had the impression that changing 47 to 1049 in terminfo (for TERM=xterm) was a relatively new thing. Well, it happened in ncurses-5.5, released 14 years ago. So my impression wasn't quite correct :D

That being said, there might be applications that have the old sequence hardcoded, I don't know.

1047 clears on the way out

Indeed, I didn't remember this detail.

In fact, pushing it one step further (I'm looking at VTE's source, although I should probably check xterm):

1049 clears on the way in, even if already on the alternate screen and this is reinforced by another 1049h (and this is xterm's behavior, too). [*]

1047 clears on the way out only if actually leaving the alternate screen, not if the normal screen is reinforced by another 1047l.

Some of these only make a difference if you're using a mixture of these numbers, e.g. 47+1047 or 49+1049. As long as you're on the normal screen, the alternate screen's contents is a Schrödinger's cat.

([*] I find this a questionable design. Shouldn't setting a boolean to its previous value guaranteed to be a no-op? Nevermind.)

If it were up to me, I wouldn't want to bloat the code with cruft like this unless it was genuinely needed.

On one hand, it should be fairly easy to implement (just a few lines of wiring; the actual features are already implemented). On the other hand, as you pointed out, we're not sure if anyone still uses these.

Let's leave this call to the Microsoft guys :)

@j4james
Copy link
Collaborator

j4james commented Oct 10, 2019

([*] I find this a questionable design. Shouldn't setting a boolean to its previous value guaranteed to be a no-op? Nevermind.)

Absolutely. They're quite clearly not modes. And once you start looking at all the edge cases, it becomes obvious how ridiculously underspecified they are. I'd be surprised to find any two terminals that implemented them identically. Even XTerm looks buggy to me in some respects, but because there's no actual specification, their implementation is technically the standard. At least until it gets an update one day, and then suddenly the standard has changed. Sorry for the rant, but I really loath these commands.

Let's leave this call to the Microsoft guys :)

Yeah. I'm happy to accept whatever they decide.

@DHowett-MSFT
Copy link
Contributor

Is it generally possible to pair the -49 series with the -47 series? Can you push 1049h and then 47l? If it's just changing the entry/exit params on the alt buffer, it's probably not a terrifically difficult problem for us to solve.

@egmontkob
Copy link
Author

There's no "-49 series" since plain 49 is not part of this game.

47/1047/1049 all toggle the same altscreen property, not different ones. So yup, e.g. 1049h enters the alt screen and then 47l leaves it. Someone has to be truly out of their minds to do this, though. :) Similarly, I think (but this is to be double checked) that DECSC/DECRC, SCOSC/SCORC, 1048 and 1049 all use the same single slot for the saved cursor.

@j4james
Copy link
Collaborator

j4james commented Oct 10, 2019

I'm just working on a PR for the DECSC/DECRC stuff at the moment and have been testing the interaction with 1049 (and a little bit of 1047/47 too). There are a number of issues with our 1049 implementation that still need fixing, and I think we should be concentrating on that first. But the way we've implemented the alt buffer is going to make it difficult to exactly match XTerm.

As for 1047/47, we can't simply implement it as 1049 without clearing the screen (ignoring the cursor issues for the moment), because we create a new alt buffer every time, so the screen is automatically cleared. Not that any of this is unsolvable - it's just not necessarily that straightforward for us as it might be for other implementations.

I should also mention I haven't found any terminals that exactly match the XTerm behaviour for these commands, so it's probably not that big a deal. Although to be fair, I don't test with the latest versions of everything, so maybe I'm seeing issues that have already been fixed.

@DHowett-MSFT DHowett-MSFT removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Oct 14, 2019
@DHowett-MSFT DHowett-MSFT added this to the Console Backlog milestone Oct 14, 2019
@zadjii-msft zadjii-msft added the Help Wanted We encourage anyone to jump in on these. label Oct 24, 2019
@egmontkob
Copy link
Author

According to Kitty 2366, elinks uses hardwired 47h/l.

As correctly pointed out there, it's a bug in elinks to go with a hardwired value rather than one taken from terminfo. That being said, it's a precious data point: yes 47 is still being used out there in the wild.

@j4james
Copy link
Collaborator

j4james commented Feb 18, 2020

@egmontkob Thanks for the heads up. I had a look at elinks, and it doesn't seem to trigger mode 47 for us. It looks like it has some kind of xterm-compatibility detection which we don't pass, and it only uses mode 47 on terminals that it considers xterm compatible.

But as you say, this does show that mode 47 is be used, so I would agree it's worth supporting.

@j4james
Copy link
Collaborator

j4james commented Aug 19, 2020

While I remember, I wanted to add a note here that private mode 47 is actually a DEC mode used on a number of their graphic terminals - Graphics Rotated Print Mode (DECGRPM). It determines the orientation in which graphic images are printed.

Knowing that, I'd now be more inclined to ignore the XTerm interpretation, unless we one day add something like a compatibility mode, that lets users choose the behaviour they expect for conflicting operations like this..

@zadjii-msft zadjii-msft modified the milestones: Console Backlog, 22H2 Jan 4, 2022
@zadjii-msft zadjii-msft modified the milestones: 22H2, Terminal v1.14 Feb 9, 2022
@zadjii-msft zadjii-msft changed the title DEC private 47/1047 not supported DEC private 47/1047 not supported (alt buffer) Feb 9, 2022
@zadjii-msft zadjii-msft modified the milestones: Terminal v1.14, Backlog Mar 10, 2022
DHowett pushed a commit that referenced this issue Apr 12, 2022
This PR allows the Terminal to actually use the alt buffer
appropriately. Currently, we just render the alt buffer state into the
main buffer and that is wild. It means things like `vim` will let the
user scroll up to see the previous history (which it shouldn't).

Very first thing this PR does: updates the
`{Trigger|Invalidate}Circling` methods to instead be
`{Trigger|Invalidate}Flush(bool circling)`. We need this so that when an
app requests the alt buffer in conpty, we can immediately flush the
frame before asking the Terminal side to switch to the other buffer. The
`Circling` methods was a great place to do this, but we don't actually
want to set the circled flag in VtRenderer when that happens just for a
flush. 

The Terminal's implementation is a little different than conhost's.
Conhost's implementation grew organically, so I had it straight up
create an entire new screen buffer for the alt buffer. The Terminal
doesn't need all that! All we need to do is have a separate `TextBuffer`
for the alt buffer contents. This makes other parts easier as well - we
don't really need to do anything with the `_mutableViewport` in the alt
buffer, because it's always in the same place. So, we can just leave it
alone and when we come back to the main buffer, there it is. Helper
methods have been updated to account for this. 

* [x] Closes #381
* [x] Closes #3492
* #3686, #3082, #3321, #3493 are all good follow-ups here.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-VT Virtual Terminal sequence support Help Wanted We encourage anyone to jump in on these. Issue-Task It's a feature request, but it doesn't really need a major design. Product-Conhost For issues in the Console codebase
Projects
None yet
Development

No branches or pull requests

4 participants