Close the gap between Terminal and Conhost ITerminalApi

[DHowett]

There's a number of TODOs in Terminal's implementation of ITerminalApi, herein captured:

58-void Terminal::SetAutoWrapMode(const bool /*wrapAtEOL*/)
59-{
60:    // TODO: This will be needed to support DECAWM.

63-void Terminal::SetScrollingRegion(const til::inclusive_rect& /*scrollMargins*/)
64-{
65:    // TODO: This will be needed to fully support DECSTBM.

73-bool Terminal::GetLineFeedMode() const
74-{
75:    // TODO: This will be needed to support LNM.

109-bool Terminal::ResizeWindow(const til::CoordType /*width*/, const til::CoordType /*height*/)
110-{
111:    // TODO: This will be needed to support various resizing sequences. See also GH#1860.

115-void Terminal::SetConsoleOutputCP(const unsigned int /*codepage*/)
116-{
117:    // TODO: This will be needed to support 8-bit charsets and DOCS sequences.

120-unsigned int Terminal::GetConsoleOutputCP() const
121-{
122:    // TODO: See SetConsoleOutputCP above.

These were introduced in the merger of TerminalDispatch and AdaptDispatch, which unified conhost's and Terminal's output state machine engine dispatchers in and around #13024.

@j4james are there any that I've missed? I was pretty hamfisted with it. 😄

Notes from @j4james:

That looks right from the point of view of ITerminalApi. But I think the main things outstanding are that the _WriteBuffer and _AdjustCursorPosition methods in Terminal need to be unified with the WriteCharsLegacy and AdjustCursorPosition functions from conhost, since the Terminal versions are missing a bunch of functionality.

And once those methods are merged into AdaptDispatch, we might also be able to get rid of some ITerminalApi methods like SetScrollingRegion, GetLineFeedMode, and LineFeed.

There are also things that Terminal supports which conhost does not. I am not sure whether it is right to track them here.

I didn't think anyone really cared about the conhost side, but these are the ones I'm aware of:

• EnableXtermBracketedPasteMode
• CopyToClipboard
• SetTaskbarProgress
• SetWorkingDirectory
• AddMark

Although in the case of SetWorkingDirectory, I'm not sure there's anything useful conhost can do with that information. And AddMark should really be moved out of ITerminalApi and implemented directly on the TextBuffer somehow. As currently implemented, it's kind of blocking the AdjustCursorPosition unification.

Also, Build-SupportedSequenceIndex still expects to find terminalDispatch. Right now, we don't have a way to tell it that Terminal doesn't support a particular control sequence

Yeah, I was thinking we should maybe come up with a new way of tracking that info - maybe with a well-defined pattern in the AdaptDispatch doc comments. That way we would have more room to add annotations like "ConHost-only" or "Terminal-only" for operations that aren't yet supported in both.

[DHowett]

There are also things that Terminal supports which conhost does not. I am not sure whether it is right to track them here.

[DHowett]

Also, Build-SupportedSequenceIndex still expects to find terminalDispatch. Right now, we don't have a way to tell it that Terminal doesn't support a particular control sequence (somewhat by design, since the dispatcher is of course identical).

[j4james]

@j4james are there any that I've missed? I was pretty hamfisted with it. 😄

That looks right from the point of view of ITerminalApi. But I think the main things outstanding are that the _WriteBuffer and _AdjustCursorPosition methods in Terminal need to be unified with the WriteCharsLegacy and AdjustCursorPosition functions from conhost, since the Terminal versions are missing a bunch of functionality.

And once those methods are merged into AdaptDispatch, we might also be able to get rid of some ITerminalApi methods like SetScrollingRegion, GetLineFeedMode, and LineFeed.

There are also things that Terminal supports which conhost does not. I am not sure whether it is right to track them here.

I didn't think anyone really cared about the conhost side, but these are the ones I'm aware of:

• EnableXtermBracketedPasteMode
• CopyToClipboard
• SetTaskbarProgress
• SetWorkingDirectory
• AddMark

Although in the case of SetWorkingDirectory, I'm not sure there's anything useful conhost can do with that information. And AddMark should really be moved out of ITerminalApi and implemented directly on the TextBuffer somehow. As currently implemented, it's kind of blocking the AdjustCursorPosition unification.

Also, Build-SupportedSequenceIndex still expects to find terminalDispatch. Right now, we don't have a way to tell it that Terminal doesn't support a particular control sequence

Yeah, I was thinking we should maybe come up with a new way of tracking that info - maybe with a well-defined pattern in the AdaptDispatch doc comments. That way we would have more room to add annotations like "ConHost-only" or "Terminal-only" for operations that aren't yet supported in both.

[j4james]

I've been experimenting with the WriteCharsLegacy/_WriteBuffer part of this, and I think I have a solution that could be turned into a PR if you're interested.

What it amounts to is a loop over the string that is being output, writing a line at a time with TextBuffer::WriteLine, and calling a slightly modified version of ITerminalApi::LineFeed to handle the line wrapping (the modification is because a LineFeed call usually clears the WrapForced flag, while in this case it needs to be set).

This way we don't need to touch WriteCharsLegacy, and AdjustCursorPosition is only used indirectly via the LineFeed API (hopefully that dependency will be removed in a later PR).

But with this functionality merged into AdaptDispatch, I'm assuming there's no longer a need for WriteCharsLegacy2ElectricBoogaloo (#780), which I see is now assigned to @lhecker . So I before I submit anything, I just want to make sure I'm not doing something that's going to mess with your existing plans.

I should also stress out that I haven't tried to address any performance issues, or fix bugs like the cursor droppings (#12739). I'm just trying to get the existing functionality moved into AdaptDispatch so I can progress with VT features that depends on that.