Panic in insert mode when undoing changes.

[adminy]

Summary

here is what I have in the config:

[keys.insert]
C-p = ["normal_mode", "file_picker"]
C-x = ["normal_mode", ":wq"]
C-f = ["normal_mode", "search"]
C-z = ["normal_mode", "earlier"]

Reproduction Steps

Now I set the editor in insert mode, I type a bunch of characters then I press Ctrl + z, says there is nothing to undo, so I keep typing more characters, then Ctrl + z then crash:

● λ RUST_BACKTRACE=full hx
thread 'main' panicked at helix-core/src/transaction.rs:293:9:
assertion failed: original_doc.len_chars() == self.len
stack backtrace:
   0:     0x5608c7f376a7 - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h4a5853078ed6ad5e
   1:     0x5608c732093c - core::fmt::write::hcb5164cc0001627e
   2:     0x5608c7f372ce - std::io::Write::write_fmt::h4f047c3956a46825
   3:     0x5608c7f37424 - std::sys_common::backtrace::print::h33b76037fd95b2fe
   4:     0x5608c7f485b7 - std::panicking::default_hook::{{closure}}::hd7fd2e4913eae4ac
   5:     0x5608c7f482b3 - std::panicking::default_hook::h13a9a2eb684db87c
   6:     0x5608c7f48b4f - std::panicking::rust_panic_with_hook::hda39260d38680d2a
   7:     0x5608c7f37cc9 - std::panicking::begin_panic_handler::{{closure}}::h150d47ece314bd47
   8:     0x5608c7f378c6 - std::sys_common::backtrace::__rust_end_short_backtrace::hf5ef31e5afaaca81
   9:     0x5608c7f486c4 - rust_begin_unwind
  10:     0x5608c7239ce5 - core::panicking::panic_fmt::h9a659e19ee4930b5
  11:     0x5608c7239da3 - core::panicking::panic::h48cfe629b6ef4fae
  12:     0x5608c7539fda - helix_core::transaction::ChangeSet::invert::h1d983015c22d4fd8
  13:     0x5608c7c3fd84 - helix_view::document::Document::apply_impl::h06ada3c7aeafa558
  14:     0x5608c7c421ec - helix_view::document::Document::earlier_later_impl::hfee7c922eb006382
  15:     0x5608c77bb914 - helix_term::commands::earlier::hf04f81dd56cc2e90
  16:     0x5608c7ace0b3 - helix_term::ui::editor::EditorView::handle_keymap_event::{{closure}}::h866d35cb8b4d50bc
  17:     0x5608c7acdf59 - helix_term::ui::editor::EditorView::handle_keymap_event::hac761f3995529829
  18:     0x5608c7ace42f - helix_term::ui::editor::EditorView::insert_mode::h93266cd28c2bfd3e
  19:     0x5608c7ad0bd1 - <helix_term::ui::editor::EditorView as helix_term::compositor::Component>::handle_event::hf6931c1f6bfc5b35
  20:     0x5608c7b14eda - helix_term::compositor::Compositor::handle_event::h4ddbd6865e1351c9
  21:     0x5608c7ce7fb2 - hx::main_impl::{{closure}}::h7ccfd2c3320597aa
  22:     0x5608c7ce546f - tokio::runtime::park::CachedParkThread::block_on::h8bf5aee73e624708
  23:     0x5608c7d3205e - tokio::runtime::runtime::Runtime::block_on::h1f2ae556e1ce9d6d
  24:     0x5608c7d49e33 - hx::main::h52cb20467dd048c1
  25:     0x5608c7d4f0c3 - std::sys_common::backtrace::__rust_begin_short_backtrace::hb92d1ce411ac36ca
  26:     0x5608c7d49f3d - std::rt::lang_start::{{closure}}::h63ff2b32f63ab2dd
  27:     0x5608c7f2a165 - std::rt::lang_start_internal::hf4417b67ea9bae52
  28:     0x5608c7d49f25 - main
  29:     0x7f4d2a98a10e - __libc_start_call_main
  30:     0x7f4d2a98a1c9 - __libc_start_main@@GLIBC_2.34
  31:     0x5608c72a2c85 - _start
  32:                0x0 - <unknown>

Helix log

~/.cache/helix/helix.log

empty?!

Platform

Linux (NixOS)

Terminal Emulator

alacritty 0.13.2

Installation Method

nixpkgs master

Helix Version

helix 24.3 (2cadec0)

[kanielrkirby]

I poked around in the code a bit, mostly to learn the codebase a little better. I'll start with the editor design side of things.

In editors like VSCode, transactions are oriented around spaces, but this doesn't follow the conventions of modal editors like (Neo)Vim and Kakoune, which only make transactions after exiting insert mode (e.g., enter insert mode, type stuff, exit insert mode, undo, everything done in insert mode is undone).

Here's the relevant code at helix-term/src/ui/editor.rs#1432:

...
Event::Key(mut key) => {
  ...
  // Store a history state if not in insert mode. This also takes care of
  // committing changes when leaving insert mode.
  if mode != Mode::Insert {
      doc.append_changes_to_history(view);
  }
  ...
}
...

There's a couple solutions, most likely of which (in my opinion) is that this won't be supported. Another option is to start committing transactions every space like VSCode does, but this will change how a very common functionality in modal editors works, so I'm not personally keen on that. A middle ground could be that we create a temporary Vec of Transaction/Revisions when entering insert mode that works like normal editors, and that gets cleared when exiting insert mode.

Now, onto the actual bug, the panic.

thread 'main' panicked at helix-core/src/transaction.rs:293:9:
assertion failed: original_doc.len_chars() == self.len
...
12:     0x56051c5dcaa7 - helix_core::transaction::ChangeSet::invert::ha8b1f55c37e72755

The bug is that (with a non-empty History), if you go into insert mode, make some changes, and try to undo them whilst in insert mode, the underlying ChangeSet hasn't been updated with the document's length, and so it panics from an assert that was added into invert (as well as other methods like this):

/// Returns a new changeset that reverts this one. Useful for `undo` implementation.
/// The document parameter expects the original document before this change was applied.
pub fn invert(&self, original_doc: &Rope) -> Self {
    assert!(original_doc.len_chars() == self.len); // panics here

#[derive(Debug, Default, Clone, PartialEq, Eq)]
pub struct ChangeSet {
    pub(crate) changes: Vec<Operation>,
    /// The required document length. Will refuse to apply changes unless it matches.
    len: usize,
    len_after: usize,
}

I'm still learning the codebase, so I'm not positive what the best way to address this is. I think it partly depends on what the preferred approach to "Undo in insert mode" is, whether that is that it's not supported, a temporary insert-specific Vec, or simply making more Transactions (though even that won't work without addressing the original_doc.len == ChangeSet.len constraint).

Another note, this also happens with :later, and there's probably some other ways to trigger this bug with other commands in insert mode that eventually trigger compose or invert.

TLDR is that the invert function is being a little too safe here, and doesn't expect earlier/later/undo/redo/etc. to be called without Revisions and the doc being in sync.

[the-mikedavis]

See undo in Kakoune's buffer.cc: we just need to call append_changes_to_history in undo_redo_impl and earlier_later_impl - it's the equivalent of Kakoune's "undo group" committing

[kanielrkirby]

Oh nice, appreciate the info @the-mikedavis! Sorry for the tangent 😅 Still trying to get a feel for the project, and learned a lot anyways. I made a PR with the changes you suggested.

[adminy]

Nice, thanks for going into details, I'm starting to understand how all this works too. I'm new to helix, have been using neovim and now I want to use something that seems to have a well thought out design and better UI. I reported this because I managed to crash this on a couple of occasions and I thought saying go to normal mode then undo actually did what it said. Maybe I've misunderstood, I'm very new to this.