Ansi parser

[tznind]

@BDisp I wanted to explore how we could detect the ansi sequences within input stream without having to put breaks on input like in #3768

This is where I have gone so far, what do you think?

The purpose is to stream text from console driver as read and 'transparently' pluck out the expected responses live.

Explanation of Ansi Parser

AnsiParser processes events from an input stream. Each time it will take 1 char and either add it to Held or Release it. Critically it sometimes releases more than 1 event.

Each event is a char which can have optional metadata attached (See T header below).

Consider the following use case:

User presses Esc key then types "Hi" then hits Esc again. While the user is typing a DAR response comes in <esc>[0c

Here is a breakdown of how AnsiParser would deal with this:

Detailed breakdown of state at each step

Stage 1 - Consume first Esc

The first call to ProcessInput is with the Esc char. This causes the parser to shift into expecting an escape code (i.e. a bracket). Because we are assuming the Esc is a response we hold it and return empty.

Stage 2 - Consume H

The next call to ProcessInput is with H. Because H is a _knownTerminators we realize that we have either come to the end of an escape sequence or the user was just typing away normally.

First we look to see if we are indeed waiting for a response to come in with terminator H by looking at expectedResponses. We do not find one so instead release both the Esc and the H for routine processing.

Stage 3 - Consume next escape sequence

This process repeats, this time for the actual response we are expecting (i.e. ending with c)

When we reach the first character that is a _knownTerminators (i.e. c) or matches one of our expectedResponses terminators we will leave the InResponse state.

If the response built up in Held matches an expectedResponses we swallow it (Release None) and raise event If the response built up does not match (instead it matches _knownTerminators) we release the whole thing back to be processed downstream.

Stage 4 - Consume last Esc

Finally we consume the last Esc which leaves us in state Expecting Bracket. Now this may be the start of a new escape sequence or it could be that the user has just pressed Esc - we will not know which till later.

BUT we don't want to sit around waiting for the rest of the escape sequence forever. Pressing Esc could be the last thing the user does and there could be no events while user sits around watiting for app to respond.

For this reason we put timestamp on state changes StateChangedAt - this lets the caller (e.g. driver main loop)

BUT we don't want to sit around waiting for the rest of the escape sequence forever. Pressing Esc could be the last thing the user does and there could be no events while user sits around watiting for app to respond.

For this reason we put timestamp on state changes StateChangedAt - this lets the caller (e.g. driver main loop) force the Parser to release the Esc after a short period of time if no new input is comming:

if (Parser.State == ParserState.ExpectingBracket &&
    DateTime.Now - Parser.StateChangedAt > _escTimeout)
{
    return Parser.Release ().Select (o => o.Item2);
}

Why <T>?

I realized working exclusively in char and string made it difficult for WindowsDriver to integrate - so I have changed to AnsiParser<T> . The class now deals with sequences of char each of which has any metadata you want (type T). For WindowsDriver this means AnsiResponseParser<WindowsConsole.InputRecord>.

The parser can pull things out of the stream and later return them and we don't loose the metadata.

I will look at the other drivers, if they are dealing just in char with no other metadata I can see about putting a non generic one too that just wraps the generic like TreeView vs TreeView<T>

Outstanding Issues

• [ ] Key down and key up means double key presses
• [x] Esc on its own as isolated keypress needs to have a release timeout (i.e. if we dont see '[' in 50ms then we need to release that back to input stream as nothing else is coming).
• [ ] deal/prevent parallel outstanding requests executing at once
• [x] Hitting Esc key twice in quick succession or users Esc+start escape sequence

Fixes

• Fixes #_____

Proposed Changes/Todos

• [ ] Todo 1

Pull Request checklist:

• [ ] I've named my PR in the form of "Fixes #issue. Terse description."
• [ ] My code follows the style guidelines of Terminal.Gui - if you use Visual Studio, hit CTRL-K-D to automatically reformat your files before committing.
• [ ] My code follows the Terminal.Gui library design guidelines
• [ ] I ran dotnet test before commit
• [ ] I have made corresponding changes to the API documentation (using /// style comments)
• [ ] My changes generate no new warnings
• [ ] I have checked my code and corrected any poor grammar or misspellings
• [ ] I conducted basic QA to assure all features are working

[tznind]

Key down and Key up events are a pain. Here is what response looks like when sending a Device Attributes Request on startup in WindowsDriver.

We get keydown and then keyup for each letter. esc[?1;0c

So we would need another 'pre step' to the parser that paired up down/up events and only passed them around in pairs (i.e. swallow both or release both). Currently I am using T as InputRecord but now maybe it needs to be a Pair (down and up).

Any idea if this is universal behaviour for WindowsDriver? or are there cases where it only sends key downs for example?

[BDisp]

Any idea if this is universal behaviour for WindowsDriver? or are there cases where it only sends key downs for example?

I think that is a behavior for Wind32 API but would be interesting to know how that work with the others drivers, because you are sending any keystroke with the prefix 0x1b, right? If the other drivers respond with the same output, then there is a possibility that all drivers can deal with key-down and keu-up.

[tznind]

Ok I have added WindowsDriverKeyPairer to match up input key down/up pairs - good news is no changes to the parser we just make the 'metadata' for each char the pair and return that to the main loop.

d642fb6

[tznind]

Key down/up is very unreliable. Just by typing fast you get random order of down/ups (this is on v2_develop):

[tznind]

Maybe we can ditch key up event for v2? it seems almost everything goes of key down anyway? And 2 of the 3 drivers are just making up down/up as pairs anyway - for example NetDriver:

                KeyCode map = MapKey (consoleKeyInfo);

                if (map == KeyCode.Null)
                {
                    break;
                }

                OnKeyDown (new Key (map));
                OnKeyUp (new Key (map));

                break;

In other news though, here is the current WIP, we can send DAR request on demand. It has issues though - crashes a lot. Issues are:

• key down/up pair processing
• multi threading/instance (enumeration modified...)
• not implemented for curses/netdriver/fakedriver

[tznind]

Now working with WindowsDriver and CursesDriver 🎉

Looks like there is already multiple bespoke systems for handling these escape sequences (each driver has its own implementation) so later on it might be worth expanding the parser to do that logic too (is mouse etc).

Update: Now mostly working for NetDriver but it has a delay because of where I have added it.

State of Drivers in v2

The current drivers use a method with this beefy signature! Combined with each driver having its own custom logic for finding the sequence of chars e.g. ProcessInputQueue in NetDriver is busy trying to read till input blocks and figure out what is a complete escape sequence.

public static void DecodeEscSeq (
    EscSeqRequests escSeqRequests,
    ref ConsoleKeyInfo newConsoleKeyInfo,
    ref ConsoleKey key,
    ConsoleKeyInfo [] cki,
    ref ConsoleModifiers mod,
    out string c1Control,
    out string code,
    out string [] values,
    out string terminator,
    out bool isMouse,
    out List<MouseFlags> buttonState,
    out Point pos,
    out bool isResponse,
    Action<MouseFlags, Point> continuousButtonPressedHandler
)
{

Any issue with me trying to refactor this stuff into the standalone parser in this PR? I think we could do away with a lot of code duplication between drivers with this new abstraction. Additionally we will gain more testability and readability/maintainability.

[tznind]

@BDisp I have built stress testing Scenario into this PR

You can type away while the system is sending ANSI requests. Currently the WindowsDriver will crash under following circumstances.

Currently I see errors on my computer around 16 requests per second.

Failure state for this test is:

• U becomes anything other than 1 (this means the DAR is getting 'responded' to with other strings). Likely a symptomn of failure
• When the app crashes - I have seen this with the 'Invalid KeyCode: Space, C is invalid. thing we saw in your PR too.

Scenario shows 4 unique responses among 415 Device Attribute Requests then crashes at 16 requests per second.

I will look to add a Scheduler component to this PR as really we should prevent 2+ requests being sent before response for first as come in (When terminator is same). With some timeouts incase a request is ignored by term or user specifies wrong terminator etc.

[BDisp]

@BDisp I have built stress testing Scenario into this PR

You can type away while the system is sending ANSI requests. Currently the WindowsDriver will crash under following circumstances.

Currently I see errors on my computer around 16 requests per second.

Failure state for this test is:

• U becomes anything other than 1 (this means the DAR is getting 'responded' to with other strings). Likely a symptomn of failure
• When the app crashes - I have seen this with the 'Invalid KeyCode: Space, C is invalid. thing we saw in your PR too.

I have fixed some of that failures in my PR. If I remind the 'Invalid KeyCode: Space, C is invalid. was occurring because I wasn't breaking the response with the terminator request, reading while key is available. Other issue was having an extra char after the terminator on the NetDriver, which I fixed in the NetEvents.ProcessInputQueue. Other issue was having an extra char before the esc key, which I fixed in the ConsoleDriver.ReadAnsiResponseDefault method.

I will look to add a Scheduler component to this PR as really we should prevent 2+ requests being sent before response for first as come in (When terminator is same). With some timeouts incase a request is ignored by term or user specifies wrong terminator etc.

In my PR I removed the control about when terminator is same. That was causing malfunctions. See how the EscSeqRequests class is working now.

[tznind]

I have fixed some of that failures in my PR. If I remind the 'Invalid KeyCode: Space, C is invalid. was occurring because I wasn't breaking the response with the terminator request,

Ok, but your code and mine are different so it may not be the same thing exactly.

I have captured everything that came in over std in and here is the last records up to the crash (see below)

I have color coded the responses. Each response should be <esc>[?1;0c however when it gets too many requests too fast we see that one response starts coming in before the last has been completely written.

Colored in blue and red we see isolated 'bits' of a response that are interrupted by new events.

The bit that causes error is the final c terminator which seems to come in as Space | C but that is really just a symptom. Problem is the terminal giving us responses in the wrong order.

I will update the parser internals to throttle requests based on how fast the responses are coming in (i.e. 1 outstanding request at once) - that should fix this I think. Although the windows terminal is really the thing at fault here IMO - it should not be doing this.

When sending ~16 Device Attribute Requests a second the responses become intermixed leading to system failure

[BDisp]

Yes I see. Maybe it isn't thread safe. When you get the response in the Action method it may have mixed keys, which make it unreadable.

[tznind]

Ok I have added a scheduler, its still a WIP - I think it makes most sense to move it to ConsoleDriver rather than instantiating in scenario. But it has 2 throttling mechanisms:

• Do not allow a send request when there is an outstanding response being waited for (for given terminator)
• Do not send more than x requests per second (e.g. 10) for any given terminator

In both cases the request is put in holding and sent when chance next becomes available (and throttle/wait conditions are met).

Responses are in red, queued messages are in green. After 10 reqeusts/second throttle kicks in.

[BDisp]

Do you also prevented if no response get back at all, e.g. in the cases where the request isn't recognized and no event is send by the mainloop, waiting indefinitely?

[tznind]

Do you also prevented if no response get back at all, e.g. in the cases where the request isn't recognized and no event is send by the mainloop, waiting indefinitely?

Well there is no waiting going on. But yes unmet requests will be forever outstanding which will also block new requests for same terminator.

I will look at adding timeout to scheduler. It shouldn't be too hard.

[BDisp]

Well there is no waiting going on. But yes unmet requests will be forever outstanding which will also block new requests for same terminator.

I think you shouldn't restrict requests for the same terminator because there are different requests that use the same terminator like the size request 't'. That why I removed it from my PR. Requests must be read sequentially, no mater having the same terminator.

I will look at adding timeout to scheduler. It shouldn't be too hard.

That's the disadvantage of non-blocking case because returning from the timeout doesn't mean there is no response at all later, but I really think there is no other way. I also had the same issue and I was looking from null input representing any response but there is no such of signal from all drivers. In my case was more easy because if there is no key available then the response will be empty. With the NetDriver if the EscSeqRequests.Statuses isn't empty and no response was returned, then I dequeue it.

[tznind]

I think you shouldn't restrict requests for the same terminator 

I am not restricting. Just ensuring that there are not 2 running at once. If 2 come along at same time the second Just gets queued till first gets its response answered e.g. in 20ms when the console finishes writing its reply to us.

returning from the timeout doesn't mean there is no response at all later,

Thinking about it, for the timeout I actually only need to evict when a new request comes in for same terminator (and old requestis past timeout). Otherwise having outstanding request does no harm.

Because my code is attached to the main loop read pump there is no suspend or looping that would stop UI etc so timeout could even be 2-3 seconds or something if desired.

Another use case for timeout is the schedule queue. If user is pushing faster than requests can be dispatched by a lot and for a long time we don't just want an ever growing list outstanding. They need to be abandoned or throw an Exception.

[BDisp]

Another issue I had with my PR was in WindowsDriver and CursesDriver when the mainloop is waiting for input. Even suspended them from reading the input to only using the Console.ReadKey, theses drivers held the first character, which is the Esc if we done a request, corrupting the response with Invalid KeyCode: Space, C is invalid. Thus in my PR I now check if there is available input before read and now that doesn't happen. But since you you're using non-blocking mode this shouldn't been happening, curious.

[tznind]

Ok I think this is more or less feature complete.

Next I will add:

• [ ] Scheduler tests
• [ ] More parser tests
• [ ] Scenario support for more diverse requests (like yours @BDisp)
• [ ] Debug NetDriver and CursesDriver more

Here is running on Windows Terminal. Red indicates the completed requests, Green is the requests sent to scheduler. The console remains sluggish but working when increasing beyond a given threshold (and typing rapidly). I think this is the refresh bug is contributing significantly to this and code is fighting to move console and spray input while simultaneously handling all the DAR responses.

Still seems quite low rate.

Stress test of ansi requests for Device Attributes Request - red is answers, green is requests scheduled

[BDisp]

Do you are having unfinished requests returning by the timeout schedule? If affirmative would also be great to include in the stress tests with the yellow color.

[tznind]

NetDriver is really difficult to integrate with AnsiParser. The main loop ProcessInputQueue is choc full of break and continue.

Ideally I wanted to start by just having AnsiParser pre filter the input then pass on the rest to the main loop like I did with WindowsDriver but its difficult with calls like Console.KeyAvailable in the middle of the handling loop code (i.e. ProcessInputQueue).

A lot of what it is doing is detecting ansi codes (mouse, key, terminal resize etc).

I will look into whether the existing code can be replaced wholesale. It should be possible just to have an outstanding expectation in parser for the mouse and key combinations and just have those come out as events like the rest.

I will use a seperate branch incase things dont work out ( ansi-parser-net-driver).

[tznind]

Ok I have added the idea of persistent expectations. Can think about refactoring as now have 3 collections (expected, late and persistent). I think this will let the existing mouse/screen parsing events parsing logic in NetEvents get nuked. Will look at doing that in ansi-parser-net-driver

[tznind]

Ok this now includes the NetDriver changes to have AnsiParser take over responsibility for response parsing instead of the NetEvents class

Only issues I now have revolve around some terminals sending interleaved responses

As described here: https://github.com/gui-cs/Terminal.Gui/pull/3791#issuecomment-2425135732

We no longer have the DAR responses tripping each other because we schedule those to have 1 in 1 out.

But sometimes we see mouse getting interrupted by DAR response (in some terminals).

Easiest way to get this to trigger - if you have a console where it is an issue. Is to run the scenario and set to 20 DAR per second then just wave mouse around frantically.

🎥 Video Reproducing Issue

It manifest in one of two ways

• Scenario closes because 2 Esc get seen in a row (DAR interleaved with Mouse both outputting Esc then both outputting [ (or some variation thereof)
• If in a text field - part of the mouse response code or DAR appearing written into text view as text

It may be possible to detect this situation in some cases although I need a large sample of cases - so a way of capturing the occurrances.

Here is what it looks like for mouse/DAR interruption:

DAR response (green) comes in mid-stream before mouse event (pink) is complete

[tznind]

@j4james thanks for the help on the Sixel stuff - maybe I can pick your brains on this also?

Have you ever seen this kind of behaviour from terminals? I am pretty sure this is coming in raw like this and not a race condition in the Terminal.Gui code.

Any advice would be appreciated.

[j4james]

@tznind I've not experienced that issue myself, but I could easily believe that was a bug in older versions of the conhost/conpty framework. And if you're getting \e[1;0c in the DA1 report, that suggest you're testing an old version of Windows Terminal, or an old version of conhost, or a third-party terminal which is relying on an old version of conhost.

Something similar was reported quite recently as a bug in Windows Terminal (https://github.com/microsoft/terminal/issues/17775), but that has since been fixed, and was identified as a regression, so I think that would only have been evident in versions 1.19 to 1.21 of Windows Terminal (which would not have returned a \e[1;0c report). However, that does suggest that a similar bug might have been present in older versions as well.

[tznind]

Interesting, thanks for linking - that does sound similar, and indeed, I have a very old version running as default terminal for Visual Studio.

The test here is pretty extreme (spamming DAR is a bit of an artificial use case) and it doesn't reproduce on more recent versions - maybe we can live with this.

I will look into it further and try with some of the Linux consoles like xterm and mlterm I have on laptop later this week.

[tznind]

@BDisp this is also ready - which do you want to merge first if we are going to have both these systems (blocking/realtime) as options?

[BDisp]

@BDisp this is also ready - which do you want to merge first if we are going to have both these systems (blocking/realtime) as options?

I prefer mine is merge first because it has all driver as non-blocking input and very raw changes. I think it would more easy to merge yours later, if you don't mind.

[tznind]

ok, I also have changes to driver but I think they are more limitted

[tznind]

@BDisp I have also added your combo box and controls to this PR so we can test other commands.

I also added the StoppedExpecting event so we can see when the parser gives up on getting a response.

[BDisp]

On the request text field put a invalid request that wont response at all and see what happen. Also with a valid request maintain the enter key pressed during a long time and see if any error come up.

[tznind]

The current state of this branch is functional and ready for merging. @BDisp and I are exploring a combined approach—see https://github.com/BDisp/Terminal.Gui/pull/207 for more details.

That branch is currently encountering significant issues with deadlocks, so we're treating it as an experimental effort for the time being.