Add a Compatibility and Terminal page to the Settings UI

[carlos-zamora]

Summary of the Pull Request

Adds a global Compatibility page to the settings UI. This page exposes several existing settings and introduces a few new settings:

• compatibility.allowHeadless
• compatibility.isolatedMode
• compatibility.textMeasurement
• debugFeatures

This also adds a Terminal subpage for profiles in the settings UI. This page includes:

• suppressApplicationTitle
• compatibility.input.forceVT
• compatibility.allowDECRQCRA

Several smaller changes were accomplished as a part of this PR:

• experimental.input.forceVT was renamed to compatibility.input.forceVT
• introduced the compatibility.allowDECRQCRA setting
• updated the schema for these new settings and compatibility.allowHeadless (which was missing)
• add Feature_DebugMode feature flag to control if debug features should be shown in the SUI

A part of #10000
Closes #16672

[lhecker]

I think we should also move the compatibility.textMeasurement setting from the rendering page over to this new one.

[j4james]

Is it too late to change the JSON names used for the VT settings? Because I think it would be nice if they could be consistently named after the control sequences that they apply to, especially since we could be adding quite a few more of these at some point in the future.

One possibility would be to use the control sequence acronyms, i.e. something like this:

• AllowW32IM instead of input.forceVT (although these are the inverse of each other)
• AllowDECRQCRA instead of allowVtChecksumReport
• AllowDECNKM instead of allowKeypadMode

Or if you prefer a more descriptive name, then maybe something that more closely matches the actual control names, like this:

• AllowWin32InputMode
• AllowRequestChecksumRectangularArea
• AllowNumericKeypadMode

That said, have you tested either of the input mode settings? Because I have a strong suspicion that they won't actually work. As far I understand it, the VT input is largely handled in conhost, so a setting that is only applied to Windows terminal isn't necessarily going to work the way you're expecting it to.

Another point I wanted to raise was whether these settings might be better at the profile level? Because I can imagine there will be some VT operations that someone might need for compatibility with a specific application, but which they wouldn't necessarily want enabled in general because of security concerns (DECRQCRA would be one example; the OSC52 clipboard operation is another possibility).

[DHowett]

Are all of the compatibility settings globals? I thought that most of them were actually profile settings. How do we resolve that? How do users set them per-profile?

FWIW about debugFeatures: that one is hidden for a reason. I do not know if we should introduce it to the SUI.

[carlos-zamora]

Is it too late to change the JSON names used for the VT settings? Because I think it would be nice if they could be consistently named after the control sequences that they apply to, especially since we could be adding quite a few more of these at some point in the future.

We're able to change them. We'd just have to still support the old keys, but we do that all the time.

If we're renaming them, I'd prefer to use the control sequence acronyms for the JSON, actually. It's shorter to type out and the schema provides additional details to make it more user friendly. We can provide a more descriptive name and help text in the settings UI.

That said, have you tested either of the input mode settings? Because I have a strong suspicion that they won't actually work. As far I understand it, the VT input is largely handled in conhost, so a setting that is only applied to Windows terminal isn't necessarily going to work the way you're expecting it to.

I haven't, but these settings have been around for a while. I'm just exposing them to the settings UI. You're talking specifically about the compatibility.input.forceVT and compatibility.allowKeypadMode?

[j4james] Another point I wanted to raise was whether these settings might be better at the profile level? Because I can imagine there will be some VT operations that someone might need for compatibility with a specific application, but which they wouldn't necessarily want enabled in general because of security concerns (DECRQCRA would be one example; the OSC52 clipboard operation is another possibility).

[DHowett] Are all of the compatibility settings globals? I thought that most of them were actually profile settings. How do we resolve that? How do users set them per-profile?

Nope. They're all globals. We could convert them to profile settings like this though:

• add keys to be a part of profile settings
• deprecate global settings (aka, basically remove them from the schema, on serialization, use the new keys)
• if a legacy global setting was found, apply it to profile.defaults instead

FWIW about debugFeatures: that one is hidden for a reason. I do not know if we should introduce it to the SUI.

What's the reason? We're already exposing this setting via the JSON. We're just adding another method to toggle it. I think it's important to carry it over so that we treat SUI as a first-class citizen like the JSON.

[j4james]

If we're renaming them, I'd prefer to use the control sequence acronyms for the JSON, actually.

Yeah, me too.

I haven't, but these settings have been around for a while. I'm just exposing them to the settings UI. You're talking specifically about the compatibility.input.forceVT and compatibility.allowKeypadMode?

Well forceVT has only been an experimental setting which I suspect nobody has used (I recently tried it and couldn't get it to work meaningfully). And keypadMode is only a build setting, which works because it applies to both conhost and WT - making that a runtime setting is where it becomes problematic.

And if it was a just a matter of bug fixing, I wouldn't have a problem in leaving that for later. But my concern is that these options may not be feasible at all, in which case it seems premature to be adding them to the UI. Although if someone else has an idea of how they could be made to work, that's fine. I might be missing something obvious.

[carlos-zamora]

Well forceVT has only been an experimental setting which I suspect nobody has used (I recently tried it and couldn't get it to work meaningfully). And keypadMode is only a build setting, which works because it applies to both conhost and WT - making that a runtime setting is where it becomes problematic.

And if it was a just a matter of bug fixing, I wouldn't have a problem in leaving that for later. But my concern is that these options may not be feasible at all, in which case it seems premature to be adding them to the UI. Although if someone else has an idea of how they could be made to work, that's fine. I might be missing something obvious.

Got it. That's fair. I'll take a closer look to see if these work tomorrow then.

[carlos-zamora]

Demo (Updated for bb1521b)

[carlos-zamora]

Well forceVT has only been an experimental setting which I suspect nobody has used (I recently tried it and couldn't get it to work meaningfully). And keypadMode is only a build setting, which works because it applies to both conhost and WT - making that a runtime setting is where it becomes problematic.
And if it was a just a matter of bug fixing, I wouldn't have a problem in leaving that for later. But my concern is that these options may not be feasible at all, in which case it seems premature to be adding them to the UI. Although if someone else has an idea of how they could be made to work, that's fine. I might be missing something obvious.

Got it. That's fair. I'll take a closer look to see if these work tomorrow then.

Alright! Got a chance to take a look at the two input mode settings. Let me preface this by saying that this isn't an area I'm too familiar with, but trying my best here (and any help/feedback would be very appreciated 🙂). Here's my findings.

compatibility.input.forceVT

Ran a simple test by running showkey -a and typing 1234 in an Ubuntu tab with debug tap enabled. The output itself is the same in each test, but the debug tap shows something different. Here's the results:

// regular output (regardless of setting)
1        49 0061 0x31
2        50 0062 0x32
3        51 0063 0x33
4        52 0064 0x34

// compatibility.input.forceVT on
11␣␉␣49␣0061␣0x31␍␊
22␣␉␣50␣0062␣0x32␍␊
33␣␉␣51␣0063␣0x33␍␊
44␣␉␣52␣0064␣0x34␍␊

// compatibility.input.forceVT off (tested in WT Canary and via hot-reload in dev build)
␛[49;2;49;1;0;1_1␣␉␣49␣0061␣0x31␍␊
␛[49;2;49;0;0;1_␛[50;3;50;1;0;1_2␣␉␣50␣0062␣0x32␍␊
␛[50;3;50;0;0;1_␛[51;4;51;1;0;1_3␣␉␣51␣0063␣0x33␍␊
␛[51;4;51;0;0;1_␛[52;5;52;1;0;1_4␣␉␣52␣0064␣0x34␍␊
␛[52;5;52;0;0;1_

Looks like the setting is doing something. Looking through the repo, looks like we generally use this as a quick way to further debug input issues. If there's any other tests I should do or expected behavior, definitely let me know. 🙂

Also! I know we talked about changing the name (considered allowW32IM). I've set it to compatibility.input.forceVT for now. I'd like to avoid inverting the logic on the setting because we want users to enable the setting, rather than disable it. If there's any ideas here to rename it, let me know.

compatibility.allowDECNKM

This one I was able to check that we're enabling/disabling application keypad mode fine. After chatting with Dustin a bit though, I now see the issue with #16672 and ConPTY. Yeah, I'll back this part out. There's plenty of good stuff in here and, yeah, the setting wouldn't work as expected.

[j4james]

Looks like the setting is doing something. Looking through the repo, looks like we generally use this as a quick way to further debug input issues. If there's any other tests I should do or expected behavior, definitely let me know. 🙂

@carlos-zamora I think perhaps it's just me not understanding what it was intended to do. I expected it would disable win32 input mode completely, so it would be impossible for an application to turn that on (which is why I was suggesting the name AllowW32IM). But if I do printf "\e[?9001h" in a WSL shell, that still ends up producing win32 input sequences, leaving the shell in an unusable state.

However, if it's just meant to control the internal conpty encoding, then I think it's fine as it is. And in that case I think the current setting name is also fine as it is.