diff options
author | Blaž Hrastnik | 2022-10-20 14:11:22 +0000 |
---|---|---|
committer | GitHub | 2022-10-20 14:11:22 +0000 |
commit | 78c0cdc519a2c76842441103b1ed716bb7c0a4e1 (patch) | |
tree | a7a551c4fd458a6dec3ccb94bc31055f7c8c9077 | |
parent | 8c9bb23650ba3c0c0bc7b25a359f997e130feb25 (diff) | |
parent | 756253b43f5ec1d8cc6fce9e6ebcf3f9fee5bc5a (diff) |
Merge pull request #2267 from dead10ck/fix-write-fail
Write path fixes
-rw-r--r-- | docs/CONTRIBUTING.md | 3 | ||||
-rw-r--r-- | helix-core/src/auto_pairs.rs | 1 | ||||
-rw-r--r-- | helix-core/src/syntax.rs | 6 | ||||
-rw-r--r-- | helix-term/src/application.rs | 208 | ||||
-rw-r--r-- | helix-term/src/commands.rs | 63 | ||||
-rw-r--r-- | helix-term/src/commands/dap.rs | 35 | ||||
-rw-r--r-- | helix-term/src/commands/typed.rs | 152 | ||||
-rw-r--r-- | helix-term/src/compositor.rs | 10 | ||||
-rw-r--r-- | helix-term/src/job.rs | 36 | ||||
-rw-r--r-- | helix-term/src/main.rs | 12 | ||||
-rw-r--r-- | helix-term/src/ui/editor.rs | 10 | ||||
-rw-r--r-- | helix-term/src/ui/mod.rs | 6 | ||||
-rw-r--r-- | helix-term/tests/integration.rs | 1 | ||||
-rw-r--r-- | helix-term/tests/test/auto_indent.rs | 1 | ||||
-rw-r--r-- | helix-term/tests/test/auto_pairs.rs | 1 | ||||
-rw-r--r-- | helix-term/tests/test/commands.rs | 36 | ||||
-rw-r--r-- | helix-term/tests/test/helpers.rs | 136 | ||||
-rw-r--r-- | helix-term/tests/test/movement.rs | 8 | ||||
-rw-r--r-- | helix-term/tests/test/prompt.rs | 4 | ||||
-rw-r--r-- | helix-term/tests/test/splits.rs | 129 | ||||
-rw-r--r-- | helix-term/tests/test/write.rs | 152 | ||||
-rw-r--r-- | helix-view/src/document.rs | 143 | ||||
-rw-r--r-- | helix-view/src/editor.rs | 140 |
23 files changed, 995 insertions, 298 deletions
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 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-core/src/syntax.rs b/helix-core/src/syntax.rs index b5083f55..a08e5084 100644 --- a/helix-core/src/syntax.rs +++ b/helix-core/src/syntax.rs @@ -61,6 +61,12 @@ pub struct Configuration { pub language: Vec<LanguageConfiguration>, } +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 4bb36b59..b4b4a675 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -1,12 +1,19 @@ 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, }; 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::DocumentSavedEventResult, + 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, @@ -102,7 +109,11 @@ fn restore_term() -> Result<(), Error> { } impl Application { - pub fn new(args: Args, config: Config) -> Result<Self, Error> { + pub fn new( + args: Args, + config: Config, + syn_loader_conf: syntax::Configuration, + ) -> Result<Self, Error> { #[cfg(feature = "integration")] setup_integration_logging(); @@ -129,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 <ENTER> 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")?; @@ -245,6 +248,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; @@ -275,9 +282,6 @@ impl Application { where S: Stream<Item = crossterm::Result<crossterm::event::Event>> + Unpin, { - #[cfg(feature = "integration")] - let mut idle_handled = false; - loop { if self.editor.should_close() { return false; @@ -294,26 +298,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,26 +306,22 @@ 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() => { + let _idle_handled = self.handle_editor_event(event).await; #[cfg(feature = "integration")] { - idle_handled = true; + if _idle_handled { + return 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 { - return true; - } - self.editor.reset_idle_timer(); } } @@ -446,6 +426,111 @@ impl Application { } } + pub fn handle_document_write(&mut self, doc_save_event: DocumentSavedEventResult) { + let doc_save_event = match doc_save_event { + Ok(event) => event, + Err(err) => { + self.editor.set_error(err.to_string()); + 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 + ); + + return; + } + Some(doc) => doc, + }; + + debug!( + "document {:?} saved with revision {}", + doc.path(), + doc_save_event.revision + ); + + doc.set_last_saved_revision(doc_save_event.revision); + + let lines = doc_save_event.text.len_lines(); + let bytes = doc_save_event.text.len_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 = doc_mut!(self.editor, &doc_save_event.doc_id); + 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)] + pub async fn handle_editor_event(&mut self, event: EditorEvent) -> bool { + log::debug!("received editor event: {:?}", event); + + match event { + EditorEvent::DocumentSaved(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<CrosstermEvent, crossterm::ErrorKind>) { let mut cx = crate::compositor::Context { editor: &mut self.editor, @@ -866,11 +951,10 @@ impl Application { self.event_loop(input_stream).await; - let err = self.close().await.err(); - + let close_errs = self.close().await; restore_term()?; - if let Some(err) = err { + for err in close_errs { self.editor.exit_code = 1; eprintln!("Error: {}", err); } @@ -878,13 +962,33 @@ impl Application { Ok(self.editor.exit_code) } - pub async fn close(&mut self) -> anyhow::Result<()> { - self.jobs.finish().await?; + pub async fn close(&mut self) -> Vec<anyhow::Error> { + // [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(); + + if let Err(err) = self + .jobs + .finish(&mut self.editor, Some(&mut self.compositor)) + .await + { + log::error!("Error executing job: {}", err); + errs.push(err); + }; + + 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"); - }; + errs.push(anyhow::format_err!( + "Timed out waiting for language servers to shutdown" + )); + } - Ok(()) + errs } } diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 5073651b..87bbd6c6 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -47,12 +47,13 @@ use movement::Movement; use crate::{ args, compositor::{self, Component, Compositor}, + job::Callback, keymap::ReverseKeymap, ui::{self, overlay::overlayed, FilePicker, Picker, Popup, Prompt, PromptEvent}, }; -use crate::job::{self, Job, Jobs}; -use futures_util::{FutureExt, StreamExt}; +use crate::job::{self, Jobs}; +use futures_util::StreamExt; use std::{collections::HashMap, fmt, future::Future}; use std::{collections::HashSet, num::NonZeroUsize}; @@ -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<FileResult> = 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); @@ -2504,13 +2507,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,11 +2515,12 @@ enum Modified { async fn make_format_callback( doc_id: DocumentId, doc_version: i32, - modified: Modified, format: impl Future<Output = Result<Transaction, FormatterError>> + Send + 'static, + write: Option<(Option<PathBuf>, bool)>, ) -> anyhow::Result<job::Callback> { - let format = format.await?; - let call: job::Callback = Box::new(move |editor, _compositor| { + let format = format.await; + + let call: job::Callback = Callback::Editor(Box::new(move |editor| { if !editor.documents.contains_key(&doc_id) { return; } @@ -2531,22 +2528,30 @@ async fn make_format_callback( let scrolloff = editor.config().scrolloff; 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 Modified::SetUnmodified = modified { - doc.reset_modified(); + + 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"); } - } else { - log::info!("discarded formatting changes because the document changed"); } - }); + + if let Some((path, force)) = write { + let id = doc.id(); + if let Err(err) = editor.save(id, path, force) { + editor.set_error(format!("Error saving: {}", err)); + } + } + })); + Ok(call) } -#[derive(PartialEq)] +#[derive(PartialEq, Eq)] pub enum Open { Below, Above, diff --git a/helix-term/src/commands/dap.rs b/helix-term/src/commands/dap.rs index 12a3fbc7..c27417e3 100644 --- a/helix-term/src/commands/dap.rs +++ b/helix-term/src/commands/dap.rs @@ -118,11 +118,14 @@ fn dap_callback<T, F>( 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,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 = Box::new(move |_editor, 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); @@ -332,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 = Box::new(move |_editor, 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); @@ -582,7 +587,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 +615,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 +629,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 +656,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 13a0adcf..f20e71c2 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<str>], event: PromptEvent) -> anyhow::Result<()> { + log::debug!("quitting..."); + if event != PromptEvent::Validate { return Ok(()); } @@ -30,6 +34,7 @@ fn quit(cx: &mut compositor::Context, args: &[Cow<str>], event: PromptEvent) -> buffers_remaining_impl(cx.editor)? } + cx.block_try_flush_writes()?; cx.editor.close(view!(cx.editor).id); Ok(()) @@ -70,14 +75,16 @@ fn open(cx: &mut compositor::Context, args: &[Cow<str>], event: PromptEvent) -> } fn buffer_close_by_ids_impl( - editor: &mut Editor, + cx: &mut compositor::Context, doc_ids: &[DocumentId], force: bool, ) -> anyhow::Result<()> { + 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 @@ -86,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: {:?}", @@ -149,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( @@ -162,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<DocumentId> { @@ -184,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( @@ -197,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<DocumentId> { @@ -214,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( @@ -227,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( @@ -261,39 +268,29 @@ fn write_impl( path: Option<&Cow<str>>, 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 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"); - } - 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(), - Modified::SetUnmodified, - shared.clone(), + fmt, + Some((path.map(Into::into), force)), ); - jobs.callback(callback); - shared + + jobs.add(Job::with_callback(callback).wait_before_exiting()); }) } else { None }; - let future = doc.format_and_save(fmt, force); - cx.jobs.add(Job::new(future).wait_before_exiting()); - if path.is_some() { + if fmt.is_none() { let id = doc.id(); - doc.detect_language(cx.editor.syn_loader.clone()); - let _ = cx.editor.refresh_language_server(id); + cx.editor.save(id, path, force)?; } Ok(()) @@ -348,8 +345,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, None); cx.jobs.callback(callback); } @@ -520,7 +516,7 @@ fn write_quit( } write_impl(cx, args.first(), false)?; - helix_lsp::block_on(cx.jobs.finish())?; + cx.block_try_flush_writes()?; quit(cx, &[], event) } @@ -534,6 +530,7 @@ fn force_write_quit( } write_impl(cx, args.first(), true)?; + cx.block_try_flush_writes()?; force_quit(cx, &[], event) } @@ -573,40 +570,50 @@ fn write_all_impl( return Ok(()); } - let mut errors = String::new(); + 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.push_str("cannot write a buffer without a filename\n"); - 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 shared = fmt.shared(); - let callback = make_format_callback( - doc.id(), - doc.version(), - Modified::SetUnmodified, - shared.clone(), - ); - jobs.callback(callback); - shared - }) - } 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 - }; - let future = doc.format_and_save(fmt, force); - jobs.add(Job::new(future).wait_before_exiting()); + }) + .collect(); + + // manually call save for the rest of docs that don't have a formatter + for id in saves { + cx.editor.save::<PathBuf>(id, None, force)?; } if quit { + cx.block_try_flush_writes()?; + if !force { buffers_remaining_impl(cx.editor)?; } @@ -618,7 +625,11 @@ fn write_all_impl( } } - bail!(errors) + if !errors.is_empty() && !force { + bail!("{:?}", errors); + } + + Ok(()) } fn write_all( @@ -680,6 +691,7 @@ fn quit_all( return Ok(()); } + cx.block_try_flush_writes()?; quit_all_impl(cx.editor, false) } @@ -708,8 +720,9 @@ fn cquit( .first() .and_then(|code| code.parse::<i32>().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) } @@ -1064,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) }; @@ -1492,12 +1506,13 @@ fn tree_sitter_subtree( contents.push_str("\n```"); 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) }; @@ -1605,8 +1620,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(), @@ -1615,7 +1630,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/compositor.rs b/helix-term/src/compositor.rs index c0898dae..971dc52d 100644 --- a/helix-term/src/compositor.rs +++ b/helix-term/src/compositor.rs @@ -27,6 +27,16 @@ 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 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()))?; + Ok(()) + } +} + pub trait Component: Any + AnyComponent { /// Process input events, return true if handled. fn handle_event(&mut self, _event: &Event, _ctx: &mut Context) -> EventResult { diff --git a/helix-term/src/job.rs b/helix-term/src/job.rs index e5147992..2888b6eb 100644 --- a/helix-term/src/job.rs +++ b/helix-term/src/job.rs @@ -5,7 +5,11 @@ use crate::compositor::Compositor; use futures_util::future::{BoxFuture, Future, FutureExt}; use futures_util::stream::{FuturesUnordered, StreamExt}; -pub type Callback = Box<dyn FnOnce(&mut Editor, &mut Compositor) + Send>; +pub enum Callback { + EditorCompositor(Box<dyn FnOnce(&mut Editor, &mut Compositor) + Send>), + Editor(Box<dyn FnOnce(&mut Editor) + Send>), +} + pub type JobFuture = BoxFuture<'static, anyhow::Result<Option<Callback>>>; pub struct Job { @@ -68,9 +72,10 @@ 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), + }, Err(e) => { editor.set_error(format!("Async job failed: {}", e)); } @@ -93,13 +98,32 @@ 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, + editor: &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 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 + _ => (), + } + } } Err(e) => { self.wait_futures = tail; 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 <ENTER> 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/src/ui/editor.rs b/helix-term/src/ui/editor.rs index 3cd2130a..73dfd52c 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,10 @@ 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| { - compositor.remove(SignatureHelp::ID); - }); + let call: job::Callback = + Callback::EditorCompositor(Box::new(|_editor, compositor| { + compositor.remove(SignatureHelp::ID); + })); Ok(call) }); } 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) }; 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/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(<esc>", "(#[|\n]#"), ) .await?; diff --git a/helix-term/tests/test/commands.rs b/helix-term/tests/test/commands.rs index f7ce9af0..5238cc69 100644 --- a/helix-term/tests/test/commands.rs +++ b/helix-term/tests/test/commands.rs @@ -1,21 +1,25 @@ -use std::{ - io::{Read, Write}, - ops::RangeInclusive, -}; +use std::ops::RangeInclusive; use helix_core::diagnostic::Severity; -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()?; + 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<esc>:wq<ret>"), 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, @@ -25,11 +29,10 @@ async fn test_write_quit_fail() -> anyhow::Result<()> { Ok(()) } -#[tokio::test] -#[ignore] +#[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, @@ -69,8 +72,12 @@ async fn test_buffer_close_concurrent() -> anyhow::Result<()> { command.push_str(":buffer<minus>close<ret>"); + 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()); @@ -82,12 +89,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 8f2501e6..5adc3354 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; @@ -56,7 +61,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 +77,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 { @@ -87,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(()) } @@ -101,7 +118,7 @@ pub async fn test_key_sequence_with_input_text<T: Into<TestCase>>( 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); @@ -125,16 +142,30 @@ pub async fn test_key_sequence_with_input_text<T: Into<TestCase>>( .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<String>) -> 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<T: Into<TestCase>>( 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), @@ -155,7 +186,13 @@ pub async fn test_with_config<T: Into<TestCase>>( } pub async fn test<T: Into<TestCase>>(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<S: AsRef<str>>( @@ -200,14 +237,75 @@ pub fn new_readonly_tempfile() -> anyhow::Result<NamedTempFile> { Ok(file) } -/// Creates a new Application with default config that opens the given file -/// path -pub fn app_with_file<P: Into<PathBuf>>(path: P) -> anyhow::Result<Application> { - 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<P: Into<PathBuf>>( + mut self, + path: P, + pos: Option<helix_core::Position>, + ) -> 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<S: Into<String>>(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<Application> { + 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<()> { + 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/movement.rs b/helix-term/tests/test/movement.rs index 45aae39e..81c66e53 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(); @@ -115,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 @@ -146,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 @@ -178,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 @@ -208,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 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<ret>:theme d<C-n><tab>"), Some(&|app| { assert!(!app.editor.is_err()); diff --git a/helix-term/tests/test/splits.rs b/helix-term/tests/test/splits.rs new file mode 100644 index 00000000..5807413a --- /dev/null +++ b/helix-term/tests/test/splits.rs @@ -0,0 +1,129 @@ +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()?; + + let mut app = helpers::AppBuilder::new() + .with_file(file1.path(), None) + .build()?; + + test_key_sequences( + &mut app, + vec![ + ( + Some(&format!( + "ihello1<esc>:sp<ret>:o {}<ret>ihello2<esc>:sp<ret>:o {}<ret>ihello3<esc>", + 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<ret>"), + 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()?; + let mut app = helpers::AppBuilder::new() + .with_file(file.path(), None) + .build()?; + + test_key_sequences( + &mut app, + vec![ + ( + Some("O<esc>ihello<esc>:sp<ret>ogoodbye<esc>"), + 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<ret>"), + 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(()) +} diff --git a/helix-term/tests/test/write.rs b/helix-term/tests/test/write.rs index 8869d881..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<ret><esc>:w<ret>"), None, false, @@ -35,12 +37,15 @@ 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()?; + 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<ret><esc>:wq<ret>"), None, true, @@ -61,25 +66,21 @@ async fn test_write_quit() -> anyhow::Result<()> { Ok(()) } -#[tokio::test] -#[ignore] +#[tokio::test(flavor = "multi_thread")] async fn test_write_concurrent() -> anyhow::Result<()> { let mut file = tempfile::NamedTempFile::new()?; let mut command = String::new(); const RANGE: RangeInclusive<i32> = 1..=5000; + let mut app = helpers::AppBuilder::new() + .with_file(file.path(), None) + .build()?; for i in RANGE { let cmd = format!("%c{}<esc>:w<ret>", 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()?; @@ -92,12 +93,14 @@ 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()?; + 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, @@ -131,12 +134,127 @@ async fn test_write_fail_mod_flag() -> anyhow::Result<()> { } #[tokio::test] -#[ignore] +async fn test_write_scratch_to_new_path() -> anyhow::Result<()> { + let mut file = tempfile::NamedTempFile::new()?; + + test_key_sequence( + &mut AppBuilder::new().build()?, + Some(format!("ihello<esc>:w {}<ret>", 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<esc>:w<ret>", "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_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<ret>"), 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 app, + vec![ + ( + Some("ii can eat glass, it will not hurt me<ret><esc>:w<ret>"), + Some(&|app| { + let doc = doc!(app.editor); + assert!(!app.editor.is_err()); + assert_eq!(file1.path(), doc.path().unwrap()); + }), + ), + ( + Some(&format!(":w {}<ret>", 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?; + + helpers::assert_file_has_content( + file1.as_file_mut(), + &helpers::platform_line("i can eat glass, it will not hurt me\n"), + )?; + + helpers::assert_file_has_content( + file2.as_file_mut(), + &helpers::platform_line("i can eat glass, it will not hurt me\n"), + )?; + + Ok(()) +} + +#[tokio::test] 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, diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 0daa983f..78c6d032 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -83,6 +83,18 @@ 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 DocumentSavedEvent { + pub revision: usize, + pub doc_id: DocumentId, + pub path: PathBuf, + pub text: Rope, +} + +pub type DocumentSavedEventResult = Result<DocumentSavedEvent, anyhow::Error>; +pub type DocumentSavedEventFuture = BoxFuture<'static, DocumentSavedEventResult>; + pub struct Document { pub(crate) id: DocumentId, text: Rope, @@ -492,45 +504,61 @@ impl Document { Some(fut.boxed()) } - pub fn save(&mut self, force: bool) -> impl Future<Output = Result<(), anyhow::Error>> { - self.save_impl::<futures_util::future::Ready<_>>(None, force) - } - - pub fn format_and_save( + pub fn save<P: Into<PathBuf>>( &mut self, - formatting: Option<impl Future<Output = Result<Transaction, FormatterError>>>, + path: Option<P>, force: bool, - ) -> impl Future<Output = anyhow::Result<()>> { - self.save_impl(formatting, force) + ) -> Result< + impl Future<Output = Result<DocumentSavedEvent, anyhow::Error>> + 'static + Send, + anyhow::Error, + > { + let path = path.map(|path| path.into()); + self.save_impl(path, force) + + // futures_util::future::Ready<_>, } - // 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 /// 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<F: Future<Output = Result<Transaction, FormatterError>>>( + fn save_impl( &mut self, - formatting: Option<F>, + path: Option<PathBuf>, force: bool, - ) -> impl Future<Output = Result<(), anyhow::Error>> { + ) -> Result< + impl Future<Output = Result<DocumentSavedEvent, anyhow::Error>> + 'static + Send, + anyhow::Error, + > { + 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 text = self.text().clone(); - 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)?, + None => { + if self.path.is_none() { + bail!("Can't save with no path set!"); + } + self.path.as_ref().unwrap().clone() + } + }; + + let identifier = self.path().map(|_| self.identifier()); 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 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 @@ -543,39 +571,34 @@ 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?; + let mut file = File::create(&path).await?; to_writer(&mut file, encoding, &text).await?; + let event = DocumentSavedEvent { + revision: current_rev, + doc_id, + path, + text: text.clone(), + }; + 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) - { - notification.await?; + + if let Some(identifier) = identifier { + if let Some(notification) = + language_server.text_document_did_save(identifier, &text) + { + notification.await?; + } } } - Ok(()) - } + Ok(event) + }; + + Ok(future) } /// Detect the programming language based on the file type. @@ -930,6 +953,12 @@ impl Document { let history = self.history.take(); let current_revision = history.current_revision(); self.history.set(history); + log::debug!( + "id {} modified - last saved: {}, current: {}", + self.id, + self.last_saved_revision, + current_revision + ); current_revision != self.last_saved_revision || !self.changes.is_empty() } @@ -941,6 +970,30 @@ 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) { + 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(); + let current_revision = history.current_revision(); + self.history.set(history); + current_revision + } + /// Corresponding language scope name. Usually `source.<lang>`. 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..cd2b1ad4 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::{DocumentSavedEventFuture, DocumentSavedEventResult, 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::{future, StreamExt}; +use helix_lsp::Call; use tokio_stream::wrappers::UnboundedReceiverStream; use std::{ @@ -28,7 +29,7 @@ use tokio::{ time::{sleep, Duration, Instant, Sleep}, }; -use anyhow::Error; +use anyhow::{anyhow, bail, Error}; pub use helix_core::diagnostic::Severity; pub use helix_core::register::Registers; @@ -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<TerminalConfig> { 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<StatusLineElement>, @@ -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, @@ -643,12 +644,21 @@ pub struct Breakpoint { pub log_message: Option<String>, } +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<DocumentId, Document>, + + // 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<DocumentId, UnboundedSender<Once<DocumentSavedEventFuture>>>, + pub save_queue: SelectAll<Flatten<UnboundedReceiverStream<Once<DocumentSavedEventFuture>>>>, + pub write_count: usize, + pub count: Option<std::num::NonZeroUsize>, pub selected_register: Option<char>, pub registers: Registers, @@ -688,6 +698,15 @@ pub struct Editor { pub config_events: (UnboundedSender<ConfigEvent>, UnboundedReceiver<ConfigEvent>), } +#[derive(Debug)] +pub enum EditorEvent { + DocumentSaved(DocumentSavedEventResult), + ConfigEvent(ConfigEvent), + LanguageServerMessage((usize, Call)), + DebuggerEvent(dap::Payload), + IdleTimer, +} + #[derive(Debug, Clone)] pub enum ConfigEvent { Refresh, @@ -719,6 +738,8 @@ pub enum CloseError { DoesNotExist, /// Buffer is modified BufferModified(String), + /// Document failed to save + SaveError(anyhow::Error), } impl Editor { @@ -739,6 +760,9 @@ impl Editor { tree: Tree::new(area), next_document_id: DocumentId::default(), documents: BTreeMap::new(), + saves: HashMap::new(), + save_queue: SelectAll::new(), + write_count: 0, count: None, selected_register: None, macro_recording: None, @@ -804,12 +828,16 @@ impl Editor { #[inline] pub fn set_status<T: Into<Cow<'static, str>>>(&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<T: Into<Cow<'static, str>>>(&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] @@ -1034,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 } @@ -1080,16 +1115,19 @@ impl Editor { } pub fn close_document(&mut self, doc_id: DocumentId, force: bool) -> Result<(), CloseError> { - let doc = match self.documents.get(&doc_id) { + let doc = match self.documents.get_mut(&doc_id) { Some(doc) => doc, None => return Err(CloseError::DoesNotExist), }; - if !force && doc.is_modified() { return Err(CloseError::BufferModified(doc.display_name().into_owned())); } + // This will also disallow any follow-up writes + self.saves.remove(&doc_id); + if let Some(language_server) = doc.language_server() { + // TODO: track error tokio::spawn(language_server.text_document_did_close(doc.identifier())); } @@ -1152,6 +1190,32 @@ impl Editor { Ok(()) } + pub fn save<P: Into<PathBuf>>( + &mut self, + doc_id: DocumentId, + path: Option<P>, + 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)?; + + use futures_util::stream; + + 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))?; + + self.write_count += 1; + + Ok(()) + } + pub fn resize(&mut self, area: Rect) { if self.tree.resize(area) { self._refresh(); @@ -1252,14 +1316,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<u64>, ) -> Result<(), tokio::time::error::Elapsed> { tokio::time::timeout( - Duration::from_millis(timeout.unwrap_or(500)), + Duration::from_millis(timeout.unwrap_or(3000)), future::join_all( self.language_servers .iter_clients() @@ -1269,4 +1333,48 @@ impl Editor { .await .map(|_| ()) } + + pub async fn wait_event(&mut self) -> EditorEvent { + tokio::select! { + biased; + + Some(event) = self.save_queue.next() => { + self.write_count -= 1; + EditorEvent::DocumentSaved(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 + } + } + } + + pub async fn flush_writes(&mut self) -> anyhow::Result<()> { + while self.write_count > 0 { + if let Some(save_event) = self.save_queue.next().await { + self.write_count -= 1; + + let save_event = match save_event { + Ok(event) => event, + Err(err) => { + self.set_error(err.to_string()); + bail!(err); + } + }; + + let doc = doc_mut!(self, &save_event.doc_id); + doc.set_last_saved_revision(save_event.revision); + } + } + + Ok(()) + } } |