helix
helix copied to clipboard
Allow entering insert mode with sequence of commands
Summary
[keys.normal]
D = ["collapse_selection", "select_mode", "goto_line_end", "delete_selection"] # << works
C = ["collapse_selection", "select_mode", "goto_line_end", "change_selection"] # << crashes with:
thread 'main' panicked at 'not implemented', helix-term/src/ui/editor.rs:1183:34
stack backtrace:
0: 0x5621345d3b2c - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h3e2b509ce2ce6007
1: 0x562133f27c6c - core::fmt::write::h753c7571fa063ecb
2: 0x5621345cbf55 - std::io::Write::write_fmt::h2815c0519c99ba09
3: 0x5621345d6cd5 - std::panicking::default_hook::{{closure}}::h78d3e6cf97fc623d
4: 0x5621345d6953 - std::panicking::default_hook::hda898f8d3ad1a5ae
5: 0x562134497a63 - helix_term::application::Application::run::{{closure}}::{{closure}}::h4b76c2240aae4f67
6: 0x5621345d7381 - std::panicking::rust_panic_with_hook::h1a5ea2d6c23051aa
7: 0x5621345d7027 - std::panicking::begin_panic_handler::{{closure}}::h07f549390938b73f
8: 0x5621345d4054 - std::sys_common::backtrace::__rust_end_short_backtrace::h5ec3758a92cfb00d
9: 0x5621345d6dad - rust_begin_unwind
10: 0x562133eb0d01 - core::panicking::panic_fmt::h3a79a6a99affe1d5
11: 0x562133eb0c4d - core::panicking::panic::h97167cd315d19cd4
12: 0x56213423746d - <helix_term::ui::editor::EditorView as helix_term::compositor::Component>::handle_event::h00bf68e58e63ad3c
13: 0x56213424c89f - helix_term::compositor::Compositor::handle_event::h2c87fb0c59cda7a2
14: 0x562134225456 - helix_term::application::Application::handle_terminal_events::hd5459c104fbdeacf
15: 0x5621344a4778 - <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll::h3ff36697f3352ace
16: 0x562134471ace - tokio::park::thread::CachedParkThread::block_on::hc59f23f9dca30781
17: 0x562134468569 - tokio::runtime::thread_pool::ThreadPool::block_on::hfe452b18475d1c6f
18: 0x56213448656c - tokio::runtime::Runtime::block_on::h03092494b2bed184
19: 0x562134470daa - hx::main::h0e3c2d144b98d229
20: 0x5621344683a3 - std::sys_common::backtrace::__rust_begin_short_backtrace::hc60ae78ab50ca768
21: 0x56213448630d - std::rt::lang_start::{{closure}}::h2620061a5cdb5fad
22: 0x5621345d318e - std::rt::lang_start_internal::h52e73755f77c7dd9
23: 0x562134474462 - main
24: 0x7f5a46fbf790 - __libc_start_main
25: 0x562133eddeda - _start
26: 0x0 - <unknown>
This is because it is unimplemented! https://github.com/helix-editor/helix/blob/78b16009433041059597cc6ccea28c17254483b7/helix-term/src/ui/editor.rs#L1180-L1184
Reproduction Steps
Setup a custom keybind:
[keys.normal]
C = ["collapse_selection", "select_mode", "goto_line_end", "change_selection"]
and then press C in normal mode.
Helix log
~/.cache/helix/helix.log
please provide a copy of `~/.cache/helix/helix.log` here if possible, you may need to redact some of the lines
Platform
Linux
Terminal Emulator
Kitty
Helix Version
helix 22.05-dev (2d4f94eb)
This is a temporary fix, at least for my particular keymap.
self.last_insert.0 = match self.keymaps.get(mode, key) {
KeymapResult::Matched(command) => command,
KeymapResult::MatchedSequence(commands) => {
commands.last().unwrap().clone()
}
// FIXME: insert mode can only be entered through single KeyCodes
_ => unimplemented!(),
};
This works well for my keymap:
[keys.normal]
C = ["collapse_selection", "extend_to_line_end", "change_selection"]
But maybe self.last_insert.0 should take the last command that switches to insert mode and not just the last command of the sequence.
@icanhazcheeze saw your implementation here too https://github.com/icanhazcheeze/helix/commit/930b4a58664067f4132c568e384d7abd281f721b
But maybe
self.last_insert.0should take the last command that switches to insert mode and not just the last command of the sequence.
Yeah exactly. Maybe the mode transition logic should happen in https://github.com/helix-editor/helix/blob/master/helix-term/src/ui/editor.rs#L803-L823= instead so we can determine the exact command that switched?
Just want to drop a note here that I run into this multiple times a day via the trigger described in #3090. Thankfully, I save obsessively, so I don't lose a substantial amount of work from it.
But it would be nice to see the priority bumped on this, since it's a crash that's very easy to accidentally trigger as a user even in Helix's default configuration. (Unfortunately, I don't have the bandwidth to try to fix this myself right now. :-/)
I stumbled upon this error as well for the same reason (trying to remap C to change to end of line). Just voicing my support for this fix. Thanks!
Being able to use https://github.com/LGUG2Z/helix-vim without code changes would be awesome...
I just want to make sure this is emphasized: this bug is easily triggered in the default configuration with no keybinding customization whatsoever. I have multiple crashes a day from this in the default configuration. I believe this is a critical bug.
I am worried that this is getting lost due to the relatively benign title and description of this issue, which makes it sound like it's only preventing certain keybinding customizations.
On the contrary, this is easily triggered (resulting in a crash) in a completely vanilla installation with zero custom configuration. The critical trigger for this bug is described in the now-marked-duplicate #3090.
(I'm not intending to be pushy here, I just want to make sure this isn't lost among the comments that (unintentionally) obscure how critical a bug this is.)
EDIT: I goofed a little. It doesn't happen in a completely default config. I thought it did, because I don't have any seemingly relevant keybinding changes in my config (e.g. mode switches) and it could be triggered without using any of my custom keys.
This is a minimal config.toml that crashes in combination with following the steps in #3090:
keys.normal.w = ["no_op"]
Importantly, you never have to use the custom binding. It just has to be defined, and then follow the steps in #3090. It seems specific to the w key, since I can have other bindings defined, and as long as w isn't custom-bound it all works fine. It also seems specific to binding w to a command sequence, because w = "no_op" is also fine.
Looks like I was too hasty in marking #3090 as a duplicate 😓. Sorry @cessen! I'll re-open that and I'll post a change to fix it too.
@the-mikedavis
No worries! I realize I may have come across a bit aggressive, which wasn't my intention. I just wanted to make sure the issue didn't get lost in the weeds.
Thanks so much for looking into it!
Merged https://github.com/helix-editor/helix/pull/2634 to resolve the panic but it would still be good to repeat the whole sequence: https://github.com/helix-editor/helix/pull/2634#issuecomment-1201970286