Terminal: Implement stream writing mechanism on buffer (a.k.a. WriteCharsLegacy2ElectricBoogaloo)

Created on 14 May 2019  路  5Comments  路  Source: microsoft/terminal

Apologies if this explanation is confusing. I've mentally internalized the problem here, but I'm not sure if I can fully articulate. Here goes...

When receiving characters in a stream fashion (in either conhost or the Terminal), there are a bunch of situations that occur that run off the golden path rails of "just put this into the buffer and move the cursor forward by one".

A few major categories of these are:

  1. C0 control characters (tab, backspace, newline, carriage return, etc.)
  2. Surrogate pairs for UTF-16 (two wchar_ts are required to form one U+ codepoint)
  3. An entire run of characters required to figure out a particular cell display or run of cells.

We need to solve 1 and 2 first. Solving 3 will probably require a bit more interaction with the CustomTextLayout in the DirectWrite renderer to perform some sort of read-ahead or multiple-pass calculation over text as it flows in stream-wise to figure out the columns it should take.

The existing manifests of this sort of work are WriteCharsLegacy in the console host (which is a thousand line or more monstrosity full of multi-pass switch cases, loops, and gotos) and the stream write thing in TerminalControl (I think, might be TerminalConnection) which @zadjii-msft affectionately warned against it becoming "WriteCharsLegacy2ElectricBoogaloo" in one of his comments.

I think what needs to be done here is:

  1. Create a third entry point to the buffer object that accepts text in a stream write fashion, potentially "buffering" before it inserts into the buffer for reals (or has the ability to look-behind at what was just written) taking over the duties of the "WriteCharsLegacy" type methods into the buffer itself so at least 1 and 2 can be handled in a unified-ish manner.
  2. Teach the buffer and/or let the buffer accept some sort of loose pfn that will connect it up to the text layout engine and let it query for column counts or glyph run information as stream characters show up and adjust the behavior of their neighboring glyphs.
Area-Output Issue-Task Priority-1 Product-Terminal v1-Scrubbed

Most helpful comment

Good on the "...2 Electric Boogaloo" reference...we don't want it breakin'...

All 5 comments

@miniksa @DHowett
It looks like that we can have a "Carriage" class that overall manipulates the cursor and backbuffer. It will take WCHARs one by one to decide what to do.

  1. C0 control characters (tab, backspace, newline, carriage return, etc.)

I don't know if this is still your plan, but I don't think this stream writer is the right place to be handling C0 control characters. The StateMachine already has to parse them out, and it already has to handle CAN, SUB and ESC, while passing the rest on individually to OutputStateMachine::ActionExecute. And in my opinion, that seems like the right place to be handling the rest of them.

We're already handling NUL there, and LF, FF and VT are due to be added there in PR #3271. So why don't we handle the rest of them there too? And by that I mean parse them in a switch statement and simply delegate the actual handling to the ITermDispatch implementation. In the case of BS and TAB, I think we already even have the dispatch methods implemented (CursorBackward and ForwardTab).

If that makes sense to you, would you be open to a PR that starts doing something along those lines? In theory that should let us pull a whole lot of VT-specific code out of the WriteCharsLegacy function, and I think there is already one known bug that that could fix (#3971).

  1. C0 control characters (tab, backspace, newline, carriage return, etc.)

I don't know if this is still your plan, but I don't think this stream writer is the right place to be handling C0 control characters. The StateMachine already has to parse them out, and it already has to handle CAN, SUB and ESC, while passing the rest on individually to OutputStateMachine::ActionExecute. And in my opinion, that seems like the right place to be handling the rest of them.

We're already handling NUL there, and LF, FF and VT are due to be added there in PR #3271. So why don't we handle the rest of them there too? And by that I mean parse them in a switch statement and simply delegate the actual handling to the ITermDispatch implementation. In the case of BS and TAB, I think we already even have the dispatch methods implemented (CursorBackward and ForwardTab).

If that makes sense to you, would you be open to a PR that starts doing something along those lines? In theory that should let us pull a whole lot of VT-specific code out of the WriteCharsLegacy function, and I think there is already one known bug that that could fix (#3971).

Yeah, @j4james, as I was reading your other PRs where you started processing the C0s outside... I think that you're right. We should probably handle them all credibly in the VT side.

I would then probably investigate the right way to direct other writers who are currently going straight into the buffer to run through the VT output instead so their C0s could be processed accordingly and also work on a legacy-compatibility adapter to make what goes into WriteCharsLegacy instead adjust itself into the appropriate VT-compatible sequences, run through the VT code, and handle its C0 movement that way. I'd probably lastly have to rework the cooked input line to stop doing all the tricky things that it does with weird provisions on WriteCharsLegacy.

@j4james, please go ahead and open the PR to finish off the C0s as you see fit on the VT stuff and I'll work around the mental assumption that that will arrive and we'll try to have that be the one source of truth on how C0s should be processed across conhost and terminal writes.

Good on the "...2 Electric Boogaloo" reference...we don't want it breakin'...

Was this page helpful?
0 / 5 - 0 ratings

Related issues

dev-logan picture dev-logan  路  3Comments

miniksa picture miniksa  路  3Comments

xmm1989218 picture xmm1989218  路  3Comments

TayYuanGeng picture TayYuanGeng  路  3Comments

zadjii-msft picture zadjii-msft  路  3Comments