Prevent flickering in nushell due to FTCS marks

[zadjii-msft]

Tl;dr: Conpty would flush a frame whenever it encountered a FTCS mark. Combine that with the whole-line redrawing that nushell does, and the Terminal would get the prompt in two frames instead of one, causing a slight flickering. This fixes that by rendering the frame, but not flushing to the pipe when we encounter one of these sequences.

Closes #13710

A complication here: there are some sequences that we passthrough immediately when we encounter them. For example, \x1b[ 2q. we need to also not flush when we encounter one of these sequences. nushell emits one of these as a part of the prompt, and that would force the buffered frame to get written anyways, before writing that to the pipe.

[DHowett]

I might need you to walk me through how the "flush to terminal" callback becoming "paint and then also flush" actually prevents excess flushes 😄

[zadjii-msft]

@DHowett Okay I'm so sorry my handwriting is atrocious, but this is faster than trying to mermaid it

(imagine an "after" beneath the dotted line)

[zadjii-msft]

For real though:

Before

sequenceDiagram
    participant API Thread
    participant Render Thread
    participant Terminal
    Note left of API Thread: {the start of the prompt}
    Note left of API Thread: ^[[2q
    activate API Thread
    opt Parser
    Note right of API Thread: StateMachine::ProcessString
    Note right of API Thread: AdaptDispatch -> failed
    Note right of API Thread: StateMachine::FlushToTerminal
    opt XtermEngine
        Note right of API Thread: WriteTerminalW
        Note right of API Thread: write "^[[2q"to _buffer
        Note right of API Thread: _Flush
        API Thread ->> Terminal: _buffer goes out the pipe
    end
    Note left of API Thread: {the rest of the prompt}
        Note right of API Thread: InvalidateFlush
        Note right of API Thread: [paint the frame]
        Note right of API Thread: _Flush
        API Thread ->> Terminal: _buffer goes out the pipe
    end
    Note left of API Thread: Unlock console
    API Thread --> -Render Thread: 
    activate Render Thread
        Note right of Render Thread: PaintFrame
        opt XtermEngine
            Note right of Render Thread: [paint the frame]
            Note right of Render Thread: _Flush
        end
        Render Thread ->> Terminal: _buffer goes out the pipe
    deactivate Render Thread

Loading

After this PR

sequenceDiagram
    participant API Thread
    participant Render Thread
    participant Terminal
    Note left of API Thread: {the start of the prompt}
    Note left of API Thread: ^[[2q
    activate API Thread
        opt Parser
            Note right of API Thread: StateMachine::ProcessString
            Note right of API Thread: AdaptDispatch -> failed
            API Thread -->> Render Thread: NotifyPaintFrame
            Note right of API Thread: StateMachine::FlushToTerminal
        opt XtermEngine
            Note right of API Thread: WriteTerminalW
            Note right of API Thread: write "^[[2q"to _buffer
        end
        end
    Note left of API Thread: {the rest of the prompt}
        opt XtermEngine
            Note right of API Thread: InvalidateFlush
            Note right of API Thread: [paint the frame]
        end
    Note left of API Thread: Unlock console
    API Thread --> -Render Thread: 
    activate Render Thread
        Note right of Render Thread: PaintFrame
        opt XtermEngine
            Note right of Render Thread: [paint the frame]
            Note right of Render Thread: _Flush
        end
        Render Thread ->> Terminal: _buffer goes out the pipe
    deactivate Render Thread

Loading

[lhecker]

The code does look good to me and I'd approve it instantly, but I'm not sure whether this is the best approach. From what I can tell, flushing during circling isn't necessarily at all and I believe it'd be better in general to instead only flush when the console API call finishes (i.e. when the entire user string has been processed), because this will avoid calling WriteFile for every line that's written. It should also solve the issue, while also improving ConPTY performance drastically.

[zadjii-msft]

From what I can tell, flushing during circling isn't necessarily at all

That's kinda my thesis too 😉 Though, I was moreso tracking that in #12336. I figured best to separate the work into

• fix nushell which we horribly broke
• make conpty go brrrrrrrrrrr

[lhecker]

FYI I don't think the [[unlikely]] is worth it here. Its implementation is super dumb in MSVC and causes any and all affected unlikely code to be moved to the end of the function, even if it's literally just a single mov [rcx+1234], 0 instruction like here. I think it's still useful, but it seems like its usage is mostly restricted to a rather "nuanced" application, especially after observing the disassembly output.