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.

Sign up for GitHub

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

Jump to bottom

Support for XTSAVE and XTRESTORE or DECRQM #10153

Open
coezbek opened this issue May 22, 2021 · 5 comments
Open

Support for XTSAVE and XTRESTORE or DECRQM #10153

coezbek opened this issue May 22, 2021 · 5 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 Product-Terminal The new Windows Terminal.
Milestone
Backlog

Comments

@coezbek
Copy link

coezbek commented May 22, 2021

Description of the new feature/enhancement

With the introduction of bracketed paste mode it is now possible to handle pasting when announcing support via DECSET 2004 (see #395), which is great! As a terminal-based user interface application the question now arises how to know which value to restore at the end of the application. This is usually done by using XTSAVE prior to setting bracketed paste mode and then calling XTRESTORE before closing the application or by querying the state of bracketed paste mode via DECRQM (as far as I understand).

Why is this important? Imagine the following:

  • Initially Bracketed Paste Mode is Off
  • One application is started which supports it and enables it
  • At the end of the application the application does not know if bracketed paste was on or off before the application started, so it opts to leave it enabled.
  • If the user next starts an application does not support bracketed paste, then the user will start seeing escape sequences \x1b[200~ etc when pasting.

Please add support for either method, so we can leave bracketed paste mode in the same state as it was before the application was started.

Proposed technical implementation details (optional)

XTSAVE and XTRESTORE need to keep a stack of values for the given CSI and either push the current value on the stack or restore the value from the stack and pop.
@coezbek coezbek added the Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. label May 22, 2021
@ghost ghost added Needs-Tag-Fix Doesn't match tag requirements Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels May 22, 2021
@j4james
Copy link
Collaborator

j4james commented May 22, 2021

I'd prefer we didn't implement XTSAVE and XTRESTORE, but DECRQM is definitely on my todo list. Ideally I would like us to clean up the various mode methods in the ConGetSet interface first (as I mentioned in #3849). We don't currently have a way to query modes, and I'd prefer that didn't require dozens of additional GetModeXXX methods.

@zadjii-msft zadjii-msft added Area-VT Virtual Terminal sequence support Issue-Task It's a feature request, but it doesn't really need a major design. Product-Conhost For issues in the Console codebase Product-Terminal The new Windows Terminal. and removed Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. labels May 24, 2021
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label May 24, 2021
@zadjii-msft zadjii-msft added the Help Wanted We encourage anyone to jump in on these. label May 24, 2021
@zadjii-msft zadjii-msft added this to the Terminal Backlog milestone May 24, 2021
@DHowett
Copy link
Member

DHowett commented May 25, 2021

So, I'm of two minds on this. It's really useful to generally know the state of the terminal, but it seems like the generally-accepted practice is that an application asserts the states that it needs before it needs them. Shells that support bracketed paste re-emit DECSET 2004 before every prompt, and DECRST before every invocation; applications that want DECCKM likewise turn it on/off when they want it.

This "revert to the default state" behavior generally works, apart from when an application is terminated unceremoniously. Is it more worth it to add a back-and-forth exchange (including over links of unknown latency?) between application and terminal emulator, or to simply allow for some minor breakage¹ when an app or shell doesn't behave as it should?

¹ and of course, in the case of OSS projects, request remediation 😄

@DHowett
Copy link
Member

DHowett commented May 25, 2021 •
edited
Loading

This is really similar to the discussions around XT{PUSH,POP}SGR: whether an application should be granted the existence of an SGR stack, or whether it is safe to assume that the terminal is left in a reasonable state even if that means sticking our heads in the sand. \_(shrug)_/

@coezbek
Copy link
Author

coezbek commented May 25, 2021

This "revert to the default state" behavior generally works,

Yes, if this is really the state of things then this is of course fine. I understand that adding state management is a big drag. I was hoping that it wouldn't be too hard to at least to provide the DECRQM for any command that is supported anyway.

I am just dabbling into VT100 and it is a bit confusing to figure out what is implicit and not in the spec or not implemented. I would have thought that querying capabilities, getting state and set/restore would be features implemented in general.

@j4james
Copy link
Collaborator

j4james commented May 25, 2021

This is really similar to the discussions around XT{PUSH,POP}SGR:

Yeah, this is one of the reasons I'm opposed to those sequences. They have all the same flaws. Also the XTRESTORE sequence conflicts with the DECPCTERM sequence, which I would actually like us to support one day.

DECRQM solves the problem of restoring state in a far more sane way in my opinion. And it's also extremely useful for feature detection. For example, if you want to detect if a terminal supports mouse modes, synchronized updates, or even our Win32 input mode, you can query all of those things with DECRQM.

DECRQSS serves a similar purpose. It's a both a better way to restore SGR state than XT{PUSH,POP}SGR, and also provides a way for applications to feature-detect things like cursor styles and all those fancy extended SGR attributes that modern apps want to use.

And with a decent level of support for feature detection, we have a better argument for not reporting version information.

@DHowett DHowett removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label May 27, 2021
@zadjii-msft zadjii-msft modified the milestones: Terminal Backlog, Backlog Jan 4, 2022
@schungx schungx mentioned this issue Jan 30, 2022
Closed
@j4james j4james mentioned this issue Nov 27, 2022
Merged
ghost pushed a commit that referenced this issue Nov 30, 2022
@j4james
Add support for the DECRQM escape sequence (#14444)
4379148 
This PR adds support for the `DECRQM` (Request Mode) escape sequence,
which allows applications to query the state of the various modes
supported by the terminal. It also adds support for the `DECNKM` mode,
which aliases the existing `DECKPAM` and `DECKPNM` operations, so they
can be queried with `DECRQM` too.

This is one solution for #10153 (saving and restoring the state of
bracketed paste mode), and should also help with #1040 (providing a way
for clients to determine the capabilities of the terminal).

Prior to adding `DECRQM`, I also did some refactoring of the mode
handling to get rid of the mode setting methods in the `ITermDispatch`
interface that had no need to be there. Most of them were essentially a
single line of code that could easily be executed directly from the
`_ModeParamsHelper` handler anyway.

As part of this refactoring I combined all the internal `AdaptDispatch`
modes into an `enumset` to allow for easier management, and made sure
all modes were correctly reset in the `HardReset` method (prior to this,
there were a number of modes that we weren't restoring when we should
have been).

And note that there are some differences in behavior between conhost and
Windows Terminal. In conhost, `DECRQM` will report bracketed paste mode
as unsupported, and in Terminal, both `DECCOLM` and `AllowDECCOLM` are
reported as unsupported. And `DECCOLM` is now explicitly ignored in
conpty mode, to avoid the conpty client and conhost getting out of sync.
 
## Validation Steps Performed

I've manually confirmed that all the supported modes are reported in the
`DECRQM` tests in Vttest, and I have my own test scripts which I've used
to confirm that `RIS` is now resetting the modes correctly.

I've also added a unit test in `AdapterTest` that iterates through the
modes, checking the responses from `DECRQM` for both the set and reset
states.

I should also mention that I had to do some refactoring of the existing
tests to compensate for methods that were removed from `ITermDispatch`,
particularly in `OutputEngineTest`. In many cases, though, these tests
weren't doing much more than testing the test framework.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
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 Product-Terminal The new Windows Terminal.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
4 participants
@DHowett @j4james @coezbek @zadjii-msft