From b6a4927f00cba3d55a6bad93bfd93346a876ca74 Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Sun, 12 Mar 2023 01:01:15 +0100 Subject: [PATCH] discard outdated workspace edits recived from the LS --- helix-term/src/application.rs | 11 ++-- helix-term/src/commands/lsp.rs | 117 ++++++++++++++++++++++++--------- 2 files changed, 94 insertions(+), 34 deletions(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 8803792aa..95faa01b0 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -999,16 +999,19 @@ impl Application { Ok(serde_json::Value::Null) } Ok(MethodCall::ApplyWorkspaceEdit(params)) => { - apply_workspace_edit( + let res = apply_workspace_edit( &mut self.editor, helix_lsp::OffsetEncoding::Utf8, ¶ms.edit, ); Ok(json!(lsp::ApplyWorkspaceEditResponse { - applied: true, - failure_reason: None, - failed_change: None, + applied: res.is_ok(), + failure_reason: res.as_ref().err().map(|err| err.kind.to_string()), + failed_change: res + .as_ref() + .err() + .map(|err| err.failed_change_idx as u32), })) } Ok(MethodCall::WorkspaceFolders) => { diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs index 13d62b5db..0b0d1db4d 100644 --- a/helix-term/src/commands/lsp.rs +++ b/helix-term/src/commands/lsp.rs @@ -651,7 +651,7 @@ pub fn code_action(cx: &mut Context) { log::debug!("code action: {:?}", code_action); if let Some(ref workspace_edit) = code_action.edit { log::debug!("edit: {:?}", workspace_edit); - apply_workspace_edit(editor, offset_encoding, workspace_edit); + let _ = apply_workspace_edit(editor, offset_encoding, workspace_edit); } // if code action provides both edit and command first the edit @@ -757,19 +757,50 @@ pub fn apply_document_resource_op(op: &lsp::ResourceOp) -> std::io::Result<()> { } } +#[derive(Debug)] +pub struct ApplyEditError { + pub kind: ApplyEditErrorKind, + pub failed_change_idx: usize, +} + +#[derive(Debug)] +pub enum ApplyEditErrorKind { + DocumentChanged, + FileNotFound, + UnknownURISchema, + IoError(std::io::Error), + // TODO: check edits before applying and propagate failure + // InvalidEdit, +} + +impl ToString for ApplyEditErrorKind { + fn to_string(&self) -> String { + match self { + ApplyEditErrorKind::DocumentChanged => "document has changed".to_string(), + ApplyEditErrorKind::FileNotFound => "file not found".to_string(), + ApplyEditErrorKind::UnknownURISchema => "URI schema not supported".to_string(), + ApplyEditErrorKind::IoError(err) => err.to_string(), + } + } +} + +///TODO make this transactional (and set failureMode to transactional) pub fn apply_workspace_edit( editor: &mut Editor, offset_encoding: OffsetEncoding, workspace_edit: &lsp::WorkspaceEdit, -) { - let mut apply_edits = |uri: &helix_lsp::Url, text_edits: Vec| { +) -> Result<(), ApplyEditError> { + let mut apply_edits = |uri: &helix_lsp::Url, + version: Option, + text_edits: Vec| + -> Result<(), ApplyEditErrorKind> { let path = match uri.to_file_path() { Ok(path) => path, Err(_) => { let err = format!("unable to convert URI to filepath: {}", uri); log::error!("{}", err); editor.set_error(err); - return; + return Err(ApplyEditErrorKind::UnknownURISchema); } }; @@ -780,11 +811,19 @@ pub fn apply_workspace_edit( let err = format!("failed to open document: {}: {}", uri, err); log::error!("{}", err); editor.set_error(err); - return; + return Err(ApplyEditErrorKind::FileNotFound); } }; let doc = doc_mut!(editor, &doc_id); + if let Some(version) = version { + if version != doc.version() { + let err = format!("outdated workspace edit for {path:?}"); + log::error!("{err}, expected {} but got {version}", doc.version()); + editor.set_error(err); + return Err(ApplyEditErrorKind::DocumentChanged); + } + } // Need to determine a view for apply/append_changes_to_history let selections = doc.selections(); @@ -808,31 +847,13 @@ pub fn apply_workspace_edit( let view = view_mut!(editor, view_id); doc.apply(&transaction, view.id); doc.append_changes_to_history(view); + Ok(()) }; - if let Some(ref changes) = workspace_edit.changes { - log::debug!("workspace changes: {:?}", changes); - for (uri, text_edits) in changes { - let text_edits = text_edits.to_vec(); - apply_edits(uri, text_edits) - } - return; - // Not sure if it works properly, it'll be safer to just panic here to avoid breaking some parts of code on which code actions will be used - // TODO: find some example that uses workspace changes, and test it - // for (url, edits) in changes.iter() { - // let file_path = url.origin().ascii_serialization(); - // let file_path = std::path::PathBuf::from(file_path); - // let file = std::fs::File::open(file_path).unwrap(); - // let mut text = Rope::from_reader(file).unwrap(); - // let transaction = edits_to_changes(&text, edits); - // transaction.apply(&mut text); - // } - } - if let Some(ref document_changes) = workspace_edit.document_changes { match document_changes { lsp::DocumentChanges::Edits(document_edits) => { - for document_edit in document_edits { + for (i, document_edit) in document_edits.iter().enumerate() { let edits = document_edit .edits .iter() @@ -844,15 +865,26 @@ pub fn apply_workspace_edit( }) .cloned() .collect(); - apply_edits(&document_edit.text_document.uri, edits); + apply_edits( + &document_edit.text_document.uri, + document_edit.text_document.version, + edits, + ) + .map_err(|kind| ApplyEditError { + kind, + failed_change_idx: i, + })?; } } lsp::DocumentChanges::Operations(operations) => { log::debug!("document changes - operations: {:?}", operations); - for operation in operations { + for (i, operation) in operations.iter().enumerate() { match operation { lsp::DocumentChangeOperation::Op(op) => { - apply_document_resource_op(op).unwrap(); + apply_document_resource_op(op).map_err(|io| ApplyEditError { + kind: ApplyEditErrorKind::IoError(io), + failed_change_idx: i, + })?; } lsp::DocumentChangeOperation::Edit(document_edit) => { @@ -867,13 +899,36 @@ pub fn apply_workspace_edit( }) .cloned() .collect(); - apply_edits(&document_edit.text_document.uri, edits); + apply_edits( + &document_edit.text_document.uri, + document_edit.text_document.version, + edits, + ) + .map_err(|kind| ApplyEditError { + kind, + failed_change_idx: i, + })?; } } } } } + + return Ok(()); } + + if let Some(ref changes) = workspace_edit.changes { + log::debug!("workspace changes: {:?}", changes); + for (i, (uri, text_edits)) in changes.iter().enumerate() { + let text_edits = text_edits.to_vec(); + apply_edits(uri, None, text_edits).map_err(|kind| ApplyEditError { + kind, + failed_change_idx: i, + })?; + } + } + + Ok(()) } fn goto_impl( @@ -1297,7 +1352,9 @@ pub fn rename_symbol(cx: &mut Context) { } }; match block_on(future) { - Ok(edits) => apply_workspace_edit(cx.editor, offset_encoding, &edits), + Ok(edits) => { + let _ = apply_workspace_edit(cx.editor, offset_encoding, &edits); + } Err(err) => cx.editor.set_error(err.to_string()), } },