Merge pull request #2267 from dead10ck/fix-write-fail

Write path fixes

Author: archseer

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
 

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)] = &[
     ('(', ')'),
     ('{', '}'),

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

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,25 +951,44 @@ 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);
         }
 
         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
     }
 }