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

Alt-modified mousewheel mode and other wheel tweaks #1234

Merged
merged 3 commits into from Oct 4, 2023

Conversation

ak2
Copy link
Contributor

@ak2 ak2 commented Sep 8, 2023

Proposing the following three main changes to mousewheel handling. Further details in the individual commit messages.

Avoid mixed line and page mousewheel scrolling

Don't convert line scrolling events to page scrolling events when they exceed the terminal height. This was my idea to start with, but in hindsight I don't think it was a good one because we don't actually know how many lines correspond to a page as far as the application is concerned, or whether it supports page up/down events in the first place. I think this simplifies the code a bit, and helps witht the following changes.

Send Alt modifier with mousewheel events

If the Alt key is pressed, modify mousewheel events sent as cursor/page up/down keycodes or as mintty's application wheel codes accordingly. (Ctrl and Shift are already used for other purposes.)

Add Alt-modified mousewheel mode

When enabled, this sends Alt-modified cursor up/down keycodes for mousewheel events when the Alt key is not pressed. If the Alt key is pressed, unmodified keycodes are sent. The mode does not affect page up/down codes.

In the nano editor from version 4.0, the Alt+Up/Down keycodes scroll the window instead of moving the cursor. Scrolling straight away is the proper action for the mousewheel, whereas moving the cursor only scrolls once the cursor reaches the top or bottom.

Less scrolls with or without the Alt modifier. Vim and mined at least tolerate Alt+Up/Down, apparently handling it the same as unmodified Up/Down, but they have proper mousewheel scrolling support using full mouse reporting mode anyway.

@ak2
Copy link
Contributor Author

ak2 commented Sep 10, 2023

Re less, sorry I forgot I had actually created a ~/ .lesskey file for the Alt+Up/Down bindings when experimenting with this previously:

\e[1;3A back-line
\e[1;3B forw-line

@mintty
Copy link
Owner

mintty commented Sep 17, 2023

Sorry for the late response; I thought I could take a break from mintty after the last release :)

Also I am having a hard time to get an overview of these changes.

I do not seem to understand what the first one actually changes, except for the one line less which is OK. Which interaction is being avoided or is it after all just some refactoring?

About the second, it is good to enrich escape sequences by modifiers (although I couldn’t find a test case to reproduce the difference) but why just Alt. Shouldn’t simply all modifiers be considered generically?
I know, some are masked by other features but that might change or get reconfigured, and also there are some more modifiers, 6 in total.

The third change reverts Alt semantics for the sake of nano if I understood.
Wouldn’t it be cleaner to reconfigure nano for this use case?
Also it kind of contradicts the idea of generic modifier handling which I suggested above. Furthermore, it needs another dynamic mode which I guess not many people will ever use. And if so, could we reserve the nice number 7777 for something more fancy in the future?
Again, however, I don’t seem to recognize the use case fully or even find a test case.

@ak2
Copy link
Contributor Author

ak2 commented Sep 23, 2023

Sorry for the late response; I thought I could take a break from mintty after the last release :)

Sorry for a late response from me too.

I'd thought I better leave this one until after 3.6.5. :)

I do not seem to understand what the first one actually changes, except for the one line less which is OK. Which interaction is being avoided or is it after all just some refactoring?

It is partly refactoring, which I'm hoping makes the code a bit clearer before the other two changes.

But it also does away with my idea of converting line scroll events to page up/down key presses when the number of lines of an event exceeds the number of rows of the terminal. That's unlikely to happen with a real mousewheel, but it can happen with touchpad scroll events, which the OS converts into mousewheel events.

For example, when the mousewheel event says scroll 40 lines down on a 25-row terminal, mintty would convert that to 1 page down and 15 cursor down key presses. With the change here, it's just 40 cursor downs.

There are a few reasons for that:

  • We don't actually know whether the application treats page up/down events similarly to cursor up/down, so it's better to just send cursor keys. For example, if you're running a shell in 'screen' (which normally uses the alternate screen), cursor up/down usually goes through the history while page up/down does nothing or even puts partial keycodes on the command line.
  • We don't know how many lines make up a page as far as the application is concerned. For example, an editor might have multiple files open in a tiled arrangement, in which case a page might just be half or less of the terminal screen, so the conversion would be incorrect.
  • It did the conversion the other way as well, i.e. if you requested page-wise mousewheel scrolling through the Windows setting or by pressing Shift, mintty would nevertheless convert pages into lines when the delta indicated partial wheel notches (as happens with touchpads). It shouldn't ignore user wishes like that.
  • With regards to the subsequent changes, modified combinations such as Alt+Up and Alt+PgUp are even less likely to do similar things, so implicitly converting between them would be even more problematic.

About the second, it is good to enrich escape sequences by modifiers (although I couldn’t find a test case to reproduce the difference) but why just Alt.

A test case is nano, where Alt+Up/Down scrolls, whereas plain Up/Down moves the cursor.

Shouldn’t simply all modifiers be considered generically? I know, some are masked by other features but that might change or get reconfigured, and also there are some more modifiers, 6 in total.

Okay, I can have a go at that. (Although I'm generally not in favour or applications messing with the Windows key.)

While looking at that, should I also look into making the currently hardcoded Shift for page-wise scrolling configurable? 'WheelPageMod', defaulting to 'shift'?

As for the use of the Ctrl modifier for single-line scrolling, I'm tempted to just do away with that. I can't remember why I implemented it as it's neither standard nor very useful, and Ctrl+wheel normally means zooming anyway.

The third change reverts Alt semantics for the sake of nano if I understood. Wouldn’t it be cleaner to reconfigure nano for this use case? Also it kind of contradicts the idea of generic modifier handling which I suggested above. Furthermore, it needs another dynamic mode which I guess not many people will ever use. And if so, could we reserve the nice number 7777 for something more fancy in the future? Again, however, I don’t seem to recognize the use case fully or even find a test case.

Sadly nano doesn't allow configuring cursor key combinations, but I think the implicitly added modifier would also be useful in other applications to do scrolling instead of cursor movement (without going full mouse mode or using the mintty-only application mousewheel codes).

For example, vim can be configured to scroll on Alt+Up/Down with this:

:map <M-Up> <C-Y>
:map <M-Down> <C-E>
:imap <M-Up> <C-O><C-Y>
:imap <M-Down> <C-O><C-E>

But maybe a 'WheelScrollMod' option configurable to any modifier but defaulting to 'none' would be a better approach than a dynamic switch?

@mintty
Copy link
Owner

mintty commented Oct 4, 2023

Hi,
I'd like to release 3.6.6 soon as it fixes a memory leak. There are 4 or 5 other issues that I want to include, including (part of?) this one.

The first, sanitizing of line vs page events, plus the minus-1 page scroll behaviour, is OK.

The second, adding Alt+wheel events, obviously adds value e.g. to nano, so yes.

Okay, I can have a go at that. (Although I'm generally not in favour or applications messing with the Windows key.)

The generalization to other modifiers as I suggested is optional for now. If you've worked out something already, that's fine of course. Yes, the Win key... I tried to compensate for its side effects by some tricky hooks. I might additionally add user-definable keys for the Meta modifier, like for Super and Hyper. This issue is about the effect of modifiers, anyway, not how they are entered.

While looking at that, should I also look into making the currently hardcoded Shift for page-wise scrolling configurable? > 'WheelPageMod', defaulting to 'shift'?

I think Shift is somewhat intuitive to make an effect stronger, while Ctrl could make it weaker, so I see only a faint use case here.

As for the use of the Ctrl modifier for single-line scrolling, I'm tempted to just do away with that. I can't remember why I implemented it as it's neither standard nor very useful, and Ctrl+wheel normally means zooming anyway.

As said above, Ctrl intuitively makes things smaller for me. The zoom modifier could be made configurable to uncover the single-line scroll variation. Please leave the code in for now.

The third change, I see now how that could implement a preference for nano. Generalizing modifiers and making everything configurable, one could in theory apply a linear algebra matrix to transform modifier configuration... just joking.

But maybe a 'WheelScrollMod' option configurable to any modifier but defaulting to 'none' would be a better approach than a dynamic switch?

I imagine a user might have a shell wrapper for nano, switching this on and off (or push/pull) as desired (although, if nano cannot switch this itself, can it spawn other programs or shells as some editors do? that would make switching a bit inconsistent). Anyway, I'd keep the dynamic sequence. An option could set the initial value, too (there are a few other options that can preset DECSET modes). But please change the sequence value to 7765 (65 = ASCII 'A' = Alt).

One of my other patches affects the same file, I've waited so far for your changes.

Don't convert line scrolling events to page scrolling events when they
exceed the terminal height, because we don't actually know how many
lines correspond to a page as far as the application is concerned, or
whether it supports page up/down events in the first place.

Also, scroll the scrollback by one line less than the terminal height
when scrolling by pages with the mousewheel, to match what we do for
PgUp/Dn presses.

Drop len argument from send_keys(), and use NUL-terminated strings
instead for more flexibility. Put the count argument first.
If the Alt key is pressed, modify mousewheel events sent as cursor/page
up/down keycodes or as mintty's application wheel codes accordingly.

In nano, Alt+up/down means scrolling the window instead of moving the
cursor.

(The Shift and Ctrl modifiers are already used for modifying the scroll
distance per wheel notch to whole pages or single lines, so don't send
those.)
When enabled, this sends Alt-modified cursor up/down keycodes for
mousewheel events. If the Alt key is pressed, unmodified keycodes are
sent. The mode does not affect page up/down codes.

In the nano editor from version 4.0, the Alt+Up/Down keycodes scroll the
window instead of moving the cursor. Scrolling straight away is the
proper action for the mousewheel, whereas moving the cursor only scrolls
once the cursor reaches the top or bottom.

Less can be configured to scroll on Alt+Up/Down with this in ~/.lesskey:
\e[1;3A back-line
\e[1;3B forw-line

Similarly, vim can be configured to do so with this in ~/.vimrc:
map <M-Up> <C-Y>
map <M-Down> <C-E>
imap <M-Up> <C-O><C-Y>
imap <M-Down> <C-O><C-E>
@ak2
Copy link
Contributor Author

ak2 commented Oct 4, 2023

Hi Thomas, I're rebased the three commits onto latest master, and amended the third one to use DECSET 7765 instead of 7777, as requested. (I had originally implemented it with 7765, before realising that I could try to claim 7777 with the 'M'odified, but you're right to reserve that.)

I'll look into adding support for the other modifiers to bring it up to par with the arrow keys.

@mintty mintty merged commit a204cca into mintty:master Oct 4, 2023
1 check passed
@mintty
Copy link
Owner

mintty commented Oct 4, 2023

Danke, Andy.
I've pulled the 3 commits. I think the description of the 7765 mode is a bit unclear in the wiki, I'll look into that later and make some unrelated other changes now.

mintty pushed a commit that referenced this pull request Oct 4, 2023
@ak2 ak2 deleted the wheel-change branch October 4, 2023 21:14
@mintty
Copy link
Owner

mintty commented Nov 14, 2023

Released 3.7.0.

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.

None yet

2 participants