fix: WpfTerminalControl allow Connection set to null

[mitchcapper]

Hides the cursor when null, shows it when not.
Clear the screen any time the connection is changed.

Summary of the Pull Request

Prevents the WPF Control from crashing when set back to null, clears the console and hides the mouse as well.

References and Relevant Issues

#15061

Detailed Description of the Pull Request / Additional comments

It say 3 ansi commands as well now:

1. When the Connection is set to null the cursor is hidden (reflects what the default state is)
2. When the Connection is set to a value and it was null before we show the cursor (not a breaking change as requires it to have been null which previously would cause a crash, except for for set)
3. When the Connection is changed the screen is cleared. A breaking change officially although not sure if there are use cases where this behavior is not desired. For added safety we could make sure we are not being set to the same value we currently are.

None of the ansi commands are needed, users could do it all themselves as well, the behavior largely seemed natural though. I didn't see any ansi constants anywhere so they are just hard coded with comments, but not sure if there is an established better practice.

Closes #15061

[mitchcapper]

@microsoft-github-policy-service agree
I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.

[zadjii-msft]

Sure

[DHowett]

Alright! I like the thrust behind this, but I'd like to put you on brief pause while we figure out what the public API for the Terminal Control is.

In general, I feel weird about using strings to have one part of an application tell another part what to do; this includes VT (the control telling itself!)

It strikes me as something that should be intrinsic to the internal native API bridging PublicTerminalCore <-> WpfTerminalControl.

In addition, we may want to surface the hard reset operation rather than a simple clear screen one. A connection terminating can leave the terminal in a weird state, where it's sending encoded mouse events or scroll events or is using DECCKM for arrow keys or somesuch. Hard reset will bring it completely back into sanity. :)

[mitchcapper]

In general, I feel weird about using strings to have one part of an application tell another part what to do; this includes VT (the control telling itself!)

Yeah certainly a bit odd but such is much of the ANSI api.

If exposing the hard reset api is something to occur sooner rather than later happy to wait and update once it is. If it might be a good bit further down the line, I can revise this to just allow setting it back to null for a temporary bandaid (if preferable). There is nothing currently that stops you from assigning another it is just setting to null that breaks things (so the bugs mentioned do occur there).

[zadjii-msft]

Discussion: maybe it should be the tput reset string

[lhecker]

C# supports \x1b for strings btw, so there's no need for the rather verbose \u001b.

[lhecker]

I personally believe that the ANSI escape sequences are fine. Using an approach like tput reset is probably a good idea, because it's more robust at resetting the state. The output of tput reset for xterm-256color is:

"\x1bc\x1b]104\a\x1b[!p\x1b[?3;4l\x1b[4l\x1b>\x1b[?69l"

Translated this means:

• \x1bc: RIS - Reset to initial state
• \x1b]104\a: OSC 104 - color palette reset (not yet implemented though Support resetting the colors via OSC 104, 110, 111... #3719)
• \x1b[!p: DECSTR - soft reset
• \x1b[?3;4l: DECRST - 80 columns (DECCOLM), fast scroll (DECSCLM)
• \x1b[4l: RM - reset mode to replace mode (IRM)
• \x1b>: DECKPNM - normal keypad
• \x1b[?69l: DECRST - disable left and right margins (DECLRMM)

Some of these don't seem to be useful for us, but I think that this string is generally fine. It's definitely more thorough than the current approach.

Chat

[DHowett]

Alright @mitchcapper, my team convinced me to see the light here. I'm not gonna let perfect be the enemy of good enough, and I think that using VT is good enough. We don't have immediate plans to give you a better API, anyway.

I was pretty curious about Leonard's recommended reset sequence, though.

"\x1bc\x1b]104\a\x1b[!p\x1b[?3;4l\x1b[4l\x1b>\x1b[?69l"

• \x1bc: RIS - Reset to initial state
• \x1b]104\a: OSC 104 - color palette reset (not yet implemented though Support resetting the colors via OSC 104, 110, 111... #3719)
• \x1b[!p: DECSTR - soft reset
• \x1b[?3;4l: DECRST - 80 columns (DECCOLM), fast scroll (DECSCLM)
• \x1b[4l: RM - reset mode to replace mode (IRM)
• \x1b>: DECKPNM - normal keypad
• \x1b[?69l: DECRST - disable left and right margins (DECLRMM)

I don't know if we need to go full overboard with this, and I'm actually quite confused as to why tput reset emits all of these. Maybe @j4james has additional info there.

My understanding is that \x1bc RIS should reset the terminal to its startup state and blow away DECCOLM DECSCLM IRM DECKPAM and DECLRMM through standard operation. Looking at our own implementation . . .

terminal/src/terminal/adapter/adaptDispatch.cpp

Lines 2594 to 2610 in f671f06

	// Full Reset - Perform a hard reset of the terminal. http://vt100.net/docs/vt220-rm/chapter4.html
	// RIS performs the following actions: (Items with sub-bullets are supported)
	// - Switches to the main screen buffer if in the alt buffer.
	// * This matches the XTerm behaviour, which is the de facto standard for the alt buffer.
	// - Performs a communications line disconnect.
	// - Clears UDKs.
	// - Clears a down-line-loaded character set.
	// * The soft font is reset in the renderer and the font buffer is deleted.
	// - Clears the screen.
	// * This is like Erase in Display (3), also clearing scrollback, as well as ED(2)
	// - Returns the cursor to the upper-left corner of the screen.
	// * CUP(1;1)
	// - Sets the SGR state to normal.
	// * SGR(Off)
	// - Sets the selective erase attribute write state to "not erasable".
	// - Sets all character sets to the default.
	// * G0(USASCII)

terminal/src/terminal/adapter/adaptDispatch.cpp

Lines 2535 to 2556 in f671f06

	// X Text cursor enable DECTCEM Cursor enabled.
	// X Insert/replace IRM Replace mode.
	// X Origin DECOM Absolute (cursor origin at upper-left of screen.)
	// X Autowrap DECAWM Autowrap enabled (matches XTerm behavior).
	// National replacement DECNRCM Multinational set.
	// character set
	// Keyboard action KAM Unlocked.
	// X Numeric keypad DECNKM Numeric characters.
	// X Cursor keys DECCKM Normal (arrow keys).
	// X Set top and bottom margins DECSTBM Top margin = 1; bottom margin = page length.
	// X All character sets G0, G1, G2, Default settings.
	// G3, GL, GR
	// X Select graphic rendition SGR Normal rendition.
	// X Select character attribute DECSCA Normal (erasable by DECSEL and DECSED).
	// X Save cursor state DECSC Home position.
	// Assign user preference DECAUPSS Set selected in Set-Up.
	// supplemental set
	// Select active DECSASD Main display.
	// status display
	// Keyboard position mode DECKPM Character codes.
	// Cursor direction DECRLM Reset (Left-to-right), regardless of NVR setting.
	// PC Term mode DECPCTERM Always reset.

. . . I believe that all we need is \x1bc\x1b]104\x1b\\ because everything else is redundant or unsupported.

[j4james]

My understanding is that \x1bc RIS should reset the terminal to its startup state and blow away DECCOLM DECSCLM IRM DECKPAM and DECLRMM through standard operation.

The issue here is that terminfo has a bunch of different reset strings (rs1, rs2, etc.). I don't know if their meaning is officially defined anywhere, but on XTerm, rs1 is a hard reset, and rs2 is a soft reset with a bunch of other things being manually reset.

I suspect one of the reasons for the split might be because some of the original hardware terminals would trigger a disconnect on a hard reset, so it would typically be preferable to use a soft reset if that option was available.

When you do a tput reset, though, it's just outputting all of the reset strings combined together, which kind of defeats the purpose. But maybe there's a good reason for that - I know very little about terminfo.

But the bottom line is, if you're going to send RIS, you really shouldn't need to send anything else. I believe that should even reset the palette, although I'll have to check how well that's supported. I know we don't do that, but it's on my TODO list.

[mitchcapper]

OK so I have updated the PR to match described. @lhecker I mostly was using the intercept sequences between ConPty and Terminal. The \x1b does save you two characters but when doing tests with VT sequences more prone to error esp in chains (at least for me:)). 0x1bG completely fine but follow it with any of the valid hex chars, say \x1bc, and oops you did a "Ƽ".

[DHowett]

Thanks James for the write-up :) and thanks Mitch for implementing