Skip to content

Commit 8102c32

Browse files
Limit the number of items in the jumplist (#4750)
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.
1 parent 4443885 commit 8102c32

File tree

2 files changed

+16
-11
lines changed

2 files changed

+16
-11
lines changed

helix-term/src/commands.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2427,7 +2427,6 @@ fn jumplist_picker(cx: &mut Context) {
24272427
.views()
24282428
.flat_map(|(view, _)| {
24292429
view.jumps
2430-
.get()
24312430
.iter()
24322431
.map(|(doc_id, selection)| new_meta(view, *doc_id, selection.clone()))
24332432
})

helix-view/src/view.rs

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,29 +3,35 @@ use helix_core::{
33
pos_at_visual_coords, visual_coords_at_pos, Position, RopeSlice, Selection, Transaction,
44
};
55

6-
use std::fmt;
6+
use std::{collections::VecDeque, fmt};
7+
8+
const JUMP_LIST_CAPACITY: usize = 30;
79

810
type Jump = (DocumentId, Selection);
911

1012
#[derive(Debug, Clone)]
1113
pub struct JumpList {
12-
jumps: Vec<Jump>,
14+
jumps: VecDeque<Jump>,
1315
current: usize,
1416
}
1517

1618
impl JumpList {
1719
pub fn new(initial: Jump) -> Self {
18-
Self {
19-
jumps: vec![initial],
20-
current: 0,
21-
}
20+
let mut jumps = VecDeque::with_capacity(JUMP_LIST_CAPACITY);
21+
jumps.push_back(initial);
22+
Self { jumps, current: 0 }
2223
}
2324

2425
pub fn push(&mut self, jump: Jump) {
2526
self.jumps.truncate(self.current);
2627
// don't push duplicates
27-
if self.jumps.last() != Some(&jump) {
28-
self.jumps.push(jump);
28+
if self.jumps.back() != Some(&jump) {
29+
// If the jumplist is full, drop the oldest item.
30+
while self.jumps.len() >= JUMP_LIST_CAPACITY {
31+
self.jumps.pop_front();
32+
}
33+
34+
self.jumps.push_back(jump);
2935
self.current = self.jumps.len();
3036
}
3137
}
@@ -57,8 +63,8 @@ impl JumpList {
5763
self.jumps.retain(|(other_id, _)| other_id != doc_id);
5864
}
5965

60-
pub fn get(&self) -> &[Jump] {
61-
&self.jumps
66+
pub fn iter(&self) -> impl Iterator<Item = &Jump> {
67+
self.jumps.iter()
6268
}
6369

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

0 commit comments

Comments
 (0)