From d706194597d462fbaeb1ef55e2e8fb6eae38d2f3 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Sun, 10 Apr 2022 11:05:47 -0400 Subject: 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/commands.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'helix-term/src/commands.rs') 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}; -- cgit v1.2.3-70-g09d2 From b8a07f7d15a10186fa2b481a3423c23f32d7d561 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Fri, 24 Jun 2022 08:39:07 -0400 Subject: 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(-) (limited to 'helix-term/src/commands.rs') 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 }) -- cgit v1.2.3-70-g09d2 From c9418582d2a6d8dbb8b5bb1d3432a9087438e61d Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Tue, 5 Jul 2022 00:15:15 -0400 Subject: 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(-) (limited to 'helix-term/src/commands.rs') 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::{ -- cgit v1.2.3-70-g09d2 From aaa145067833c41686b7cdde9fb999018735bc04 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Mon, 11 Jul 2022 23:38:26 -0400 Subject: 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(-) (limited to 'helix-term/src/commands.rs') 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) }); } -- cgit v1.2.3-70-g09d2 From b3fc31a211293f48696d26855781577d1859c2c6 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Fri, 2 Sep 2022 09:20:28 -0400 Subject: 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(-) (limited to 'helix-term/src/commands.rs') 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 { -- cgit v1.2.3-70-g09d2 From 3f07885b351748c5b8225aadb165f8ef7066f047 Mon Sep 17 00:00:00 2001 From: Skyler Hawthorne Date: Fri, 16 Sep 2022 23:17:48 -0400 Subject: 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(-) (limited to 'helix-term/src/commands.rs') 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, -- cgit v1.2.3-70-g09d2 From 30c93994b50888aaeb32c65c90426e997800ccea Mon Sep 17 00:00:00 2001 From: Blaž Hrastnik Date: Fri, 14 Oct 2022 16:22:21 +0900 Subject: 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(-) (limited to 'helix-term/src/commands.rs') 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() => { -- cgit v1.2.3-70-g09d2