5406e9f · helix · git.iko.soy

git.iko.soy/helix

correctly handle completion rerequest

commit5406e9f629313221c8ae97583393dfd6221d3dfc authorPascal Kuthe <pascal.kuthe@semimod.de> dateTue 4 Apr 2023, 23:34 (4 years ago) parent91da0dc

5 files changed+127−56

compare with master

unifiedsplit

helix-term/src/commands.rs+7−2 helix-term/src/ui/completion.rs+29−17 helix-term/src/ui/editor.rs+42−15 helix-view/src/document.rs+40−18 helix-view/src/editor.rs+9−4

helix-term/src/commands.rs+7−2

@@ -33,7 +33,7 @@ use helix_core::{

3333use helix_view::{

3434 clipboard::ClipboardType,

3535 document::{FormatterError, Mode, SCRATCH_BUFFER_NAME},

36− editor::{Action, Motion},

36+ editor::{Action, CompleteAction, Motion},

3737 info::Info,

3838 input::KeyEvent,

3939 keyboard::KeyCode,

@@ -4254,7 +4254,12 @@ pub fn completion(cx: &mut Context) {

42544254 iter.reverse();

42554255 let offset = iter.take_while(|ch| chars::char_is_word(*ch)).count();

42564256 let start_offset = cursor.saturating_sub(offset);

4257− let savepoint = doc.savepoint(view);

4257+ let savepoint = if let Some(CompleteAction::Selected { savepoint }) = &cx.editor.last_completion

4258+ {

4259+ savepoint.clone()

4260+ } else {

4261+ doc.savepoint(view)

4262+ };

42584263

42594264 let trigger_doc = doc.id();

42604265 let trigger_view = view.id;

helix-term/src/ui/completion.rs+29−17

@@ -209,14 +209,27 @@ impl Completion {

209209

210210 let (view, doc) = current!(editor);

211211

212− // if more text was entered, remove it

213− doc.restore(view, &savepoint);

214−

215212 match event {

216− PromptEvent::Abort => {

217− editor.last_completion = None;

218− }

213+ PromptEvent::Abort => {}

219214 PromptEvent::Update => {

215+ // Update creates "ghost" transactiosn which are not send to the

216+ // lsp server to avoid messing up rerequesting completions. Once a

217+ // completion has been selected (with) tab it's always accepted whenever anything

218+ // is typed. The only way to avoid that is to explicitly abort the completion

219+ // with esc/c-c. This will remove the "ghost" transaction.

220+ //

221+ // The ghost transaction is modeled with a transaction that is not send to the LS.

222+ // (apply_temporary) and a savepoint. It's extremly important this savepoint is restored

223+ // (also without sending the transaction to the LS) *before any further transaction is applied*.

224+ // Otherwise incremental sync breaks (since the state of the LS doesn't match the state the transaction

225+ // is applied to).

226+ if editor.last_completion.is_none() {

227+ editor.last_completion = Some(CompleteAction::Selected {

228+ savepoint: doc.savepoint(view),

229+ })

230+ }

231+ // if more text was entered, remove it

232+ doc.restore(view, &savepoint, false);

220233 // always present here

221234 let item = item.unwrap();

222235

@@ -229,19 +242,20 @@ impl Completion {

229242 true,

230243 replace_mode,

231244 );

232−

233− // initialize a savepoint

234− doc.apply(&transaction, view.id);

235−

236− editor.last_completion = Some(CompleteAction {

237− trigger_offset,

238− changes: completion_changes(&transaction, trigger_offset),

239− });

245+ doc.apply_temporary(&transaction, view.id);

240246 }

241247 PromptEvent::Validate => {

248+ if let Some(CompleteAction::Selected { savepoint }) =

249+ editor.last_completion.take()

250+ {

251+ doc.restore(view, &savepoint, false);

252+ }

242253 // always present here

243254 let item = item.unwrap();

244255

256+

257+ // if more text was entered, remove it

258+ doc.restore(view, &savepoint, true);

245259 let transaction = item_to_transaction(

246260 doc,

247261 view.id,

@@ -251,10 +265,9 @@ impl Completion {

251265 false,

252266 replace_mode,

253267 );

254−

255268 doc.apply(&transaction, view.id);

256269

257− editor.last_completion = Some(CompleteAction {

270+ editor.last_completion = Some(CompleteAction::Applied {

258271 trigger_offset,

259272 changes: completion_changes(&transaction, trigger_offset),

260273 });

@@ -270,7 +283,6 @@ impl Completion {

270283 } else {

271284 Self::resolve_completion_item(doc, item.clone())

272285 };

273−

274286 if let Some(additional_edits) = resolved_item

275287 .as_ref()

276288 .and_then(|item| item.additional_text_edits.as_ref())

helix-term/src/ui/editor.rs+42−15

@@ -19,7 +19,7 @@ use helix_core::{

1919 syntax::{self, HighlightEvent},

2020 text_annotations::TextAnnotations,

2121 unicode::width::UnicodeWidthStr,

22− visual_offset_from_block, Position, Range, Selection, Transaction,

22+ visual_offset_from_block, Change, Position, Range, Selection, Transaction,

2323};

2424use helix_view::{

2525 document::{Mode, SavePoint, SCRATCH_BUFFER_NAME},

@@ -48,7 +48,10 @@ pub struct EditorView {

4848#[derive(Debug, Clone)]

4949pub enum InsertEvent {

5050 Key(KeyEvent),

51− CompletionApply(CompleteAction),

51+ CompletionApply {

52+ trigger_offset: usize,

53+ changes: Vec<Change>,

54+ },

5255 TriggerCompletion,

5356 RequestCompletion,

5457}

@@ -813,7 +816,7 @@ impl EditorView {

813816 }

814817 (Mode::Insert, Mode::Normal) => {

815818 // if exiting insert mode, remove completion

816− self.completion = None;

819+ self.clear_completion(cxt.editor);

817820 cxt.editor.completion_request_handle = None;

818821

819822 // TODO: Use an on_mode_change hook to remove signature help

@@ -891,22 +894,25 @@ impl EditorView {

891894 for key in self.last_insert.1.clone() {

892895 match key {

893896 InsertEvent::Key(key) => self.insert_mode(cxt, key),

894− InsertEvent::CompletionApply(compl) => {

897+ InsertEvent::CompletionApply {

898+ trigger_offset,

899+ changes,

900+ } => {

895901 let (view, doc) = current!(cxt.editor);

896902

897903 if let Some(last_savepoint) = last_savepoint.as_deref() {

898− doc.restore(view, last_savepoint);

904+ doc.restore(view, last_savepoint, true);

899905 }

900906

901907 let text = doc.text().slice(..);

902908 let cursor = doc.selection(view.id).primary().cursor(text);

903909

904910 let shift_position =

905− |pos: usize| -> usize { pos + cursor - compl.trigger_offset };

911+ |pos: usize| -> usize { pos + cursor - trigger_offset };

906912

907913 let tx = Transaction::change(

908914 doc.text(),

909− compl.changes.iter().cloned().map(|(start, end, t)| {

915+ changes.iter().cloned().map(|(start, end, t)| {

910916 (shift_position(start), shift_position(end), t)

911917 }),

912918 );

@@ -979,6 +985,21 @@ impl EditorView {

979985

980986 pub fn clear_completion(&mut self, editor: &mut Editor) {

981987 self.completion = None;

988+ if let Some(last_completion) = editor.last_completion.take() {

989+ match last_completion {

990+ CompleteAction::Applied {

991+ trigger_offset,

992+ changes,

993+ } => self.last_insert.1.push(InsertEvent::CompletionApply {

994+ trigger_offset,

995+ changes,

996+ }),

997+ CompleteAction::Selected { savepoint } => {

998+ let (view, doc) = current!(editor);

999+ doc.restore(view, &savepoint, false);

1000+ }

1001+ }

1002+ }

9821003

9831004 // Clear any savepoints

9841005 editor.clear_idle_timer(); // don't retrigger

@@ -1265,12 +1286,22 @@ impl Component for EditorView {

12651286 jobs: cx.jobs,

12661287 scroll: None,

12671288 };

1268− completion.handle_event(event, &mut cx)

1269− };

12701289

1271− if let EventResult::Consumed(callback) = res {

1272− consumed = true;

1290+ if let EventResult::Consumed(callback) =

1291+ completion.handle_event(event, &mut cx)

1292+ {

1293+ consumed = true;

1294+ Some(callback)

1295+ } else if let EventResult::Consumed(callback) =

1296+ completion.handle_event(&Event::Key(key!(Enter)), &mut cx)

1297+ {

1298+ Some(callback)

1299+ } else {

1300+ None

1301+ }

1302+ };

12731303

1304+ if let Some(callback) = res {

12741305 if callback.is_some() {

12751306 // assume close_fn

12761307 self.clear_completion(cx.editor);

@@ -1286,10 +1317,6 @@ impl Component for EditorView {

12861317

12871318 // if completion didn't take the event, we pass it onto commands

12881319 if !consumed {

1289− if let Some(compl) = cx.editor.last_completion.take() {

1290− self.last_insert.1.push(InsertEvent::CompletionApply(compl));

1291− }

1292−

12931320 self.insert_mode(&mut cx, key);

12941321

12951322 // record last_insert key

helix-view/src/document.rs+40−18

@@ -1034,7 +1034,12 @@ impl Document {

10341034 }

10351035

10361036 /// Apply a [`Transaction`] to the [`Document`] to change its text.

1037− fn apply_impl(&mut self, transaction: &Transaction, view_id: ViewId) -> bool {

1037+ fn apply_impl(

1038+ &mut self,

1039+ transaction: &Transaction,

1040+ view_id: ViewId,

1041+ emit_lsp_notification: bool,

1042+ ) -> bool {

10381043 use helix_core::Assoc;

10391044

10401045 let old_doc = self.text().clone();

@@ -1130,25 +1135,31 @@ impl Document {

11301135 apply_inlay_hint_changes(padding_after_inlay_hints);

11311136 }

11321137

1133− // emit lsp notification

1134− if let Some(language_server) = self.language_server() {

1135− let notify = language_server.text_document_did_change(

1136− self.versioned_identifier(),

1137− &old_doc,

1138− self.text(),

1139− changes,

1140− );

1138+ if emit_lsp_notification {

1139+ // emit lsp notification

1140+ if let Some(language_server) = self.language_server() {

1141+ let notify = language_server.text_document_did_change(

1142+ self.versioned_identifier(),

1143+ &old_doc,

1144+ self.text(),

1145+ changes,

1146+ );

11411147

1142− if let Some(notify) = notify {

1143− tokio::spawn(notify);

1148+ if let Some(notify) = notify {

1149+ tokio::spawn(notify);

1150+ }

11441151 }

11451152 }

11461153 }

11471154 success

11481155 }

11491156

1150− /// Apply a [`Transaction`] to the [`Document`] to change its text.

1151− pub fn apply(&mut self, transaction: &Transaction, view_id: ViewId) -> bool {

1157+ fn apply_inner(

1158+ &mut self,

1159+ transaction: &Transaction,

1160+ view_id: ViewId,

1161+ emit_lsp_notification: bool,

1162+ ) -> bool {

11521163 // store the state just before any changes are made. This allows us to undo to the

11531164 // state just before a transaction was applied.

11541165 if self.changes.is_empty() && !transaction.changes().is_empty() {

@@ -1158,7 +1169,7 @@ impl Document {

11581169 });

11591170 }

11601171

1161− let success = self.apply_impl(transaction, view_id);

1172+ let success = self.apply_impl(transaction, view_id, emit_lsp_notification);

11621173

11631174 if !transaction.changes().is_empty() {

11641175 // Compose this transaction with the previous one

@@ -1168,12 +1179,23 @@ impl Document {

11681179 }

11691180 success

11701181 }

1182+ /// Apply a [`Transaction`] to the [`Document`] to change its text.

1183+ pub fn apply(&mut self, transaction: &Transaction, view_id: ViewId) -> bool {

1184+ self.apply_inner(transaction, view_id, true)

1185+ }

1186+

1187+ /// Apply a [`Transaction`] to the [`Document`] to change its text.

1188+ /// without notifying the language servers. This is useful for temporary transactions

1189+ /// that must not influence the server.

1190+ pub fn apply_temporary(&mut self, transaction: &Transaction, view_id: ViewId) -> bool {

1191+ self.apply_inner(transaction, view_id, false)

1192+ }

11711193

11721194 fn undo_redo_impl(&mut self, view: &mut View, undo: bool) -> bool {

11731195 let mut history = self.history.take();

11741196 let txn = if undo { history.undo() } else { history.redo() };

11751197 let success = if let Some(txn) = txn {

1176− self.apply_impl(txn, view.id)

1198+ self.apply_impl(txn, view.id, true)

11771199 } else {

11781200 false

11791201 };

@@ -1213,7 +1235,7 @@ impl Document {

12131235 savepoint

12141236 }

12151237

1216− pub fn restore(&mut self, view: &mut View, savepoint: &SavePoint) {

1238+ pub fn restore(&mut self, view: &mut View, savepoint: &SavePoint, emit_lsp_notification: bool) {

12171239 assert_eq!(

12181240 savepoint.view, view.id,

12191241 "Savepoint must not be used with a different view!"

@@ -1228,7 +1250,7 @@ impl Document {

12281250

12291251 let savepoint_ref = self.savepoints.remove(savepoint_idx);

12301252 let mut revert = savepoint.revert.lock();

1231− self.apply(&revert, view.id);

1253+ self.apply_inner(&revert, view.id, emit_lsp_notification);

12321254 *revert = Transaction::new(self.text()).with_selection(self.selection(view.id).clone());

12331255 self.savepoints.push(savepoint_ref)

12341256 }

@@ -1241,7 +1263,7 @@ impl Document {

12411263 };

12421264 let mut success = false;

12431265 for txn in txns {

1244− if self.apply_impl(&txn, view.id) {

1266+ if self.apply_impl(&txn, view.id, true) {

12451267 success = true;

12461268 }

12471269 }

helix-view/src/editor.rs+9−4

@@ -1,7 +1,7 @@

11use crate::{

22 align_view,

33 clipboard::{get_clipboard_provider, ClipboardProvider},

4− document::{DocumentSavedEventFuture, DocumentSavedEventResult, Mode},

4+ document::{DocumentSavedEventFuture, DocumentSavedEventResult, Mode, SavePoint},

55 graphics::{CursorKind, Rect},

66 info::Info,

77 input::KeyEvent,

@@ -906,9 +906,14 @@ enum ThemeAction {

906906}

907907

908908#[derive(Debug, Clone)]

909−pub struct CompleteAction {

910− pub trigger_offset: usize,

911− pub changes: Vec<Change>,

909+pub enum CompleteAction {

910+ Applied {

911+ trigger_offset: usize,

912+ changes: Vec<Change>,

913+ },

914+ /// A savepoint of the currently active completion. The completion

915+ /// MUST be restored before sending any event to the LSP

916+ Selected { savepoint: Arc<SavePoint> },

912917}

913918

914919#[derive(Debug, Copy, Clone)]