Fixes #3516. Changes default QuitKey to Esc

[tig]

Fixes

• Fixes Change default Appliation.QuitKey to Esc from Ctrl-Q #3516

Proposed Changes/Todos

• Change to Esc
• Fix tests
• Fix scenarios (if any)
• Update docs

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

[BDisp]

There is something strange when clicking a button in a Dialog. Try opening the InteractiveTree scenario and clicking Add Root in the status bar. The Dialog will open and after typing the node name click on the Ok button. The Dialog closes and opens again despite Application.RequestStop having been called. Therefore, the Dialog should not open and the same happens with the Cancel button.
It is a good idea to create a unit test to prevent this bad behavior from happening.

[tig]

Do you think this has something to do with this PR?

Does this reproduce on v2_develop. I'm not a computer to check.

[BDisp]

Do you think this has something to do with this PR?

Does this reproduce on v2_develop. I'm not a computer to check.

Yes it reproduce on v2_develop branch and so has nothing with this PR.

[dodexahedron]

I have a question.

What happens when an escape literal is received on stdin? Quit, by default?

Because I ran into stuff today that has drastically altered my stance, if that's the proposed change to default behavior, which I'm glad to share if that is, in fact, what it is.

[BDisp]

I have a question.

What happens when an escape literal is received on stdin? Quit, by default?

Because I ran into stuff today that has drastically altered my stance, if that's the proposed change to default behavior, which I'm glad to share if that is, in fact, what it is.

If only an escape is received it will Quit, otherwise the ANSI escape sequence will be processed if the terminal supports it, otherwise it will be echoed as text.

[dodexahedron]

OK. That I think mostly jives with expectations.

I think it would be good to include some tests for it that do some things such as redirecting stdin and writing esc to it, to test/prove at least that particular case.

Most other ways of testing scenarios and environments that I thought up aren't really things that can be done in that unit test project, but that one is simple and good enough for a basic case that should cover at least some of them, I think.

Here were scenarios I thought up last night (food for thought, maybe, but not hard suggestions really):

• Asymmetric input if multiple processes are watching stdin (loggers, input filters, password managers, certain keyboard drivers and macros, and a whole range of other things)
• What if they're kibitzing with another user (That's almost the same as the previous point, but now multiple humans are directly involved)
• What if they feed it from a pipe or other stream for automation?
• What if there's an automated process involved? That includes people's UI testing.
• What if they redirect stdin? Maybe that input comes from a formatted file or even a remote session?
• What if their mouse input is dependent on it?
  • This isn't uncommon, and is part of why sometimes terminals get screwed up and mouse input causes garbage at the prompt.
• What if they want to actually format text they're storing in a text field?
• What if they're using UI theming software (such as Oh My Posh, even), which might want to use escape sequences to change cursor shapes and stuff like that, depending on themes and preferences of course?

And if you've already got that covered by your planned implementation, awesome! 🥳

[dodexahedron]

Just a quick scan taken over the changes to the Terminal.Gui code.

Didn't do in-depth analysis yet and didn't bother with the tests or samples either.

Nothing critical popped out at me anyway on scan.

Only things that prompted any questions were a nullable bool return and a hidden method. But both are pretty minor.

Oh, and some empty XmlDoc elements.

[dodexahedron]

Is the nullable necessary here?

[tig]

I think so.

Part of the design of Command/KeyBinding is described here. For this, I believe we need to support propogating "don't know/yes/no" up and down the stack. OnAccept is right in the middle of all this.

Terminal.Gui/Terminal.Gui/View/ViewKeyboard.cs

Lines 653 to 658 in fd683d8

	// * If no key binding was found, `InvokeKeyBindings` returns `null`.
	// Continue passing the event (return `false` from `OnInvokeKeyBindings`).
	// * If key bindings were found, but none handled the key (all `Command`s returned `false`),
	// `InvokeKeyBindings` returns `false`. Continue passing the event (return `false` from `OnInvokeKeyBindings`)..
	// * If key bindings were found, and any handled the key (at least one `Command` returned `true`),
	// `InvokeKeyBindings` returns `true`. Continue passing the event (return `false` from `OnInvokeKeyBindings`).

[dodexahedron]

Works for me.

An opportunity for a small future optimization is there, but it's fairly minor.

I also have been starting to think more about accessibility modifiers, as we get closer to completion. This one, being a kinda non-standard handler, because of not being void, might be a candidate for private protected instead of just protected.

The resulting visibility is the intersection of protected and internal, so it's visible to derived types in the same assembly only.

[dodexahedron]

That also lets it continue to do what it needs to do, without having to worry about it being overridden outside of the library.