Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #3692++ - Rearchitects drivers #3837

Open
wants to merge 143 commits into
base: v2_develop
Choose a base branch
from

Conversation

tznind
Copy link
Collaborator

@tznind tznind commented Nov 19, 2024

This is a super set of #3791 and exists to see diff and direction of v2 drivers as discussed in #3821

Fixes

Progress

  • Keyboard
    • Exit
    • Typing
    • Backspace
  • Views
    • Run/RequestStop
    • MenuBar
  • Mouse (see MouseInterpreter)
    • Button 1 click
    • Multi clicks
    • All buttons
    • Scroll wheel
  • Other
    • Clipboard

Proposed Changes/Todos

Splits drivers up into logical components, simplifies and centralizes shared code patterns.

image

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

@tig tig changed the title V2 drivers Fixes #3692++ - Rearchitects drivers Nov 23, 2024
@tig
Copy link
Collaborator

tig commented Nov 23, 2024

Hope you're ok with me updating the title & first post...

@tznind
Copy link
Collaborator Author

tznind commented Nov 23, 2024

Hope you're ok with me updating the title & first post...

No problem! Always appreciate a clear naming

Now we have fledgling support for

  • Keyboard
  • Mouse
  • Screen size

Net (i.e. native dotnet) and Windows (i.e win 32 api) are the two implementations - I assume we can drop curses and don't need to port?

resize-etc

@tznind tznind mentioned this pull request Nov 23, 2024
@tznind
Copy link
Collaborator Author

tznind commented Dec 29, 2024

Once this is merged and well tested we can have an epic deleting session to remove the old drivers - if we want to that is.

It would mean finally getting rid of the old curses dependency too which would be nice!

@tznind tznind marked this pull request as ready for review December 30, 2024 19:47
@tznind tznind requested a review from tig as a code owner December 30, 2024 19:47
@tznind
Copy link
Collaborator Author

tznind commented Dec 31, 2024

@BDisp can you see if you can see why right click context menus are not popping? I guess something about the mouse event(s) in v1 vs v2?

ok looks like Released+Clicked flags together is bad. TG code wants 2 seperate events I think.

Got it, is just about order of events

@BDisp
Copy link
Collaborator

BDisp commented Jan 1, 2025

ok looks like Released+Clicked flags together is bad. TG code wants 2 seperate events I think.

Got it, is just about order of events

Glad you found the solution but that's really the cause of the bad behavior that was causing the right click wasn't working. Of course that each escape sequence must raise separate events, for them to work as expected. Good work and Happy New Year.

@tznind
Copy link
Collaborator Author

tznind commented Jan 1, 2025

Of course that each escape sequence must raise separate events, for them to work as expected. Good work and Happy New Year.

Happy new year!

There is actually no escape sequence for click so you must in fact create one manually in interpreter:

Input:

�[<0;3;3m
�[<35;3;3m
�[<0;4;4M

We see mouse is moved with no button
Mouse is down (button 35)
Mouse is released (button is back to 0 - also we have 'M' instead of 'm;)

This must currently be translated into 4 events for Terminal.Gui right click menu to work properly:

ReportMousePosition
Button1Pressed
Button1Released
Button1Clicked

So it is not a 1 to 1 mapping between escape codes and events, click is a 'supplemental' event that we inject into the event stream.

Combining flags e.g. Released+Click as one event does not work with current codebase (which is the original thing I was doing).

@tznind
Copy link
Collaborator Author

tznind commented Jan 1, 2025

@BDisp EscSeqUtils.MapKey does not handle backspace. Here is the ConsoleKeyInfo that comes in direct from console when reading with console read:

image

Apparently:

When working with ConsoleKeyInfo in a console application (e.g., in .NET or similar environments), the behavior you're describing is common. Here's why:
Backspace and 127 (0x7F) Behavior

Historically, many systems followed terminal standards where the Backspace key sent the DELETE (0x7F) control character instead of the Backspace (0x08) character.

When you press the Backspace key, the console might interpret it as sending the 127 (0x7F) code instead of 0x08 (Backspace).

This happens because of differences in how the terminal emulator or console environment maps keyboard keys to control codes.

How is this handled in NetDriver? is MapKey the best place to put this new logic?

@BDisp
Copy link
Collaborator

BDisp commented Jan 1, 2025

The DEL (127) is handled on the EscSeqUtils.MapConsoleKeyInfo method.

image

@BDisp
Copy link
Collaborator

BDisp commented Jan 1, 2025

This code fixes the right button click to popup the context menu.

image

@tznind
Copy link
Collaborator Author

tznind commented Jan 2, 2025

The DEL (127) is handled on the EscSeqUtils.MapConsoleKeyInfo method.

Great, I have added in call to this function in the new architecture, see aa8bdd7

This code fixes the right button click to popup the context menu.

Right click works for v2 net but not v2 win, I will have to explore your suggestion further when I have some time.

@tznind
Copy link
Collaborator Author

tznind commented Jan 2, 2025

Windows right click working thanks! 6c55e11

@BDisp
Copy link
Collaborator

BDisp commented Jan 2, 2025

Great, I have added in call to this function in the new architecture, see aa8bdd7

Nice.

Right click works for v2 net but not v2 win, I will have to explore your suggestion further when I have some time.

Works with NetDriver because it rely in the EscSeqUtils API and WindowsDriver don't, and it's needed to point the right way how to handle it. Don't forget to deal with the warning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants