Skip to content
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.

Sign up for GitHub

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

Reset mode when changing buffers #5072

Merged
merged 2 commits into from
Dec 11, 2022
Merged

Reset mode when changing buffers #5072

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1c399e4
Reset mode when changing buffers
the-mikedavis Dec 8, 2022
4e09497
Move normal mode entering code to Editor
the-mikedavis Dec 9, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions helix-term/src/commands.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -694,6 +694,7 @@ fn goto_buffer(editor: &mut Editor, direction: Direction) {

let id = *id;

normal_mode_impl(editor);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it makes more sense to move this into editor.switch instead of adding this only here? I imagine the bug could appear for other commands aswell. editor.switch is called many times and it seems like a huge footgut for it to cause crahses it the editor is not in normal mode. I think this bug could already be produced by other commands if you created a custom insert mode binding for them. I can't think of a situation where this wouldn't want to be a normal mode after a switch.

This will require moving normal_mode_impl to helix-view but that should be okay since the function is fairly small.

What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah good call. I think we currently miss some steps when switching modes in Editor::focus that would be resolved by that change as well

editor.switch(id, Action::Replace);
}

Expand Down Expand Up @@ -2672,12 +2673,16 @@ fn open_above(cx: &mut Context) {
}

fn normal_mode(cx: &mut Context) {
if cx.editor.mode == Mode::Normal {
normal_mode_impl(cx.editor);
}

fn normal_mode_impl(editor: &mut Editor) {
if editor.mode == Mode::Normal {
return;
}

cx.editor.mode = Mode::Normal;
let (view, doc) = current!(cx.editor);
editor.mode = Mode::Normal;
let (view, doc) = current!(editor);

try_restore_indent(doc, view);

Expand Down
3 changes: 3 additions & 0 deletions helix-view/src/editor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1025,6 +1025,9 @@ impl Editor {
let (view, doc) = current!(self);
let view_id = view.id;

// Append any outstanding changes to history in the old document.
doc.append_changes_to_history(view);

if remove_empty_scratch {
// Copy `doc.id` into a variable before calling `self.documents.remove`, which requires a mutable
// borrow, invalidating direct access to `doc.id`.
Expand Down