Kakoune: Multiple cursors when undo-ing complex command

[RianFuro]

Tried to get used to scripting the editor to my liking by implementing some naiive surround solution:

def surround %! on-key %~ %sh*
    case $kak_key in
        ")") BEGIN="("; END=")";;
        "(") BEGIN="( "; END=" )";;
        "]") BEGIN="["; END="]";;
        "[") BEGIN="[ "; END=" ]";;
        "}") BEGIN='{'; END='}';;
        "{") BEGIN='{'; END=' }';;
    esac

    echo "exec 'a$END<esc>i$BEGIN<esc>'";
*~!

However, after executing that on a selection and then pressing u for undo, I am left with 2 cursors - one for each insert the command did I assume.
No idea if it's intended and it's not that annoying since I can just space out of it, but it seems undesirable.

[lenormf]

Dupe of #753.

When we undo, we do not restore the previous selections, as we dont save them for each modifications. What we do is finding the modified ranges provoked by the undo. In the first case, we see 3 deletions, starting at the begining of each even lines. So we just select the position of the deletions. the last one ends up past the buffer end, so it gets clamped to the last char. which gives us the 3 selections you saw. In other words, we deleted from (1,0), (3,0) and (5,0), compensating for deleted lines before each deletions, we get (1,0),(2,0) and (3,0). (3,0) is not valid anymore so we clamp it to buffer end (2,5) (actually seems we avoid the end of line as well so (2,4)).
I am not sure if we can do much better without saving selections in the undo data, which might be a bit coslty memory wise.

[occivink]

Depending on how much memory it would cost, I think it could be worth it to restore the actual selections. It's certainly an annoyance to get completely unexpected selections in some cases.

[lenormf]

It's fine when you're dealing with a dozen short selections, but think about the cost when doing something like the following on a a few megabytes:

%<a-s>u

Unless there are arbitrary limits (which wouldn't make sense) set on how many selections can be kept in memory, I don't see a generic way to handle this.

[occivink]

With a BufferRange being 16 bytes, you'd be looking at an additional 16MB per undo operation if you have 1M selections.
That doesn't strike me as unacceptable considering that kakoune already uses a substantial amount of memory with such big buffers, but maybe my estimation is wrong.

[lenormf]

I don't know what solution was considered in the first issue that would lead to an over-consumption of memory, but even assuming that it only involved a vector of BufferRange instances, 16MB is actually pretty big. Not all users use editors on a desktop that has several gigabytes of RAM, so implementing a feature that could use arbitrary amounts of memory in extreme cases just because it's way below the average user's system's capacity is a slippery slope.

Then again I don't know what the choices are there, so we might actually have a way to implement that (because I do want this feature as well!).

[lenormf]

Complimentary information:

ironzorg | https://github.com/mawww/kakoune/issues/1184 // it's back again, is there really nothing to be done about this feature?
   mawww | There is always things to be done, the real question is do we want to do them, are they worth it
ironzorg | I'm talking about specifically the remarks about memory
ironzorg | what solution did you consider in the first place that lead you to say it would take too much memory
   mawww | Thats the whole point of "worth it", its going to use some memory, for sure, if we want to save the actual selections instead of infering them from the changes, it might be, or not, worth it to do
         | that
   mawww | I'd expect we need to store, for each undo operation, 2 lists of selections, the selections before (for undo) and the selections after (for redo)
ironzorg | wait, wouldn't you need two lists for each node in the undo-tree too ?
   mawww | Yep
   mawww | We can compact selections to 3 ints per selection (line,column,len)
ironzorg | didn't implementing the undo-tree also cause a massive spike in memory use, for long trees?
ironzorg | I never check, but since I have kakoune on a mediacenter that should have as much RAM as possible for video decoding, it might be important to close kak any chance I get
ironzorg | checked
   mawww | Kakoune should not use that much memory for undo
   mawww | for some definition of that much

[RianFuro]

Woah, looks like my sloppy searching revived some discussion there; @lenormf thanks for clarifying, makes sense if you put it that way.

My opnion on this, although I haven't yet looked at the codebase (still evaluating if kakoune beats nvim for me):
From what I can see, you have an algorithm for finding the final cursors after an undo. In my presented case the issue then is that it composes multiple basic operations (a and i). These are undone simultanously, the algorithm cannot see that it's just composition an assumes it has to put a cursor in both places; guessing because it looks the same like an operation with multiple cursors in data.
If the undo operation was granular enough to split up my command into it's basic operations (and I guess it already is), it could hide all but the last undone basic operation from the cursor-finding-algorithm which then in turn would only produce cursors for the last operation, because it doesn't know any better.

Unless I'm too far off with my guesswork this would yield a bearable amount of memory and performance overhead (seperating composite undo-groups from multi-cursor undo-groups and filtering in the former case).
Also there is a big change this would break something I haven't yet discovered, in which case everything I just said is moot :smile:

[lenormf]

I think this issue can be closed, also dupe of #898.