Fix consecutive cursors being filtered out #3823
Conversation
The code in `DoTextEvent` is quite hard to understand. Why do we clamp an existing event here? Because we recalculate the end location which should have been assigned before in `ExecuteTextEvent`. The end locations for all `TextEventInsert` are incorrectly being stored in the undo stack and are hot-fixed while being processed. This also fixes potential issues with `TextEventReplace` in regards to multi-line strings.
Getting the remaining characters after line breaks is a common task when dealing with TextEvents. So make it a utility function and expose it to Lua. This is very useful when processing these events in Lua plugins.
…Remove`, `TextEventInsert`) When pasting from the clipboard with multiple cursors, cursors that are directly next to each other are filtered out. The reason for this is that `EventHandler.Replace` triggers two separate events to mimic the replacement. These two events are processed sequentially in `DoTextEvent`, which means that these two consecutive cursors have the same position after `TextEventRemove`. The second event `TextEventInsert`, can no longer distinguish between them, and Buffer.MergeCursors filters out one cursor later.
After replacing text, it is (imho) helpful to undo/redo changes individually. With a single cursor, nothing changes, but with multiple cursors, this gives the user a greater sense of control.
| if h.Cursor.HasSelection() { | ||
| h.Cursor.DeleteSelection() | ||
| h.Cursor.ResetSelection() | ||
| h.Buf.Replace(h.Cursor.CurSelection[0], h.Cursor.CurSelection[1], clip) | ||
| } else { | ||
| h.Buf.Insert(h.Cursor.Loc, clip) |
There was a problem hiding this comment.
Previously, when you have multiple selections and perform a paste, the undo will undo all of them at one go.
With this branch, the undo only undos one replacement at a time, which will be undesirable if I paste replaced many selections and decide to undo them.
My guess is because of this change here.
There was a problem hiding this comment.
Did you see this commit 8b32508?
The last commit (Always stop Undo(), Redo() after TextEventReplace) is totally subjective and up for debate. For me personally it feels much better to have more control for TextEventReplace undos/redos. Let me know what you think.
There was a problem hiding this comment.
Missed it, mb.
I can sort of see scenarios for both cases.
Though I would prefer an undo in one go since it is a single "action", at least from the user's perspective.
I can't imagine having to smash my undo button many times if I have many multi-cursors.
I would prefer it to be a toggle option instead if we decide to keep it.
There was a problem hiding this comment.
I can't imagine having to smash my undo button many times if I have many multi-cursors.
Once you pressed it your finger is already on the right key, so pressing it again is not too bad.
For me, it felt good to have more control. But that's completely subjective, which is why I was interested in other people's opinions.
I would prefer it to be a toggle option instead if we decide to keep it.
I'm not sure if we really need to make this an option. I'm fine with removing it again.
Sometimes you experiment with something, put it up for discussion, and then realize that it was actually a good idea. But I also can imagine that there are people who prefer the original functionality.
2 participants
When pasting from the clipboard (or inserting) with multiple cursors, cursors that are directly next to each other are filtered out.
The reason for this is that
EventHandler.Replacetriggers two separate events to mimic the replacement. These two events are processed sequentially inDoTextEvent, which means that these two consecutive cursors have the same position afterTextEventRemove. The second eventTextEventInsert, can no longer distinguish between them, andBuffer.MergeCursorsfilters out one cursor later.How to reproduce:
Create 2 cursors with selections directly next to each other. Each cursor selects one
c.Insert any character, e.g.
xor paste your clipboard.Expectation:
Actual behavior:
So, one cursor gets lost.
The last commit (
Always stop Undo(), Redo() after TextEventReplace) is totally subjective and up for debate. For me personally it feels much better to have more control forTextEventReplaceundos/redos. Let me know what you think.