Add options to control scrollwheel behavior (zoom and scroll lines) #3835

xiota · 2024-04-13T11:26:24Z

This PR adds two options to control scrollwheel behavior.

scrollwheel_lines: How many lines to scroll.
scrollwheel_zoom_disable: To disable zooming by control + scrollwheel.

Control of vertical scrolling is taken away from Scintilla to provide consistent behavior when the control key is or is not pressed. Lines option added because I don't see a way to get or set the value used by Scintilla.

A possible modification is to allow number of lines when control is pressed to differ from when it is not. This could allow for single line scrolling or half-page scrolling. The option would need to be renamed. (eg, scrollwheel_control_scrolls, scrollwheel_control_scroll_lines)

Supersedes #2954

b4n · 2024-04-13T23:02:17Z

doc/geany.txt

@@ -2681,6 +2681,9 @@ indent_hard_tab_width             The size of a tab character. Don't change    8
 editor_ime_interaction            Input method editor (IME)'s candidate        0           to new
                                  window behaviour. May be 0 (windowed) or                 documents
                                  1 (inline)
+scrollwheel_lines                 Number of lines the scrollwheel scrolls      3           immediately


First: why?
Second: this is NOT the current value (which is 4, or a system value). Also, as suggested in the previous sentence, Scintilla's default can depend on the system configuration (currently, it does on Windows), so it's not necessarily a great idea to force another value, at least not with a good rationale.
And, why change it from 4 to 3? or is the Windows's default 3? :)

I guess I see the reason why you added this: because you need a value when transforming Ctrl+Scroll to a "regular" scroll event, right? And then instead of hard-coding it, you added a setting.
I guess that's a fair reason, but there's sill the issues mentioned above. And is it important that Ctrl+Scroll scrolls, or could it just do nothing?

At any rate, at least the default should be the same as the current one. I don't care as much for Windows I admit, but it's probably easy enough to mimic what Scintilla does there as well, isn't it? The semantic of the option becomes a bit hairy though, because how to conceal a dynamic system value with a user setting? I guess it could be 0 means use system default (and fallback to 4), and any other value this amount of lines or something like that…

I miscounted and the historical value when mouse wheels were introduced to consumers was 3. So I didn't bother recounting to confirm. Would be better to use the system default, but I don't know how.

b4n · 2024-04-13T23:10:34Z

src/editor.c

 		sci_scroll_columns(editor->sci, amount);
 		return TRUE;
 	}
+	else if (!event->state || (editor_prefs.scrollwheel_zoom_disable && (event->state & GDK_CONTROL_MASK)))


This doesn't actually do what you think it does (in some situations). event->state contains all modifier states, including e.g. NumLock and others, so in the non-disabled case it'll fallback to Scintilla's handling if e.g. NumLock is on, and your code if it's not.
The proper solution is to filter through keybindings_get_modifiers():

GdkModifierType modifiers = keybindings_get_modifiers(event->state); if (modifiers & GDK_MOD1_MASK) … else if (modifiers & …

Some existing code will need to be revised then.

Possibly, did you see some? This very function was fine because it only checked whether some modifiers were present, not that they were specific -- and the ones checked weren't in the special cases for macos -- but I can totally imagine we have issues some other places (although I'd think they'd be discovered by now -- your case was a lot more subtle to see given that if your test failed the behavior was still very similar)

I didn't understand the purpose earlier. It makes more sense now that I see it's to work around how Macs interpret some keys.

If I want to remove the control mask, would either of the following work for mac?

event->state = event->state & !GDK_CONTROL_MASK;

GdkModifierType modifiers = keybindings_get_modifiers(event->state); event->state = modifiers & !GDK_CONTROL_MASK;

It's also to filter out uninteresting modifiers like NumLock.

For removing, you could try event->state &= ~GDK_CONTROL_MASK;, or if inside the if (event->state & GDK_CONTOL_MASK), just event->state ^= GDK_CONTROL_MASK;.

src/editor.c

b4n · 2024-04-13T23:12:33Z

src/sciwrappers.c

+void sci_scroll_lines(ScintillaObject *sci, gint lines)
+{
+	SSM(sci, SCI_LINESCROLL, 0, (uptr_t) lines);
+}
+
+


I don't think it's worth adding a new Scintilla wrapper function for the single caller, given the call is pretty simple. Just inline the equivalent in the calling code.

Same could be said of...

geany/src/sciwrappers.c

Lines 1015 to 1018 in a44df8b

void sci_scroll_columns(ScintillaObject *sci, gint columns)

{

SSM(sci, SCI_LINESCROLL, (uptr_t) columns, 0);

}

Disagree with @b4n, all Scintilla interfacing should go through wrappers so that changes Scintilla makes can be abstracted away, for example the recent string length change.

The performance difference will be immaterial unless the Scintilla call is being hammered in a loop (which it never is AFAICT). And if it is a problem anywhere, define the function inline in sciwrappers.h (C99).

@xiota sure there might be some already very seldom used, not sure it's a trend to follow…
But if @elextr has a strong opinion, fine by me. FWIW, IMO it doesn't make much sense where it's a single cas that's just as easy to fix in the caller or refactor out when there are more users.

@elextr I'm not concerned by any speed issues, just that although it looks "clean", it also makes the code somewhat more complex, adding a tiny bit of an additional constraints through new interfaces just to transform that interface (the wrapper doesn't have any logic of its own)

@b4n I am normally the one complaining about single use functions where a lump of functionality is broken out for no good reason, and that case totally matches your reasoning.

But for APIs wrapping is a useful way of getting encapsulation and hiding, and the Geany plugin API should have been done that way, but well, that ship has sailed, hit stormy seas, and sunk ;-D

Probably doesn't matter at this stage which this individual call uses but every little drop of water erodes the stone [proverb].

elextr · 2024-04-14T00:35:27Z

Why is it transforming ctrl+scroll to scroll differently (as @b4n pointed out on the code) instead of doing nothing when scrollwheel_zoom_disable is set?

On #2954 you say:

I have the editor font sent to a comfortable size. Many shortcuts consist of combining the Control key with others. And of course, the touchpad is used for moving the cursor. So very frequently (multiple times per day), I accidentally engage the scrolling function of the touchpad while the control key is pressed. This causes the editor view to zoom, which is disruptive.

Simply disabling the ctrl+zoom will achieve your aim and won't unexpectedly scroll the view either.

xiota · 2024-04-14T02:42:16Z

Why is it transforming ctrl+scroll to scroll differently ... instead of doing nothing ...?

The mouse wheel would look broken if scrolling stopped when control is accidentally pressed.

elextr · 2024-04-14T03:24:18Z

The mouse wheel would look broken if scrolling stopped when control is accidentally pressed.

Hmm, ok, in that case is it safe for us to simply edit the event to remove the control mask then let it go to Scintilla, maybe @b4n knows, or you could try it.

b4n · 2024-04-14T06:58:37Z

The mouse wheel would look broken if scrolling stopped when control is accidentally pressed.

I'd argue that it's just as true with any incorrect usage that doesn't do what you wanted. For a more unreasonable example, if I hit Del when I meant to write "hello", I wouldn't blame the software but myself.

I get that it's annoying the zoom changes by mistake, but is it truly a problem for you to re-scroll without control pressed?

@elextr hum… sounds hacky, but might happen to work, I don't know.

elextr · 2024-04-14T07:33:44Z

hum… sounds hacky, but might happen to work, I don't know.

Totally hacky :-). But if it works it keeps the scrolling under Scintilla's control and IIUC Scintilla makes attempts to match the environment scrolling speed.

xiota · 2024-04-14T08:41:46Z

Changed the options and how they interact because I wanted to see what it would be like.

scrollwheel_lines (4): Number of lines for normal scrolling. Still don't know how to get the system default.
scrollwheel_alt_factor (6): Multiplier when alt is pressed, instead of pgup/pgdown. This is like a scrolling accelerator. Users can set it to scroll less than a full page so that context from jump to jump is still visible.
scrollwheel_ctrl_lines (4): Number of lines to scroll when control is pressed, when it doesn't activate zooming. Fine scrolling is easier if set to a low number, like 1.
scrollwheel_ctrl_zoom (T): Whether ctrl+scroll = zoom.

Multiple modifiers can be pressed simultaneously, and they affect each other. For example:

shift = horizontal scrolling.
shift + alt = accelerated horizontal scrolling.
control = slow scrolling (when set to low number).
shift + control = slow horizontal scrolling

xiota added 2 commits April 13, 2024 10:48

Add option to block scroll-wheel zooming

41ffdd8

Add option to change number of scroll lines

f6efbf6

b4n requested changes Apr 13, 2024

View reviewed changes

Change filtering algorithm, add more options

33e4ade

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add options to control scrollwheel behavior (zoom and scroll lines) #3835

Add options to control scrollwheel behavior (zoom and scroll lines) #3835

xiota commented Apr 13, 2024 •

edited

b4n Apr 13, 2024

b4n Apr 13, 2024

xiota Apr 14, 2024

b4n Apr 13, 2024

xiota Apr 14, 2024

b4n Apr 14, 2024

xiota Apr 14, 2024

xiota Apr 14, 2024

b4n Apr 14, 2024

b4n Apr 13, 2024

xiota Apr 14, 2024

elextr Apr 14, 2024

b4n Apr 14, 2024

b4n Apr 14, 2024

elextr Apr 14, 2024

elextr commented Apr 14, 2024

xiota commented Apr 14, 2024

elextr commented Apr 14, 2024

b4n commented Apr 14, 2024

elextr commented Apr 14, 2024

xiota commented Apr 14, 2024 •

edited

	void sci_scroll_columns(ScintillaObject *sci, gint columns)
	{
	SSM(sci, SCI_LINESCROLL, (uptr_t) columns, 0);
	}

Add options to control scrollwheel behavior (zoom and scroll lines) #3835

Are you sure you want to change the base?

Add options to control scrollwheel behavior (zoom and scroll lines) #3835

Conversation

xiota commented Apr 13, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elextr commented Apr 14, 2024

xiota commented Apr 14, 2024

elextr commented Apr 14, 2024

b4n commented Apr 14, 2024

elextr commented Apr 14, 2024

xiota commented Apr 14, 2024 • edited

xiota commented Apr 13, 2024 •

edited

xiota commented Apr 14, 2024 •

edited