store multiple snapshots on the document at once

Fixing autocomplete required moving the document savepoint before the
asynchronous completion request. However, this in turn causes new bugs:

If the completion popup is open, the savepoint is restored when the
popup closes (or another entry is selected). However, at that point
a new completion request might already have been created which
would have replaced the new savepoint (therefore leading to incorrectly
applied complies).

This commit fixes that bug by allowing in arbitrary number of
savepoints to be tracked on the document. The savepoints are reference
counted and therefore remain valid as long as any reference to them
remains. Weak reference are stored on the document and any reference
that can not be upgraded anymore (hence no strong reference remain)
are automatically discarded.
pull/6242/head
Pascal Kuthe 2 years ago committed by Blaž Hrastnik
parent 2588fa3710
commit e8898fd9a8

1
Cargo.lock generated

@ -1241,6 +1241,7 @@ dependencies = [
"libc", "libc",
"log", "log",
"once_cell", "once_cell",
"parking_lot",
"serde", "serde",
"serde_json", "serde_json",
"slotmap", "slotmap",

@ -4181,7 +4181,7 @@ pub fn completion(cx: &mut Context) {
iter.reverse(); iter.reverse();
let offset = iter.take_while(|ch| chars::char_is_word(*ch)).count(); let offset = iter.take_while(|ch| chars::char_is_word(*ch)).count();
let start_offset = cursor.saturating_sub(offset); let start_offset = cursor.saturating_sub(offset);
doc.savepoint(&view); let savepoint = doc.savepoint(view);
cx.callback( cx.callback(
future, future,
@ -4209,6 +4209,7 @@ pub fn completion(cx: &mut Context) {
let ui = compositor.find::<ui::EditorView>().unwrap(); let ui = compositor.find::<ui::EditorView>().unwrap();
ui.set_completion( ui.set_completion(
editor, editor,
savepoint,
items, items,
offset_encoding, offset_encoding,
start_offset, start_offset,

@ -1,12 +1,13 @@
use crate::compositor::{Component, Context, Event, EventResult}; use crate::compositor::{Component, Context, Event, EventResult};
use helix_view::{ use helix_view::{
document::SavePoint,
editor::CompleteAction, editor::CompleteAction,
theme::{Modifier, Style}, theme::{Modifier, Style},
ViewId, ViewId,
}; };
use tui::{buffer::Buffer as Surface, text::Span}; use tui::{buffer::Buffer as Surface, text::Span};
use std::borrow::Cow; use std::{borrow::Cow, sync::Arc};
use helix_core::{Change, Transaction}; use helix_core::{Change, Transaction};
use helix_view::{graphics::Rect, Document, Editor}; use helix_view::{graphics::Rect, Document, Editor};
@ -101,6 +102,7 @@ impl Completion {
pub fn new( pub fn new(
editor: &Editor, editor: &Editor,
savepoint: Arc<SavePoint>,
mut items: Vec<CompletionItem>, mut items: Vec<CompletionItem>,
offset_encoding: helix_lsp::OffsetEncoding, offset_encoding: helix_lsp::OffsetEncoding,
start_offset: usize, start_offset: usize,
@ -213,11 +215,10 @@ impl Completion {
let (view, doc) = current!(editor); let (view, doc) = current!(editor);
// if more text was entered, remove it // if more text was entered, remove it
doc.restore(view); doc.restore(view, &savepoint);
match event { match event {
PromptEvent::Abort => { PromptEvent::Abort => {
doc.restore(view);
editor.last_completion = None; editor.last_completion = None;
} }
PromptEvent::Update => { PromptEvent::Update => {
@ -235,7 +236,6 @@ impl Completion {
); );
// initialize a savepoint // initialize a savepoint
doc.savepoint(&view);
doc.apply(&transaction, view.id); doc.apply(&transaction, view.id);
editor.last_completion = Some(CompleteAction { editor.last_completion = Some(CompleteAction {

@ -940,17 +940,25 @@ impl EditorView {
} }
} }
#[allow(clippy::too_many_arguments)]
pub fn set_completion( pub fn set_completion(
&mut self, &mut self,
editor: &mut Editor, editor: &mut Editor,
savepoint: Arc<SavePoint>,
items: Vec<helix_lsp::lsp::CompletionItem>, items: Vec<helix_lsp::lsp::CompletionItem>,
offset_encoding: helix_lsp::OffsetEncoding, offset_encoding: helix_lsp::OffsetEncoding,
start_offset: usize, start_offset: usize,
trigger_offset: usize, trigger_offset: usize,
size: Rect, size: Rect,
) { ) {
let mut completion = let mut completion = Completion::new(
Completion::new(editor, items, offset_encoding, start_offset, trigger_offset); editor,
savepoint,
items,
offset_encoding,
start_offset,
trigger_offset,
);
if completion.is_empty() { if completion.is_empty() {
// skip if we got no completion results // skip if we got no completion results
@ -969,8 +977,6 @@ impl EditorView {
self.completion = None; self.completion = None;
// Clear any savepoints // Clear any savepoints
let doc = doc_mut!(editor);
doc.savepoint = None;
editor.clear_idle_timer(); // don't retrigger editor.clear_idle_timer(); // don't retrigger
} }

@ -43,6 +43,7 @@ toml = "0.7"
log = "~0.4" log = "~0.4"
which = "4.4" which = "4.4"
parking_lot = "0.12.1"
[target.'cfg(windows)'.dependencies] [target.'cfg(windows)'.dependencies]

@ -9,6 +9,7 @@ use helix_core::text_annotations::TextAnnotations;
use helix_core::Range; use helix_core::Range;
use helix_vcs::{DiffHandle, DiffProviderRegistry}; use helix_vcs::{DiffHandle, DiffProviderRegistry};
use ::parking_lot::Mutex;
use serde::de::{self, Deserialize, Deserializer}; use serde::de::{self, Deserialize, Deserializer};
use serde::Serialize; use serde::Serialize;
use std::borrow::Cow; use std::borrow::Cow;
@ -18,7 +19,7 @@ use std::fmt::Display;
use std::future::Future; use std::future::Future;
use std::path::{Path, PathBuf}; use std::path::{Path, PathBuf};
use std::str::FromStr; use std::str::FromStr;
use std::sync::Arc; use std::sync::{Arc, Weak};
use std::time::SystemTime; use std::time::SystemTime;
use helix_core::{ use helix_core::{
@ -105,6 +106,13 @@ pub struct DocumentSavedEvent {
pub type DocumentSavedEventResult = Result<DocumentSavedEvent, anyhow::Error>; pub type DocumentSavedEventResult = Result<DocumentSavedEvent, anyhow::Error>;
pub type DocumentSavedEventFuture = BoxFuture<'static, DocumentSavedEventResult>; pub type DocumentSavedEventFuture = BoxFuture<'static, DocumentSavedEventResult>;
#[derive(Debug)]
pub struct SavePoint {
/// The view this savepoint is associated with
pub view: ViewId,
revert: Mutex<Transaction>,
}
pub struct Document { pub struct Document {
pub(crate) id: DocumentId, pub(crate) id: DocumentId,
text: Rope, text: Rope,
@ -136,7 +144,7 @@ pub struct Document {
pub history: Cell<History>, pub history: Cell<History>,
pub config: Arc<dyn DynAccess<Config>>, pub config: Arc<dyn DynAccess<Config>>,
pub savepoint: Option<Transaction>, savepoints: Vec<Weak<SavePoint>>,
// Last time we wrote to the file. This will carry the time the file was last opened if there // Last time we wrote to the file. This will carry the time the file was last opened if there
// were no saves. // were no saves.
@ -389,7 +397,7 @@ impl Document {
diagnostics: Vec::new(), diagnostics: Vec::new(),
version: 0, version: 0,
history: Cell::new(History::default()), history: Cell::new(History::default()),
savepoint: None, savepoints: Vec::new(),
last_saved_time: SystemTime::now(), last_saved_time: SystemTime::now(),
last_saved_revision: 0, last_saved_revision: 0,
modified_since_accessed: false, modified_since_accessed: false,
@ -846,11 +854,18 @@ impl Document {
} }
// generate revert to savepoint // generate revert to savepoint
if self.savepoint.is_some() { if !self.savepoints.is_empty() {
take_with(&mut self.savepoint, |prev_revert| { let revert = transaction.invert(&old_doc);
let revert = transaction.invert(&old_doc); self.savepoints
Some(revert.compose(prev_revert.unwrap())) .retain_mut(|save_point| match save_point.upgrade() {
}); Some(savepoint) => {
let mut revert_to_savepoint = savepoint.revert.lock();
*revert_to_savepoint =
revert.clone().compose(mem::take(&mut revert_to_savepoint));
true
}
None => false,
})
} }
// update tree-sitter syntax tree // update tree-sitter syntax tree
@ -940,15 +955,39 @@ impl Document {
self.undo_redo_impl(view, false) self.undo_redo_impl(view, false)
} }
pub fn savepoint(&mut self) { /// Creates a reference counted snapshot (called savpepoint) of the document.
self.savepoint = ///
Some(Transaction::new(self.text()).with_selection(self.selection(view.id).clone())); /// The snapshot will remain valid (and updated) idenfinitly as long as ereferences to it exist.
/// Restoring the snapshot will restore the selection and the contents of the document to
/// the state it had when this function was called.
pub fn savepoint(&mut self, view: &View) -> Arc<SavePoint> {
let revert = Transaction::new(self.text()).with_selection(self.selection(view.id).clone());
let savepoint = Arc::new(SavePoint {
view: view.id,
revert: Mutex::new(revert),
});
self.savepoints.push(Arc::downgrade(&savepoint));
savepoint
} }
pub fn restore(&mut self, view: &mut View) { pub fn restore(&mut self, view: &mut View, savepoint: &SavePoint) {
if let Some(revert) = self.savepoint.take() { assert_eq!(
self.apply(&revert, view.id); savepoint.view, view.id,
} "Savepoint must not be used with a different view!"
);
// search and remove savepoint using a ptr comparison
// this avoids a deadlock as we need to lock the mutex
let savepoint_idx = self
.savepoints
.iter()
.position(|savepoint_ref| savepoint_ref.as_ptr() == savepoint as *const _)
.expect("Savepoint must belong to this document");
let savepoint_ref = self.savepoints.remove(savepoint_idx);
let mut revert = savepoint.revert.lock();
self.apply(&revert, view.id);
*revert = Transaction::new(self.text()).with_selection(self.selection(view.id).clone());
self.savepoints.push(savepoint_ref)
} }
fn earlier_later_impl(&mut self, view: &mut View, uk: UndoKind, earlier: bool) -> bool { fn earlier_later_impl(&mut self, view: &mut View, uk: UndoKind, earlier: bool) -> bool {

Loading…
Cancel
Save