@b4n requested changes on this pull request.
@@ -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? :)
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. <kbd>NumLock</kbd> and others, so in the non-disabled case it'll fallback to Scintilla's handling if e.g. <kbd>NumLock</kbd> is on, and your code if it's not. The proper solution is to filter through `keybindings_get_modifiers()`:
```c GdkModifierType modifiers = keybindings_get_modifiers(event->state);
if (modifiers & GDK_MOD1_MASK) … else if (modifiers & … ```
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);
```suggestion gint amount = (event->direction == GDK_SCROLL_DOWN) ? lines : -lines; ```
+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.
@@ -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 <kbd>Ctrl+Scroll</kbd> 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 <kbd>Ctrl+Scroll</kbd> 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…