@b4n requested changes on this pull request.

In 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? :)

In 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 & …

In 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)))
+	{
+		/* scroll normally */
+		gint lines = editor_prefs.scrollwheel_lines;
+		gint amount = (event->direction == GDK_SCROLL_DOWN) ? lines : (-1 * lines);

⬇️ Suggested change

-		gint amount = (event->direction == GDK_SCROLL_DOWN) ? lines : (-1 * lines);
+		gint amount = (event->direction == GDK_SCROLL_DOWN) ? lines : -lines;

In 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.

In 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

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…

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.