From d706194597d462fbaeb1ef55e2e8fb6eae38d2f3 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Sun, 10 Apr 2022 11:05:47 -0400 Subject: [PATCH 01/37] chore(write): serialize write operations within a Document The way that document writes are handled are by submitting them to the async job pool, which are all executed opportunistically out of order. It was discovered that this can lead to write inconsistencies when there are multiple writes to the same file in quick succession. This seeks to fix this problem by removing document writes from the general pool of jobs and into its own specialized event. Now when a user submits a write with one of the write commands, a request is simply queued up in a new mpsc channel that each Document makes to handle its own writes. This way, if multiple writes are submitted on the same document, they are executed in order, while still allowing concurrent writes for different documents. --- helix-term/src/application.rs | 135 +++++++++++++++++++++++-------- helix-term/src/commands.rs | 2 +- helix-term/src/commands/typed.rs | 13 +-- helix-term/tests/test/write.rs | 2 - helix-view/src/document.rs | 128 ++++++++++++++++++++++++++--- helix-view/src/editor.rs | 81 ++++++++++++++++--- 6 files changed, 297 insertions(+), 64 deletions(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 4bb36b59..a9e25d08 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -6,7 +6,14 @@ use helix_core::{ pos_at_coords, syntax, Selection, }; use helix_lsp::{lsp, util::lsp_pos_to_pos, LspProgressMap}; -use helix_view::{align_view, editor::ConfigEvent, theme, tree::Layout, Align, Editor}; +use helix_view::{ + align_view, + document::DocumentSaveEventResult, + editor::{ConfigEvent, EditorEvent}, + theme, + tree::Layout, + Align, Editor, +}; use serde_json::json; use crate::{ @@ -19,7 +26,7 @@ use crate::{ ui::{self, overlay::overlayed}, }; -use log::{error, warn}; +use log::{debug, error, warn}; use std::{ io::{stdin, stdout, Write}, sync::Arc, @@ -294,26 +301,6 @@ impl Application { Some(signal) = self.signals.next() => { self.handle_signals(signal).await; } - Some((id, call)) = self.editor.language_servers.incoming.next() => { - self.handle_language_server_message(call, id).await; - // limit render calls for fast language server messages - let last = self.editor.language_servers.incoming.is_empty(); - - if last || self.last_render.elapsed() > LSP_DEADLINE { - self.render(); - self.last_render = Instant::now(); - } - } - Some(payload) = self.editor.debugger_events.next() => { - let needs_render = self.editor.handle_debugger_message(payload).await; - if needs_render { - self.render(); - } - } - Some(config_event) = self.editor.config_events.1.recv() => { - self.handle_config_events(config_event); - self.render(); - } Some(callback) = self.jobs.futures.next() => { self.jobs.handle_callback(&mut self.editor, &mut self.compositor, callback); self.render(); @@ -322,20 +309,47 @@ impl Application { self.jobs.handle_callback(&mut self.editor, &mut self.compositor, callback); self.render(); } - _ = &mut self.editor.idle_timer => { - // idle timeout - self.editor.clear_idle_timer(); - self.handle_idle_timeout(); + event = self.editor.wait_event() => { + match event { + EditorEvent::DocumentSave(event) => { + self.handle_document_write(event); + self.render(); + } + EditorEvent::ConfigEvent(event) => { + self.handle_config_events(event); + self.render(); + } + EditorEvent::LanguageServerMessage((id, call)) => { + self.handle_language_server_message(call, id).await; + // limit render calls for fast language server messages + let last = self.editor.language_servers.incoming.is_empty(); + + if last || self.last_render.elapsed() > LSP_DEADLINE { + self.render(); + self.last_render = Instant::now(); + } + } + EditorEvent::DebuggerEvent(payload) => { + let needs_render = self.editor.handle_debugger_message(payload).await; + if needs_render { + self.render(); + } + } + EditorEvent::IdleTimer => { + self.editor.clear_idle_timer(); + self.handle_idle_timeout(); - #[cfg(feature = "integration")] - { - idle_handled = true; + #[cfg(feature = "integration")] + { + idle_handled = true; + } + } } } } // for integration tests only, reset the idle timer after every - // event to make a signal when test events are done processing + // event to signal when test events are done processing #[cfg(feature = "integration")] { if idle_handled { @@ -446,6 +460,46 @@ impl Application { } } + pub fn handle_document_write(&mut self, doc_save_event: DocumentSaveEventResult) { + if let Err(err) = doc_save_event { + self.editor.set_error(err.to_string()); + return; + } + + let doc_save_event = doc_save_event.unwrap(); + let doc = self.editor.document_mut(doc_save_event.doc_id); + + if doc.is_none() { + warn!( + "received document saved event for non-existent doc id: {}", + doc_save_event.doc_id + ); + + return; + } + + let doc = doc.unwrap(); + + debug!( + "document {:?} saved with revision {}", + doc.path(), + doc_save_event.revision + ); + + doc.set_last_saved_revision(doc_save_event.revision); + let lines = doc.text().len_lines(); + let bytes = doc.text().len_bytes(); + + let path_str = doc + .path() + .expect("document written without path") + .to_string_lossy() + .into_owned(); + + self.editor + .set_status(format!("'{}' written, {}L {}B", path_str, lines, bytes)); + } + pub fn handle_terminal_events(&mut self, event: Result) { let mut cx = crate::compositor::Context { editor: &mut self.editor, @@ -866,11 +920,28 @@ impl Application { self.event_loop(input_stream).await; - let err = self.close().await.err(); + let mut save_errs = Vec::new(); + + for doc in self.editor.documents_mut() { + if let Some(Err(err)) = doc.close().await { + save_errs.push(( + doc.path() + .map(|path| path.to_string_lossy().into_owned()) + .unwrap_or_else(|| "".into()), + err, + )); + } + } + let close_err = self.close().await.err(); restore_term()?; - if let Some(err) = err { + for (path, err) in save_errs { + self.editor.exit_code = 1; + eprintln!("Error closing '{}': {}", path, err); + } + + if let Some(err) = close_err { self.editor.exit_code = 1; eprintln!("Error: {}", err); } diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 5073651b..f38434e2 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -51,7 +51,7 @@ use crate::{ ui::{self, overlay::overlayed, FilePicker, Picker, Popup, Prompt, PromptEvent}, }; -use crate::job::{self, Job, Jobs}; +use crate::job::{self, Jobs}; use futures_util::{FutureExt, StreamExt}; use std::{collections::HashMap, fmt, future::Future}; use std::{collections::HashSet, num::NonZeroUsize}; diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index 1bfc8153..d82dd7fe 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -77,7 +77,9 @@ fn buffer_close_by_ids_impl( let (modified_ids, modified_names): (Vec<_>, Vec<_>) = doc_ids .iter() .filter_map(|&doc_id| { - if let Err(CloseError::BufferModified(name)) = editor.close_document(doc_id, force) { + if let Err(CloseError::BufferModified(name)) = + helix_lsp::block_on(editor.close_document(doc_id, force)) + { Some((doc_id, name)) } else { None @@ -269,6 +271,7 @@ fn write_impl( doc.set_path(Some(path.as_ref().as_ref())) .context("invalid filepath")?; } + if doc.path().is_none() { bail!("cannot write a buffer without a filename"); } @@ -287,8 +290,8 @@ fn write_impl( } else { None }; - let future = doc.format_and_save(fmt, force); - cx.jobs.add(Job::new(future).wait_before_exiting()); + + doc.format_and_save(fmt, force)?; if path.is_some() { let id = doc.id(); @@ -602,8 +605,8 @@ fn write_all_impl( } else { None }; - let future = doc.format_and_save(fmt, force); - jobs.add(Job::new(future).wait_before_exiting()); + + doc.format_and_save(fmt, force)?; } if quit { diff --git a/helix-term/tests/test/write.rs b/helix-term/tests/test/write.rs index 8869d881..4ac850c1 100644 --- a/helix-term/tests/test/write.rs +++ b/helix-term/tests/test/write.rs @@ -62,7 +62,6 @@ async fn test_write_quit() -> anyhow::Result<()> { } #[tokio::test] -#[ignore] async fn test_write_concurrent() -> anyhow::Result<()> { let mut file = tempfile::NamedTempFile::new()?; let mut command = String::new(); @@ -92,7 +91,6 @@ async fn test_write_concurrent() -> anyhow::Result<()> { } #[tokio::test] -#[ignore] async fn test_write_fail_mod_flag() -> anyhow::Result<()> { let file = helpers::new_readonly_tempfile()?; diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 0daa983f..d6480b32 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -3,6 +3,7 @@ use futures_util::future::BoxFuture; use futures_util::FutureExt; use helix_core::auto_pairs::AutoPairs; use helix_core::Range; +use log::debug; use serde::de::{self, Deserialize, Deserializer}; use serde::Serialize; use std::borrow::Cow; @@ -13,6 +14,8 @@ use std::future::Future; use std::path::{Path, PathBuf}; use std::str::FromStr; use std::sync::Arc; +use tokio::sync::mpsc::{UnboundedReceiver, UnboundedSender}; +use tokio::sync::Mutex; use helix_core::{ encoding, @@ -83,6 +86,16 @@ impl Serialize for Mode { } } +/// A snapshot of the text of a document that we want to write out to disk +#[derive(Debug, Clone)] +pub struct DocumentSaveEvent { + pub revision: usize, + pub doc_id: DocumentId, +} + +pub type DocumentSaveEventResult = Result; +pub type DocumentSaveEventFuture = BoxFuture<'static, DocumentSaveEventResult>; + pub struct Document { pub(crate) id: DocumentId, text: Rope, @@ -118,6 +131,9 @@ pub struct Document { last_saved_revision: usize, version: i32, // should be usize? pub(crate) modified_since_accessed: bool, + save_sender: Option>, + save_receiver: Option>, + current_save: Arc>>, diagnostics: Vec, language_server: Option>, @@ -338,6 +354,7 @@ impl Document { let encoding = encoding.unwrap_or(encoding::UTF_8); let changes = ChangeSet::new(&text); let old_state = None; + let (save_sender, save_receiver) = tokio::sync::mpsc::unbounded_channel(); Self { id: DocumentId::default(), @@ -358,6 +375,9 @@ impl Document { savepoint: None, last_saved_revision: 0, modified_since_accessed: false, + save_sender: Some(save_sender), + save_receiver: Some(save_receiver), + current_save: Arc::new(Mutex::new(None)), language_server: None, } } @@ -492,29 +512,34 @@ impl Document { Some(fut.boxed()) } - pub fn save(&mut self, force: bool) -> impl Future> { + pub fn save(&mut self, force: bool) -> Result<(), anyhow::Error> { self.save_impl::>(None, force) } pub fn format_and_save( &mut self, - formatting: Option>>, + formatting: Option< + impl Future> + 'static + Send, + >, force: bool, - ) -> impl Future> { + ) -> anyhow::Result<()> { self.save_impl(formatting, force) } - // TODO: do we need some way of ensuring two save operations on the same doc can't run at once? - // or is that handled by the OS/async layer + // TODO: impl Drop to handle ensuring writes when closed /// The `Document`'s text is encoded according to its encoding and written to the file located /// at its `path()`. /// /// If `formatting` is present, it supplies some changes that we apply to the text before saving. - fn save_impl>>( + fn save_impl> + 'static + Send>( &mut self, formatting: Option, force: bool, - ) -> impl Future> { + ) -> Result<(), anyhow::Error> { + if self.save_sender.is_none() { + bail!("saves are closed for this document!"); + } + // we clone and move text + path into the future so that we asynchronously save the current // state without blocking any further edits. @@ -525,12 +550,13 @@ impl Document { let language_server = self.language_server.clone(); // mark changes up to now as saved - self.reset_modified(); + let current_rev = self.get_current_revision(); + let doc_id = self.id(); let encoding = self.encoding; // We encode the file according to the `Document`'s encoding. - async move { + let save_event = async move { use tokio::fs::File; if let Some(parent) = path.parent() { // TODO: display a prompt asking the user if the directories should be created @@ -563,9 +589,14 @@ impl Document { let mut file = File::create(path).await?; to_writer(&mut file, encoding, &text).await?; + let event = DocumentSaveEvent { + revision: current_rev, + doc_id, + }; + if let Some(language_server) = language_server { if !language_server.is_initialized() { - return Ok(()); + return Ok(event); } if let Some(notification) = language_server.text_document_did_save(identifier, &text) @@ -574,8 +605,70 @@ impl Document { } } - Ok(()) + Ok(event) + }; + + self.save_sender + .as_mut() + .unwrap() + .send(Box::pin(save_event)) + .map_err(|err| anyhow!("failed to send save event: {}", err)) + } + + pub async fn await_save(&mut self) -> Option { + let mut current_save = self.current_save.lock().await; + if let Some(ref mut save) = *current_save { + let result = save.await; + *current_save = None; + debug!("save of '{:?}' result: {:?}", self.path(), result); + return Some(result); + } + + // return early if the receiver is closed + self.save_receiver.as_ref()?; + + let save = match self.save_receiver.as_mut().unwrap().recv().await { + Some(save) => save, + None => { + self.save_receiver = None; + return None; + } + }; + + // save a handle to the future so that when a poll on this + // function gets cancelled, we don't lose it + *current_save = Some(save); + debug!("awaiting save of '{:?}'", self.path()); + + let result = (*current_save).as_mut().unwrap().await; + *current_save = None; + + debug!("save of '{:?}' result: {:?}", self.path(), result); + + Some(result) + } + + /// Prepares the Document for being closed by stopping any new writes + /// and flushing through the queue of pending writes. If any fail, + /// it stops early before emptying the rest of the queue. Callers + /// should keep calling until it returns None. + pub async fn close(&mut self) -> Option { + if self.save_sender.is_some() { + self.save_sender = None; } + + let mut final_result = None; + + while let Some(save_event) = self.await_save().await { + let is_err = save_event.is_err(); + final_result = Some(save_event); + + if is_err { + break; + } + } + + final_result } /// Detect the programming language based on the file type. @@ -941,6 +1034,19 @@ impl Document { self.last_saved_revision = current_revision; } + /// Set the document's latest saved revision to the given one. + pub fn set_last_saved_revision(&mut self, rev: usize) { + self.last_saved_revision = rev; + } + + /// Get the current revision number + pub fn get_current_revision(&mut self) -> usize { + let history = self.history.take(); + let current_revision = history.current_revision(); + self.history.set(history); + current_revision + } + /// Corresponding language scope name. Usually `source.`. pub fn language_scope(&self) -> Option<&str> { self.language diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index e9a3c639..ec6119a4 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -1,6 +1,6 @@ use crate::{ clipboard::{get_clipboard_provider, ClipboardProvider}, - document::Mode, + document::{DocumentSaveEventResult, Mode}, graphics::{CursorKind, Rect}, info::Info, input::KeyEvent, @@ -9,8 +9,9 @@ use crate::{ Document, DocumentId, View, ViewId, }; -use futures_util::future; -use futures_util::stream::select_all::SelectAll; +use futures_util::stream::{select_all::SelectAll, FuturesUnordered}; +use futures_util::{future, StreamExt}; +use helix_lsp::Call; use tokio_stream::wrappers::UnboundedReceiverStream; use std::{ @@ -65,7 +66,7 @@ where ) } -#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "kebab-case", default, deny_unknown_fields)] pub struct FilePickerConfig { /// IgnoreOptions @@ -172,7 +173,7 @@ pub struct Config { pub color_modes: bool, } -#[derive(Debug, Default, Clone, PartialEq, Serialize, Deserialize)] +#[derive(Debug, Default, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(default, rename_all = "kebab-case", deny_unknown_fields)] pub struct TerminalConfig { pub command: String, @@ -225,7 +226,7 @@ pub fn get_terminal_provider() -> Option { None } -#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(default, rename_all = "kebab-case", deny_unknown_fields)] pub struct LspConfig { /// Display LSP progress messages below statusline @@ -246,7 +247,7 @@ impl Default for LspConfig { } } -#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "kebab-case", default, deny_unknown_fields)] pub struct SearchConfig { /// Smart case: Case insensitive searching unless pattern contains upper case characters. Defaults to true. @@ -255,7 +256,7 @@ pub struct SearchConfig { pub wrap_around: bool, } -#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "kebab-case", default, deny_unknown_fields)] pub struct StatusLineConfig { pub left: Vec, @@ -279,7 +280,7 @@ impl Default for StatusLineConfig { } } -#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "kebab-case", default, deny_unknown_fields)] pub struct ModeConfig { pub normal: String, @@ -458,7 +459,7 @@ impl std::str::FromStr for GutterType { } } -#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(default)] pub struct WhitespaceConfig { pub render: WhitespaceRender, @@ -688,6 +689,15 @@ pub struct Editor { pub config_events: (UnboundedSender, UnboundedReceiver), } +#[derive(Debug)] +pub enum EditorEvent { + DocumentSave(DocumentSaveEventResult), + ConfigEvent(ConfigEvent), + LanguageServerMessage((usize, Call)), + DebuggerEvent(dap::Payload), + IdleTimer, +} + #[derive(Debug, Clone)] pub enum ConfigEvent { Refresh, @@ -719,6 +729,8 @@ pub enum CloseError { DoesNotExist, /// Buffer is modified BufferModified(String), + /// Document failed to save + SaveError(anyhow::Error), } impl Editor { @@ -1079,8 +1091,12 @@ impl Editor { self._refresh(); } - pub fn close_document(&mut self, doc_id: DocumentId, force: bool) -> Result<(), CloseError> { - let doc = match self.documents.get(&doc_id) { + pub async fn close_document( + &mut self, + doc_id: DocumentId, + force: bool, + ) -> Result<(), CloseError> { + let doc = match self.documents.get_mut(&doc_id) { Some(doc) => doc, None => return Err(CloseError::DoesNotExist), }; @@ -1089,8 +1105,19 @@ impl Editor { return Err(CloseError::BufferModified(doc.display_name().into_owned())); } + if let Some(Err(err)) = doc.close().await { + return Err(CloseError::SaveError(err)); + } + + // Don't fail the whole write because the language server could not + // acknowledge the close if let Some(language_server) = doc.language_server() { - tokio::spawn(language_server.text_document_did_close(doc.identifier())); + if let Err(err) = language_server + .text_document_did_close(doc.identifier()) + .await + { + log::error!("Error closing doc in language server: {}", err); + } } enum Action { @@ -1269,4 +1296,32 @@ impl Editor { .await .map(|_| ()) } + + pub async fn wait_event(&mut self) -> EditorEvent { + let mut saves: FuturesUnordered<_> = self + .documents + .values_mut() + .map(Document::await_save) + .collect(); + + tokio::select! { + biased; + + Some(Some(event)) = saves.next() => { + EditorEvent::DocumentSave(event) + } + Some(config_event) = self.config_events.1.recv() => { + EditorEvent::ConfigEvent(config_event) + } + Some(message) = self.language_servers.incoming.next() => { + EditorEvent::LanguageServerMessage(message) + } + Some(event) = self.debugger_events.next() => { + EditorEvent::DebuggerEvent(event) + } + _ = &mut self.idle_timer => { + EditorEvent::IdleTimer + } + } + } } From a5a93182cd5ccf88bc95b68044aa05d746ded35e Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Sat, 23 Apr 2022 18:38:55 -0400 Subject: [PATCH 02/37] fix: buffer-close ensuring writes Make sure buffer-close waits for the document to finish its writes. --- helix-term/src/application.rs | 2 + helix-term/src/commands/typed.rs | 1 + helix-term/tests/test/commands.rs | 1 - helix-view/src/document.rs | 66 +++++++++++++++++++++++++------ helix-view/src/editor.rs | 5 +++ 5 files changed, 63 insertions(+), 12 deletions(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index a9e25d08..2c1047da 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -310,6 +310,8 @@ impl Application { self.render(); } event = self.editor.wait_event() => { + log::debug!("received editor event: {:?}", event); + match event { EditorEvent::DocumentSave(event) => { self.handle_document_write(event); diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index d82dd7fe..375e7b4f 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -151,6 +151,7 @@ fn buffer_close( } let document_ids = buffer_gather_paths_impl(cx.editor, args); + log::debug!("closing buffers: {:?}", document_ids); buffer_close_by_ids_impl(cx.editor, &document_ids, false) } diff --git a/helix-term/tests/test/commands.rs b/helix-term/tests/test/commands.rs index f7ce9af0..8aea144b 100644 --- a/helix-term/tests/test/commands.rs +++ b/helix-term/tests/test/commands.rs @@ -26,7 +26,6 @@ async fn test_write_quit_fail() -> anyhow::Result<()> { } #[tokio::test] -#[ignore] async fn test_buffer_close_concurrent() -> anyhow::Result<()> { test_key_sequences( &mut Application::new(Args::default(), Config::default())?, diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index d6480b32..3045e3b7 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -616,6 +616,10 @@ impl Document { } pub async fn await_save(&mut self) -> Option { + self.await_save_impl(true).await + } + + async fn await_save_impl(&mut self, block: bool) -> Option { let mut current_save = self.current_save.lock().await; if let Some(ref mut save) = *current_save { let result = save.await; @@ -627,7 +631,15 @@ impl Document { // return early if the receiver is closed self.save_receiver.as_ref()?; - let save = match self.save_receiver.as_mut().unwrap().recv().await { + let rx = self.save_receiver.as_mut().unwrap(); + + let save_req = if block { + rx.recv().await + } else { + rx.try_recv().ok() + }; + + let save = match save_req { Some(save) => save, None => { self.save_receiver = None; @@ -648,19 +660,24 @@ impl Document { Some(result) } - /// Prepares the Document for being closed by stopping any new writes - /// and flushing through the queue of pending writes. If any fail, - /// it stops early before emptying the rest of the queue. Callers - /// should keep calling until it returns None. - pub async fn close(&mut self) -> Option { - if self.save_sender.is_some() { - self.save_sender = None; - } + /// Flushes the queue of pending writes. If any fail, + /// it stops early before emptying the rest of the queue. + pub async fn try_flush_saves(&mut self) -> Option { + self.flush_saves_impl(false).await + } + async fn flush_saves_impl(&mut self, block: bool) -> Option { let mut final_result = None; - while let Some(save_event) = self.await_save().await { - let is_err = save_event.is_err(); + while let Some(save_event) = self.await_save_impl(block).await { + let is_err = match &save_event { + Ok(event) => { + self.set_last_saved_revision(event.revision); + false + } + Err(_) => true, + }; + final_result = Some(save_event); if is_err { @@ -671,6 +688,17 @@ impl Document { final_result } + /// Prepares the Document for being closed by stopping any new writes + /// and flushing through the queue of pending writes. If any fail, + /// it stops early before emptying the rest of the queue. + pub async fn close(&mut self) -> Option { + if self.save_sender.is_some() { + self.save_sender = None; + } + + self.flush_saves_impl(true).await + } + /// Detect the programming language based on the file type. pub fn detect_language(&mut self, config_loader: Arc) { if let Some(path) = &self.path { @@ -1023,6 +1051,11 @@ impl Document { let history = self.history.take(); let current_revision = history.current_revision(); self.history.set(history); + log::debug!( + "modified - last saved: {}, current: {}", + self.last_saved_revision, + current_revision + ); current_revision != self.last_saved_revision || !self.changes.is_empty() } @@ -1036,9 +1069,20 @@ impl Document { /// Set the document's latest saved revision to the given one. pub fn set_last_saved_revision(&mut self, rev: usize) { + log::debug!( + "doc {} revision updated {} -> {}", + self.id, + self.last_saved_revision, + rev + ); self.last_saved_revision = rev; } + /// Get the document's latest saved revision. + pub fn get_last_saved_revision(&mut self) -> usize { + self.last_saved_revision + } + /// Get the current revision number pub fn get_current_revision(&mut self) -> usize { let history = self.history.take(); diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index ec6119a4..e038a82d 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -1101,6 +1101,11 @@ impl Editor { None => return Err(CloseError::DoesNotExist), }; + // flush out any pending writes first to clear the modified status + if let Some(save_result) = doc.try_flush_saves().await { + save_result?; + } + if !force && doc.is_modified() { return Err(CloseError::BufferModified(doc.display_name().into_owned())); } From 83b6042b97d13fca751e3d5d0c32f04e3ad04c9a Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Sun, 24 Apr 2022 15:33:43 -0400 Subject: [PATCH 03/37] fix(write): do not set new path on document until write succeeds If a document is written with a new path, currently, in the event that the write fails, the document still gets its path changed. This fixes it so that the path is not updated unless the write succeeds. --- helix-term/src/application.rs | 26 +++++++++----- helix-term/src/commands/typed.rs | 9 ++--- helix-term/tests/test/write.rs | 61 ++++++++++++++++++++++++++++++-- helix-view/src/document.rs | 52 +++++++++++++++++++-------- 4 files changed, 116 insertions(+), 32 deletions(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 2c1047da..0640de3c 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -3,6 +3,7 @@ use futures_util::Stream; use helix_core::{ config::{default_syntax_loader, user_syntax_loader}, diagnostic::{DiagnosticTag, NumberOrString}, + path::get_relative_path, pos_at_coords, syntax, Selection, }; use helix_lsp::{lsp, util::lsp_pos_to_pos, LspProgressMap}; @@ -489,17 +490,26 @@ impl Application { ); doc.set_last_saved_revision(doc_save_event.revision); + let lines = doc.text().len_lines(); let bytes = doc.text().len_bytes(); - let path_str = doc - .path() - .expect("document written without path") - .to_string_lossy() - .into_owned(); - - self.editor - .set_status(format!("'{}' written, {}L {}B", path_str, lines, bytes)); + if let Err(err) = doc.set_path(Some(&doc_save_event.path)) { + log::error!( + "error setting path for doc '{:?}': {}", + doc.path(), + err.to_string(), + ); + self.editor.set_error(err.to_string()); + } else { + // TODO: fix being overwritten by lsp + self.editor.set_status(format!( + "'{}' written, {}L {}B", + get_relative_path(&doc_save_event.path).to_string_lossy(), + lines, + bytes + )); + } } pub fn handle_terminal_events(&mut self, event: Result) { diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index 375e7b4f..35c84601 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -268,11 +268,6 @@ fn write_impl( let jobs = &mut cx.jobs; let doc = doc_mut!(cx.editor); - if let Some(ref path) = path { - doc.set_path(Some(path.as_ref().as_ref())) - .context("invalid filepath")?; - } - if doc.path().is_none() { bail!("cannot write a buffer without a filename"); } @@ -292,7 +287,7 @@ fn write_impl( None }; - doc.format_and_save(fmt, force)?; + doc.format_and_save(fmt, path.map(AsRef::as_ref), force)?; if path.is_some() { let id = doc.id(); @@ -607,7 +602,7 @@ fn write_all_impl( None }; - doc.format_and_save(fmt, force)?; + doc.format_and_save::<_, PathBuf>(fmt, None, force)?; } if quit { diff --git a/helix-term/tests/test/write.rs b/helix-term/tests/test/write.rs index 4ac850c1..d2e6922f 100644 --- a/helix-term/tests/test/write.rs +++ b/helix-term/tests/test/write.rs @@ -35,7 +35,7 @@ async fn test_write() -> anyhow::Result<()> { Ok(()) } -#[tokio::test] +#[tokio::test(flavor = "multi_thread")] async fn test_write_quit() -> anyhow::Result<()> { let mut file = tempfile::NamedTempFile::new()?; @@ -129,7 +129,64 @@ async fn test_write_fail_mod_flag() -> anyhow::Result<()> { } #[tokio::test] -#[ignore] +async fn test_write_new_path() -> anyhow::Result<()> { + let mut file1 = tempfile::NamedTempFile::new().unwrap(); + let mut file2 = tempfile::NamedTempFile::new().unwrap(); + + test_key_sequences( + &mut Application::new( + Args { + files: vec![(file1.path().to_path_buf(), Position::default())], + ..Default::default() + }, + Config::default(), + )?, + vec![ + ( + Some("ii can eat glass, it will not hurt me:w"), + Some(&|app| { + let doc = doc!(app.editor); + assert!(!app.editor.is_err()); + assert_eq!(file1.path(), doc.path().unwrap()); + }), + ), + ( + Some(&format!(":w {}", file2.path().to_string_lossy())), + Some(&|app| { + let doc = doc!(app.editor); + assert!(!app.editor.is_err()); + assert_eq!(file2.path(), doc.path().unwrap()); + assert!(app.editor.document_by_path(file1.path()).is_none()); + }), + ), + ], + false, + ) + .await?; + + file1.as_file_mut().flush()?; + file1.as_file_mut().sync_all()?; + file2.as_file_mut().flush()?; + file2.as_file_mut().sync_all()?; + + let mut file1_content = String::new(); + file1.as_file_mut().read_to_string(&mut file1_content)?; + assert_eq!( + helpers::platform_line("i can eat glass, it will not hurt me\n"), + file1_content + ); + + let mut file2_content = String::new(); + file2.as_file_mut().read_to_string(&mut file2_content)?; + assert_eq!( + helpers::platform_line("i can eat glass, it will not hurt me\n"), + file2_content + ); + + Ok(()) +} + +#[tokio::test] async fn test_write_fail_new_path() -> anyhow::Result<()> { let file = helpers::new_readonly_tempfile()?; diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 3045e3b7..82d526a9 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -91,6 +91,7 @@ impl Serialize for Mode { pub struct DocumentSaveEvent { pub revision: usize, pub doc_id: DocumentId, + pub path: PathBuf, } pub type DocumentSaveEventResult = Result; @@ -512,41 +513,61 @@ impl Document { Some(fut.boxed()) } - pub fn save(&mut self, force: bool) -> Result<(), anyhow::Error> { - self.save_impl::>(None, force) + pub fn save>( + &mut self, + path: Option

, + force: bool, + ) -> Result<(), anyhow::Error> { + self.save_impl::, _>(None, path, force) } - pub fn format_and_save( + pub fn format_and_save( &mut self, - formatting: Option< - impl Future> + 'static + Send, - >, + formatting: Option, + path: Option

, force: bool, - ) -> anyhow::Result<()> { - self.save_impl(formatting, force) + ) -> anyhow::Result<()> + where + F: Future> + 'static + Send, + P: Into, + { + self.save_impl(formatting, path, force) } - // TODO: impl Drop to handle ensuring writes when closed /// The `Document`'s text is encoded according to its encoding and written to the file located /// at its `path()`. /// /// If `formatting` is present, it supplies some changes that we apply to the text before saving. - fn save_impl> + 'static + Send>( + fn save_impl( &mut self, formatting: Option, + path: Option

, force: bool, - ) -> Result<(), anyhow::Error> { + ) -> Result<(), anyhow::Error> + where + F: Future> + 'static + Send, + P: Into, + { if self.save_sender.is_none() { bail!("saves are closed for this document!"); } // we clone and move text + path into the future so that we asynchronously save the current // state without blocking any further edits. - let mut text = self.text().clone(); - let path = self.path.clone().expect("Can't save with no path set!"); - let identifier = self.identifier(); + let path = match path { + Some(path) => helix_core::path::get_canonicalized_path(&path.into())?, + None => { + if self.path.is_none() { + bail!("Can't save with no path set!"); + } + + self.path.as_ref().unwrap().clone() + } + }; + + let identifier = self.identifier(); let language_server = self.language_server.clone(); // mark changes up to now as saved @@ -586,12 +607,13 @@ impl Document { } } - let mut file = File::create(path).await?; + let mut file = File::create(&path).await?; to_writer(&mut file, encoding, &text).await?; let event = DocumentSaveEvent { revision: current_rev, doc_id, + path, }; if let Some(language_server) = language_server { From e1f7bdb1d2ef68c0de38e768080291901ff4662e Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Mon, 9 May 2022 23:08:12 -0400 Subject: [PATCH 04/37] fix buffer-close --- helix-term/src/application.rs | 1 + helix-term/src/commands/typed.rs | 6 +++--- helix-term/tests/test/commands.rs | 2 +- helix-term/tests/test/helpers.rs | 6 ++++-- helix-term/tests/test/write.rs | 2 +- helix-view/src/document.rs | 14 ++++++-------- helix-view/src/editor.rs | 4 ++-- 7 files changed, 18 insertions(+), 17 deletions(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 0640de3c..5c25e8aa 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -344,6 +344,7 @@ impl Application { #[cfg(feature = "integration")] { + log::debug!("idle handled"); idle_handled = true; } } diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index 35c84601..efe693b9 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -77,9 +77,9 @@ fn buffer_close_by_ids_impl( let (modified_ids, modified_names): (Vec<_>, Vec<_>) = doc_ids .iter() .filter_map(|&doc_id| { - if let Err(CloseError::BufferModified(name)) = + if let Err(CloseError::BufferModified(name)) = tokio::task::block_in_place(|| { helix_lsp::block_on(editor.close_document(doc_id, force)) - { + }) { Some((doc_id, name)) } else { None @@ -151,7 +151,6 @@ fn buffer_close( } let document_ids = buffer_gather_paths_impl(cx.editor, args); - log::debug!("closing buffers: {:?}", document_ids); buffer_close_by_ids_impl(cx.editor, &document_ids, false) } @@ -519,6 +518,7 @@ fn write_quit( } write_impl(cx, args.first(), false)?; + // TODO: change to use document close helix_lsp::block_on(cx.jobs.finish())?; quit(cx, &[], event) } diff --git a/helix-term/tests/test/commands.rs b/helix-term/tests/test/commands.rs index 8aea144b..1f1bd6a9 100644 --- a/helix-term/tests/test/commands.rs +++ b/helix-term/tests/test/commands.rs @@ -25,7 +25,7 @@ async fn test_write_quit_fail() -> anyhow::Result<()> { Ok(()) } -#[tokio::test] +#[tokio::test(flavor = "multi_thread")] async fn test_buffer_close_concurrent() -> anyhow::Result<()> { test_key_sequences( &mut Application::new(Args::default(), Config::default())?, diff --git a/helix-term/tests/test/helpers.rs b/helix-term/tests/test/helpers.rs index 8f2501e6..bbcc6632 100644 --- a/helix-term/tests/test/helpers.rs +++ b/helix-term/tests/test/helpers.rs @@ -56,7 +56,9 @@ pub async fn test_key_sequences( for (i, (in_keys, test_fn)) in inputs.into_iter().enumerate() { if let Some(in_keys) = in_keys { for key_event in parse_macro(in_keys)?.into_iter() { - tx.send(Ok(Event::Key(KeyEvent::from(key_event))))?; + let key = Event::Key(KeyEvent::from(key_event)); + log::trace!("sending key: {:?}", key); + tx.send(Ok(key))?; } } @@ -70,7 +72,7 @@ pub async fn test_key_sequences( // verify if it exited on the last iteration if it should have and // the inverse if i == num_inputs - 1 && app_exited != should_exit { - bail!("expected app to exit: {} != {}", app_exited, should_exit); + bail!("expected app to exit: {} != {}", should_exit, app_exited); } if let Some(test) = test_fn { diff --git a/helix-term/tests/test/write.rs b/helix-term/tests/test/write.rs index d2e6922f..544f1ba1 100644 --- a/helix-term/tests/test/write.rs +++ b/helix-term/tests/test/write.rs @@ -61,7 +61,7 @@ async fn test_write_quit() -> anyhow::Result<()> { Ok(()) } -#[tokio::test] +#[tokio::test(flavor = "multi_thread")] async fn test_write_concurrent() -> anyhow::Result<()> { let mut file = tempfile::NamedTempFile::new()?; let mut command = String::new(); diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 82d526a9..b6e42065 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -3,7 +3,6 @@ use futures_util::future::BoxFuture; use futures_util::FutureExt; use helix_core::auto_pairs::AutoPairs; use helix_core::Range; -use log::debug; use serde::de::{self, Deserialize, Deserializer}; use serde::Serialize; use std::borrow::Cow; @@ -644,16 +643,15 @@ impl Document { async fn await_save_impl(&mut self, block: bool) -> Option { let mut current_save = self.current_save.lock().await; if let Some(ref mut save) = *current_save { + log::trace!("reawaiting save of '{:?}'", self.path()); let result = save.await; *current_save = None; - debug!("save of '{:?}' result: {:?}", self.path(), result); + log::trace!("reawait save of '{:?}' result: {:?}", self.path(), result); return Some(result); } // return early if the receiver is closed - self.save_receiver.as_ref()?; - - let rx = self.save_receiver.as_mut().unwrap(); + let rx = self.save_receiver.as_mut()?; let save_req = if block { rx.recv().await @@ -672,12 +670,12 @@ impl Document { // save a handle to the future so that when a poll on this // function gets cancelled, we don't lose it *current_save = Some(save); - debug!("awaiting save of '{:?}'", self.path()); + log::trace!("awaiting save of '{:?}'", self.path()); let result = (*current_save).as_mut().unwrap().await; *current_save = None; - debug!("save of '{:?}' result: {:?}", self.path(), result); + log::trace!("save of '{:?}' result: {:?}", self.path(), result); Some(result) } @@ -715,7 +713,7 @@ impl Document { /// it stops early before emptying the rest of the queue. pub async fn close(&mut self) -> Option { if self.save_sender.is_some() { - self.save_sender = None; + self.save_sender.take(); } self.flush_saves_impl(true).await diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index e038a82d..e54aa7fa 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -1102,8 +1102,8 @@ impl Editor { }; // flush out any pending writes first to clear the modified status - if let Some(save_result) = doc.try_flush_saves().await { - save_result?; + if let Some(Err(err)) = doc.try_flush_saves().await { + return Err(CloseError::SaveError(err)); } if !force && doc.is_modified() { From 69c9e44ef205a81c112dfb14d5f2e67e5ce9c300 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Tue, 10 May 2022 23:41:44 -0400 Subject: [PATCH 05/37] update write-quit to wait for saves --- helix-term/src/commands/typed.rs | 8 ++++++-- helix-term/tests/test/commands.rs | 7 ++++++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index efe693b9..bc254146 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -518,8 +518,12 @@ fn write_quit( } write_impl(cx, args.first(), false)?; - // TODO: change to use document close - helix_lsp::block_on(cx.jobs.finish())?; + let doc = doc_mut!(cx.editor); + + tokio::task::block_in_place(|| helix_lsp::block_on(doc.try_flush_saves())) + .map(|result| result.map(|_| ())) + .unwrap_or(Ok(()))?; + quit(cx, &[], event) } diff --git a/helix-term/tests/test/commands.rs b/helix-term/tests/test/commands.rs index 1f1bd6a9..b7c0f7cc 100644 --- a/helix-term/tests/test/commands.rs +++ b/helix-term/tests/test/commands.rs @@ -8,7 +8,7 @@ use helix_term::application::Application; use super::*; -#[tokio::test] +#[tokio::test(flavor = "multi_thread")] async fn test_write_quit_fail() -> anyhow::Result<()> { let file = helpers::new_readonly_tempfile()?; @@ -16,6 +16,11 @@ async fn test_write_quit_fail() -> anyhow::Result<()> { &mut helpers::app_with_file(file.path())?, Some("ihello:wq"), Some(&|app| { + let mut docs: Vec<_> = app.editor.documents().collect(); + assert_eq!(1, docs.len()); + + let doc = docs.pop().unwrap(); + assert_eq!(Some(file.path()), doc.path().map(PathBuf::as_path)); assert_eq!(&Severity::Error, app.editor.get_status().unwrap().1); }), false, From b8a07f7d15a10186fa2b481a3423c23f32d7d561 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Fri, 24 Jun 2022 08:39:07 -0400 Subject: [PATCH 06/37] add conditional noop render back It makes it much slower without stubbing this out --- helix-core/src/auto_pairs.rs | 1 - helix-term/src/application.rs | 4 ++++ helix-term/src/commands.rs | 14 ++------------ helix-term/src/commands/typed.rs | 17 +++-------------- 4 files changed, 9 insertions(+), 27 deletions(-) diff --git a/helix-core/src/auto_pairs.rs b/helix-core/src/auto_pairs.rs index ff680a77..edc404ac 100644 --- a/helix-core/src/auto_pairs.rs +++ b/helix-core/src/auto_pairs.rs @@ -7,7 +7,6 @@ use std::collections::HashMap; use smallvec::SmallVec; // Heavily based on https://github.com/codemirror/closebrackets/ - pub const DEFAULT_PAIRS: &[(char, char)] = &[ ('(', ')'), ('{', '}'), diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 5c25e8aa..fd9b7c3e 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -253,6 +253,10 @@ impl Application { Ok(app) } + #[cfg(feature = "integration")] + fn render(&mut self) {} + + #[cfg(not(feature = "integration"))] fn render(&mut self) { let compositor = &mut self.compositor; diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index f38434e2..a4421f03 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -2504,13 +2504,6 @@ fn insert_at_line_end(cx: &mut Context) { doc.set_selection(view.id, selection); } -/// Sometimes when applying formatting changes we want to mark the buffer as unmodified, for -/// example because we just applied the same changes while saving. -enum Modified { - SetUnmodified, - LeaveModified, -} - // Creates an LspCallback that waits for formatting changes to be computed. When they're done, // it applies them, but only if the doc hasn't changed. // @@ -2519,7 +2512,6 @@ enum Modified { async fn make_format_callback( doc_id: DocumentId, doc_version: i32, - modified: Modified, format: impl Future> + Send + 'static, ) -> anyhow::Result { let format = format.await?; @@ -2536,17 +2528,15 @@ async fn make_format_callback( doc.append_changes_to_history(view.id); doc.detect_indent_and_line_ending(); view.ensure_cursor_in_view(doc, scrolloff); - if let Modified::SetUnmodified = modified { - doc.reset_modified(); - } } else { log::info!("discarded formatting changes because the document changed"); } }); + Ok(call) } -#[derive(PartialEq)] +#[derive(PartialEq, Eq)] pub enum Open { Below, Above, diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index bc254146..14b23f2a 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -273,12 +273,7 @@ fn write_impl( let fmt = if auto_format { doc.auto_format().map(|fmt| { let shared = fmt.shared(); - let callback = make_format_callback( - doc.id(), - doc.version(), - Modified::SetUnmodified, - shared.clone(), - ); + let callback = make_format_callback(doc.id(), doc.version(), shared.clone()); jobs.callback(callback); shared }) @@ -346,8 +341,7 @@ fn format( let doc = doc!(cx.editor); if let Some(format) = doc.format() { - let callback = - make_format_callback(doc.id(), doc.version(), Modified::LeaveModified, format); + let callback = make_format_callback(doc.id(), doc.version(), format); cx.jobs.callback(callback); } @@ -593,12 +587,7 @@ fn write_all_impl( let fmt = if auto_format { doc.auto_format().map(|fmt| { let shared = fmt.shared(); - let callback = make_format_callback( - doc.id(), - doc.version(), - Modified::SetUnmodified, - shared.clone(), - ); + let callback = make_format_callback(doc.id(), doc.version(), shared.clone()); jobs.callback(callback); shared }) From cb23399dee723cec67f1a04dbe6514dfddfd7f5f Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Sat, 9 Jul 2022 22:39:40 -0400 Subject: [PATCH 07/37] improve reliability of shutdown --- helix-term/src/application.rs | 72 +++++++++++++++++++++++--------- helix-term/src/commands/typed.rs | 8 +++- helix-view/src/document.rs | 17 +++++++- helix-view/src/editor.rs | 8 +++- 4 files changed, 80 insertions(+), 25 deletions(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index fd9b7c3e..e84739cd 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -937,26 +937,26 @@ impl Application { self.event_loop(input_stream).await; - let mut save_errs = Vec::new(); - - for doc in self.editor.documents_mut() { - if let Some(Err(err)) = doc.close().await { - save_errs.push(( - doc.path() - .map(|path| path.to_string_lossy().into_owned()) - .unwrap_or_else(|| "".into()), - err, - )); - } - } + // let mut save_errs = Vec::new(); + + // for doc in self.editor.documents_mut() { + // if let Some(Err(err)) = doc.close().await { + // save_errs.push(( + // doc.path() + // .map(|path| path.to_string_lossy().into_owned()) + // .unwrap_or_else(|| "".into()), + // err, + // )); + // } + // } let close_err = self.close().await.err(); restore_term()?; - for (path, err) in save_errs { - self.editor.exit_code = 1; - eprintln!("Error closing '{}': {}", path, err); - } + // for (path, err) in save_errs { + // self.editor.exit_code = 1; + // eprintln!("Error closing '{}': {}", path, err); + // } if let Some(err) = close_err { self.editor.exit_code = 1; @@ -967,12 +967,44 @@ impl Application { } pub async fn close(&mut self) -> anyhow::Result<()> { - self.jobs.finish().await?; + // [NOTE] we intentionally do not return early for errors because we + // want to try to run as much cleanup as we can, regardless of + // errors along the way - if self.editor.close_language_servers(None).await.is_err() { - log::error!("Timed out waiting for language servers to shutdown"); + let mut result = match self.jobs.finish().await { + Ok(_) => Ok(()), + Err(err) => { + log::error!("Error executing job: {}", err); + Err(err) + } }; - Ok(()) + for doc in self.editor.documents_mut() { + if let Some(save_result) = doc.close().await { + result = match save_result { + Ok(_) => result, + Err(err) => { + if let Some(path) = doc.path() { + log::error!( + "Error saving document '{}': {}", + path.to_string_lossy(), + err + ); + } + Err(err) + } + }; + } + } + + match self.editor.close_language_servers(None).await { + Ok(_) => result, + Err(_) => { + log::error!("Timed out waiting for language servers to shutdown"); + Err(anyhow::format_err!( + "Timed out waiting for language servers to shutdown" + )) + } + } } } diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index 14b23f2a..650ff75d 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -1,5 +1,7 @@ use std::ops::Deref; +use crate::job::Job; + use super::*; use helix_view::{ @@ -19,6 +21,8 @@ pub struct TypableCommand { } fn quit(cx: &mut compositor::Context, args: &[Cow], event: PromptEvent) -> anyhow::Result<()> { + log::info!("quitting..."); + if event != PromptEvent::Validate { return Ok(()); } @@ -274,7 +278,7 @@ fn write_impl( doc.auto_format().map(|fmt| { let shared = fmt.shared(); let callback = make_format_callback(doc.id(), doc.version(), shared.clone()); - jobs.callback(callback); + jobs.add(Job::with_callback(callback).wait_before_exiting()); shared }) } else { @@ -512,8 +516,10 @@ fn write_quit( } write_impl(cx, args.first(), false)?; + let doc = doc_mut!(cx.editor); + tokio::task::block_in_place(|| helix_lsp::block_on(cx.jobs.finish()))?; tokio::task::block_in_place(|| helix_lsp::block_on(doc.try_flush_saves())) .map(|result| result.map(|_| ())) .unwrap_or(Ok(()))?; diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index b6e42065..1743fac2 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -551,6 +551,11 @@ impl Document { bail!("saves are closed for this document!"); } + log::debug!( + "submitting save of doc '{:?}'", + self.path().map(|path| path.to_string_lossy()) + ); + // we clone and move text + path into the future so that we asynchronously save the current // state without blocking any further edits. let mut text = self.text().clone(); @@ -695,7 +700,14 @@ impl Document { self.set_last_saved_revision(event.revision); false } - Err(_) => true, + Err(err) => { + log::error!( + "error saving document {:?}: {}", + self.path().map(|path| path.to_string_lossy()), + err + ); + true + } }; final_result = Some(save_event); @@ -1072,7 +1084,8 @@ impl Document { let current_revision = history.current_revision(); self.history.set(history); log::debug!( - "modified - last saved: {}, current: {}", + "id {} modified - last saved: {}, current: {}", + self.id, self.last_saved_revision, current_revision ); diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index e54aa7fa..58fcf238 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -816,12 +816,16 @@ impl Editor { #[inline] pub fn set_status>>(&mut self, status: T) { - self.status_msg = Some((status.into(), Severity::Info)); + let status = status.into(); + log::debug!("editor status: {}", status); + self.status_msg = Some((status, Severity::Info)); } #[inline] pub fn set_error>>(&mut self, error: T) { - self.status_msg = Some((error.into(), Severity::Error)); + let error = error.into(); + log::error!("editor error: {}", error); + self.status_msg = Some((error, Severity::Error)); } #[inline] From c9418582d2a6d8dbb8b5bb1d3432a9087438e61d Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Tue, 5 Jul 2022 00:15:15 -0400 Subject: [PATCH 08/37] fix modified status with auto format --- helix-term/src/commands.rs | 17 +++++++++++++++- helix-term/src/commands/typed.rs | 35 +++++++++++++++++--------------- helix-view/src/document.rs | 1 + 3 files changed, 36 insertions(+), 17 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index a4421f03..e76e0280 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -52,7 +52,7 @@ use crate::{ }; use crate::job::{self, Jobs}; -use futures_util::{FutureExt, StreamExt}; +use futures_util::StreamExt; use std::{collections::HashMap, fmt, future::Future}; use std::{collections::HashSet, num::NonZeroUsize}; @@ -2513,6 +2513,7 @@ async fn make_format_callback( doc_id: DocumentId, doc_version: i32, format: impl Future> + Send + 'static, + write: Option<(Option, bool)>, ) -> anyhow::Result { let format = format.await?; let call: job::Callback = Box::new(move |editor, _compositor| { @@ -2523,11 +2524,25 @@ async fn make_format_callback( let scrolloff = editor.config().scrolloff; let doc = doc_mut!(editor, &doc_id); let view = view_mut!(editor); + let loader = editor.syn_loader.clone(); + if doc.version() == doc_version { apply_transaction(&format, doc, view); doc.append_changes_to_history(view.id); doc.detect_indent_and_line_ending(); view.ensure_cursor_in_view(doc, scrolloff); + + if let Some((path, force)) = write { + let refresh_lang = path.is_some(); + + if let Err(err) = doc.save(path, force) { + editor.set_error(format!("Error saving: {}", err)); + } else if refresh_lang { + let id = doc.id(); + doc.detect_language(loader); + let _ = editor.refresh_language_server(id); + } + } } else { log::info!("discarded formatting changes because the document changed"); } diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index 650ff75d..955b3b58 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -267,30 +267,32 @@ fn write_impl( path: Option<&Cow>, force: bool, ) -> anyhow::Result<()> { - let auto_format = cx.editor.config().auto_format; + let editor_auto_fmt = cx.editor.config().auto_format; let jobs = &mut cx.jobs; let doc = doc_mut!(cx.editor); + let path = path.map(AsRef::as_ref); if doc.path().is_none() { bail!("cannot write a buffer without a filename"); } - let fmt = if auto_format { + + let fmt = if editor_auto_fmt { doc.auto_format().map(|fmt| { - let shared = fmt.shared(); - let callback = make_format_callback(doc.id(), doc.version(), shared.clone()); + let callback = make_format_callback( + doc.id(), + doc.version(), + fmt, + Some((path.map(Into::into), force)), + ); + jobs.add(Job::with_callback(callback).wait_before_exiting()); - shared }) } else { None }; - doc.format_and_save(fmt, path.map(AsRef::as_ref), force)?; - - if path.is_some() { - let id = doc.id(); - doc.detect_language(cx.editor.syn_loader.clone()); - let _ = cx.editor.refresh_language_server(id); + if fmt.is_none() { + doc.save(path, force)?; } Ok(()) @@ -345,7 +347,7 @@ fn format( let doc = doc!(cx.editor); if let Some(format) = doc.format() { - let callback = make_format_callback(doc.id(), doc.version(), format); + let callback = make_format_callback(doc.id(), doc.version(), format, None); cx.jobs.callback(callback); } @@ -592,16 +594,17 @@ fn write_all_impl( let fmt = if auto_format { doc.auto_format().map(|fmt| { - let shared = fmt.shared(); - let callback = make_format_callback(doc.id(), doc.version(), shared.clone()); + let callback = + make_format_callback(doc.id(), doc.version(), fmt, Some((None, force))); jobs.callback(callback); - shared }) } else { None }; - doc.format_and_save::<_, PathBuf>(fmt, None, force)?; + if fmt.is_none() { + doc.save::(None, force)?; + } } if quit { diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 1743fac2..fe081442 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -14,6 +14,7 @@ use std::path::{Path, PathBuf}; use std::str::FromStr; use std::sync::Arc; use tokio::sync::mpsc::{UnboundedReceiver, UnboundedSender}; + use tokio::sync::Mutex; use helix_core::{ From aaa145067833c41686b7cdde9fb999018735bc04 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Mon, 11 Jul 2022 23:38:26 -0400 Subject: [PATCH 09/37] fix write-quit with auto format write-quit will now save all files successfully even when there is auto formatting --- helix-term/src/application.rs | 6 +++- helix-term/src/commands.rs | 19 +++++++------ helix-term/src/commands/dap.rs | 25 +++++++++-------- helix-term/src/commands/typed.rs | 17 +++++++----- helix-term/src/job.rs | 47 ++++++++++++++++++++++++++++---- helix-term/src/ui/editor.rs | 7 +++-- 6 files changed, 85 insertions(+), 36 deletions(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index e84739cd..84eba22a 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -971,7 +971,11 @@ impl Application { // want to try to run as much cleanup as we can, regardless of // errors along the way - let mut result = match self.jobs.finish().await { + let mut result = match self + .jobs + .finish(Some(&mut self.editor), Some(&mut self.compositor)) + .await + { Ok(_) => Ok(()), Err(err) => { log::error!("Error executing job: {}", err); diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index e76e0280..afd94564 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -47,6 +47,7 @@ use movement::Movement; use crate::{ args, compositor::{self, Component, Compositor}, + job::Callback, keymap::ReverseKeymap, ui::{self, overlay::overlayed, FilePicker, Picker, Popup, Prompt, PromptEvent}, }; @@ -107,10 +108,11 @@ impl<'a> Context<'a> { let callback = Box::pin(async move { let json = call.await?; let response = serde_json::from_value(json)?; - let call: job::Callback = - Box::new(move |editor: &mut Editor, compositor: &mut Compositor| { + let call: job::Callback = Callback::EditorCompositor(Box::new( + move |editor: &mut Editor, compositor: &mut Compositor| { callback(editor, compositor, response) - }); + }, + )); Ok(call) }); self.jobs.callback(callback); @@ -1925,8 +1927,8 @@ fn global_search(cx: &mut Context) { let show_picker = async move { let all_matches: Vec = UnboundedReceiverStream::new(all_matches_rx).collect().await; - let call: job::Callback = - Box::new(move |editor: &mut Editor, compositor: &mut Compositor| { + let call: job::Callback = Callback::EditorCompositor(Box::new( + move |editor: &mut Editor, compositor: &mut Compositor| { if all_matches.is_empty() { editor.set_status("No matches found"); return; @@ -1962,7 +1964,8 @@ fn global_search(cx: &mut Context) { }, ); compositor.push(Box::new(overlayed(picker))); - }); + }, + )); Ok(call) }; cx.jobs.callback(show_picker); @@ -2516,7 +2519,7 @@ async fn make_format_callback( write: Option<(Option, bool)>, ) -> anyhow::Result { let format = format.await?; - let call: job::Callback = Box::new(move |editor, _compositor| { + let call: job::Callback = Callback::EditorCompositor(Box::new(move |editor, _compositor| { if !editor.documents.contains_key(&doc_id) { return; } @@ -2546,7 +2549,7 @@ async fn make_format_callback( } else { log::info!("discarded formatting changes because the document changed"); } - }); + })); Ok(call) } diff --git a/helix-term/src/commands/dap.rs b/helix-term/src/commands/dap.rs index 12a3fbc7..9c067eba 100644 --- a/helix-term/src/commands/dap.rs +++ b/helix-term/src/commands/dap.rs @@ -118,11 +118,14 @@ fn dap_callback( let callback = Box::pin(async move { let json = call.await?; let response = serde_json::from_value(json)?; - let call: Callback = Box::new(move |editor: &mut Editor, compositor: &mut Compositor| { - callback(editor, compositor, response) - }); + let call: Callback = Callback::EditorCompositor(Box::new( + move |editor: &mut Editor, compositor: &mut Compositor| { + callback(editor, compositor, response) + }, + )); Ok(call) }); + jobs.callback(callback); } @@ -274,10 +277,10 @@ pub fn dap_launch(cx: &mut Context) { let completions = template.completion.clone(); let name = template.name.clone(); let callback = Box::pin(async move { - let call: Callback = Box::new(move |_editor, compositor| { + let call: Callback = Callback::Compositor(Box::new(move |compositor| { let prompt = debug_parameter_prompt(completions, name, Vec::new()); compositor.push(Box::new(prompt)); - }); + })); Ok(call) }); cx.jobs.callback(callback); @@ -332,10 +335,10 @@ fn debug_parameter_prompt( let config_name = config_name.clone(); let params = params.clone(); let callback = Box::pin(async move { - let call: Callback = Box::new(move |_editor, compositor| { + let call: Callback = Callback::Compositor(Box::new(move |compositor| { let prompt = debug_parameter_prompt(completions, config_name, params); compositor.push(Box::new(prompt)); - }); + })); Ok(call) }); cx.jobs.callback(callback); @@ -582,7 +585,7 @@ pub fn dap_edit_condition(cx: &mut Context) { None => return, }; let callback = Box::pin(async move { - let call: Callback = Box::new(move |editor, compositor| { + let call: Callback = Callback::EditorCompositor(Box::new(move |editor, compositor| { let mut prompt = Prompt::new( "condition:".into(), None, @@ -610,7 +613,7 @@ pub fn dap_edit_condition(cx: &mut Context) { prompt.insert_str(&condition, editor) } compositor.push(Box::new(prompt)); - }); + })); Ok(call) }); cx.jobs.callback(callback); @@ -624,7 +627,7 @@ pub fn dap_edit_log(cx: &mut Context) { None => return, }; let callback = Box::pin(async move { - let call: Callback = Box::new(move |editor, compositor| { + let call: Callback = Callback::EditorCompositor(Box::new(move |editor, compositor| { let mut prompt = Prompt::new( "log-message:".into(), None, @@ -651,7 +654,7 @@ pub fn dap_edit_log(cx: &mut Context) { prompt.insert_str(&log_message, editor); } compositor.push(Box::new(prompt)); - }); + })); Ok(call) }); cx.jobs.callback(callback); diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index 955b3b58..a687b854 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -519,9 +519,10 @@ fn write_quit( write_impl(cx, args.first(), false)?; + tokio::task::block_in_place(|| helix_lsp::block_on(cx.jobs.finish(Some(cx.editor), None)))?; + let doc = doc_mut!(cx.editor); - tokio::task::block_in_place(|| helix_lsp::block_on(cx.jobs.finish()))?; tokio::task::block_in_place(|| helix_lsp::block_on(doc.try_flush_saves())) .map(|result| result.map(|_| ())) .unwrap_or(Ok(()))?; @@ -1491,12 +1492,13 @@ fn tree_sitter_subtree( let contents = format!("```tsq\n{}\n```", selected_node.to_sexp()); let callback = async move { - let call: job::Callback = - Box::new(move |editor: &mut Editor, compositor: &mut Compositor| { + let call: job::Callback = Callback::EditorCompositor(Box::new( + move |editor: &mut Editor, compositor: &mut Compositor| { let contents = ui::Markdown::new(contents, editor.syn_loader.clone()); let popup = Popup::new("hover", contents).auto_close(true); compositor.replace_or_push("hover", popup); - }); + }, + )); Ok(call) }; @@ -1604,8 +1606,8 @@ fn run_shell_command( if !output.is_empty() { let callback = async move { - let call: job::Callback = - Box::new(move |editor: &mut Editor, compositor: &mut Compositor| { + let call: job::Callback = Callback::EditorCompositor(Box::new( + move |editor: &mut Editor, compositor: &mut Compositor| { let contents = ui::Markdown::new( format!("```sh\n{}\n```", output), editor.syn_loader.clone(), @@ -1614,7 +1616,8 @@ fn run_shell_command( helix_core::Position::new(editor.cursor().0.unwrap_or_default().row, 2), )); compositor.replace_or_push("shell", popup); - }); + }, + )); Ok(call) }; diff --git a/helix-term/src/job.rs b/helix-term/src/job.rs index e5147992..a997653d 100644 --- a/helix-term/src/job.rs +++ b/helix-term/src/job.rs @@ -5,7 +5,12 @@ use crate::compositor::Compositor; use futures_util::future::{BoxFuture, Future, FutureExt}; use futures_util::stream::{FuturesUnordered, StreamExt}; -pub type Callback = Box; +pub enum Callback { + EditorCompositor(Box), + Editor(Box), + Compositor(Box), +} + pub type JobFuture = BoxFuture<'static, anyhow::Result>>; pub struct Job { @@ -68,9 +73,11 @@ impl Jobs { ) { match call { Ok(None) => {} - Ok(Some(call)) => { - call(editor, compositor); - } + Ok(Some(call)) => match call { + Callback::EditorCompositor(call) => call(editor, compositor), + Callback::Editor(call) => call(editor), + Callback::Compositor(call) => call(compositor), + }, Err(e) => { editor.set_error(format!("Async job failed: {}", e)); } @@ -93,13 +100,41 @@ impl Jobs { } /// Blocks until all the jobs that need to be waited on are done. - pub async fn finish(&mut self) -> anyhow::Result<()> { + pub async fn finish( + &mut self, + mut editor: Option<&mut Editor>, + mut compositor: Option<&mut Compositor>, + ) -> anyhow::Result<()> { log::debug!("waiting on jobs..."); let mut wait_futures = std::mem::take(&mut self.wait_futures); while let (Some(job), tail) = wait_futures.into_future().await { match job { - Ok(_) => { + Ok(callback) => { wait_futures = tail; + + if let Some(callback) = callback { + // clippy doesn't realize this is an error without the derefs + #[allow(clippy::needless_option_as_deref)] + match callback { + Callback::EditorCompositor(call) + if editor.is_some() && compositor.is_some() => + { + call( + editor.as_deref_mut().unwrap(), + compositor.as_deref_mut().unwrap(), + ) + } + Callback::Editor(call) if editor.is_some() => { + call(editor.as_deref_mut().unwrap()) + } + Callback::Compositor(call) if compositor.is_some() => { + call(compositor.as_deref_mut().unwrap()) + } + + // skip callbacks for which we don't have the necessary references + _ => (), + } + } } Err(e) => { self.wait_futures = tail; diff --git a/helix-term/src/ui/editor.rs b/helix-term/src/ui/editor.rs index 3cd2130a..abed7f9b 100644 --- a/helix-term/src/ui/editor.rs +++ b/helix-term/src/ui/editor.rs @@ -1,7 +1,8 @@ use crate::{ commands, compositor::{Component, Context, Event, EventResult}, - job, key, + job::{self, Callback}, + key, keymap::{KeymapResult, Keymaps}, ui::{Completion, ProgressSpinners}, }; @@ -944,9 +945,9 @@ impl EditorView { // TODO: Use an on_mode_change hook to remove signature help cxt.jobs.callback(async { - let call: job::Callback = Box::new(|_editor, compositor| { + let call: job::Callback = Callback::Compositor(Box::new(|compositor| { compositor.remove(SignatureHelp::ID); - }); + })); Ok(call) }); } From 8c667ef8deea1311a8e9569909ac11d79cc993ed Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Fri, 22 Jul 2022 01:07:31 -0400 Subject: [PATCH 10/37] factor editor event handling into function --- helix-term/src/application.rs | 101 +++++++++++++++++----------------- helix-term/src/ui/mod.rs | 6 +- 2 files changed, 55 insertions(+), 52 deletions(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 84eba22a..bae3f192 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -287,9 +287,6 @@ impl Application { where S: Stream> + Unpin, { - #[cfg(feature = "integration")] - let mut idle_handled = false; - loop { if self.editor.should_close() { return false; @@ -315,56 +312,19 @@ impl Application { self.render(); } event = self.editor.wait_event() => { - log::debug!("received editor event: {:?}", event); + let _idle_handled = self.handle_editor_event(event).await; - match event { - EditorEvent::DocumentSave(event) => { - self.handle_document_write(event); - self.render(); - } - EditorEvent::ConfigEvent(event) => { - self.handle_config_events(event); - self.render(); - } - EditorEvent::LanguageServerMessage((id, call)) => { - self.handle_language_server_message(call, id).await; - // limit render calls for fast language server messages - let last = self.editor.language_servers.incoming.is_empty(); - - if last || self.last_render.elapsed() > LSP_DEADLINE { - self.render(); - self.last_render = Instant::now(); - } - } - EditorEvent::DebuggerEvent(payload) => { - let needs_render = self.editor.handle_debugger_message(payload).await; - if needs_render { - self.render(); - } - } - EditorEvent::IdleTimer => { - self.editor.clear_idle_timer(); - self.handle_idle_timeout(); - - #[cfg(feature = "integration")] - { - log::debug!("idle handled"); - idle_handled = true; - } + // for integration tests only, reset the idle timer after every + // event to signal when test events are done processing + #[cfg(feature = "integration")] + { + if _idle_handled { + return true; } - } - } - } - // for integration tests only, reset the idle timer after every - // event to signal when test events are done processing - #[cfg(feature = "integration")] - { - if idle_handled { - return true; + self.editor.reset_idle_timer(); + } } - - self.editor.reset_idle_timer(); } } } @@ -517,6 +477,49 @@ impl Application { } } + #[inline(always)] + pub async fn handle_editor_event(&mut self, event: EditorEvent) -> bool { + log::debug!("received editor event: {:?}", event); + + match event { + EditorEvent::DocumentSave(event) => { + self.handle_document_write(event); + self.render(); + } + EditorEvent::ConfigEvent(event) => { + self.handle_config_events(event); + self.render(); + } + EditorEvent::LanguageServerMessage((id, call)) => { + self.handle_language_server_message(call, id).await; + // limit render calls for fast language server messages + let last = self.editor.language_servers.incoming.is_empty(); + + if last || self.last_render.elapsed() > LSP_DEADLINE { + self.render(); + self.last_render = Instant::now(); + } + } + EditorEvent::DebuggerEvent(payload) => { + let needs_render = self.editor.handle_debugger_message(payload).await; + if needs_render { + self.render(); + } + } + EditorEvent::IdleTimer => { + self.editor.clear_idle_timer(); + self.handle_idle_timeout(); + + #[cfg(feature = "integration")] + { + return true; + } + } + } + + false + } + pub fn handle_terminal_events(&mut self, event: Result) { let mut cx = crate::compositor::Context { editor: &mut self.editor, diff --git a/helix-term/src/ui/mod.rs b/helix-term/src/ui/mod.rs index 6ac4dbb7..f99dea0b 100644 --- a/helix-term/src/ui/mod.rs +++ b/helix-term/src/ui/mod.rs @@ -14,7 +14,7 @@ mod statusline; mod text; use crate::compositor::{Component, Compositor}; -use crate::job; +use crate::job::{self, Callback}; pub use completion::Completion; pub use editor::EditorView; pub use markdown::Markdown; @@ -121,7 +121,7 @@ pub fn regex_prompt( if event == PromptEvent::Validate { let callback = async move { - let call: job::Callback = Box::new( + let call: job::Callback = Callback::EditorCompositor(Box::new( move |_editor: &mut Editor, compositor: &mut Compositor| { let contents = Text::new(format!("{}", err)); let size = compositor.size(); @@ -135,7 +135,7 @@ pub fn regex_prompt( compositor.replace_or_push("invalid-regex", popup); }, - ); + )); Ok(call) }; From faa00d4cc3de89a89679f6315c0345182929beb2 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Sun, 7 Aug 2022 00:00:15 -0400 Subject: [PATCH 11/37] increase LSP shutdown timeout The Clang LAP takes a long time to shut down on Windows --- helix-view/src/editor.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 58fcf238..27b4458f 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -1288,14 +1288,14 @@ impl Editor { } } - /// Closes language servers with timeout. The default timeout is 500 ms, use + /// Closes language servers with timeout. The default timeout is 10000 ms, use /// `timeout` parameter to override this. pub async fn close_language_servers( &self, timeout: Option, ) -> Result<(), tokio::time::error::Elapsed> { tokio::time::timeout( - Duration::from_millis(timeout.unwrap_or(500)), + Duration::from_millis(timeout.unwrap_or(10000)), future::join_all( self.language_servers .iter_clients() From e5fd5e2a9c78a3f391cda24cb57d187e248f06d1 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Tue, 9 Aug 2022 00:32:04 -0400 Subject: [PATCH 12/37] fix panic when view of pending write is closed --- helix-term/src/commands/typed.rs | 18 ++++++++---------- helix-term/src/compositor.rs | 18 ++++++++++++++++++ 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index a687b854..fa2ba5e6 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -34,6 +34,7 @@ fn quit(cx: &mut compositor::Context, args: &[Cow], event: PromptEvent) -> buffers_remaining_impl(cx.editor)? } + cx.block_try_flush_writes()?; cx.editor.close(view!(cx.editor).id); Ok(()) @@ -518,15 +519,7 @@ fn write_quit( } write_impl(cx, args.first(), false)?; - - tokio::task::block_in_place(|| helix_lsp::block_on(cx.jobs.finish(Some(cx.editor), None)))?; - - let doc = doc_mut!(cx.editor); - - tokio::task::block_in_place(|| helix_lsp::block_on(doc.try_flush_saves())) - .map(|result| result.map(|_| ())) - .unwrap_or(Ok(()))?; - + cx.block_try_flush_writes()?; quit(cx, &[], event) } @@ -540,6 +533,7 @@ fn force_write_quit( } write_impl(cx, args.first(), true)?; + cx.block_try_flush_writes()?; force_quit(cx, &[], event) } @@ -613,6 +607,8 @@ fn write_all_impl( buffers_remaining_impl(cx.editor)?; } + cx.block_try_flush_writes()?; + // close all views let views: Vec<_> = cx.editor.tree.views().map(|(view, _)| view.id).collect(); for view_id in views { @@ -682,6 +678,7 @@ fn quit_all( return Ok(()); } + cx.block_try_flush_writes()?; quit_all_impl(cx.editor, false) } @@ -710,8 +707,9 @@ fn cquit( .first() .and_then(|code| code.parse::().ok()) .unwrap_or(1); - cx.editor.exit_code = exit_code; + cx.editor.exit_code = exit_code; + cx.block_try_flush_writes()?; quit_all_impl(cx.editor, false) } diff --git a/helix-term/src/compositor.rs b/helix-term/src/compositor.rs index c0898dae..6ef77341 100644 --- a/helix-term/src/compositor.rs +++ b/helix-term/src/compositor.rs @@ -27,6 +27,24 @@ pub struct Context<'a> { pub jobs: &'a mut Jobs, } +impl<'a> Context<'a> { + /// Waits on all pending jobs, and then tries to flush all pending write + /// operations for the current document. + pub fn block_try_flush_writes(&mut self) -> anyhow::Result<()> { + tokio::task::block_in_place(|| { + helix_lsp::block_on(self.jobs.finish(Some(self.editor), None)) + })?; + + let doc = doc_mut!(self.editor); + + tokio::task::block_in_place(|| helix_lsp::block_on(doc.try_flush_saves())) + .map(|result| result.map(|_| ())) + .unwrap_or(Ok(()))?; + + Ok(()) + } +} + pub trait Component: Any + AnyComponent { /// Process input events, return true if handled. fn handle_event(&mut self, _event: &Event, _ctx: &mut Context) -> EventResult { From d544376590e9e9d649cc7406282b5ebd54dc6f0a Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Tue, 9 Aug 2022 23:32:01 -0400 Subject: [PATCH 13/37] reset idle timer for all events --- helix-term/src/application.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index bae3f192..70eae18a 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -314,18 +314,21 @@ impl Application { event = self.editor.wait_event() => { let _idle_handled = self.handle_editor_event(event).await; - // for integration tests only, reset the idle timer after every - // event to signal when test events are done processing #[cfg(feature = "integration")] { if _idle_handled { return true; } - - self.editor.reset_idle_timer(); } } } + + // for integration tests only, reset the idle timer after every + // event to signal when test events are done processing + #[cfg(feature = "integration")] + { + self.editor.reset_idle_timer(); + } } } From 7b11e9ac6941188c6f6148961fdd97e34988490e Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Tue, 23 Aug 2022 17:21:26 -0400 Subject: [PATCH 14/37] fix erroneous write sender close This was not distinguishing the error types when trying a receive on an empty receiver, which was erroneously causing the sender to be closed when trying to flush the writes when there were none --- helix-view/src/document.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index fe081442..86a1d6d2 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -13,6 +13,7 @@ use std::future::Future; use std::path::{Path, PathBuf}; use std::str::FromStr; use std::sync::Arc; +use tokio::sync::mpsc::error::TryRecvError; use tokio::sync::mpsc::{UnboundedReceiver, UnboundedSender}; use tokio::sync::Mutex; @@ -662,7 +663,16 @@ impl Document { let save_req = if block { rx.recv().await } else { - rx.try_recv().ok() + let msg = rx.try_recv(); + + if let Err(err) = msg { + match err { + TryRecvError::Empty => return None, + TryRecvError::Disconnected => None, + } + } else { + msg.ok() + } }; let save = match save_req { From 57de4e62519a59aece104867569c2b9ad044af54 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Wed, 24 Aug 2022 01:13:41 -0400 Subject: [PATCH 15/37] various fixes in write-all path --- helix-term/src/commands/typed.rs | 25 ++++-- helix-term/src/compositor.rs | 12 +-- helix-term/tests/integration.rs | 1 + helix-term/tests/test/commands.rs | 12 +-- helix-term/tests/test/helpers.rs | 28 ++++++- helix-term/tests/test/splits.rs | 122 ++++++++++++++++++++++++++++++ 6 files changed, 175 insertions(+), 25 deletions(-) create mode 100644 helix-term/tests/test/splits.rs diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index fa2ba5e6..7fd619d9 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -573,13 +573,20 @@ fn write_all_impl( return Ok(()); } - let mut errors = String::new(); + let mut errors: Option = None; let auto_format = cx.editor.config().auto_format; let jobs = &mut cx.jobs; + // save all documents for doc in &mut cx.editor.documents.values_mut() { if doc.path().is_none() { - errors.push_str("cannot write a buffer without a filename\n"); + errors = errors + .or_else(|| Some(String::new())) + .map(|mut errs: String| { + errs.push_str("cannot write a buffer without a filename\n"); + errs + }); + continue; } @@ -591,7 +598,7 @@ fn write_all_impl( doc.auto_format().map(|fmt| { let callback = make_format_callback(doc.id(), doc.version(), fmt, Some((None, force))); - jobs.callback(callback); + jobs.add(Job::with_callback(callback).wait_before_exiting()); }) } else { None @@ -603,12 +610,12 @@ fn write_all_impl( } if quit { + cx.block_try_flush_writes()?; + if !force { buffers_remaining_impl(cx.editor)?; } - cx.block_try_flush_writes()?; - // close all views let views: Vec<_> = cx.editor.tree.views().map(|(view, _)| view.id).collect(); for view_id in views { @@ -616,7 +623,13 @@ fn write_all_impl( } } - bail!(errors) + if let Some(errs) = errors { + if !force { + bail!(errs); + } + } + + Ok(()) } fn write_all( diff --git a/helix-term/src/compositor.rs b/helix-term/src/compositor.rs index 6ef77341..5077807d 100644 --- a/helix-term/src/compositor.rs +++ b/helix-term/src/compositor.rs @@ -29,17 +29,17 @@ pub struct Context<'a> { impl<'a> Context<'a> { /// Waits on all pending jobs, and then tries to flush all pending write - /// operations for the current document. + /// operations for all documents. pub fn block_try_flush_writes(&mut self) -> anyhow::Result<()> { tokio::task::block_in_place(|| { helix_lsp::block_on(self.jobs.finish(Some(self.editor), None)) })?; - let doc = doc_mut!(self.editor); - - tokio::task::block_in_place(|| helix_lsp::block_on(doc.try_flush_saves())) - .map(|result| result.map(|_| ())) - .unwrap_or(Ok(()))?; + for doc in &mut self.editor.documents.values_mut() { + tokio::task::block_in_place(|| helix_lsp::block_on(doc.try_flush_saves())) + .map(|result| result.map(|_| ())) + .unwrap_or(Ok(()))?; + } Ok(()) } diff --git a/helix-term/tests/integration.rs b/helix-term/tests/integration.rs index 8969e976..e3754c43 100644 --- a/helix-term/tests/integration.rs +++ b/helix-term/tests/integration.rs @@ -22,5 +22,6 @@ mod test { mod commands; mod movement; mod prompt; + mod splits; mod write; } diff --git a/helix-term/tests/test/commands.rs b/helix-term/tests/test/commands.rs index b7c0f7cc..0279e348 100644 --- a/helix-term/tests/test/commands.rs +++ b/helix-term/tests/test/commands.rs @@ -1,7 +1,4 @@ -use std::{ - io::{Read, Write}, - ops::RangeInclusive, -}; +use std::ops::RangeInclusive; use helix_core::diagnostic::Severity; use helix_term::application::Application; @@ -86,12 +83,7 @@ async fn test_buffer_close_concurrent() -> anyhow::Result<()> { ) .await?; - file.as_file_mut().flush()?; - file.as_file_mut().sync_all()?; - - let mut file_content = String::new(); - file.as_file_mut().read_to_string(&mut file_content)?; - assert_eq!(RANGE.end().to_string(), file_content); + helpers::assert_file_has_content(file.as_file_mut(), &RANGE.end().to_string())?; Ok(()) } diff --git a/helix-term/tests/test/helpers.rs b/helix-term/tests/test/helpers.rs index bbcc6632..ed1a0331 100644 --- a/helix-term/tests/test/helpers.rs +++ b/helix-term/tests/test/helpers.rs @@ -1,10 +1,15 @@ -use std::{io::Write, path::PathBuf, time::Duration}; +use std::{ + fs::File, + io::{Read, Write}, + path::PathBuf, + time::Duration, +}; use anyhow::bail; use crossterm::event::{Event, KeyEvent}; -use helix_core::{test, Selection, Transaction}; +use helix_core::{diagnostic::Severity, test, Selection, Transaction}; use helix_term::{application::Application, args::Args, config::Config}; -use helix_view::{doc, input::parse_macro}; +use helix_view::{doc, input::parse_macro, Editor}; use tempfile::NamedTempFile; use tokio_stream::wrappers::UnboundedReceiverStream; @@ -213,3 +218,20 @@ pub fn app_with_file>(path: P) -> anyhow::Result { Config::default(), ) } + +pub fn assert_file_has_content(file: &mut File, content: &str) -> anyhow::Result<()> { + file.flush()?; + file.sync_all()?; + + let mut file_content = String::new(); + file.read_to_string(&mut file_content)?; + assert_eq!(content, file_content); + + Ok(()) +} + +pub fn assert_status_not_error(editor: &Editor) { + if let Some((_, sev)) = editor.get_status() { + assert_ne!(&Severity::Error, sev); + } +} diff --git a/helix-term/tests/test/splits.rs b/helix-term/tests/test/splits.rs new file mode 100644 index 00000000..70a517be --- /dev/null +++ b/helix-term/tests/test/splits.rs @@ -0,0 +1,122 @@ +use super::*; + +#[tokio::test(flavor = "multi_thread")] +async fn test_split_write_quit_all() -> anyhow::Result<()> { + let mut file1 = tempfile::NamedTempFile::new()?; + let mut file2 = tempfile::NamedTempFile::new()?; + let mut file3 = tempfile::NamedTempFile::new()?; + + test_key_sequences( + &mut helpers::app_with_file(file1.path())?, + vec![ + ( + Some(&format!( + "ihello1:sp:o {}ihello2:sp:o {}ihello3", + file2.path().to_string_lossy(), + file3.path().to_string_lossy() + )), + Some(&|app| { + let docs: Vec<_> = app.editor.documents().collect(); + assert_eq!(3, docs.len()); + + let doc1 = docs + .iter() + .find(|doc| doc.path().unwrap() == file1.path()) + .unwrap(); + + assert_eq!("hello1", doc1.text().to_string()); + + let doc2 = docs + .iter() + .find(|doc| doc.path().unwrap() == file2.path()) + .unwrap(); + + assert_eq!("hello2", doc2.text().to_string()); + + let doc3 = docs + .iter() + .find(|doc| doc.path().unwrap() == file3.path()) + .unwrap(); + + assert_eq!("hello3", doc3.text().to_string()); + + helpers::assert_status_not_error(&app.editor); + assert_eq!(3, app.editor.tree.views().count()); + }), + ), + ( + Some(":wqa"), + Some(&|app| { + helpers::assert_status_not_error(&app.editor); + assert_eq!(0, app.editor.tree.views().count()); + }), + ), + ], + true, + ) + .await?; + + helpers::assert_file_has_content(file1.as_file_mut(), "hello1")?; + helpers::assert_file_has_content(file2.as_file_mut(), "hello2")?; + helpers::assert_file_has_content(file3.as_file_mut(), "hello3")?; + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_split_write_quit_same_file() -> anyhow::Result<()> { + let mut file = tempfile::NamedTempFile::new()?; + + test_key_sequences( + &mut helpers::app_with_file(file.path())?, + vec![ + ( + Some("Oihello:spogoodbye"), + Some(&|app| { + assert_eq!(2, app.editor.tree.views().count()); + helpers::assert_status_not_error(&app.editor); + + let mut docs: Vec<_> = app.editor.documents().collect(); + assert_eq!(1, docs.len()); + + let doc = docs.pop().unwrap(); + + assert_eq!( + helpers::platform_line("hello\ngoodbye"), + doc.text().to_string() + ); + + assert!(doc.is_modified()); + }), + ), + ( + Some(":wq"), + Some(&|app| { + helpers::assert_status_not_error(&app.editor); + assert_eq!(1, app.editor.tree.views().count()); + + let mut docs: Vec<_> = app.editor.documents().collect(); + assert_eq!(1, docs.len()); + + let doc = docs.pop().unwrap(); + + assert_eq!( + helpers::platform_line("hello\ngoodbye"), + doc.text().to_string() + ); + + assert!(!doc.is_modified()); + }), + ), + ], + false, + ) + .await?; + + helpers::assert_file_has_content( + file.as_file_mut(), + &helpers::platform_line("hello\ngoodbye"), + )?; + + Ok(()) +} From 6cffc7f05d0ca983690b46300cc82933389172c8 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Sun, 28 Aug 2022 23:23:00 -0400 Subject: [PATCH 16/37] Add note about log level for integration tests --- docs/CONTRIBUTING.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md index 353cb4fd..491cd424 100644 --- a/docs/CONTRIBUTING.md +++ b/docs/CONTRIBUTING.md @@ -35,7 +35,8 @@ to `cargo install` anything either). Integration tests for helix-term can be run with `cargo integration-test`. Code contributors are strongly encouraged to write integration tests for their code. Existing tests can be used as examples. Helpers can be found in -[helpers.rs][helpers.rs] +[helpers.rs][helpers.rs]. The log level can be set with the `HELIX_LOG_LEVEL` +environment variable, e.g. `HELIX_LOG_LEVEL=debug cargo integration-test`. ## Minimum Stable Rust Version (MSRV) Policy From f82a551b98cec165ac91aae15ba656a811229fde Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Tue, 30 Aug 2022 23:08:15 -0400 Subject: [PATCH 17/37] Rename doc save event names to past tense --- helix-term/src/application.rs | 6 +++--- helix-term/src/job.rs | 1 + helix-view/src/document.rs | 24 ++++++++++++------------ helix-view/src/editor.rs | 6 +++--- 4 files changed, 19 insertions(+), 18 deletions(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 70eae18a..a06460de 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -9,7 +9,7 @@ use helix_core::{ use helix_lsp::{lsp, util::lsp_pos_to_pos, LspProgressMap}; use helix_view::{ align_view, - document::DocumentSaveEventResult, + document::DocumentSavedEventResult, editor::{ConfigEvent, EditorEvent}, theme, tree::Layout, @@ -431,7 +431,7 @@ impl Application { } } - pub fn handle_document_write(&mut self, doc_save_event: DocumentSaveEventResult) { + pub fn handle_document_write(&mut self, doc_save_event: DocumentSavedEventResult) { if let Err(err) = doc_save_event { self.editor.set_error(err.to_string()); return; @@ -485,7 +485,7 @@ impl Application { log::debug!("received editor event: {:?}", event); match event { - EditorEvent::DocumentSave(event) => { + EditorEvent::DocumentSaved(event) => { self.handle_document_write(event); self.render(); } diff --git a/helix-term/src/job.rs b/helix-term/src/job.rs index a997653d..3c9e4511 100644 --- a/helix-term/src/job.rs +++ b/helix-term/src/job.rs @@ -107,6 +107,7 @@ impl Jobs { ) -> anyhow::Result<()> { log::debug!("waiting on jobs..."); let mut wait_futures = std::mem::take(&mut self.wait_futures); + while let (Some(job), tail) = wait_futures.into_future().await { match job { Ok(callback) => { diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 86a1d6d2..91d5f8aa 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -89,14 +89,14 @@ impl Serialize for Mode { /// A snapshot of the text of a document that we want to write out to disk #[derive(Debug, Clone)] -pub struct DocumentSaveEvent { +pub struct DocumentSavedEvent { pub revision: usize, pub doc_id: DocumentId, pub path: PathBuf, } -pub type DocumentSaveEventResult = Result; -pub type DocumentSaveEventFuture = BoxFuture<'static, DocumentSaveEventResult>; +pub type DocumentSavedEventResult = Result; +pub type DocumentSavedEventFuture = BoxFuture<'static, DocumentSavedEventResult>; pub struct Document { pub(crate) id: DocumentId, @@ -133,9 +133,9 @@ pub struct Document { last_saved_revision: usize, version: i32, // should be usize? pub(crate) modified_since_accessed: bool, - save_sender: Option>, - save_receiver: Option>, - current_save: Arc>>, + save_sender: Option>, + save_receiver: Option>, + current_save: Arc>>, diagnostics: Vec, language_server: Option>, @@ -616,7 +616,7 @@ impl Document { let mut file = File::create(&path).await?; to_writer(&mut file, encoding, &text).await?; - let event = DocumentSaveEvent { + let event = DocumentSavedEvent { revision: current_rev, doc_id, path, @@ -643,11 +643,11 @@ impl Document { .map_err(|err| anyhow!("failed to send save event: {}", err)) } - pub async fn await_save(&mut self) -> Option { + pub async fn await_save(&mut self) -> Option { self.await_save_impl(true).await } - async fn await_save_impl(&mut self, block: bool) -> Option { + async fn await_save_impl(&mut self, block: bool) -> Option { let mut current_save = self.current_save.lock().await; if let Some(ref mut save) = *current_save { log::trace!("reawaiting save of '{:?}'", self.path()); @@ -698,11 +698,11 @@ impl Document { /// Flushes the queue of pending writes. If any fail, /// it stops early before emptying the rest of the queue. - pub async fn try_flush_saves(&mut self) -> Option { + pub async fn try_flush_saves(&mut self) -> Option { self.flush_saves_impl(false).await } - async fn flush_saves_impl(&mut self, block: bool) -> Option { + async fn flush_saves_impl(&mut self, block: bool) -> Option { let mut final_result = None; while let Some(save_event) = self.await_save_impl(block).await { @@ -734,7 +734,7 @@ impl Document { /// Prepares the Document for being closed by stopping any new writes /// and flushing through the queue of pending writes. If any fail, /// it stops early before emptying the rest of the queue. - pub async fn close(&mut self) -> Option { + pub async fn close(&mut self) -> Option { if self.save_sender.is_some() { self.save_sender.take(); } diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 27b4458f..fbd0b2b0 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -1,6 +1,6 @@ use crate::{ clipboard::{get_clipboard_provider, ClipboardProvider}, - document::{DocumentSaveEventResult, Mode}, + document::{DocumentSavedEventResult, Mode}, graphics::{CursorKind, Rect}, info::Info, input::KeyEvent, @@ -691,7 +691,7 @@ pub struct Editor { #[derive(Debug)] pub enum EditorEvent { - DocumentSave(DocumentSaveEventResult), + DocumentSaved(DocumentSavedEventResult), ConfigEvent(ConfigEvent), LanguageServerMessage((usize, Call)), DebuggerEvent(dap::Payload), @@ -1317,7 +1317,7 @@ impl Editor { biased; Some(Some(event)) = saves.next() => { - EditorEvent::DocumentSave(event) + EditorEvent::DocumentSaved(event) } Some(config_event) = self.config_events.1.recv() => { EditorEvent::ConfigEvent(config_event) From 18c32118b1df63895b662c1b37ada28ad0d5c9b5 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Tue, 30 Aug 2022 23:10:33 -0400 Subject: [PATCH 18/37] Save text in document saved events, use in status message --- helix-term/src/application.rs | 4 ++-- helix-view/src/document.rs | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index a06460de..793993ee 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -459,8 +459,8 @@ impl Application { doc.set_last_saved_revision(doc_save_event.revision); - let lines = doc.text().len_lines(); - let bytes = doc.text().len_bytes(); + let lines = doc_save_event.text.len_lines(); + let bytes = doc_save_event.text.len_bytes(); if let Err(err) = doc.set_path(Some(&doc_save_event.path)) { log::error!( diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 91d5f8aa..61bea527 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -93,6 +93,7 @@ pub struct DocumentSavedEvent { pub revision: usize, pub doc_id: DocumentId, pub path: PathBuf, + pub text: Rope, } pub type DocumentSavedEventResult = Result; @@ -620,6 +621,7 @@ impl Document { revision: current_rev, doc_id, path, + text: text.clone(), }; if let Some(language_server) = language_server { From af03df3413f04ce7079d14388ce42fe70bd1397e Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Wed, 31 Aug 2022 15:08:00 -0400 Subject: [PATCH 19/37] fix write scratch buffer to file --- helix-term/src/commands/typed.rs | 4 -- helix-term/tests/test/write.rs | 71 ++++++++++++++++++++++++-------- helix-view/src/document.rs | 18 +++++--- 3 files changed, 67 insertions(+), 26 deletions(-) diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index 7fd619d9..d312c45f 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -273,10 +273,6 @@ fn write_impl( let doc = doc_mut!(cx.editor); let path = path.map(AsRef::as_ref); - if doc.path().is_none() { - bail!("cannot write a buffer without a filename"); - } - let fmt = if editor_auto_fmt { doc.auto_format().map(|fmt| { let callback = make_format_callback( diff --git a/helix-term/tests/test/write.rs b/helix-term/tests/test/write.rs index 544f1ba1..7d105431 100644 --- a/helix-term/tests/test/write.rs +++ b/helix-term/tests/test/write.rs @@ -128,6 +128,52 @@ async fn test_write_fail_mod_flag() -> anyhow::Result<()> { Ok(()) } +#[tokio::test] +async fn test_write_scratch_to_new_path() -> anyhow::Result<()> { + let mut file = tempfile::NamedTempFile::new()?; + + test_key_sequence( + &mut Application::new(Args::default(), Config::default())?, + Some(format!("ihello:w {}", file.path().to_string_lossy()).as_ref()), + Some(&|app| { + assert!(!app.editor.is_err()); + + let mut docs: Vec<_> = app.editor.documents().collect(); + assert_eq!(1, docs.len()); + + let doc = docs.pop().unwrap(); + assert_eq!(Some(&file.path().to_path_buf()), doc.path()); + }), + false, + ) + .await?; + + helpers::assert_file_has_content(file.as_file_mut(), &helpers::platform_line("hello"))?; + + Ok(()) +} + +#[tokio::test] +async fn test_write_scratch_no_path_fails() -> anyhow::Result<()> { + helpers::test_key_sequence_with_input_text( + None, + ("#[\n|]#", "ihello:w", "hello#[\n|]#"), + &|app| { + assert!(app.editor.is_err()); + + let mut docs: Vec<_> = app.editor.documents().collect(); + assert_eq!(1, docs.len()); + + let doc = docs.pop().unwrap(); + assert_eq!(None, doc.path()); + }, + false, + ) + .await?; + + Ok(()) +} + #[tokio::test] async fn test_write_new_path() -> anyhow::Result<()> { let mut file1 = tempfile::NamedTempFile::new().unwrap(); @@ -164,24 +210,15 @@ async fn test_write_new_path() -> anyhow::Result<()> { ) .await?; - file1.as_file_mut().flush()?; - file1.as_file_mut().sync_all()?; - file2.as_file_mut().flush()?; - file2.as_file_mut().sync_all()?; + helpers::assert_file_has_content( + file1.as_file_mut(), + &helpers::platform_line("i can eat glass, it will not hurt me\n"), + )?; - let mut file1_content = String::new(); - file1.as_file_mut().read_to_string(&mut file1_content)?; - assert_eq!( - helpers::platform_line("i can eat glass, it will not hurt me\n"), - file1_content - ); - - let mut file2_content = String::new(); - file2.as_file_mut().read_to_string(&mut file2_content)?; - assert_eq!( - helpers::platform_line("i can eat glass, it will not hurt me\n"), - file2_content - ); + helpers::assert_file_has_content( + file2.as_file_mut(), + &helpers::platform_line("i can eat glass, it will not hurt me\n"), + )?; Ok(()) } diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 61bea527..ff64689e 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -574,7 +574,12 @@ impl Document { } }; - let identifier = self.identifier(); + let identifier = if self.path().is_some() { + Some(self.identifier()) + } else { + None + }; + let language_server = self.language_server.clone(); // mark changes up to now as saved @@ -628,10 +633,13 @@ impl Document { if !language_server.is_initialized() { return Ok(event); } - if let Some(notification) = - language_server.text_document_did_save(identifier, &text) - { - notification.await?; + + if let Some(identifier) = identifier { + if let Some(notification) = + language_server.text_document_did_save(identifier, &text) + { + notification.await?; + } } } From b3fc31a211293f48696d26855781577d1859c2c6 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Fri, 2 Sep 2022 09:20:28 -0400 Subject: [PATCH 20/37] move language server refresh to document saved event handler --- helix-term/src/application.rs | 42 ++++++++++++++++++++++------------- helix-term/src/commands.rs | 9 +------- 2 files changed, 28 insertions(+), 23 deletions(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 793993ee..60610c1d 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -462,22 +462,34 @@ impl Application { let lines = doc_save_event.text.len_lines(); let bytes = doc_save_event.text.len_bytes(); - if let Err(err) = doc.set_path(Some(&doc_save_event.path)) { - log::error!( - "error setting path for doc '{:?}': {}", - doc.path(), - err.to_string(), - ); - self.editor.set_error(err.to_string()); - } else { - // TODO: fix being overwritten by lsp - self.editor.set_status(format!( - "'{}' written, {}L {}B", - get_relative_path(&doc_save_event.path).to_string_lossy(), - lines, - bytes - )); + if doc.path() != Some(&doc_save_event.path) { + if let Err(err) = doc.set_path(Some(&doc_save_event.path)) { + log::error!( + "error setting path for doc '{:?}': {}", + doc.path(), + err.to_string(), + ); + + self.editor.set_error(err.to_string()); + return; + } + + let loader = self.editor.syn_loader.clone(); + + // borrowing the same doc again to get around the borrow checker + let doc = self.editor.document_mut(doc_save_event.doc_id).unwrap(); + let id = doc.id(); + doc.detect_language(loader); + let _ = self.editor.refresh_language_server(id); } + + // TODO: fix being overwritten by lsp + self.editor.set_status(format!( + "'{}' written, {}L {}B", + get_relative_path(&doc_save_event.path).to_string_lossy(), + lines, + bytes + )); } #[inline(always)] diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index afd94564..6deecbe2 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -2519,7 +2519,7 @@ async fn make_format_callback( write: Option<(Option, bool)>, ) -> anyhow::Result { let format = format.await?; - let call: job::Callback = Callback::EditorCompositor(Box::new(move |editor, _compositor| { + let call: job::Callback = Callback::Editor(Box::new(move |editor| { if !editor.documents.contains_key(&doc_id) { return; } @@ -2527,7 +2527,6 @@ async fn make_format_callback( let scrolloff = editor.config().scrolloff; let doc = doc_mut!(editor, &doc_id); let view = view_mut!(editor); - let loader = editor.syn_loader.clone(); if doc.version() == doc_version { apply_transaction(&format, doc, view); @@ -2536,14 +2535,8 @@ async fn make_format_callback( view.ensure_cursor_in_view(doc, scrolloff); if let Some((path, force)) = write { - let refresh_lang = path.is_some(); - if let Err(err) = doc.save(path, force) { editor.set_error(format!("Error saving: {}", err)); - } else if refresh_lang { - let id = doc.id(); - doc.detect_language(loader); - let _ = editor.refresh_language_server(id); } } } else { From b530a86d1f15cc7df0e1ae8aa4bd02109ac33a8f Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Fri, 2 Sep 2022 23:38:38 -0400 Subject: [PATCH 21/37] remove Callback::Compositor variant To reduce likelihood of accidental discarding of important callbacks --- helix-term/src/application.rs | 2 +- helix-term/src/commands/dap.rs | 18 ++++++++++-------- helix-term/src/compositor.rs | 4 +--- helix-term/src/job.rs | 20 ++++---------------- helix-term/src/ui/editor.rs | 7 ++++--- 5 files changed, 20 insertions(+), 31 deletions(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 60610c1d..fe53d73d 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -991,7 +991,7 @@ impl Application { let mut result = match self .jobs - .finish(Some(&mut self.editor), Some(&mut self.compositor)) + .finish(&mut self.editor, Some(&mut self.compositor)) .await { Ok(_) => Ok(()), diff --git a/helix-term/src/commands/dap.rs b/helix-term/src/commands/dap.rs index 9c067eba..c27417e3 100644 --- a/helix-term/src/commands/dap.rs +++ b/helix-term/src/commands/dap.rs @@ -277,10 +277,11 @@ pub fn dap_launch(cx: &mut Context) { let completions = template.completion.clone(); let name = template.name.clone(); let callback = Box::pin(async move { - let call: Callback = Callback::Compositor(Box::new(move |compositor| { - let prompt = debug_parameter_prompt(completions, name, Vec::new()); - compositor.push(Box::new(prompt)); - })); + let call: Callback = + Callback::EditorCompositor(Box::new(move |_editor, compositor| { + let prompt = debug_parameter_prompt(completions, name, Vec::new()); + compositor.push(Box::new(prompt)); + })); Ok(call) }); cx.jobs.callback(callback); @@ -335,10 +336,11 @@ fn debug_parameter_prompt( let config_name = config_name.clone(); let params = params.clone(); let callback = Box::pin(async move { - let call: Callback = Callback::Compositor(Box::new(move |compositor| { - let prompt = debug_parameter_prompt(completions, config_name, params); - compositor.push(Box::new(prompt)); - })); + let call: Callback = + Callback::EditorCompositor(Box::new(move |_editor, compositor| { + let prompt = debug_parameter_prompt(completions, config_name, params); + compositor.push(Box::new(prompt)); + })); Ok(call) }); cx.jobs.callback(callback); diff --git a/helix-term/src/compositor.rs b/helix-term/src/compositor.rs index 5077807d..35b9d054 100644 --- a/helix-term/src/compositor.rs +++ b/helix-term/src/compositor.rs @@ -31,9 +31,7 @@ impl<'a> Context<'a> { /// Waits on all pending jobs, and then tries to flush all pending write /// operations for all documents. pub fn block_try_flush_writes(&mut self) -> anyhow::Result<()> { - tokio::task::block_in_place(|| { - helix_lsp::block_on(self.jobs.finish(Some(self.editor), None)) - })?; + tokio::task::block_in_place(|| helix_lsp::block_on(self.jobs.finish(self.editor, None)))?; for doc in &mut self.editor.documents.values_mut() { tokio::task::block_in_place(|| helix_lsp::block_on(doc.try_flush_saves())) diff --git a/helix-term/src/job.rs b/helix-term/src/job.rs index 3c9e4511..2888b6eb 100644 --- a/helix-term/src/job.rs +++ b/helix-term/src/job.rs @@ -8,7 +8,6 @@ use futures_util::stream::{FuturesUnordered, StreamExt}; pub enum Callback { EditorCompositor(Box), Editor(Box), - Compositor(Box), } pub type JobFuture = BoxFuture<'static, anyhow::Result>>; @@ -76,7 +75,6 @@ impl Jobs { Ok(Some(call)) => match call { Callback::EditorCompositor(call) => call(editor, compositor), Callback::Editor(call) => call(editor), - Callback::Compositor(call) => call(compositor), }, Err(e) => { editor.set_error(format!("Async job failed: {}", e)); @@ -102,7 +100,7 @@ impl Jobs { /// Blocks until all the jobs that need to be waited on are done. pub async fn finish( &mut self, - mut editor: Option<&mut Editor>, + editor: &mut Editor, mut compositor: Option<&mut Compositor>, ) -> anyhow::Result<()> { log::debug!("waiting on jobs..."); @@ -117,20 +115,10 @@ impl Jobs { // clippy doesn't realize this is an error without the derefs #[allow(clippy::needless_option_as_deref)] match callback { - Callback::EditorCompositor(call) - if editor.is_some() && compositor.is_some() => - { - call( - editor.as_deref_mut().unwrap(), - compositor.as_deref_mut().unwrap(), - ) - } - Callback::Editor(call) if editor.is_some() => { - call(editor.as_deref_mut().unwrap()) - } - Callback::Compositor(call) if compositor.is_some() => { - call(compositor.as_deref_mut().unwrap()) + Callback::EditorCompositor(call) if compositor.is_some() => { + call(editor, compositor.as_deref_mut().unwrap()) } + Callback::Editor(call) => call(editor), // skip callbacks for which we don't have the necessary references _ => (), diff --git a/helix-term/src/ui/editor.rs b/helix-term/src/ui/editor.rs index abed7f9b..73dfd52c 100644 --- a/helix-term/src/ui/editor.rs +++ b/helix-term/src/ui/editor.rs @@ -945,9 +945,10 @@ impl EditorView { // TODO: Use an on_mode_change hook to remove signature help cxt.jobs.callback(async { - let call: job::Callback = Callback::Compositor(Box::new(|compositor| { - compositor.remove(SignatureHelp::ID); - })); + let call: job::Callback = + Callback::EditorCompositor(Box::new(|_editor, compositor| { + compositor.remove(SignatureHelp::ID); + })); Ok(call) }); } From 3f07885b351748c5b8225aadb165f8ef7066f047 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Fri, 16 Sep 2022 23:17:48 -0400 Subject: [PATCH 22/37] document should save even if formatter fails --- helix-core/src/syntax.rs | 6 ++ helix-term/src/application.rs | 15 ++--- helix-term/src/commands.rs | 27 +++++---- helix-term/src/main.rs | 12 +++- helix-term/tests/test/auto_indent.rs | 1 + helix-term/tests/test/auto_pairs.rs | 1 + helix-term/tests/test/commands.rs | 14 +++-- helix-term/tests/test/helpers.rs | 90 ++++++++++++++++++++++++---- helix-term/tests/test/movement.rs | 4 +- helix-term/tests/test/prompt.rs | 4 +- helix-term/tests/test/splits.rs | 11 +++- helix-term/tests/test/write.rs | 66 +++++++++++++------- 12 files changed, 185 insertions(+), 66 deletions(-) diff --git a/helix-core/src/syntax.rs b/helix-core/src/syntax.rs index 61d382fd..f907629f 100644 --- a/helix-core/src/syntax.rs +++ b/helix-core/src/syntax.rs @@ -61,6 +61,12 @@ pub struct Configuration { pub language: Vec, } +impl Default for Configuration { + fn default() -> Self { + crate::config::default_syntax_loader() + } +} + // largely based on tree-sitter/cli/src/loader.rs #[derive(Debug, Serialize, Deserialize)] #[serde(rename_all = "kebab-case", deny_unknown_fields)] diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index fe53d73d..4fde2a66 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -1,7 +1,6 @@ use arc_swap::{access::Map, ArcSwap}; use futures_util::Stream; use helix_core::{ - config::{default_syntax_loader, user_syntax_loader}, diagnostic::{DiagnosticTag, NumberOrString}, path::get_relative_path, pos_at_coords, syntax, Selection, @@ -110,7 +109,11 @@ fn restore_term() -> Result<(), Error> { } impl Application { - pub fn new(args: Args, config: Config) -> Result { + pub fn new( + args: Args, + config: Config, + syn_loader_conf: syntax::Configuration, + ) -> Result { #[cfg(feature = "integration")] setup_integration_logging(); @@ -137,14 +140,6 @@ impl Application { }) .unwrap_or_else(|| theme_loader.default_theme(true_color)); - let syn_loader_conf = user_syntax_loader().unwrap_or_else(|err| { - eprintln!("Bad language config: {}", err); - eprintln!("Press to continue with default language config"); - use std::io::Read; - // This waits for an enter press. - let _ = std::io::stdin().read(&mut []); - default_syntax_loader() - }); let syn_loader = std::sync::Arc::new(syntax::Loader::new(syn_loader_conf)); let mut compositor = Compositor::new().context("build compositor")?; diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 6deecbe2..f6d583f5 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -2518,7 +2518,8 @@ async fn make_format_callback( format: impl Future> + Send + 'static, write: Option<(Option, bool)>, ) -> anyhow::Result { - let format = format.await?; + let format = format.await; + let call: job::Callback = Callback::Editor(Box::new(move |editor| { if !editor.documents.contains_key(&doc_id) { return; @@ -2528,19 +2529,21 @@ async fn make_format_callback( let doc = doc_mut!(editor, &doc_id); let view = view_mut!(editor); - if doc.version() == doc_version { - apply_transaction(&format, doc, view); - doc.append_changes_to_history(view.id); - doc.detect_indent_and_line_ending(); - view.ensure_cursor_in_view(doc, scrolloff); + if let Ok(format) = format { + if doc.version() == doc_version { + apply_transaction(&format, doc, view); + doc.append_changes_to_history(view.id); + doc.detect_indent_and_line_ending(); + view.ensure_cursor_in_view(doc, scrolloff); + } else { + log::info!("discarded formatting changes because the document changed"); + } + } - if let Some((path, force)) = write { - if let Err(err) = doc.save(path, force) { - editor.set_error(format!("Error saving: {}", err)); - } + if let Some((path, force)) = write { + if let Err(err) = doc.save(path, force) { + editor.set_error(format!("Error saving: {}", err)); } - } else { - log::info!("discarded formatting changes because the document changed"); } })); diff --git a/helix-term/src/main.rs b/helix-term/src/main.rs index 726bf9e3..96b695c6 100644 --- a/helix-term/src/main.rs +++ b/helix-term/src/main.rs @@ -139,8 +139,18 @@ FLAGS: Err(err) => return Err(Error::new(err)), }; + let syn_loader_conf = helix_core::config::user_syntax_loader().unwrap_or_else(|err| { + eprintln!("Bad language config: {}", err); + eprintln!("Press to continue with default language config"); + use std::io::Read; + // This waits for an enter press. + let _ = std::io::stdin().read(&mut []); + helix_core::config::default_syntax_loader() + }); + // TODO: use the thread local executor to spawn the application task separately from the work pool - let mut app = Application::new(args, config).context("unable to create new application")?; + let mut app = Application::new(args, config, syn_loader_conf) + .context("unable to create new application")?; let exit_code = app.run(&mut EventStream::new()).await?; diff --git a/helix-term/tests/test/auto_indent.rs b/helix-term/tests/test/auto_indent.rs index 2f638893..5c093a5d 100644 --- a/helix-term/tests/test/auto_indent.rs +++ b/helix-term/tests/test/auto_indent.rs @@ -8,6 +8,7 @@ async fn auto_indent_c() -> anyhow::Result<()> { ..Default::default() }, Config::default(), + helpers::test_syntax_conf(None), // switches to append mode? ( helpers::platform_line("void foo() {#[|}]#").as_ref(), diff --git a/helix-term/tests/test/auto_pairs.rs b/helix-term/tests/test/auto_pairs.rs index ec47a5b4..caf80bd4 100644 --- a/helix-term/tests/test/auto_pairs.rs +++ b/helix-term/tests/test/auto_pairs.rs @@ -13,6 +13,7 @@ async fn auto_pairs_basic() -> anyhow::Result<()> { }, ..Default::default() }, + helpers::test_syntax_conf(None), ("#[\n|]#", "i(", "(#[|\n]#"), ) .await?; diff --git a/helix-term/tests/test/commands.rs b/helix-term/tests/test/commands.rs index 0279e348..5238cc69 100644 --- a/helix-term/tests/test/commands.rs +++ b/helix-term/tests/test/commands.rs @@ -1,16 +1,18 @@ use std::ops::RangeInclusive; use helix_core::diagnostic::Severity; -use helix_term::application::Application; use super::*; #[tokio::test(flavor = "multi_thread")] async fn test_write_quit_fail() -> anyhow::Result<()> { let file = helpers::new_readonly_tempfile()?; + let mut app = helpers::AppBuilder::new() + .with_file(file.path(), None) + .build()?; test_key_sequence( - &mut helpers::app_with_file(file.path())?, + &mut app, Some("ihello:wq"), Some(&|app| { let mut docs: Vec<_> = app.editor.documents().collect(); @@ -30,7 +32,7 @@ async fn test_write_quit_fail() -> anyhow::Result<()> { #[tokio::test(flavor = "multi_thread")] async fn test_buffer_close_concurrent() -> anyhow::Result<()> { test_key_sequences( - &mut Application::new(Args::default(), Config::default())?, + &mut helpers::AppBuilder::new().build()?, vec![ ( None, @@ -70,8 +72,12 @@ async fn test_buffer_close_concurrent() -> anyhow::Result<()> { command.push_str(":bufferclose"); + let mut app = helpers::AppBuilder::new() + .with_file(file.path(), None) + .build()?; + test_key_sequence( - &mut helpers::app_with_file(file.path())?, + &mut app, Some(&command), Some(&|app| { assert!(!app.editor.is_err(), "error: {:?}", app.editor.get_status()); diff --git a/helix-term/tests/test/helpers.rs b/helix-term/tests/test/helpers.rs index ed1a0331..c2fbe953 100644 --- a/helix-term/tests/test/helpers.rs +++ b/helix-term/tests/test/helpers.rs @@ -108,7 +108,7 @@ pub async fn test_key_sequence_with_input_text>( let test_case = test_case.into(); let mut app = match app { Some(app) => app, - None => Application::new(Args::default(), Config::default())?, + None => Application::new(Args::default(), Config::default(), test_syntax_conf(None))?, }; let (view, doc) = helix_view::current!(app.editor); @@ -132,16 +132,30 @@ pub async fn test_key_sequence_with_input_text>( .await } +/// Generates language configs that merge in overrides, like a user language +/// config. The argument string must be a raw TOML document. +pub fn test_syntax_conf(overrides: Option) -> helix_core::syntax::Configuration { + let mut lang = helix_loader::config::default_lang_config(); + + if let Some(overrides) = overrides { + let override_toml = toml::from_str(&overrides).unwrap(); + lang = helix_loader::merge_toml_values(lang, override_toml, 3); + } + + lang.try_into().unwrap() +} + /// Use this for very simple test cases where there is one input /// document, selection, and sequence of key presses, and you just /// want to verify the resulting document and selection. pub async fn test_with_config>( args: Args, config: Config, + syn_conf: helix_core::syntax::Configuration, test_case: T, ) -> anyhow::Result<()> { let test_case = test_case.into(); - let app = Application::new(args, config)?; + let app = Application::new(args, config, syn_conf)?; test_key_sequence_with_input_text( Some(app), @@ -162,7 +176,13 @@ pub async fn test_with_config>( } pub async fn test>(test_case: T) -> anyhow::Result<()> { - test_with_config(Args::default(), Config::default(), test_case).await + test_with_config( + Args::default(), + Config::default(), + test_syntax_conf(None), + test_case, + ) + .await } pub fn temp_file_with_contents>( @@ -207,16 +227,60 @@ pub fn new_readonly_tempfile() -> anyhow::Result { Ok(file) } -/// Creates a new Application with default config that opens the given file -/// path -pub fn app_with_file>(path: P) -> anyhow::Result { - Application::new( - Args { - files: vec![(path.into(), helix_core::Position::default())], - ..Default::default() - }, - Config::default(), - ) +#[derive(Default)] +pub struct AppBuilder { + args: Args, + config: Config, + syn_conf: helix_core::syntax::Configuration, + input: Option<(String, Selection)>, +} + +impl AppBuilder { + pub fn new() -> Self { + AppBuilder::default() + } + + pub fn with_file>( + mut self, + path: P, + pos: Option, + ) -> Self { + self.args.files.push((path.into(), pos.unwrap_or_default())); + self + } + + pub fn with_config(mut self, config: Config) -> Self { + self.config = config; + self + } + + pub fn with_input_text>(mut self, input_text: S) -> Self { + self.input = Some(test::print(&input_text.into())); + self + } + + pub fn with_lang_config(mut self, syn_conf: helix_core::syntax::Configuration) -> Self { + self.syn_conf = syn_conf; + self + } + + pub fn build(self) -> anyhow::Result { + let mut app = Application::new(self.args, self.config, self.syn_conf)?; + + if let Some((text, selection)) = self.input { + let (view, doc) = helix_view::current!(app.editor); + let sel = doc.selection(view.id).clone(); + let trans = Transaction::change_by_selection(doc.text(), &sel, |_| { + (0, doc.text().len_chars(), Some((text.clone()).into())) + }) + .with_selection(selection); + + // replace the initial text with the input text + doc.apply(&trans, view.id); + } + + Ok(app) + } } pub fn assert_file_has_content(file: &mut File, content: &str) -> anyhow::Result<()> { diff --git a/helix-term/tests/test/movement.rs b/helix-term/tests/test/movement.rs index 45aae39e..7212d026 100644 --- a/helix-term/tests/test/movement.rs +++ b/helix-term/tests/test/movement.rs @@ -70,7 +70,9 @@ async fn insert_to_normal_mode_cursor_position() -> anyhow::Result<()> { async fn cursor_position_newly_opened_file() -> anyhow::Result<()> { let test = |content: &str, expected_sel: Selection| -> anyhow::Result<()> { let file = helpers::temp_file_with_contents(content)?; - let mut app = helpers::app_with_file(file.path())?; + let mut app = helpers::AppBuilder::new() + .with_file(file.path(), None) + .build()?; let (view, doc) = helix_view::current!(app.editor); let sel = doc.selection(view.id).clone(); diff --git a/helix-term/tests/test/prompt.rs b/helix-term/tests/test/prompt.rs index 2ab9604c..62ec03f1 100644 --- a/helix-term/tests/test/prompt.rs +++ b/helix-term/tests/test/prompt.rs @@ -1,11 +1,9 @@ use super::*; -use helix_term::application::Application; - #[tokio::test] async fn test_history_completion() -> anyhow::Result<()> { test_key_sequence( - &mut Application::new(Args::default(), Config::default())?, + &mut AppBuilder::new().build()?, Some(":asdf:theme d"), Some(&|app| { assert!(!app.editor.is_err()); diff --git a/helix-term/tests/test/splits.rs b/helix-term/tests/test/splits.rs index 70a517be..5807413a 100644 --- a/helix-term/tests/test/splits.rs +++ b/helix-term/tests/test/splits.rs @@ -6,8 +6,12 @@ async fn test_split_write_quit_all() -> anyhow::Result<()> { let mut file2 = tempfile::NamedTempFile::new()?; let mut file3 = tempfile::NamedTempFile::new()?; + let mut app = helpers::AppBuilder::new() + .with_file(file1.path(), None) + .build()?; + test_key_sequences( - &mut helpers::app_with_file(file1.path())?, + &mut app, vec![ ( Some(&format!( @@ -66,9 +70,12 @@ async fn test_split_write_quit_all() -> anyhow::Result<()> { #[tokio::test(flavor = "multi_thread")] async fn test_split_write_quit_same_file() -> anyhow::Result<()> { let mut file = tempfile::NamedTempFile::new()?; + let mut app = helpers::AppBuilder::new() + .with_file(file.path(), None) + .build()?; test_key_sequences( - &mut helpers::app_with_file(file.path())?, + &mut app, vec![ ( Some("Oihello:spogoodbye"), diff --git a/helix-term/tests/test/write.rs b/helix-term/tests/test/write.rs index 7d105431..6aa51a31 100644 --- a/helix-term/tests/test/write.rs +++ b/helix-term/tests/test/write.rs @@ -4,7 +4,6 @@ use std::{ }; use helix_core::diagnostic::Severity; -use helix_term::application::Application; use helix_view::doc; use super::*; @@ -12,9 +11,12 @@ use super::*; #[tokio::test] async fn test_write() -> anyhow::Result<()> { let mut file = tempfile::NamedTempFile::new()?; + let mut app = helpers::AppBuilder::new() + .with_file(file.path(), None) + .build()?; test_key_sequence( - &mut helpers::app_with_file(file.path())?, + &mut app, Some("ithe gostak distims the doshes:w"), None, false, @@ -38,9 +40,12 @@ async fn test_write() -> anyhow::Result<()> { #[tokio::test(flavor = "multi_thread")] async fn test_write_quit() -> anyhow::Result<()> { let mut file = tempfile::NamedTempFile::new()?; + let mut app = helpers::AppBuilder::new() + .with_file(file.path(), None) + .build()?; test_key_sequence( - &mut helpers::app_with_file(file.path())?, + &mut app, Some("ithe gostak distims the doshes:wq"), None, true, @@ -66,19 +71,16 @@ async fn test_write_concurrent() -> anyhow::Result<()> { let mut file = tempfile::NamedTempFile::new()?; let mut command = String::new(); const RANGE: RangeInclusive = 1..=5000; + let mut app = helpers::AppBuilder::new() + .with_file(file.path(), None) + .build()?; for i in RANGE { let cmd = format!("%c{}:w", i); command.push_str(&cmd); } - test_key_sequence( - &mut helpers::app_with_file(file.path())?, - Some(&command), - None, - false, - ) - .await?; + test_key_sequence(&mut app, Some(&command), None, false).await?; file.as_file_mut().flush()?; file.as_file_mut().sync_all()?; @@ -93,9 +95,12 @@ async fn test_write_concurrent() -> anyhow::Result<()> { #[tokio::test] async fn test_write_fail_mod_flag() -> anyhow::Result<()> { let file = helpers::new_readonly_tempfile()?; + let mut app = helpers::AppBuilder::new() + .with_file(file.path(), None) + .build()?; test_key_sequences( - &mut helpers::app_with_file(file.path())?, + &mut app, vec![ ( None, @@ -133,7 +138,7 @@ async fn test_write_scratch_to_new_path() -> anyhow::Result<()> { let mut file = tempfile::NamedTempFile::new()?; test_key_sequence( - &mut Application::new(Args::default(), Config::default())?, + &mut AppBuilder::new().build()?, Some(format!("ihello:w {}", file.path().to_string_lossy()).as_ref()), Some(&|app| { assert!(!app.editor.is_err()); @@ -174,19 +179,40 @@ async fn test_write_scratch_no_path_fails() -> anyhow::Result<()> { Ok(()) } +#[tokio::test] +async fn test_write_auto_format_fails_still_writes() -> anyhow::Result<()> { + let mut file = tempfile::Builder::new().suffix(".rs").tempfile()?; + + let lang_conf = indoc! {r#" + [[language]] + name = "rust" + formatter = { command = "bash", args = [ "-c", "exit 1" ] } + "#}; + + let mut app = helpers::AppBuilder::new() + .with_file(file.path(), None) + .with_input_text("#[l|]#et foo = 0;\n") + .with_lang_config(helpers::test_syntax_conf(Some(lang_conf.into()))) + .build()?; + + test_key_sequences(&mut app, vec![(Some(":w"), None)], false).await?; + + // file still saves + helpers::assert_file_has_content(file.as_file_mut(), "let foo = 0;\n")?; + + Ok(()) +} + #[tokio::test] async fn test_write_new_path() -> anyhow::Result<()> { let mut file1 = tempfile::NamedTempFile::new().unwrap(); let mut file2 = tempfile::NamedTempFile::new().unwrap(); + let mut app = helpers::AppBuilder::new() + .with_file(file1.path(), None) + .build()?; test_key_sequences( - &mut Application::new( - Args { - files: vec![(file1.path().to_path_buf(), Position::default())], - ..Default::default() - }, - Config::default(), - )?, + &mut app, vec![ ( Some("ii can eat glass, it will not hurt me:w"), @@ -228,7 +254,7 @@ async fn test_write_fail_new_path() -> anyhow::Result<()> { let file = helpers::new_readonly_tempfile()?; test_key_sequences( - &mut Application::new(Args::default(), Config::default())?, + &mut AppBuilder::new().build()?, vec![ ( None, From 9e64974f13e91470be7b411285b7a33c698da3aa Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Fri, 16 Sep 2022 23:32:25 -0400 Subject: [PATCH 23/37] remove Document::format_and_save --- helix-view/src/document.rs | 43 +++----------------------------------- 1 file changed, 3 insertions(+), 40 deletions(-) diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index ff64689e..4a09bedc 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -520,32 +520,12 @@ impl Document { path: Option

, force: bool, ) -> Result<(), anyhow::Error> { - self.save_impl::, _>(None, path, force) - } - - pub fn format_and_save( - &mut self, - formatting: Option, - path: Option

, - force: bool, - ) -> anyhow::Result<()> - where - F: Future> + 'static + Send, - P: Into, - { - self.save_impl(formatting, path, force) + self.save_impl::, _>(path, force) } /// The `Document`'s text is encoded according to its encoding and written to the file located /// at its `path()`. - /// - /// If `formatting` is present, it supplies some changes that we apply to the text before saving. - fn save_impl( - &mut self, - formatting: Option, - path: Option

, - force: bool, - ) -> Result<(), anyhow::Error> + fn save_impl(&mut self, path: Option

, force: bool) -> Result<(), anyhow::Error> where F: Future> + 'static + Send, P: Into, @@ -561,7 +541,7 @@ impl Document { // we clone and move text + path into the future so that we asynchronously save the current // state without blocking any further edits. - let mut text = self.text().clone(); + let text = self.text().clone(); let path = match path { Some(path) => helix_core::path::get_canonicalized_path(&path.into())?, @@ -602,23 +582,6 @@ impl Document { } } - if let Some(fmt) = formatting { - match fmt.await { - Ok(transaction) => { - let success = transaction.changes().apply(&mut text); - if !success { - // This shouldn't happen, because the transaction changes were generated - // from the same text we're saving. - log::error!("failed to apply format changes before saving"); - } - } - Err(err) => { - // formatting failed: report error, and save file without modifications - log::error!("{}", err); - } - } - } - let mut file = File::create(&path).await?; to_writer(&mut file, encoding, &text).await?; From 31d1bbfddb112a1e38cf974793afc427a3614ecf Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Wed, 21 Sep 2022 18:34:56 -0400 Subject: [PATCH 24/37] review comments --- helix-term/src/application.rs | 35 ++++++++++++++++---------------- helix-term/src/commands/typed.rs | 2 +- helix-view/src/document.rs | 12 ++--------- 3 files changed, 21 insertions(+), 28 deletions(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 4fde2a66..694c55c0 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -427,24 +427,25 @@ impl Application { } pub fn handle_document_write(&mut self, doc_save_event: DocumentSavedEventResult) { - if let Err(err) = doc_save_event { - self.editor.set_error(err.to_string()); - return; - } - - let doc_save_event = doc_save_event.unwrap(); - let doc = self.editor.document_mut(doc_save_event.doc_id); - - if doc.is_none() { - warn!( - "received document saved event for non-existent doc id: {}", - doc_save_event.doc_id - ); + let doc_save_event = match doc_save_event { + Ok(event) => event, + Err(err) => { + self.editor.set_error(err.to_string()); + return; + } + }; - return; - } + let doc = match self.editor.document_mut(doc_save_event.doc_id) { + None => { + warn!( + "received document saved event for non-existent doc id: {}", + doc_save_event.doc_id + ); - let doc = doc.unwrap(); + return; + } + Some(doc) => doc, + }; debug!( "document {:?} saved with revision {}", @@ -472,7 +473,7 @@ impl Application { let loader = self.editor.syn_loader.clone(); // borrowing the same doc again to get around the borrow checker - let doc = self.editor.document_mut(doc_save_event.doc_id).unwrap(); + let doc = doc_mut!(self.editor, &doc_save_event.doc_id); let id = doc.id(); doc.detect_language(loader); let _ = self.editor.refresh_language_server(id); diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index d312c45f..070215cb 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -21,7 +21,7 @@ pub struct TypableCommand { } fn quit(cx: &mut compositor::Context, args: &[Cow], event: PromptEvent) -> anyhow::Result<()> { - log::info!("quitting..."); + log::debug!("quitting..."); if event != PromptEvent::Validate { return Ok(()); diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 4a09bedc..0774e516 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -554,12 +554,7 @@ impl Document { } }; - let identifier = if self.path().is_some() { - Some(self.identifier()) - } else { - None - }; - + let identifier = self.path().map(|_| self.identifier()); let language_server = self.language_server.clone(); // mark changes up to now as saved @@ -708,10 +703,7 @@ impl Document { /// and flushing through the queue of pending writes. If any fail, /// it stops early before emptying the rest of the queue. pub async fn close(&mut self) -> Option { - if self.save_sender.is_some() { - self.save_sender.take(); - } - + self.save_sender.take(); self.flush_saves_impl(true).await } From bf378e71b012b4c108d11825cee6eea7b69778c2 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Tue, 4 Oct 2022 01:37:02 -0400 Subject: [PATCH 25/37] fix tests --- helix-term/tests/test/movement.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/helix-term/tests/test/movement.rs b/helix-term/tests/test/movement.rs index 7212d026..81c66e53 100644 --- a/helix-term/tests/test/movement.rs +++ b/helix-term/tests/test/movement.rs @@ -117,6 +117,7 @@ async fn select_mode_tree_sitter_next_function_is_union_of_objects() -> anyhow:: ..Default::default() }, Config::default(), + helpers::test_syntax_conf(None), ( helpers::platform_line(indoc! {"\ #[/|]#// Increments @@ -148,6 +149,7 @@ async fn select_mode_tree_sitter_prev_function_unselects_object() -> anyhow::Res ..Default::default() }, Config::default(), + helpers::test_syntax_conf(None), ( helpers::platform_line(indoc! {"\ /// Increments @@ -180,6 +182,7 @@ async fn select_mode_tree_sitter_prev_function_goes_backwards_to_object() -> any ..Default::default() }, Config::default(), + helpers::test_syntax_conf(None), ( helpers::platform_line(indoc! {"\ /// Increments @@ -210,6 +213,7 @@ async fn select_mode_tree_sitter_prev_function_goes_backwards_to_object() -> any ..Default::default() }, Config::default(), + helpers::test_syntax_conf(None), ( helpers::platform_line(indoc! {"\ /// Increments From beb3427bfbaa88bec8b4c683e342f85eb53ad77d Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Tue, 4 Oct 2022 10:35:15 -0400 Subject: [PATCH 26/37] improve app close failure display --- helix-term/src/application.rs | 72 ++++++++++---------------------- helix-term/tests/test/helpers.rs | 12 +++++- 2 files changed, 34 insertions(+), 50 deletions(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 694c55c0..2e49e6d1 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -951,28 +951,10 @@ impl Application { self.event_loop(input_stream).await; - // let mut save_errs = Vec::new(); - - // for doc in self.editor.documents_mut() { - // if let Some(Err(err)) = doc.close().await { - // save_errs.push(( - // doc.path() - // .map(|path| path.to_string_lossy().into_owned()) - // .unwrap_or_else(|| "".into()), - // err, - // )); - // } - // } - - let close_err = self.close().await.err(); + let close_errs = self.close().await; restore_term()?; - // for (path, err) in save_errs { - // self.editor.exit_code = 1; - // eprintln!("Error closing '{}': {}", path, err); - // } - - if let Some(err) = close_err { + for err in close_errs { self.editor.exit_code = 1; eprintln!("Error: {}", err); } @@ -980,49 +962,41 @@ impl Application { Ok(self.editor.exit_code) } - pub async fn close(&mut self) -> anyhow::Result<()> { + pub async fn close(&mut self) -> Vec { // [NOTE] we intentionally do not return early for errors because we // want to try to run as much cleanup as we can, regardless of // errors along the way + let mut errs = Vec::new(); - let mut result = match self + if let Err(err) = self .jobs .finish(&mut self.editor, Some(&mut self.compositor)) .await { - Ok(_) => Ok(()), - Err(err) => { - log::error!("Error executing job: {}", err); - Err(err) - } + log::error!("Error executing job: {}", err); + errs.push(err); }; for doc in self.editor.documents_mut() { - if let Some(save_result) = doc.close().await { - result = match save_result { - Ok(_) => result, - Err(err) => { - if let Some(path) = doc.path() { - log::error!( - "Error saving document '{}': {}", - path.to_string_lossy(), - err - ); - } - Err(err) - } - }; + if let Some(Err(err)) = doc.close().await { + if let Some(path) = doc.path() { + log::error!( + "Error saving document '{}': {}", + path.to_string_lossy(), + err + ); + } + errs.push(err); } } - match self.editor.close_language_servers(None).await { - Ok(_) => result, - Err(_) => { - log::error!("Timed out waiting for language servers to shutdown"); - Err(anyhow::format_err!( - "Timed out waiting for language servers to shutdown" - )) - } + if self.editor.close_language_servers(None).await.is_err() { + log::error!("Timed out waiting for language servers to shutdown"); + errs.push(anyhow::format_err!( + "Timed out waiting for language servers to shutdown" + )); } + + errs } } diff --git a/helix-term/tests/test/helpers.rs b/helix-term/tests/test/helpers.rs index c2fbe953..5adc3354 100644 --- a/helix-term/tests/test/helpers.rs +++ b/helix-term/tests/test/helpers.rs @@ -94,7 +94,17 @@ pub async fn test_key_sequences( tokio::time::timeout(TIMEOUT, event_loop).await?; } - app.close().await?; + let errs = app.close().await; + + if !errs.is_empty() { + log::error!("Errors closing app"); + + for err in errs { + log::error!("{}", err); + } + + bail!("Error closing app"); + } Ok(()) } From 30c93994b50888aaeb32c65c90426e997800ccea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bla=C5=BE=20Hrastnik?= Date: Fri, 14 Oct 2022 16:22:21 +0900 Subject: [PATCH 27/37] Use a single save_queue on the editor --- helix-term/src/application.rs | 20 ++++- helix-term/src/commands.rs | 3 +- helix-term/src/commands/typed.rs | 88 +++++++++++-------- helix-term/src/compositor.rs | 22 +++-- helix-view/src/document.rs | 140 +++++-------------------------- helix-view/src/editor.rs | 79 ++++++++++------- 6 files changed, 160 insertions(+), 192 deletions(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 2e49e6d1..6010e745 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -1,5 +1,5 @@ use arc_swap::{access::Map, ArcSwap}; -use futures_util::Stream; +use futures_util::{Stream, StreamExt}; use helix_core::{ diagnostic::{DiagnosticTag, NumberOrString}, path::get_relative_path, @@ -968,6 +968,24 @@ impl Application { // errors along the way let mut errs = Vec::new(); + // TODO: deduplicate with ctx.block_try_flush_writes + tokio::task::block_in_place(|| { + helix_lsp::block_on(async { + while let Some(save_event) = self.editor.save_queue.next().await { + match &save_event { + Ok(event) => { + let doc = doc_mut!(self.editor, &event.doc_id); + doc.set_last_saved_revision(event.revision); + } + Err(err) => { + log::error!("error saving document: {}", err); + } + }; + // TODO: if is_err: break? + } + }) + }); + if let Err(err) = self .jobs .finish(&mut self.editor, Some(&mut self.compositor)) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index f6d583f5..87bbd6c6 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -2541,7 +2541,8 @@ async fn make_format_callback( } if let Some((path, force)) = write { - if let Err(err) = doc.save(path, force) { + let id = doc.id(); + if let Err(err) = editor.save(id, path, force) { editor.set_error(format!("Error saving: {}", err)); } } diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index 070215cb..ef774256 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -79,12 +79,28 @@ fn buffer_close_by_ids_impl( doc_ids: &[DocumentId], force: bool, ) -> anyhow::Result<()> { + // TODO: deduplicate with ctx.block_try_flush_writes + tokio::task::block_in_place(|| { + helix_lsp::block_on(async { + while let Some(save_event) = editor.save_queue.next().await { + match &save_event { + Ok(event) => { + let doc = doc_mut!(editor, &event.doc_id); + doc.set_last_saved_revision(event.revision); + } + Err(err) => { + log::error!("error saving document: {}", err); + } + }; + // TODO: if is_err: break? + } + }) + }); + let (modified_ids, modified_names): (Vec<_>, Vec<_>) = doc_ids .iter() .filter_map(|&doc_id| { - if let Err(CloseError::BufferModified(name)) = tokio::task::block_in_place(|| { - helix_lsp::block_on(editor.close_document(doc_id, force)) - }) { + if let Err(CloseError::BufferModified(name)) = editor.close_document(doc_id, force) { Some((doc_id, name)) } else { None @@ -289,7 +305,8 @@ fn write_impl( }; if fmt.is_none() { - doc.save(path, force)?; + let id = doc.id(); + cx.editor.save(id, path, force)?; } Ok(()) @@ -569,40 +586,45 @@ fn write_all_impl( return Ok(()); } - let mut errors: Option = None; + let mut errors: Vec<&'static str> = Vec::new(); let auto_format = cx.editor.config().auto_format; let jobs = &mut cx.jobs; // save all documents - for doc in &mut cx.editor.documents.values_mut() { - if doc.path().is_none() { - errors = errors - .or_else(|| Some(String::new())) - .map(|mut errs: String| { - errs.push_str("cannot write a buffer without a filename\n"); - errs - }); - - continue; - } + let saves: Vec<_> = cx + .editor + .documents + .values() + .filter_map(|doc| { + if doc.path().is_none() { + errors.push("cannot write a buffer without a filename\n"); + return None; + } - if !doc.is_modified() { - continue; - } + if !doc.is_modified() { + return None; + } - let fmt = if auto_format { - doc.auto_format().map(|fmt| { - let callback = - make_format_callback(doc.id(), doc.version(), fmt, Some((None, force))); - jobs.add(Job::with_callback(callback).wait_before_exiting()); - }) - } else { + let fmt = if auto_format { + doc.auto_format().map(|fmt| { + let callback = + make_format_callback(doc.id(), doc.version(), fmt, Some((None, force))); + jobs.add(Job::with_callback(callback).wait_before_exiting()); + }) + } else { + None + }; + + if fmt.is_none() { + return Some(doc.id()); + } None - }; + }) + .collect(); - if fmt.is_none() { - doc.save::(None, force)?; - } + // manually call save for the rest of docs that don't have a formatter + for id in saves { + cx.editor.save::(id, None, force)?; } if quit { @@ -619,10 +641,8 @@ fn write_all_impl( } } - if let Some(errs) = errors { - if !force { - bail!(errs); - } + if !errors.is_empty() && !force { + bail!("{:?}", errors); } Ok(()) diff --git a/helix-term/src/compositor.rs b/helix-term/src/compositor.rs index 35b9d054..a4ffaff2 100644 --- a/helix-term/src/compositor.rs +++ b/helix-term/src/compositor.rs @@ -1,3 +1,4 @@ +use futures_util::StreamExt; // Each component declares it's own size constraints and gets fitted based on it's parent. // Q: how does this work with popups? // cursive does compositor.screen_mut().add_layer_at(pos::absolute(x, y), ) @@ -33,11 +34,22 @@ impl<'a> Context<'a> { pub fn block_try_flush_writes(&mut self) -> anyhow::Result<()> { tokio::task::block_in_place(|| helix_lsp::block_on(self.jobs.finish(self.editor, None)))?; - for doc in &mut self.editor.documents.values_mut() { - tokio::task::block_in_place(|| helix_lsp::block_on(doc.try_flush_saves())) - .map(|result| result.map(|_| ())) - .unwrap_or(Ok(()))?; - } + tokio::task::block_in_place(|| { + helix_lsp::block_on(async { + while let Some(save_event) = self.editor.save_queue.next().await { + match &save_event { + Ok(event) => { + let doc = doc_mut!(self.editor, &event.doc_id); + doc.set_last_saved_revision(event.revision); + } + Err(err) => { + log::error!("error saving document: {}", err); + } + }; + // TODO: if is_err: break? + } + }) + }); Ok(()) } diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 0774e516..9fa1241e 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -13,10 +13,6 @@ use std::future::Future; use std::path::{Path, PathBuf}; use std::str::FromStr; use std::sync::Arc; -use tokio::sync::mpsc::error::TryRecvError; -use tokio::sync::mpsc::{UnboundedReceiver, UnboundedSender}; - -use tokio::sync::Mutex; use helix_core::{ encoding, @@ -134,9 +130,6 @@ pub struct Document { last_saved_revision: usize, version: i32, // should be usize? pub(crate) modified_since_accessed: bool, - save_sender: Option>, - save_receiver: Option>, - current_save: Arc>>, diagnostics: Vec, language_server: Option>, @@ -357,7 +350,6 @@ impl Document { let encoding = encoding.unwrap_or(encoding::UTF_8); let changes = ChangeSet::new(&text); let old_state = None; - let (save_sender, save_receiver) = tokio::sync::mpsc::unbounded_channel(); Self { id: DocumentId::default(), @@ -378,9 +370,6 @@ impl Document { savepoint: None, last_saved_revision: 0, modified_since_accessed: false, - save_sender: Some(save_sender), - save_receiver: Some(save_receiver), - current_save: Arc::new(Mutex::new(None)), language_server: None, } } @@ -519,21 +508,26 @@ impl Document { &mut self, path: Option

, force: bool, - ) -> Result<(), anyhow::Error> { - self.save_impl::, _>(path, force) + ) -> Result< + impl Future> + 'static + Send, + anyhow::Error, + > { + let path = path.map(|path| path.into()); + self.save_impl(path, force) + + // futures_util::future::Ready<_>, } /// The `Document`'s text is encoded according to its encoding and written to the file located /// at its `path()`. - fn save_impl(&mut self, path: Option

, force: bool) -> Result<(), anyhow::Error> - where - F: Future> + 'static + Send, - P: Into, - { - if self.save_sender.is_none() { - bail!("saves are closed for this document!"); - } - + fn save_impl( + &mut self, + path: Option, + force: bool, + ) -> Result< + impl Future> + 'static + Send, + anyhow::Error, + > { log::debug!( "submitting save of doc '{:?}'", self.path().map(|path| path.to_string_lossy()) @@ -544,7 +538,7 @@ impl Document { let text = self.text().clone(); let path = match path { - Some(path) => helix_core::path::get_canonicalized_path(&path.into())?, + Some(path) => helix_core::path::get_canonicalized_path(&path)?, None => { if self.path.is_none() { bail!("Can't save with no path set!"); @@ -564,7 +558,7 @@ impl Document { let encoding = self.encoding; // We encode the file according to the `Document`'s encoding. - let save_event = async move { + let future = async move { use tokio::fs::File; if let Some(parent) = path.parent() { // TODO: display a prompt asking the user if the directories should be created @@ -604,107 +598,15 @@ impl Document { Ok(event) }; - self.save_sender - .as_mut() - .unwrap() - .send(Box::pin(save_event)) - .map_err(|err| anyhow!("failed to send save event: {}", err)) - } - - pub async fn await_save(&mut self) -> Option { - self.await_save_impl(true).await - } - - async fn await_save_impl(&mut self, block: bool) -> Option { - let mut current_save = self.current_save.lock().await; - if let Some(ref mut save) = *current_save { - log::trace!("reawaiting save of '{:?}'", self.path()); - let result = save.await; - *current_save = None; - log::trace!("reawait save of '{:?}' result: {:?}", self.path(), result); - return Some(result); - } - - // return early if the receiver is closed - let rx = self.save_receiver.as_mut()?; - - let save_req = if block { - rx.recv().await - } else { - let msg = rx.try_recv(); - - if let Err(err) = msg { - match err { - TryRecvError::Empty => return None, - TryRecvError::Disconnected => None, - } - } else { - msg.ok() - } - }; - - let save = match save_req { - Some(save) => save, - None => { - self.save_receiver = None; - return None; - } - }; - - // save a handle to the future so that when a poll on this - // function gets cancelled, we don't lose it - *current_save = Some(save); - log::trace!("awaiting save of '{:?}'", self.path()); - - let result = (*current_save).as_mut().unwrap().await; - *current_save = None; - - log::trace!("save of '{:?}' result: {:?}", self.path(), result); - - Some(result) - } - - /// Flushes the queue of pending writes. If any fail, - /// it stops early before emptying the rest of the queue. - pub async fn try_flush_saves(&mut self) -> Option { - self.flush_saves_impl(false).await - } - - async fn flush_saves_impl(&mut self, block: bool) -> Option { - let mut final_result = None; - - while let Some(save_event) = self.await_save_impl(block).await { - let is_err = match &save_event { - Ok(event) => { - self.set_last_saved_revision(event.revision); - false - } - Err(err) => { - log::error!( - "error saving document {:?}: {}", - self.path().map(|path| path.to_string_lossy()), - err - ); - true - } - }; - - final_result = Some(save_event); - - if is_err { - break; - } - } - - final_result + Ok(future) } /// Prepares the Document for being closed by stopping any new writes /// and flushing through the queue of pending writes. If any fail, /// it stops early before emptying the rest of the queue. pub async fn close(&mut self) -> Option { - self.save_sender.take(); - self.flush_saves_impl(true).await + // TODO + None } /// Detect the programming language based on the file type. diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index fbd0b2b0..c4789ee2 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -1,6 +1,6 @@ use crate::{ clipboard::{get_clipboard_provider, ClipboardProvider}, - document::{DocumentSavedEventResult, Mode}, + document::{DocumentSavedEventFuture, DocumentSavedEventResult, Mode}, graphics::{CursorKind, Rect}, info::Info, input::KeyEvent, @@ -9,7 +9,7 @@ use crate::{ Document, DocumentId, View, ViewId, }; -use futures_util::stream::{select_all::SelectAll, FuturesUnordered}; +use futures_util::stream::select_all::SelectAll; use futures_util::{future, StreamExt}; use helix_lsp::Call; use tokio_stream::wrappers::UnboundedReceiverStream; @@ -29,7 +29,7 @@ use tokio::{ time::{sleep, Duration, Instant, Sleep}, }; -use anyhow::Error; +use anyhow::{anyhow, Error}; pub use helix_core::diagnostic::Severity; pub use helix_core::register::Registers; @@ -644,12 +644,20 @@ pub struct Breakpoint { pub log_message: Option, } +use futures_util::stream::{Flatten, Once}; + pub struct Editor { /// Current editing mode. pub mode: Mode, pub tree: Tree, pub next_document_id: DocumentId, pub documents: BTreeMap, + + // We Flatten<> to resolve the inner DocumentSavedEventFuture. For that we need a stream of streams, hence the Once<>. + // https://stackoverflow.com/a/66875668 + pub saves: HashMap>>, + pub save_queue: SelectAll>>>, + pub count: Option, pub selected_register: Option, pub registers: Registers, @@ -751,6 +759,8 @@ impl Editor { tree: Tree::new(area), next_document_id: DocumentId::default(), documents: BTreeMap::new(), + saves: HashMap::new(), + save_queue: SelectAll::new(), count: None, selected_register: None, macro_recording: None, @@ -1083,6 +1093,12 @@ impl Editor { self.new_document(doc) }; + let (save_sender, save_receiver) = tokio::sync::mpsc::unbounded_channel(); + self.saves.insert(id, save_sender); + + let stream = UnboundedReceiverStream::new(save_receiver).flatten(); + self.save_queue.push(stream); + self.switch(id, action); Ok(id) } @@ -1095,38 +1111,21 @@ impl Editor { self._refresh(); } - pub async fn close_document( - &mut self, - doc_id: DocumentId, - force: bool, - ) -> Result<(), CloseError> { + pub fn close_document(&mut self, doc_id: DocumentId, force: bool) -> Result<(), CloseError> { let doc = match self.documents.get_mut(&doc_id) { Some(doc) => doc, None => return Err(CloseError::DoesNotExist), }; - - // flush out any pending writes first to clear the modified status - if let Some(Err(err)) = doc.try_flush_saves().await { - return Err(CloseError::SaveError(err)); - } - if !force && doc.is_modified() { return Err(CloseError::BufferModified(doc.display_name().into_owned())); } - if let Some(Err(err)) = doc.close().await { - return Err(CloseError::SaveError(err)); - } + // This will also disallow any follow-up writes + self.saves.remove(&doc_id); - // Don't fail the whole write because the language server could not - // acknowledge the close if let Some(language_server) = doc.language_server() { - if let Err(err) = language_server - .text_document_did_close(doc.identifier()) - .await - { - log::error!("Error closing doc in language server: {}", err); - } + // TODO: track error + tokio::spawn(language_server.text_document_did_close(doc.identifier())); } enum Action { @@ -1188,6 +1187,28 @@ impl Editor { Ok(()) } + pub fn save>( + &mut self, + doc_id: DocumentId, + path: Option

, + force: bool, + ) -> anyhow::Result<()> { + // convert a channel of futures to pipe into main queue one by one + // via stream.then() ? then push into main future + + let path = path.map(|path| path.into()); + let doc = doc_mut!(self, &doc_id); + let future = doc.save(path, force)?; + // TODO: if no self.saves for that doc id then bail + // bail!("saves are closed for this document!"); + use futures_util::stream; + self.saves[&doc_id] + .send(stream::once(Box::pin(future))) + .map_err(|err| anyhow!("failed to send save event: {}", err))?; + + Ok(()) + } + pub fn resize(&mut self, area: Rect) { if self.tree.resize(area) { self._refresh(); @@ -1307,16 +1328,10 @@ impl Editor { } pub async fn wait_event(&mut self) -> EditorEvent { - let mut saves: FuturesUnordered<_> = self - .documents - .values_mut() - .map(Document::await_save) - .collect(); - tokio::select! { biased; - Some(Some(event)) = saves.next() => { + Some(event) = self.save_queue.next() => { EditorEvent::DocumentSaved(event) } Some(config_event) = self.config_events.1.recv() => { From b0212b36118f7a4d596510dabb0c011144c7af69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bla=C5=BE=20Hrastnik?= Date: Sun, 16 Oct 2022 18:01:31 +0900 Subject: [PATCH 28/37] Deduplicate flush_writes --- helix-term/src/application.rs | 19 ++----------------- helix-term/src/commands/typed.rs | 17 +---------------- helix-term/src/compositor.rs | 18 +----------------- helix-view/src/editor.rs | 15 +++++++++++++++ 4 files changed, 19 insertions(+), 50 deletions(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 6010e745..719683bf 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -1,5 +1,5 @@ use arc_swap::{access::Map, ArcSwap}; -use futures_util::{Stream, StreamExt}; +use futures_util::Stream; use helix_core::{ diagnostic::{DiagnosticTag, NumberOrString}, path::get_relative_path, @@ -969,22 +969,7 @@ impl Application { let mut errs = Vec::new(); // TODO: deduplicate with ctx.block_try_flush_writes - tokio::task::block_in_place(|| { - helix_lsp::block_on(async { - while let Some(save_event) = self.editor.save_queue.next().await { - match &save_event { - Ok(event) => { - let doc = doc_mut!(self.editor, &event.doc_id); - doc.set_last_saved_revision(event.revision); - } - Err(err) => { - log::error!("error saving document: {}", err); - } - }; - // TODO: if is_err: break? - } - }) - }); + tokio::task::block_in_place(|| helix_lsp::block_on(self.editor.flush_writes())); if let Err(err) = self .jobs diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index ef774256..c18f3c39 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -80,22 +80,7 @@ fn buffer_close_by_ids_impl( force: bool, ) -> anyhow::Result<()> { // TODO: deduplicate with ctx.block_try_flush_writes - tokio::task::block_in_place(|| { - helix_lsp::block_on(async { - while let Some(save_event) = editor.save_queue.next().await { - match &save_event { - Ok(event) => { - let doc = doc_mut!(editor, &event.doc_id); - doc.set_last_saved_revision(event.revision); - } - Err(err) => { - log::error!("error saving document: {}", err); - } - }; - // TODO: if is_err: break? - } - }) - }); + tokio::task::block_in_place(|| helix_lsp::block_on(editor.flush_writes())); let (modified_ids, modified_names): (Vec<_>, Vec<_>) = doc_ids .iter() diff --git a/helix-term/src/compositor.rs b/helix-term/src/compositor.rs index a4ffaff2..76703780 100644 --- a/helix-term/src/compositor.rs +++ b/helix-term/src/compositor.rs @@ -1,4 +1,3 @@ -use futures_util::StreamExt; // Each component declares it's own size constraints and gets fitted based on it's parent. // Q: how does this work with popups? // cursive does compositor.screen_mut().add_layer_at(pos::absolute(x, y), ) @@ -34,22 +33,7 @@ impl<'a> Context<'a> { pub fn block_try_flush_writes(&mut self) -> anyhow::Result<()> { tokio::task::block_in_place(|| helix_lsp::block_on(self.jobs.finish(self.editor, None)))?; - tokio::task::block_in_place(|| { - helix_lsp::block_on(async { - while let Some(save_event) = self.editor.save_queue.next().await { - match &save_event { - Ok(event) => { - let doc = doc_mut!(self.editor, &event.doc_id); - doc.set_last_saved_revision(event.revision); - } - Err(err) => { - log::error!("error saving document: {}", err); - } - }; - // TODO: if is_err: break? - } - }) - }); + tokio::task::block_in_place(|| helix_lsp::block_on(self.editor.flush_writes())); Ok(()) } diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index c4789ee2..c0efad05 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -1348,4 +1348,19 @@ impl Editor { } } } + + pub async fn flush_writes(&mut self) { + while let Some(save_event) = self.save_queue.next().await { + match &save_event { + Ok(event) => { + let doc = doc_mut!(self, &event.doc_id); + doc.set_last_saved_revision(event.revision); + } + Err(err) => { + log::error!("error saving document: {}", err); + } + }; + // TODO: if is_err: break? + } + } } From b155e861adea1a29c5e4f9e717df8ff85a36052d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bla=C5=BE=20Hrastnik?= Date: Sun, 16 Oct 2022 18:12:26 +0900 Subject: [PATCH 29/37] Use a write_count to determine how many writes left to flush --- helix-view/src/editor.rs | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index c0efad05..ac0864e1 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -657,6 +657,7 @@ pub struct Editor { // https://stackoverflow.com/a/66875668 pub saves: HashMap>>, pub save_queue: SelectAll>>>, + pub write_count: usize, pub count: Option, pub selected_register: Option, @@ -761,6 +762,7 @@ impl Editor { documents: BTreeMap::new(), saves: HashMap::new(), save_queue: SelectAll::new(), + write_count: 0, count: None, selected_register: None, macro_recording: None, @@ -1206,6 +1208,8 @@ impl Editor { .send(stream::once(Box::pin(future))) .map_err(|err| anyhow!("failed to send save event: {}", err))?; + self.write_count += 1; + Ok(()) } @@ -1332,6 +1336,7 @@ impl Editor { biased; Some(event) = self.save_queue.next() => { + self.write_count -= 1; EditorEvent::DocumentSaved(event) } Some(config_event) = self.config_events.1.recv() => { @@ -1350,17 +1355,21 @@ impl Editor { } pub async fn flush_writes(&mut self) { - while let Some(save_event) = self.save_queue.next().await { - match &save_event { - Ok(event) => { - let doc = doc_mut!(self, &event.doc_id); - doc.set_last_saved_revision(event.revision); - } - Err(err) => { - log::error!("error saving document: {}", err); - } - }; - // TODO: if is_err: break? + while self.write_count > 0 { + if let Some(save_event) = self.save_queue.next().await { + match &save_event { + Ok(event) => { + let doc = doc_mut!(self, &event.doc_id); + doc.set_last_saved_revision(event.revision); + } + Err(err) => { + log::error!("error saving document: {}", err); + } + }; + // TODO: if is_err: break? + + self.write_count -= 1; + } } } } From 55b50d9e8368793d764d36c0f363f31252900b87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bla=C5=BE=20Hrastnik?= Date: Sun, 16 Oct 2022 23:39:58 +0900 Subject: [PATCH 30/37] Seems like this flush is unnecessary --- helix-term/src/application.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 719683bf..2e49e6d1 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -968,9 +968,6 @@ impl Application { // errors along the way let mut errs = Vec::new(); - // TODO: deduplicate with ctx.block_try_flush_writes - tokio::task::block_in_place(|| helix_lsp::block_on(self.editor.flush_writes())); - if let Err(err) = self .jobs .finish(&mut self.editor, Some(&mut self.compositor)) From 1b6f7319cd605366de109f25821c6b84860f2a11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bla=C5=BE=20Hrastnik?= Date: Sun, 16 Oct 2022 23:44:36 +0900 Subject: [PATCH 31/37] Wire up save_queue as a part of new_document rather than open --- helix-view/src/editor.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index ac0864e1..dc1a800e 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -1062,6 +1062,13 @@ impl Editor { DocumentId(unsafe { NonZeroUsize::new_unchecked(self.next_document_id.0.get() + 1) }); doc.id = id; self.documents.insert(id, doc); + + let (save_sender, save_receiver) = tokio::sync::mpsc::unbounded_channel(); + self.saves.insert(id, save_sender); + + let stream = UnboundedReceiverStream::new(save_receiver).flatten(); + self.save_queue.push(stream); + id } @@ -1095,12 +1102,6 @@ impl Editor { self.new_document(doc) }; - let (save_sender, save_receiver) = tokio::sync::mpsc::unbounded_channel(); - self.saves.insert(id, save_sender); - - let stream = UnboundedReceiverStream::new(save_receiver).flatten(); - self.save_queue.push(stream); - self.switch(id, action); Ok(id) } From 2a43ee016453e39e2cc4cae510ce7d75606e7199 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bla=C5=BE=20Hrastnik?= Date: Sun, 16 Oct 2022 23:46:34 +0900 Subject: [PATCH 32/37] doc.close() now unused --- helix-view/src/document.rs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 9fa1241e..78c6d032 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -601,14 +601,6 @@ impl Document { Ok(future) } - /// Prepares the Document for being closed by stopping any new writes - /// and flushing through the queue of pending writes. If any fail, - /// it stops early before emptying the rest of the queue. - pub async fn close(&mut self) -> Option { - // TODO - None - } - /// Detect the programming language based on the file type. pub fn detect_language(&mut self, config_loader: Arc) { if let Some(path) = &self.path { From 52ba550098745b463f59211a0172c334daef350b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bla=C5=BE=20Hrastnik?= Date: Mon, 17 Oct 2022 00:02:14 +0900 Subject: [PATCH 33/37] Use flush_writes in application.close() --- helix-term/src/application.rs | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 2e49e6d1..6ca5b657 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -977,18 +977,7 @@ impl Application { errs.push(err); }; - for doc in self.editor.documents_mut() { - if let Some(Err(err)) = doc.close().await { - if let Some(path) = doc.path() { - log::error!( - "Error saving document '{}': {}", - path.to_string_lossy(), - err - ); - } - errs.push(err); - } - } + self.editor.flush_writes().await; if self.editor.close_language_servers(None).await.is_err() { log::error!("Timed out waiting for language servers to shutdown"); From e645804b0a8e42c9fb9ef8259a9b2ad47a57a6e6 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Sun, 16 Oct 2022 21:40:40 -0400 Subject: [PATCH 34/37] Editor::flush_writes returns an error --- helix-term/src/application.rs | 5 ++++- helix-term/src/commands/typed.rs | 23 +++++++++++------------ helix-term/src/compositor.rs | 4 +--- helix-view/src/editor.rs | 22 ++++++++++++---------- 4 files changed, 28 insertions(+), 26 deletions(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 6ca5b657..b4b4a675 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -977,7 +977,10 @@ impl Application { errs.push(err); }; - self.editor.flush_writes().await; + if let Err(err) = self.editor.flush_writes().await { + log::error!("Error writing: {}", err); + errs.push(err); + } if self.editor.close_language_servers(None).await.is_err() { log::error!("Timed out waiting for language servers to shutdown"); diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index c18f3c39..eeeb4625 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -75,17 +75,16 @@ fn open(cx: &mut compositor::Context, args: &[Cow], event: PromptEvent) -> } fn buffer_close_by_ids_impl( - editor: &mut Editor, + cx: &mut compositor::Context, doc_ids: &[DocumentId], force: bool, ) -> anyhow::Result<()> { - // TODO: deduplicate with ctx.block_try_flush_writes - tokio::task::block_in_place(|| helix_lsp::block_on(editor.flush_writes())); + cx.block_try_flush_writes()?; let (modified_ids, modified_names): (Vec<_>, Vec<_>) = doc_ids .iter() .filter_map(|&doc_id| { - if let Err(CloseError::BufferModified(name)) = editor.close_document(doc_id, force) { + if let Err(CloseError::BufferModified(name)) = cx.editor.close_document(doc_id, force) { Some((doc_id, name)) } else { None @@ -94,11 +93,11 @@ fn buffer_close_by_ids_impl( .unzip(); if let Some(first) = modified_ids.first() { - let current = doc!(editor); + let current = doc!(cx.editor); // If the current document is unmodified, and there are modified // documents, switch focus to the first modified doc. if !modified_ids.contains(¤t.id()) { - editor.switch(*first, Action::Replace); + cx.editor.switch(*first, Action::Replace); } bail!( "{} unsaved buffer(s) remaining: {:?}", @@ -157,7 +156,7 @@ fn buffer_close( } let document_ids = buffer_gather_paths_impl(cx.editor, args); - buffer_close_by_ids_impl(cx.editor, &document_ids, false) + buffer_close_by_ids_impl(cx, &document_ids, false) } fn force_buffer_close( @@ -170,7 +169,7 @@ fn force_buffer_close( } let document_ids = buffer_gather_paths_impl(cx.editor, args); - buffer_close_by_ids_impl(cx.editor, &document_ids, true) + buffer_close_by_ids_impl(cx, &document_ids, true) } fn buffer_gather_others_impl(editor: &mut Editor) -> Vec { @@ -192,7 +191,7 @@ fn buffer_close_others( } let document_ids = buffer_gather_others_impl(cx.editor); - buffer_close_by_ids_impl(cx.editor, &document_ids, false) + buffer_close_by_ids_impl(cx, &document_ids, false) } fn force_buffer_close_others( @@ -205,7 +204,7 @@ fn force_buffer_close_others( } let document_ids = buffer_gather_others_impl(cx.editor); - buffer_close_by_ids_impl(cx.editor, &document_ids, true) + buffer_close_by_ids_impl(cx, &document_ids, true) } fn buffer_gather_all_impl(editor: &mut Editor) -> Vec { @@ -222,7 +221,7 @@ fn buffer_close_all( } let document_ids = buffer_gather_all_impl(cx.editor); - buffer_close_by_ids_impl(cx.editor, &document_ids, false) + buffer_close_by_ids_impl(cx, &document_ids, false) } fn force_buffer_close_all( @@ -235,7 +234,7 @@ fn force_buffer_close_all( } let document_ids = buffer_gather_all_impl(cx.editor); - buffer_close_by_ids_impl(cx.editor, &document_ids, true) + buffer_close_by_ids_impl(cx, &document_ids, true) } fn buffer_next( diff --git a/helix-term/src/compositor.rs b/helix-term/src/compositor.rs index 76703780..971dc52d 100644 --- a/helix-term/src/compositor.rs +++ b/helix-term/src/compositor.rs @@ -32,9 +32,7 @@ impl<'a> Context<'a> { /// operations for all documents. pub fn block_try_flush_writes(&mut self) -> anyhow::Result<()> { tokio::task::block_in_place(|| helix_lsp::block_on(self.jobs.finish(self.editor, None)))?; - - tokio::task::block_in_place(|| helix_lsp::block_on(self.editor.flush_writes())); - + tokio::task::block_in_place(|| helix_lsp::block_on(self.editor.flush_writes()))?; Ok(()) } } diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index dc1a800e..9f74c5ec 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -29,7 +29,7 @@ use tokio::{ time::{sleep, Duration, Instant, Sleep}, }; -use anyhow::{anyhow, Error}; +use anyhow::{anyhow, bail, Error}; pub use helix_core::diagnostic::Severity; pub use helix_core::register::Registers; @@ -1355,22 +1355,24 @@ impl Editor { } } - pub async fn flush_writes(&mut self) { + pub async fn flush_writes(&mut self) -> anyhow::Result<()> { while self.write_count > 0 { if let Some(save_event) = self.save_queue.next().await { - match &save_event { - Ok(event) => { - let doc = doc_mut!(self, &event.doc_id); - doc.set_last_saved_revision(event.revision); - } + self.write_count -= 1; + + let save_event = match save_event { + Ok(event) => event, Err(err) => { - log::error!("error saving document: {}", err); + self.set_error(err.to_string()); + bail!(err); } }; - // TODO: if is_err: break? - self.write_count -= 1; + let doc = doc_mut!(self, &save_event.doc_id); + doc.set_last_saved_revision(save_event.revision); } } + + Ok(()) } } From 759d55cc81da10c93f6cbfcbe605806b9dc37d2f Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Sun, 16 Oct 2022 22:41:01 -0400 Subject: [PATCH 35/37] fail if doc save sender is closed --- helix-view/src/editor.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 9f74c5ec..ee84dbd3 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -1202,10 +1202,12 @@ impl Editor { let path = path.map(|path| path.into()); let doc = doc_mut!(self, &doc_id); let future = doc.save(path, force)?; - // TODO: if no self.saves for that doc id then bail - // bail!("saves are closed for this document!"); + use futures_util::stream; - self.saves[&doc_id] + + self.saves + .get(&doc_id) + .ok_or_else(|| anyhow::format_err!("saves are closed for this document!"))? .send(stream::once(Box::pin(future))) .map_err(|err| anyhow!("failed to send save event: {}", err))?; From 9a406b569b1b75c1970b81aac068110232ead2fd Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Tue, 18 Oct 2022 21:37:28 -0400 Subject: [PATCH 36/37] reduce LSP timeout to 3s --- helix-view/src/editor.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index ee84dbd3..cd2b1ad4 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -1323,7 +1323,7 @@ impl Editor { timeout: Option, ) -> Result<(), tokio::time::error::Elapsed> { tokio::time::timeout( - Duration::from_millis(timeout.unwrap_or(10000)), + Duration::from_millis(timeout.unwrap_or(3000)), future::join_all( self.language_servers .iter_clients() From 756253b43f5ec1d8cc6fce9e6ebcf3f9fee5bc5a Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Wed, 19 Oct 2022 00:01:00 -0400 Subject: [PATCH 37/37] fix tree_sitter_scopes --- helix-term/src/commands/typed.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index eeeb4625..c1971d81 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -1077,12 +1077,13 @@ fn tree_sitter_scopes( let contents = format!("```json\n{:?}\n````", scopes); let callback = async move { - let call: job::Callback = - Box::new(move |editor: &mut Editor, compositor: &mut Compositor| { + let call: job::Callback = Callback::EditorCompositor(Box::new( + move |editor: &mut Editor, compositor: &mut Compositor| { let contents = ui::Markdown::new(contents, editor.syn_loader.clone()); let popup = Popup::new("hover", contents).auto_close(true); compositor.replace_or_push("hover", popup); - }); + }, + )); Ok(call) };