Query SGR 4:X support from Terminal before consuming it in the adapter

[tusharsnx]

Currently, we unconditionally allow SGR 4:X in our adapter. This works for terminals that support SGR 4:X, but others have to face missing underlines if the application thinks the terminal does support extended underline style.

On Windows, an application might send out a DECRQSS query to check for extended style support before enabling it, but this won't work because it is the Conpty that would respond and not the actual terminal. To fix this issue, Conpty will send a DERQSS SGR query to the terminal during the startup. The query first sets the undercurl (4:3) and queries the SGR attributes. When a DECRPSS response is received in the input state machine engine, we'll check for the presence of the undercurl attribute in the response. If undercurl is present, we'll tell the adapter to start consuming SGR 4:X sequences.

Conpty will ignore SGR 4:X sequences if the terminal doesn't support extended styles. This way, if the terminal never responded or the response didn't include the (4:3), and an application sent us (Conpty) a DECRQSS query \x1b[0m;\x1b[4:3m\x1bP$qm to detect support for extended style, our DECRPSS response will also not contain SGR 4:X either.

References and Relevant Issues

neovim/neovim#26744
neovim/neovim#28052

Validation Steps Performed

• TBA

PR Checklist

• Tests added/passed

[DHowett]

So far, we have avoided introducing any additional synchronous VT-based requests from ConPTY to the connected terminal; after all, there is a chance that the request is ignored or lost, and if we don't handle that with an appropriate timeout (which is troublesome for other reasons...) then we'll just hang indefinitely like we do during DSR-CPR. @j4james has been down this road before, and may know more or have some ideas! Temporarily blocking 'til we have a chat :)

[tusharsnx]

synchronous VT-based requests

Maybe I'm missing something, but this PR only sends a query during the startup. It doesn't wait for the response though. If we ever see a DECRPSS reponse coming back in the input state machine engine, that's when we apply the check, and until then adapter will ignore any SGR 4:X sequence.

Let me know if you think this is not the case (because I really never intended it to be synchronous 😅)

[j4james]

I'd love to have a solution for this, because this affects a lot more than just the 4:X support, but I don't think an asynchronous approach is the right answer.

The problem is when starting a terminal app from a shortcut, that app is likely to initiate any query sequences almost immediately. But a that point conhost may still be waiting for a response from the conpty client, so potentially won't report the correct state.

The end result is the app will misdetect the feature, and won't make use of the extended underlines. This seems worse than just claiming support for all conpty terminals, since most of them probably already do support this feature.

So unless you can think of another way to address this, I'd much rather we tried to get a synchronous solution to work. The typical way to deal with terminals that may not support a particular query is by following it up with something that everyone does support (e.g. a DSR-CPR or DSR-OS). If you receive that second query response without the first, you know the first isn't supported, so you don't need to wait for it.

The only reason I gave up on that approach is because it triggered a failure in one of our feature tests, and that gave me the impression that we were expecting conpty to work with connections that don't respond to any queries. I can't imagine there's a usable terminal with that limitation, but I couldn't get anyone to explain why else we had that test, so I just gave up in the end.

One other thing I considered was adding a command line option that the conpty client could use to indicate that it was capable of responding to at least a DSR-OS query. But that's not ideal, and if the only way to make this work is with an additional command line option, you might as well add one that lets the conpty client list all its capabilities, so we wouldn't need to query it at all.

[tusharsnx]

Thanks @j4james for the insights. I did realize (by reading your comment) that the query-based approach won't work here.

I then tried to achieve a similar goal as this PR using DECRQSS SGR passthrough, but that also has its own problems. So, I guess I'll close this PR for now.

For the purpose of documenting, the problem with DECRQSS SGR passthrough is that if two DECRQSS queries involving an SGR query and any other DECRQSS query type are received together, the responses received by the application may be out-of-sync. SGR query is passed through to the terminal, while the other query's response is generated by Conpty and written directly to the input buffer.

I don't know how sensitive applications are to this kind of out-of-sync DECRQSS response, but I'm sure VT is supposed to be synchronous, and any out-of-sync responses are clearly invalid.

[j4james]

I don't know how sensitive applications are to this kind of out-of-sync DECRQSS response,

One of the big problems with out-of-sync responses is that apps tend to send multiple queries at the same time, and expect them back in the same order. This is especially true of queries like DECRQSS that may not be supported on all terminals. As I mentioned above, the standard way to deal with that is by sending the DECRQSS followed by something like DSR-CPR, so if the CPR arrives back first, it'll be assumed that the DECRQSS isn't supported.

That said, some form of query pass-through may be necessary if we want to support things like palette queries (#3718). But if we're going to do that, we would have to block in some way while waiting for the response from the conpty client. One idea I considered was to buffer any conhost query responses while waiting for the conpty response, and then you could eventually replay all the responses back the host in the correct order. I'm not positive that will work though.