Commit to history after executing a command from the palette

[nrabulinski]

Fixes #5261

While this does fix the underlying issue of that bug, which was that appending to history only happened in editor.rs:1424 which meant that undo-ing after executing a command from the palette would call undo before the change was committed, it does not address the bigger issue - that it's way too easy to mutate a document and have document and history desync.
I'm opening this PR not necessairly to have this fix merged (although we could do it to fix the issue temporarily) but to document and highlight the root cause and possibly discuss a long-term fix, which I'd gladly take on implementing.

Discussion from the matrix channel about this issue:

Pascal Kuthe
Generally any transaction should be appended immidietly to the history. We had quite a few bugs caused from missing that. You would need to find out how a history entry is normally created for a command and why its not happening from the picker.

I do not know the commands area of the editor aswell as I would like (I usually focus more on the render system with what I work on). Therefore I can't tell you how it works off the top of my head without looking at the source (and I am at a Christmas party rn :D) but looking around a bit could be a good way to get into the codebase a bit.
The interesting part here is that the same commands append to history when called normally but not when called trough the picker, if I understood the issue correctly
So I taught it would be some generic mechanism

niko (me)
Yeah the problem here is that we append to history after handling event in the Component impl, but that doesn’t get executed for the picker callback so there’s a couple options for solving this issue:
• Adding the code for appending history changes to command picker
• Adding the code for appending history changes to MappableCommand::execute
• Adding the code for appending history changes somewhere else entirely?

Ideally we wouldn’t want to repeat the same code multiple times in multiple places which is why I feel reluctant to just pasting that code into command palette handler but maybe that is the way forward

gabydd
Some commands like format make changes to the document asynchronously so they have to manually append that to the history
If I remember correctly we don't want to append to history if we are still in insert mode only on change to normal mode but that might be able to be checked not sure, Micheal Davis or archseer would probably have know what the best way to do this is

archseer
yeah that's how it works, I also was thinking commands/keybinds could return an Option<Transaction> that would get auto applied, but that's slightly more code in commands that don't do anything
and you wouldn't be able to apply multiple in a row then

[the-mikedavis]

I think the ideal fix for this has some overlap with #4013, #4244 and #4709: we should be able to queue up command(s) for the compositor to execute in the same way that we can queue up key events. I'm not sure how that should look yet though.

This should fix most of the behavior with the command palette though so we might want to merge this as-is.

[the-mikedavis]

I left some notes on an idea for a long-term fix: #4709 (comment) (note: that would be a very large change that should probably be split across multiple PRs).

It's pretty easy to trigger panics without this fix though so I'll pull this in as-is 👍