-
Notifications
You must be signed in to change notification settings - Fork 43
Prevent many hyperlinks from launching while mouse moving #86
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
Prevent many hyperlinks from launching while mouse moving #86
Conversation
stefanhaller
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, the change makes sense to me, just a small suggestion below.
I suspect the reason this is only a partial fix is due to how we detect mouse drag events in tcell_driver.go (the logic around the MAYBE_DRAGGING state). This is because with the legacy terminal protocol there are no mouse-up events, so we have to poll the button states, which is not really reliable. I'm not sure this really explains the issue (which I can't reproduce with ghostty btw), but I don't have time to investigate this in more depth right now.
Anyway, tcell now supports the kitty protocol for better terminal events, which will make it possible to actually receive real mouse-up events. Hopefully this will make it more reliable. I'm working on upgrading tcell, and will look into this as part of that work. Unfortunately it's a lot more work than I originally anticipated, so it will take a while.
gui.go
Outdated
| } | ||
|
|
||
| if ev.Key == MouseLeft && !v.Editable && g.openHyperlink != nil { | ||
| if ev.Key == MouseLeft && ev.Mod != ModMotion && !v.Editable && g.openHyperlink != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we say (ev.Mod & ModMotion) != 0 instead? It's a bit mask.
(It's true that we currently don't include any other bits for mouse events, but this might change in the future.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point on being a bitmask, although I tested that we want it == 0 instead of != 0. Implemented!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yes of course.
This is reproduced when clicking and dragging over a link, which is a common way that people try to select them to copy
66928bf to
b67a047
Compare
|
@ChrisMcD1 In case you are wondering about bumping the gocui dependency in lazygit: no need for that, I'm working on other changes to gocui right now and will bump it soon anyway. |
|
@ChrisMcD1 I decided to make a separate PR for bumping the gocui dependency after all; my other changes might take longer, and also this makes it get its own change log entry. #5133 |
Update our gocui dependency, which brings in the fix that was made there; see jesseduffield/gocui#86. (Fix by `Chris McDonnell <c.a.mcdonn@gmail.com>`)
This is reproduced when clicking and dragging over a link, which is a common way that people try to select them to copy.
Partially fixes jesseduffield/lazygit#5057