-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Panic in insert mode when undoing changes. #11077
Comments
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 ...
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 Now, onto the actual bug, the panic.
The bug is that (with a non-empty /// 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 Another note, this also happens with TLDR is that the |
See |
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. |
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. |
* Add changes before insert mode undo Fixes #11077 * Address edge cases for undo like Kakoune does --------- Co-authored-by: Kaniel Kirby <pirate7007@runbox.com> Co-authored-by: Michael Davis <mcarsondavis@gmail.com>
* Add changes before insert mode undo Fixes helix-editor#11077 * Address edge cases for undo like Kakoune does --------- Co-authored-by: Kaniel Kirby <pirate7007@runbox.com> Co-authored-by: Michael Davis <mcarsondavis@gmail.com>
* Add changes before insert mode undo Fixes helix-editor#11077 * Address edge cases for undo like Kakoune does --------- Co-authored-by: Kaniel Kirby <pirate7007@runbox.com> Co-authored-by: Michael Davis <mcarsondavis@gmail.com>
* Add changes before insert mode undo Fixes helix-editor#11077 * Address edge cases for undo like Kakoune does --------- Co-authored-by: Kaniel Kirby <pirate7007@runbox.com> Co-authored-by: Michael Davis <mcarsondavis@gmail.com>
Summary
here is what I have in the config:
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, thenCtrl
+z
then crash:Helix log
~/.cache/helix/helix.log
Platform
Linux (NixOS)
Terminal Emulator
alacritty 0.13.2
Installation Method
nixpkgs master
Helix Version
helix 24.3 (2cadec0)
The text was updated successfully, but these errors were encountered: