Skip to content

Commit

Permalink
Limit the number of items in the jumplist (helix-editor#4750)
Browse files Browse the repository at this point in the history
Previously, jumplists could grow unchecked. Every transaction is
applied to jumplist selections to ensure that they are up to date
and within document bounds, so this would cause every edit to become
more expensive as jumplist lengths increased throughout a session.
Setting a maximum number of entries limits the cost.

Vim and Neovim limit their jumplists:

* https://github.com/vim/vim/blob/b298fe6cbae3b240b10dbd55d9c38d0cc8e033d3/src/structs.h#L141
* https://github.com/neovim/neovim/blob/e8cc489accc435076afb4fdf89778b64f0a48473/src/nvim/mark_defs.h#L57

Notably, Kakoune does not. In Kakoune, changes are applied to jumplist
entries lazily as you hit `<C-o>`/`<C-i>` though, so Kakoune doesn't
have the same growing cost concerns. Kakoune also does not have a
concept of a View which limits the cost further.

Vim and Neovim limit to 100. This seems unreasonably high to me so I've
set this to 30 to start. We can increase if this is problematically
low.
  • Loading branch information
the-mikedavis authored and Frederik Vestre committed Feb 6, 2023
1 parent b499d45 commit e38a279
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 11 deletions.
1 change: 0 additions & 1 deletion helix-term/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2427,7 +2427,6 @@ fn jumplist_picker(cx: &mut Context) {
.views()
.flat_map(|(view, _)| {
view.jumps
.get()
.iter()
.map(|(doc_id, selection)| new_meta(view, *doc_id, selection.clone()))
})
Expand Down
26 changes: 16 additions & 10 deletions helix-view/src/view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,29 +3,35 @@ use helix_core::{
pos_at_visual_coords, visual_coords_at_pos, Position, RopeSlice, Selection, Transaction,
};

use std::fmt;
use std::{collections::VecDeque, fmt};

const JUMP_LIST_CAPACITY: usize = 30;

type Jump = (DocumentId, Selection);

#[derive(Debug, Clone)]
pub struct JumpList {
jumps: Vec<Jump>,
jumps: VecDeque<Jump>,
current: usize,
}

impl JumpList {
pub fn new(initial: Jump) -> Self {
Self {
jumps: vec![initial],
current: 0,
}
let mut jumps = VecDeque::with_capacity(JUMP_LIST_CAPACITY);
jumps.push_back(initial);
Self { jumps, current: 0 }
}

pub fn push(&mut self, jump: Jump) {
self.jumps.truncate(self.current);
// don't push duplicates
if self.jumps.last() != Some(&jump) {
self.jumps.push(jump);
if self.jumps.back() != Some(&jump) {
// If the jumplist is full, drop the oldest item.
while self.jumps.len() >= JUMP_LIST_CAPACITY {
self.jumps.pop_front();
}

self.jumps.push_back(jump);
self.current = self.jumps.len();
}
}
Expand Down Expand Up @@ -57,8 +63,8 @@ impl JumpList {
self.jumps.retain(|(other_id, _)| other_id != doc_id);
}

pub fn get(&self) -> &[Jump] {
&self.jumps
pub fn iter(&self) -> impl Iterator<Item = &Jump> {
self.jumps.iter()
}

/// Applies a [`Transaction`] of changes to the jumplist.
Expand Down

0 comments on commit e38a279

Please sign in to comment.