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

Allow mouse coordinate updates outside of terminal window #2741

Closed
wants to merge 2 commits into from

Conversation

neurocyte
Copy link

Most terminals send mouse updates even when the cursor is outside of the terminal window if a button is pressed. This makes dragging operations potentially much easier for the user because they are not required keep the mouse cursor precisely inside the window while dragging.

For example, when dragging a scrollbar it may be easy click intially, but quite difficult to drag without touching the edge of the terminal window.

These two commit fix related issues in notcurses.

Comments very welcome.

Copy link
Owner

@dankamongmen dankamongmen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let me look at how these are used, and make sure there are no documented guarantees this violates.

@@ -1768,6 +1742,12 @@ build_cflow_automaton(inputctx* ictx){
{ "[E", simple_cb_begin, },
{ "[<\\N;\\N;\\NM", mouse_press_cb, },
{ "[<\\N;\\N;\\Nm", mouse_release_cb, },
{ "[<\\N;-\\N;\\NM", mouse_press_cb, },
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

who's sending these?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From a quick test it seems kitty, ghostty, xterm and urxvt do. alacritty, wezterm, gnome-terminal don't.

@@ -567,19 +567,6 @@ pixelmouse_click(inputctx* ictx, ncinput* ni, long y, long x){
x /= ictx->ti->cellpxx;
x -= ictx->lmargin;
y -= ictx->tmargin;
// convert from 1- to 0-indexing, and account for margins
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ugh, i should really probably have factored these out. if you're getting rid of them, i guess it doesn't matter.

{ "[<\\N;\\N;-\\NM", mouse_press_cb, },
{ "[<\\N;\\N;-\\Nm", mouse_release_cb, },
{ "[<\\N;-\\N;-\\NM", mouse_press_cb, },
{ "[<\\N;-\\N;-\\Nm", mouse_release_cb, },
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

erp, i need some unit tests on this badly. i know touching it sometimes has unexpected results, due to a bug or bugs of mine.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been using it daily for a few months without noticing any wierdness, but yeah, it didn't look like the "correct" fix to me.

@dankamongmen
Copy link
Owner

just wanted to say i've finally got time to work on Notcurses again, and you can expect this PR to be reviewed/tested/merged/reworked/spindled/folded/mutilated this week. sorry for the delay!

@neurocyte neurocyte closed this Feb 8, 2024
@neurocyte neurocyte deleted the dragging-fixes branch February 8, 2024 19:36
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