helix icon indicating copy to clipboard operation
helix copied to clipboard

Allow entering insert mode with sequence of commands

Open David-Else opened this issue 3 years ago • 5 comments

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)

David-Else avatar Apr 09 '22 12:04 David-Else

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.

lucypero avatar May 29 '22 02:05 lucypero

@icanhazcheeze saw your implementation here too https://github.com/icanhazcheeze/helix/commit/930b4a58664067f4132c568e384d7abd281f721b

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.

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?

archseer avatar May 30 '22 08:05 archseer

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. :-/)

cessen avatar Aug 01 '22 21:08 cessen

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!

mike-lloyd03 avatar Aug 02 '22 21:08 mike-lloyd03

Being able to use https://github.com/LGUG2Z/helix-vim without code changes would be awesome...

luisdavim avatar Aug 09 '22 23:08 luisdavim

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.

cessen avatar Aug 22 '22 19:08 cessen

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 avatar Aug 22 '22 22:08 the-mikedavis

@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!

cessen avatar Aug 23 '22 02:08 cessen

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

archseer avatar Aug 31 '22 01:08 archseer