5406e9f · helix · git.iko.soy

git.iko.soy/helix

on master help sign in

correctly handle completion rerequest

5406e9fPascal Kuthe4 years ago

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

compare with master

unifiedsplit

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

33use helix_view::{

34 clipboard::ClipboardType,

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

36− editor::{Action, Motion},

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

37 info::Info,

38 input::KeyEvent,

39 keyboard::KeyCode,

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

4254 iter.reverse();

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

4256 let start_offset = cursor.saturating_sub(offset);

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

4258+ {

4259+ savepoint.clone()

4260+ } else {

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

4261+ doc.savepoint(view)

4262+ };

4258

4263

4259 let trigger_doc = doc.id();

4264 let trigger_doc = doc.id();

4260 let trigger_view = view.id;

4265 let trigger_view = view.id;

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

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

209

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

211

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

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

214−

215 match event {

212 match event {

216− PromptEvent::Abort => {

213+ PromptEvent::Abort => {}

217− editor.last_completion = None;

218− }

219 PromptEvent::Update => {

214 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);

220 // always present here

233 // always present here

221 let item = item.unwrap();

234 let item = item.unwrap();

222

235

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

229 true,

242 true,

230 replace_mode,

243 replace_mode,

231 );

244 );

232−

233− // initialize a savepoint

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

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

235−

236− editor.last_completion = Some(CompleteAction {

237− trigger_offset,

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

239− });

240 }

246 }

241 PromptEvent::Validate => {

247 PromptEvent::Validate => {

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

249+ editor.last_completion.take()

250+ {

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

252+ }

242 // always present here

253 // always present here

243 let item = item.unwrap();

254 let item = item.unwrap();

244

255

256+

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

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

245 let transaction = item_to_transaction(

259 let transaction = item_to_transaction(

246 doc,

260 doc,

247 view.id,

261 view.id,

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

251 false,

265 false,

252 replace_mode,

266 replace_mode,

253 );

267 );

254−

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

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

256

269

257− editor.last_completion = Some(CompleteAction {

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

258 trigger_offset,

271 trigger_offset,

259 changes: completion_changes(&transaction, trigger_offset),

272 changes: completion_changes(&transaction, trigger_offset),

260 });

273 });

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

270 } else {

283 } else {

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

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

272 };

285 };

273−

274 if let Some(additional_edits) = resolved_item

286 if let Some(additional_edits) = resolved_item

275 .as_ref()

287 .as_ref()

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

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

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

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

19 syntax::{self, HighlightEvent},

20 text_annotations::TextAnnotations,

21 unicode::width::UnicodeWidthStr,

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

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

23};

24use helix_view::{

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

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

48#[derive(Debug, Clone)]

49pub enum InsertEvent {

50 Key(KeyEvent),

51− CompletionApply(CompleteAction),

51+ CompletionApply {

52+ trigger_offset: usize,

53+ changes: Vec<Change>,

54+ },

52 TriggerCompletion,

55 TriggerCompletion,

53 RequestCompletion,

56 RequestCompletion,

54}

57}

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

813 }

816 }

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

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

815 // if exiting insert mode, remove completion

818 // if exiting insert mode, remove completion

816− self.completion = None;

819+ self.clear_completion(cxt.editor);

817 cxt.editor.completion_request_handle = None;

820 cxt.editor.completion_request_handle = None;

818

821

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

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

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

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

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

892 match key {

895 match key {

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

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

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

897+ InsertEvent::CompletionApply {

898+ trigger_offset,

899+ changes,

900+ } => {

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

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

896

902

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

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

898− doc.restore(view, last_savepoint);

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

899 }

905 }

900

906

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

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

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

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

903

909

904 let shift_position =

910 let shift_position =

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

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

906

912

907 let tx = Transaction::change(

913 let tx = Transaction::change(

908 doc.text(),

914 doc.text(),

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

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

910 (shift_position(start), shift_position(end), t)

916 (shift_position(start), shift_position(end), t)

911 }),

917 }),

912 );

918 );

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

979

985

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

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

981 self.completion = None;

987 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+ }

982

1003

983 // Clear any savepoints

1004 // Clear any savepoints

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

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

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

1265 jobs: cx.jobs,

1286 jobs: cx.jobs,

1266 scroll: None,

1287 scroll: None,

1267 };

1288 };

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

1269− };

1270

1289

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

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

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

1292+ {

1272− consumed = true;

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+ };

1273

1303

1304+ if let Some(callback) = res {

1274 if callback.is_some() {

1305 if callback.is_some() {

1275 // assume close_fn

1306 // assume close_fn

1276 self.clear_completion(cx.editor);

1307 self.clear_completion(cx.editor);

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

1286

1317

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

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

1288 if !consumed {

1319 if !consumed {

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

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

1291− }

1292−

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

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

1294

1321

1295 // record last_insert key

1322 // record last_insert key

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

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

1034 }

1035

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

1037+ fn apply_impl(

1038+ &mut self,

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

1039+ transaction: &Transaction,

1040+ view_id: ViewId,

1041+ emit_lsp_notification: bool,

1042+ ) -> bool {

1038 use helix_core::Assoc;

1043 use helix_core::Assoc;

1039

1044

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

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

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

1130 apply_inlay_hint_changes(padding_after_inlay_hints);

1135 apply_inlay_hint_changes(padding_after_inlay_hints);

1131 }

1136 }

1132

1137

1138+ if emit_lsp_notification {

1133− // emit lsp notification

1139+ // emit lsp notification

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

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

1135− let notify = language_server.text_document_did_change(

1141+ let notify = language_server.text_document_did_change(

1136− self.versioned_identifier(),

1142+ self.versioned_identifier(),

1137− &old_doc,

1143+ &old_doc,

1138− self.text(),

1144+ self.text(),

1139− changes,

1145+ changes,

1140− );

1146+ );

1141

1147

1142− if let Some(notify) = notify {

1148+ if let Some(notify) = notify {

1143− tokio::spawn(notify);

1149+ tokio::spawn(notify);

1150+ }

1144 }

1151 }

1145 }

1152 }

1146 }

1153 }

1147 success

1154 success

1148 }

1155 }

1149

1156

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

1157+ fn apply_inner(

1158+ &mut self,

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

1159+ transaction: &Transaction,

1160+ view_id: ViewId,

1161+ emit_lsp_notification: bool,

1162+ ) -> bool {

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

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

1153 // state just before a transaction was applied.

1164 // state just before a transaction was applied.

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

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

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

1158 });

1169 });

1159 }

1170 }

1160

1171

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

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

1162

1173

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

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

1164 // Compose this transaction with the previous one

1175 // Compose this transaction with the previous one

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

1168 }

1179 }

1169 success

1180 success

1170 }

1181 }

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+ }

1171

1193

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

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

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

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

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

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

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

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

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

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

1177 } else {

1199 } else {

1178 false

1200 false

1179 };

1201 };

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

1213 savepoint

1235 savepoint

1214 }

1236 }

1215

1237

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) {

1217 assert_eq!(

1239 assert_eq!(

1218 savepoint.view, view.id,

1240 savepoint.view, view.id,

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

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

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

1228

1250

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

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

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

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

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

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

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

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

1233 self.savepoints.push(savepoint_ref)

1255 self.savepoints.push(savepoint_ref)

1234 }

1256 }

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

1241 };

1263 };

1242 let mut success = false;

1264 let mut success = false;

1243 for txn in txns {

1265 for txn in txns {

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

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

1245 success = true;

1267 success = true;

1246 }

1268 }

1247 }

1269 }

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

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

1use crate::{

2 align_view,

3 clipboard::{get_clipboard_provider, ClipboardProvider},

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

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

5 graphics::{CursorKind, Rect},

6 info::Info,

7 input::KeyEvent,

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

906}

907

908#[derive(Debug, Clone)]

909−pub struct CompleteAction {

909+pub enum CompleteAction {

910+ Applied {

910− pub trigger_offset: usize,

911+ trigger_offset: usize,

911− pub changes: Vec<Change>,

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> },

912}

917}

913

918

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

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