From 8102c3224f8cbf4a8e679377fef6ea98ed337cba Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Mon, 21 Nov 2022 19:54:50 -0600 Subject: [PATCH] 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 ``/`` 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. --- helix-term/src/commands.rs | 1 - helix-view/src/view.rs | 26 ++++++++++++++++---------- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index bf8229441..f384612bf 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -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())) }) diff --git a/helix-view/src/view.rs b/helix-view/src/view.rs index 6da4df1f0..8aa4760dc 100644 --- a/helix-view/src/view.rs +++ b/helix-view/src/view.rs @@ -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, + jumps: VecDeque, 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(); } } @@ -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 { + self.jumps.iter() } /// Applies a [`Transaction`] of changes to the jumplist.