This is just a quick test of multiple carets for Geany. IMO we don't need a full support of everything working on multiple carets, users typically just need to insert/delete things at multiple places simultaneously for which this is sufficient.
Ideally, this should be mapped to alt+click as this is what vscode does and also we use alt+shift for the block caret. I only did run into a problem on macOS where (currently in a virtual machine) I get `GDK_MODIFIER_RESERVED_25_MASK` instead of `GDK_MOD1_MASK` but I'm always confused with GDK events so I'm maybe doing something wrong.
(Comparing to the LSP plugin, the ratio of the number of thumbs up for this feature to the amount of time spent on implementing it is quite favorable ;-)
Fixes #1141 You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/3899
-- Commit Summary --
* Preliminary support of multiple cursors
-- File Changes --
M src/editor.c (5)
-- Patch Links --
https://github.com/geany/geany/pull/3899.patch https://github.com/geany/geany/pull/3899.diff
Ideally, this should be mapped to alt+click as this is what vscode does and also we use alt+shift for the block caret.
Alright, I'm confused because I have command+control swapped on macOS - in fact it's ctrl+click for the block caret on Geany now. In this case I'd suggest that we use block caret (in addition to the current ctrl+click) also for alt+click so one doesn't have to remember which one is ctrl and which one is alt.
I'd suggest that we use block caret (in addition to the current ctrl+click) also for alt+click so one doesn't have to remember which one is ctrl and which one is alt.
I just tried ``` SSM(sci, SCI_SETRECTANGULARSELECTIONMODIFIER, SCMOD_CTRL | SCMOD_ALT, 0); ``` and sadly, it doesn't work. But since `SCMOD_CTRL=2, SCMOD_ALT=4`, maybe Scintilla could be modified to support both of them in parallel.
Are you confused about rectangular selection, its https://www.scintilla.org/ScintillaDoc.html#SCI_CHANGESELECTIONMODE not multiple selections.
Are you confused about rectangular selection, its https://www.scintilla.org/ScintillaDoc.html#SCI_CHANGESELECTIONMODE not multiple selections.
No, I'm talking about the fact that if we want to have multiple carets on Alt+click, it would be also good to have rectangular selection on Alt+Shift+click so one doesn't have to remember which of these multi-caret modes is alt and which one is ctrl. At the same time though, we should probably support the existing Ctrl+Shift+click keybinding because existing Geany users are used to it already. That's why I suggested `SSM(sci, SCI_SETRECTANGULARSELECTIONMODIFIER, SCMOD_CTRL | SCMOD_ALT, 0)` for the rectangular selection (which doesn't work right now but I had a look at the Scintilla code and it should be quite trivial to modify it).
Ok, that makes sense if Neil accepts the "trivial" change (@elextr fears @techee "trivial", could be up to 3000 lines change :-) if not I would have alt+click for multiple cursors since as you say its pretty much universal, and leave the more niche rectangular selection as Ctrl+Shift+click since only Scintilla apps do it and its their default IIUC.
(@elextr fears @techee "trivial", could be up to 3000 lines change :-)
Fear not :-). It's a matter of replacing
https://github.com/techee/geany-lsp/blob/c56de66cecf4dff2c1b1a98ea37b22bd2af...
with
```C int modifierTranslated(int sciModifier) noexcept { int translated = 0; if (sciModifier & SCMOD_SHIFT) translated |= GDK_SHIFT_MASK; if (sciModifier & SCMOD_CTRL) translated |= GDK_CONTROL_MASK; if (sciModifier & SCMOD_ALT) translated |= GDK_MOD1_MASK; if (sciModifier & SCMOD_SUPER) translated |= GDK_MOD4_MASK; return translated; } ```
(Tested, it works.)
Now, granted, this is for GTK only, there's also Windows and macOS but maybe Neil would accept it even if it weren't possible to implement on other platforms.
In any case, I'd just wait for some feedback here if this is something we want so I don't submit patches upstream that are not used.
if this is something we want
As I said several times over the years on #1141 _so long as it is confirmed that no Geany or core plugin or GP plugin functionality breaks_ it can be introduced in stages, basic Scintilla provided capability, then expand Geany and plugin functionality that can sensibly apply to multiple selections to do so over time.
But I have to point out Ctrl+click is the multiple caret shortcut in Vscode on Linux not alt+click, maybe thats a Mac thing, and Scite is also Ctrl+click[^1].
So maybe click for one selection, Ctrl+click for multiple selections (after first), and Ctrl+shift+click for rectangular selection, all nice and consistent and in agreement with at least a couple of other apps, and no Scintilla patch needed.
[^1]: not a wide sample I agree, but its all I have on this machine, and I wonder where I got the idea it was alt+click?
But I have to point out Ctrl+click is the multiple caret shortcut in Vscode on Linux not alt+click, maybe thats a Mac thing, and Scite is also Ctrl+click[1](https://github.com/geany/geany/pull/3899#user-content-fn-1-6f72b32fdc21112dc...).
Hmm, I can't reproduce it on linux (real machine, not a VM). Ctrl+click in vscode behaves like in Geany and it is goto definition/declaration. Even it were different in vscode though, we shouldn't change the current goto definition/declaration keybinding to something else - everyone is used to that already.
on macOS where (currently in a virtual machine) I get GDK_MODIFIER_RESERVED_25_MASK instead of GDK_MOD1_MASK
Just tried outside the VM and there it works correctly and I get `GDK_MOD1_MASK`. So IMO this patch (OK, the documentation too) is really all it's needed to support multiple carets.
Dropping the work-in-progress tag.
Hmmm, for Vscode its documented as `alt+click` but also documented that it can be changed to `Ctrl` by a setting "for Windows and Linux". Maybe because I get vscode from the MS deb repository they set that since they know its Linux?
Anyway, if ctrl+click is taken by default on Geany the it will have to be Alt+click.
Finally, you put that WIP back until the documentation is done [wags finger at @techee] :grin:
Finally, you put that WIP back until the documentation is done [wags finger at @techee] 😁
Also, I've just realized that currently it's possible to only place multiple carets, but not to make multiple selections by dragging while holding Alt which vscode allows. Should be doable I think.
@techee pushed 1 commit.
be7c8c9a73930104e111e6ead069e8d2e3fbe74c Support making multiple selections
I just added a commit allowing multiple selections. Alternatively we coul ask Neil to add a new Scintilla API to assign a different modifier for creating new carets than the hard-coded Ctrl that Scintilla uses (we need Alt).
<kbd>Alt+Click</kbd> is likely gonna be problematic on Linux, because many WMs use that keybinding to move windows (mine does). So IMO it at least can't be hard-coded (and probably shouldn't be default on Linux)
@b4n commented on this pull request.
@@ -4968,6 +4990,7 @@ static ScintillaObject *create_new_sci(GeanyEditor *editor)
g_signal_connect(sci, "button-press-event", G_CALLBACK(on_editor_button_press_event), editor); g_signal_connect(sci, "scroll-event", G_CALLBACK(on_editor_scroll_event), editor); g_signal_connect(sci, "motion-notify-event", G_CALLBACK(on_motion_event), NULL); + g_signal_connect(sci, "motion-notify-event", G_CALLBACK(motion_notify_event), editor);
shouldn't the two `motion-notify-event` handlers be merged?
@b4n commented on this pull request.
@@ -324,6 +339,18 @@ static gboolean on_editor_button_press_event(GtkWidget *widget, GdkEventButton *
keybindings_send_command(GEANY_KEY_GROUP_GOTO, GEANY_KEYS_GOTO_MATCHINGBRACE); return TRUE; } + if (event->type == GDK_BUTTON_PRESS && event->state == GDK_MOD1_MASK)
don't use `event->state` directly (at least not when comparing to equality), because it can contain a whole lot of problematic things (numlock state, etc.). Luckily, there's `state` already available here :)
@b4n commented on this pull request.
@@ -288,6 +288,21 @@ void editor_snippets_init(void)
}
+gboolean motion_notify_event(GtkWidget* widget, GdkEventMotion event, gpointer data) +{ + GeanyEditor *editor = data; + + if (event.state & GDK_BUTTON1_MASK && event.state & GDK_MOD1_MASK)
This includes *any* set of modifiers that contain <kbd>MOD1</kbd>, is that wanted?
```suggestion if (keybindings_get_modifiers(event.state) == (GDK_BUTTON1_MASK | GDK_MOD1_MASK)) ```
Alt+Click is likely gonna be problematic on Linux, because many WMs use that keybinding to move windows (mine does). So IMO it at least can't be hard-coded (and probably shouldn't be default on Linux)
That's quite bad - what other modifiers do we have left?
@techee commented on this pull request.
@@ -324,6 +339,18 @@ static gboolean on_editor_button_press_event(GtkWidget *widget, GdkEventButton *
keybindings_send_command(GEANY_KEY_GROUP_GOTO, GEANY_KEYS_GOTO_MATCHINGBRACE); return TRUE; } + if (event->type == GDK_BUTTON_PRESS && event->state == GDK_MOD1_MASK)
don't use event->state directly (at least not when comparing to equality), because it can contain a whole lot of problematic things (numlock state, etc.). Luckily, there's state already available here :)
Yes, but `keybindings_get_modifiers()` filters-out Alt (`GDK_MOD1_MASK`) I wanted to use.
@techee commented on this pull request.
@@ -288,6 +288,21 @@ void editor_snippets_init(void)
}
+gboolean motion_notify_event(GtkWidget* widget, GdkEventMotion event, gpointer data) +{ + GeanyEditor *editor = data; + + if (event.state & GDK_BUTTON1_MASK && event.state & GDK_MOD1_MASK)
Yeah, you're right.
@techee commented on this pull request.
@@ -4968,6 +4990,7 @@ static ScintillaObject *create_new_sci(GeanyEditor *editor)
g_signal_connect(sci, "button-press-event", G_CALLBACK(on_editor_button_press_event), editor); g_signal_connect(sci, "scroll-event", G_CALLBACK(on_editor_scroll_event), editor); g_signal_connect(sci, "motion-notify-event", G_CALLBACK(on_motion_event), NULL); + g_signal_connect(sci, "motion-notify-event", G_CALLBACK(motion_notify_event), editor);
`on_motion_event()` is in some other file than `editor.c` and probably needs access to some stuff defined there but I could call it from within `motion_notify_event()` so the signal is connected just once.
Hum… not much, maybe something with <kbd>Super</kbd>, but e.g. GNOME-Shell does use some of that (<kbd>Super+LMB</kbd> and <kbd>Super+Shift+LBM</kbd> -- actually the same as my WM but with <kbd>Super</kbd> instead of <kbd>Alt</kbd>). <kbd>Super+Ctrl</kbd>? Meh, not enough modifiers.
I do not believe that replacing a single modifier like `Alt` with a modifier combination like `Alt+Shift` is trivial. Various chording combinations already have meanings. This has been tried before on macOS with bad, confusing results.
@b4n commented on this pull request.
@@ -324,6 +339,18 @@ static gboolean on_editor_button_press_event(GtkWidget *widget, GdkEventButton *
keybindings_send_command(GEANY_KEY_GROUP_GOTO, GEANY_KEYS_GOTO_MATCHINGBRACE); return TRUE; } + if (event->type == GDK_BUTTON_PRESS && event->state == GDK_MOD1_MASK)
Does it? It filters <kbd>MOD2</kbd>, but I don't see any kind that it does this for MOD1? Actually quite the contrary, it's in the default modmask for all platforms, and even again for macos (looking at GTK3's `gtk_accelerator_get_default_mod_mask()` implementation -- and the respective platform vfuncs).
@b4n commented on this pull request.
@@ -4968,6 +4990,7 @@ static ScintillaObject *create_new_sci(GeanyEditor *editor)
g_signal_connect(sci, "button-press-event", G_CALLBACK(on_editor_button_press_event), editor); g_signal_connect(sci, "scroll-event", G_CALLBACK(on_editor_scroll_event), editor); g_signal_connect(sci, "motion-notify-event", G_CALLBACK(on_motion_event), NULL); + g_signal_connect(sci, "motion-notify-event", G_CALLBACK(motion_notify_event), editor);
Ah right, the other one is the generic "let's focus stuff" thingy. But yeah, probably having a single handler would be better -- although it doesn't change a thing save confusing me a tad :slightly_smiling_face:
@techee commented on this pull request.
@@ -324,6 +339,18 @@ static gboolean on_editor_button_press_event(GtkWidget *widget, GdkEventButton *
keybindings_send_command(GEANY_KEY_GROUP_GOTO, GEANY_KEYS_GOTO_MATCHINGBRACE); return TRUE; } + if (event->type == GDK_BUTTON_PRESS && event->state == GDK_MOD1_MASK)
Alright, it's possible it was filtering-out the strange modifier I was getting on my linux VM running under macOS where I can't properly develop this (which will be the case in the upcoming week when I'm out of home with my macOS machine only).
Well, maybe <kbd>Ctrl+click</kbd> is going to have to sacrifice its special usage, "goto definition" is <kbd>F12</kbd> elsewhere.
Well, maybe Ctrl+click is going to have to sacrifice its special usage, "goto definition" is F12 elsewhere.
I think I'd stop using Geany then :-)
I think it just has to be configurable and have something like ``` Click modifiers: Go to definition [combo box] Rectangular selection [combo box] Multiple carets [combo box] ``` Where each combo box would contain the modifiers that seem to work.
Now the question is where such settings should go. Maybe it would still fit into Preferences->Editor->Features, or there could be a new tab under Preferences->Editor, or Preferences->Keybindings could have a separate tab too.
Thoughts?
I think I'd stop using Geany then :-)
Since its `right-click->goto definition` (or a keyboard shortcut) in all other IDEs that I checked where ya gonna go? :stuck_out_tongue_winking_eye:
And remember that LSP has added `goto typedefinition` and `find references` as well as `goto definition`, and probably could add `goto declaration` to match Vscode. Retrain your muscle memory to right click and you can have a cornucopia of options [end marketing] :grinning:
Thoughts?
I'm not against infinite flexibility, but as @nyamatongwe pointed out, various combinations work or don't work and are available or not available depending on the platform and as @b4n pointed out on Linux on the desktop being used.
So the user is going to have to choose random combinations and test them until they see what works for them, not a good UI.
Also note that `Alt` is the "present mnemonics" key in GTK, its getting very overloaded on Linux with desktops using it as well.
Google finds some hits on queries about how to return `Ctrl+click` to something after some app repurposed it, so it seems we are not be alone in this quandary. Having assigned a keybinding long ago that has now become common on some platforms for a different operation there will always be some users (you ;-) who will complain when its repurposed to match the common use. And of course its not common on all platforms, sigh.
My thought is still that the odd one out, `Ctrl+click` for `goto definition`, is the thing to go on Linux and Windows. On Macos if you can use `Alt+click` for multiselect you could keep it, but its gonna be confusing if users ask questions when the operation is different between platforms.
I'm not against infinite flexibility, but as @nyamatongwe pointed out, various combinations work or don't work and are available or not available depending on the platform and as @b4n pointed out on Linux it also depends on the desktop being used.
My point isn't an infinite flexibility, it's just a matter of being able to "sacrifice" one feature for another in case there are not enough modifiers left. If I had to choose to either have multiple carets or goto definition/declaration and the only modifier available were Ctrl, I would choose goto definition/declaration without any question. But someone else may pick multiple carets. Ctrl+click has been used for goto definition/declaration forever in Geany and it should stay this way by default.
The GUI I proposed could be also "inverted" and look this way: ``` Modifier configuration Control+click [combo containing: Goto definition, Multiple carets] Alt+click [combo containing: Goto definition, Multiple carets] ```
Scintilla treats mouse modifiers as a group. When `Alt` is used for rectangular selection and `Shift` for extending a selection, `Alt+Shift+Click` will create or extend a rectangular selection from the current caret to the click. Scintilla's default mapping scheme uses Extend=Shift; Rectangular=Alt; Multiple=Ctrl. Where possible, the same modifier scheme is used for keyboard commands.
There could be an API to completely specify the logical from physical mouse modifier mapping: [Extend=Shift|Ctrl|Alt|Super|Nothing|...;Rectangular=...;Multiple=...] while retaining combinations. This should reject ambiguous definitions like assigning both Extend and Rectangular to the Alt key. Difficulties here include defining the set of modifiers as they differ between platforms.
While Scintilla does some processing of right mouse button and middle mouse button clicks, these don't use modifiers except for right clicks in the margin which may be forwarded to the app with a specific notification. Thus, it would be fairly simple to assign goto-definition to `Ctrl+RightClick` or similar chord,
The GUI I proposed could be also "inverted" and look this way:
Thats a better UI, but as @b4n pointed out `Alt+click` is stolen by some WMs on Linux (mine too), so how do you know if its available as an option to show?
Thus, it would be fairly simple to assign goto-definition to Ctrl+RightClick or similar chord
Thats a possibility. But since goto definition is already in the right click menu its not much of a saving, and still needs @techee to retrain his muscle memory :grin: but maybe existing `Ctrl_click` users would find it useful.
The LSP plugin repositions goto definition to the top of the menu, we could do that to the standard menu too.
Thats a better UI, but as @b4n pointed out Alt+click is stolen by some WMs on Linux (mine too), so how do you know if its available as an option to show?
It can be shown but unassigned to anything or assigned depending on the platform.
But I actually don't think this is the better UI - the better one was the one I proposed before: we have 3 actions we need to assign some key+click combinations to and then a set of modifiers and it should rather be the modifiers that are selectable: - Ctrl, Alt, Super for goto definition and multiple carets - Ctrl+Shift, Alt+Shift, Super+Shift (which are already configurable in Scintilla) for rectangular selection
Reasonal defaults would be - Ctrl for goto definition/declaration everywhere as this has always been used - Linux: Super for multiple carets, Ctrl+Shift for rectangular selection - Windows: Alt for multiple carets, Alt+Shift for rectangular selection - macOS: depending on what other editors use, I think it will be the same as on Windows
Note that the Windows/Linux rectangular selection is already hard-coded here: https://github.com/geany/geany/blob/7909ce299350e1ea882be3a559240aa374cec454...
Thats a possibility. But since goto definition is already in the right click menu its not much of a saving, and still needs @techee to retrain his muscle memory 😁 but maybe existing Ctrl_click users would find it useful.
I really won't use it this way - I'll much rather abandon this PR ;-) (but seriously, in such a case this is a much better option for me personally)
Well, its not much difference between the UIs for something this small, although I think mapping fixed click modifier to selectable action is the way round it should work (and the whole Geany keybinding UI should be changed to fixed key combo selectable action, but I won't mention it to avoid @b4n throwing things at me ;-).
It can be shown but unassigned to anything or assigned depending on the platform.
So the user can assign it and then wonder why it doesn't work?
Just to verify, I assume where you say "multiple carets" you also mean multiple selection?
If you map action to selectable click modifier you need to check if that it isn't assigned elsewhere since each modifier can only trigger one action and popup a "its already assigned, do you want me to unassign it" dialog like the Geany keybinding does because its backwards too. (Inverting it would be the way to go, "press keysequence to change then select action for it" but I won't mention it because @b4n will throw stuff at me, but it would allow multiple keybindings for the same action which the keypress handler supports, just not the GUI or keybindings file ;-)
I guess for something this small (a couple of modifiers a few actions) it doesn't matter which way.
Anyway so long as I can make it operate the same as [pick some other random application like say Vscode] so I can switch back and forward I can live with it.
I'll much rather abandon this PR ;-)
Well, by the time you add a UI and store and restore the settings it won't be "this" PR any more anyway :-D
Hmmm, sorry about the multiple post, for some reason the first didn't show up until I posted the second
I don't have a whole lot to add to the discussion just yet (but allowing e.g. <kbd>RMB</kbd> or even <kbd>MMB</kbd> is a neat idea to double or triple the options at hand), but:
Reasonal defaults would be
[…] * Linux: Super for multiple carets, Ctrl+Shift for rectangular selection
Actually, given GNOME uses <kbd>Super</kbd>, <kbd>Alt</kbd> might be a better default. I can actually change the modifier in my WM, and maybe others (maybe even GNOME Shell??), so that's also an option. I don't even remember what the default is for mine, I might very well have changed it 10+ years ago and forgotten about it. If it's configurable, I don't think it's a big deal, but it should be reasonable for "most" users.
(and the whole Geany keybinding UI should be changed to fixed key combo selectable action, but I won't mention it to avoid @b4n throwing things at me ;-).
Well, it's just unreasonable to have "fixed key combo" :slightly_smiling_face: : not everybody has a 105 keyboard with a boring keymap. And -- sit down, it's gonna be huge -- keyboards got even weirder keys since the early 70s (media keys, random accelerators and whatnot), so not only is it unrealistic to list them all, but just think about the sheer amount of entries -- even with your 105 US layout keyboard, you still got 105 keys and at *least* 3 modifiers if not 4 (I have like ~5). So… no, I don't think it's a good idea indeed :grin:
We could even argue something similar for mice, there can be more than 3 buttons (or less *cough* macos *cough*) I hear.
@b4n I wasn't mentioning it remember :grin:, so didn't go into details ... but I said it better in the second post, press keybinding (as you do now) then select action for it, so anything GTK can understand (and will get through the barrier of the WM ;-) can be assigned to any action. And the Help->Keyboard Shortcuts would have to be able to show multiple shortcuts.
As a general point its actually probably a good thing to add a "clickbinding" dialog as well as the "keybinding" one rather than the current ifdefed hard coded values. They could just be defaults.
Actually, given GNOME uses Super, Alt might be a better default.
Cinnamon uses Alt, so Super is better (ie no right answer)
Just to verify, I assume where you say "multiple carets" you also mean multiple selection?
Yes, it's the same thing. You either Modifier+click to add a new caret or Modifier+drag (or Modifier+double-click on an identifier) to make a new selection
@techee pushed 1 commit.
e01bd7fe10324eddc4219bf96a33b68e041d567b Make click modifiers configurable
@techee pushed 1 commit.
fc5bdd6f969e84f94ae00302ee4a974aad16b0d9 Disallow overlapping selections
Alright, I added the configurability of modifiers - for now it's under Editor->Features, could be placed somewhere else if someone has a better idea:
<img width="314" alt="Screenshot 2024-06-18 at 14 39 19" src="https://github.com/geany/geany/assets/713965/7ceb31e3-b9fa-4a59-9553-bd82256da463">
For - "Goto definition" and "Multiple carets", one can choose from Ctrl, Alt, Super, Ctrl+Shift, Alt+Shift, Super+Shift, Ctrl+Alt, Ctrl+Super - "Rectangular selection" one can use Ctrl+Shift, Alt+Shift, Super+Shift (as supported by Scintilla)
I've also updated the selection logic not to create multiple overlapping selections and, instead, drop the previous selections at the overlapping positions (something similar is also done by vscode).
As the default value for "Multiple carets" I used "Ctrl+Alt" which I first thought might be pretty universal, but on Raspberry Pi this is used for switching desktops using mouse. Not sure if we manage to find something that works out of the box everywhere.
@techee commented on this pull request.
@@ -288,6 +288,21 @@ void editor_snippets_init(void)
}
+gboolean motion_notify_event(GtkWidget* widget, GdkEventMotion event, gpointer data) +{ + GeanyEditor *editor = data; + + if (event.state & GDK_BUTTON1_MASK && event.state & GDK_MOD1_MASK)
`keybindings_get_modifiers(event.state)` filters-out `GDK_BUTTON1_MASK` so it cannot be used for it. What is the correct way to handle this situation? ```C if (event.state == (GDK_BUTTON1_MASK | GDK_MOD1_MASK)) ``` works for me though it might break if there's some extra stuff in `state`, one could also use ```C if ((event.state & GDK_BUTTON1_MASK) && keybindings_get_modifiers(event.state) == GDK_MOD1_MASK)) ``` but the condition might be satisfied also when there is more than `GDK_BUTTON1_MASK` pressed.
@techee pushed 1 commit.
e221a9886cc146e4483364b984b04e627c677b53 Don't use event->state directly
Also, shouldn't we enable these for multiple carets (also affects rectangular selection): - `SCI_SETMULTIPASTE` - paste is performed at all caret positions instead of the main one - `SCI_AUTOCSETMULTI` - autocompletion is performed at all caret positions
When someone creates a multiple selection, I guess the desired behavior is to perform one thing everywhere so both paste and autocompletion should be performed at all caret positions IMO.
@techee pushed 1 commit.
979a79a5faffbd01bdc6c154abdbb64d515c0291 Merge branch 'master' into multi_cursor
@techee pushed 1 commit.
58557119565ef34fb17b3ffac712c96ba45159b6 Fix defines in keyfile.c and indent them a bit to avoid similar problem in the future
Also, shouldn't we enable these for multiple carets (also affects rectangular selection):
If I understand correctly #625, this should fix it.
github-comments@lists.geany.org