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

Workaround for touchpad/mouse issue with MacOS default Terminal #56

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

penrods
Copy link

@penrods penrods commented Apr 1, 2021

The default terminal application that comes with the MacOS sends a tap on
the mousepad as rapid-fire left-button down/left-button up. When the
get_input() method reads in the key buffer, it ends up with both codes
in the buffer at once. Due to the check for a buffer with a mouse
code being 6 bytes (and this double-code containing 12), the mouse
code was rejected. As a result, you couldn't click on anything in the
Mac terminal.

This hacky fix just allows the double-code. The left-button up is
consumed in the process, but since the framework only pays attention
to the left-button down, this loss isn't important.

The default terminal application that comes with the MacOS sends a tap on
the mousepad as rapid-fire left-button down/left-button up.  When the
get_input() method reads in the key buffer, it ends up with both codes
in the buffer at once.  Due to the check for a buffer with a mouse
code being 6 bytes (and this double-code containing 12), the mouse
code was rejected.  As a result, you couldn't click on anything in the
Mac terminal.

This hacky fix just allows the double-code.  The left-button up is
consumed in the process, but since the framework only pays attention
to the left-button down, this loss isn't important.
@penrods penrods changed the title Fix bug with MacOS default Terminal Fix touchpad bug with MacOS default Terminal Apr 2, 2021
@pfalcon
Copy link
Owner

pfalcon commented Apr 3, 2021

Thanks for bringing up this issue. I have to admit, that proper handling of multi-byte input sequences is a known big issue in picotui. For now it indeed assumes that a single read from stdin returns single, and complete, sequence for one key. That's of course not adequate, but it happens to work most of the time on Linux. I saw that approach failing on Linux too (e.g., on heavily loaded system), but again, most of the time it works, and it's simple (the whole idea of picotui is minimalism and simplicity), so resolving that stays in backlog.

Now, this patch isn't really a solution of the issue, it's workaround. And a pretty crude one - you don't even check what's in the last 6 bytes of those 12 bytes, and just discard them, what if it's something else than mouse up sequence?

So again, thanks for bringing up this issue and proposing a workaround which other affected people may use in the meantime. But instead of adding adhoc workarounds, I'd rather find a way to resolve this "properly" - which may need some consideration and time to do.

@pfalcon pfalcon changed the title Fix touchpad bug with MacOS default Terminal Workaround for touchpad/mouse issue with MacOS default Terminal Apr 3, 2021
@pfalcon
Copy link
Owner

pfalcon commented Apr 3, 2021

I have to admit, that proper handling of multi-byte input sequences is a known big issue in picotui.

#23

@penrods
Copy link
Author

penrods commented Apr 6, 2021

Yep, this is absolutely a crude workaround -- hence the HACK comment! I started with this to minimize the number of side effects -- the current code was already throwing away the extra bytes, it was just throwing them away without also recognizing it as mouse input. I thought it worth it given the large number of users potentially impacted by this, since it pretty much breaks mouse input on the standard Mac terminal.

A better approach would probably be to reorg the function to use the self.kbuf (if it is preloaded) or fill it with the results of os.read(). Then peel off either a single key or a command sequence from that buf. I can code that up if you'd like, but it probably requires a little more validation to make sure there are no unexpected behaviors.

@penrods
Copy link
Author

penrods commented Apr 6, 2021

I do fear there could be complication in identifying how many bytes make up "one command" for that approach. There are a lot of multi-byte sequences and variations. Looking at: https://invisible-island.net/xterm/ctlseqs/ctlseqs.html#h2-Mouse-Tracking I can see some scenarios were even the 6-byte assumption being used in the current mouse code might be incorrect (if I'm reading this right). I was hesitant to make those changes without a broad selection of terminals / emulators / OSes to test against. But game if you have a good way of testing.

@pfalcon
Copy link
Owner

pfalcon commented Apr 11, 2021

I do fear there could be complication

Sure, if there weren't complications, it would have been done long ago and there wouldn't be a problem now ;-).

As I mentioned, #23 is dedicated to this issue, ideas on how to deal with it are welcome there.

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

Successfully merging this pull request may close these issues.

2 participants