Merge pull request #653 from algesten/progress-params

window/progress notification

Author: nrc

contributing.md

@@ -306,14 +306,14 @@ The RLS uses some custom extensions to the Language Server Protocol.
 These are all sent from the RLS to an LSP client and are only used to improve
 the user experience by showing progress indicators.
 
-
-* `rustDocument/beginBuild`: notification, no arguments. Sent before a
-  build starts.
-* `rustDocument/diagnosticsBegin`: notification, no arguments. Sent before
-  indexing or any diagnostics from a build are sent (build is likely in progress).
-* `rustDocument/diagnosticsEnd`: notification, no arguments. Sent when a build
-  is complete (successfully or not, or even skipped) and all post-build analysis
-  by the RLS is complete.
+* `window/progress`: notification, `title: "Build"`. Sent before build starts.
+* `window/progress`: notification with `title: "Build"`, repeated for each compile target.
+  * When total amount of work is not known, has field `message` set to the current crate name.
+  * When total amount of work is known, has field `percentage` set to how much of build has started.
+* `window/progress`: notification, `title: "Build"`, `"done": true`. Sent when build ends.
+* `window/progress`: notification, `title: "Diagnostics"`. Sent before analysis of build starts.
+* ... standard LSP `publishDiagnostics`
+* `window/progress`: notification, `title: "Diagnostics"`, `"done": true`. Sent when analysis ends.
 
 #### LSP Client to RLS
 

src/actions/mod.rs

@@ -21,12 +21,12 @@ use url::Url;
 use span;
 use Span;
 
-use actions::post_build::{BuildResults, PostBuildHandler, Notifier};
+use actions::post_build::{BuildResults, PostBuildHandler};
+use actions::progress::{BuildProgressNotifier, BuildDiagnosticsNotifier};
 use build::*;
 use lsp_data;
 use lsp_data::*;
-use lsp_data::notification::{PublishDiagnostics, ShowMessage};
-use server::{Output, Notification};
+use server::Output;
 
 use std::collections::HashMap;
 use std::path::{Path, PathBuf};
@@ -57,6 +57,7 @@ pub mod work_pool;
 pub mod post_build;
 pub mod requests;
 pub mod notifications;
+pub mod progress;
 
 /// Persistent context shared across all requests and notifications.
 pub enum ActionContext {
@@ -214,29 +215,6 @@ impl InitActionContext {
     }
 
     fn build<O: Output>(&self, project_path: &Path, priority: BuildPriority, out: &O) {
-        struct BuildNotifier<O: Output> {
-            out: O,
-            active_build_count: Arc<AtomicUsize>,
-        }
-
-        impl<O: Output> Notifier for BuildNotifier<O> {
-            fn notify_begin(&self) {
-                self.out.notify(Notification::<DiagnosticsBegin>::new(()));
-            }
-            fn notify_end(&self) {
-                self.active_build_count.fetch_sub(1, Ordering::SeqCst);
-                self.out.notify(Notification::<DiagnosticsEnd>::new(()));
-            }
-            fn notify_publish(&self, params: PublishDiagnosticsParams) {
-                self.out.notify(Notification::<PublishDiagnostics>::new(params));
-            }
-            fn notify_error(&self, msg: &str) {
-                self.out.notify(Notification::<ShowMessage>::new(lsp_data::ShowMessageParams {
-                    typ: lsp_data::MessageType::Error,
-                    message: msg.to_owned(),
-                }));
-            }
-        }
 
         let pbh = {
             let config = self.config.lock().unwrap();
@@ -246,19 +224,18 @@ impl InitActionContext {
                 project_path: project_path.to_owned(),
                 show_warnings: config.show_warnings,
                 shown_cargo_error: self.shown_cargo_error.clone(),
+                active_build_count: self.active_build_count.clone(),
                 use_black_list: config.use_crate_blacklist,
-                notifier: Box::new(BuildNotifier {
-                    out: out.clone(),
-                    active_build_count: self.active_build_count.clone(),
-                }),
+                notifier: Box::new(BuildDiagnosticsNotifier::new(out.clone())),
                 blocked_threads: vec![],
             }
         };
 
-        out.notify(Notification::<BeginBuild>::new(()));
+        let notifier = Box::new(BuildProgressNotifier::new(out.clone()));
+
         self.active_build_count.fetch_add(1, Ordering::SeqCst);
         self.build_queue
-            .request_build(project_path, priority, pbh);
+            .request_build(project_path, priority, notifier, pbh);
     }
 
     fn build_current_project<O: Output>(&self, priority: BuildPriority, out: &O) {
@@ -392,7 +369,6 @@ impl<'ctx> FileWatch<'ctx> {
     }
 }
 
-
 #[cfg(test)]
 mod test {
     use super::*;

src/actions/post_build.rs

@@ -15,11 +15,12 @@
 use std::collections::HashMap;
 use std::path::{Path, PathBuf};
 use std::sync::{Arc, Mutex};
-use std::sync::atomic::{AtomicBool, Ordering};
+use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering};
 use std::thread;
 
 use build::BuildResult;
 use lsp_data::{ls_util, PublishDiagnosticsParams};
+use actions::progress::DiagnosticsNotifier;
 
 use analysis::AnalysisHost;
 use data::Analysis;
@@ -37,28 +38,20 @@ pub struct PostBuildHandler {
     pub show_warnings: bool,
     pub use_black_list: bool,
     pub shown_cargo_error: Arc<AtomicBool>,
-    pub notifier: Box<Notifier>,
+    pub active_build_count: Arc<AtomicUsize>,
+    pub notifier: Box<DiagnosticsNotifier>,
     pub blocked_threads: Vec<thread::Thread>,
 }
 
-/// Trait for communication back to the rest of the RLS (and on to the client).
-// This trait only really exists to work around the object safety rules (Output
-// is not object-safe).
-pub trait Notifier: Send {
-    fn notify_begin(&self);
-    fn notify_end(&self);
-    fn notify_publish(&self, PublishDiagnosticsParams);
-    fn notify_error(&self, &str);
-}
 
 impl PostBuildHandler {
     pub fn handle(mut self, result: BuildResult) {
-        self.notifier.notify_begin();
 
         match result {
             BuildResult::Success(cwd, messages, new_analysis, _) => {
                 thread::spawn(move || {
                     trace!("build - Success");
+                    self.notifier.notify_begin_diagnostics();
 
                     // Emit appropriate diagnostics using the ones from build.
                     self.handle_messages(&cwd, &messages);
@@ -71,26 +64,33 @@ impl PostBuildHandler {
                         self.reload_analysis_from_memory(&cwd, new_analysis);
                     }
 
+                    // the end message must be dispatched before waking up
+                    // the blocked threads, or we might see "done":true message
+                    // first in the next action invocation.
+                    self.notifier.notify_end_diagnostics();
+
                     // Wake up any threads blocked on this analysis.
                     for t in self.blocked_threads.drain(..) {
                         t.unpark();
                     }
 
-                    self.notifier.notify_end();
                     self.shown_cargo_error.store(false, Ordering::SeqCst);
+                    self.active_build_count.fetch_sub(1, Ordering::SeqCst);
                 });
             }
             BuildResult::Squashed => {
                 trace!("build - Squashed");
-                self.notifier.notify_end();
+                self.active_build_count.fetch_sub(1, Ordering::SeqCst);
             }
             BuildResult::Err(cause, cmd) => {
                 trace!("build - Error {} when running {:?}", cause, cmd);
+                self.notifier.notify_begin_diagnostics();
                 if !self.shown_cargo_error.swap(true, Ordering::SeqCst) {
                     let msg = format!("There was an error trying to build, RLS features will be limited: {}", cause);
-                    self.notifier.notify_error(&msg);
+                    self.notifier.notify_error_diagnostics(&msg);
                 }
-                self.notifier.notify_end();
+                self.notifier.notify_end_diagnostics();
+                self.active_build_count.fetch_sub(1, Ordering::SeqCst);
             }
         }
     }
@@ -165,7 +165,7 @@ impl PostBuildHandler {
                     .collect(),
             };
 
-            self.notifier.notify_publish(params);
+            self.notifier.notify_publish_diagnostics(params);
         }
     }
 }

src/actions/progress.rs

@@ -0,0 +1,135 @@
+// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+use std::sync::atomic::{AtomicUsize, Ordering};
+
+use lsp_data::{ProgressParams, PublishDiagnosticsParams, Progress, ShowMessageParams, MessageType};
+use server::{Output, Notification};
+use ls_types::notification::{PublishDiagnostics, ShowMessage};
+
+/// Trait for communication of build progress back to the client.
+pub trait ProgressNotifier: Send {
+    fn notify_begin_progress(&self);
+    fn notify_progress(&self, update: ProgressUpdate);
+    fn notify_end_progress(&self);
+}
+
+/// Kinds of progress updates
+pub enum ProgressUpdate {
+    Message(String),
+    Percentage(f64),
+}
+
+/// Trait for communication of diagnostics (i.e. build results) back to the rest of
+/// the RLS (and on to the client).
+// This trait only really exists to work around the object safety rules (Output
+// is not object-safe).
+pub trait DiagnosticsNotifier: Send {
+    fn notify_begin_diagnostics(&self);
+    fn notify_publish_diagnostics(&self, PublishDiagnosticsParams);
+    fn notify_end_diagnostics(&self);
+    fn notify_error_diagnostics(&self, msg: &str);
+}
+
+/// Generate a new progress params with a unique ID and the given title.
+fn new_progress_params(title: String) -> ProgressParams {
+
+    // counter to generate unique ID for each chain-of-progress notifications.
+    lazy_static! {
+        static ref PROGRESS_ID_COUNTER: AtomicUsize = {
+            AtomicUsize::new(0)
+        };
+    }
+
+    ProgressParams {
+        id: format!("progress_{}", PROGRESS_ID_COUNTER.fetch_add(1, Ordering::SeqCst)),
+        title: Some(title),
+        message: None,
+        percentage: None,
+        done: None,
+    }
+}
+
+/// Notifier of progress for the build (window/progress notifications).
+/// the same instance is used for the entirety of one single build.
+pub struct BuildProgressNotifier<O: Output> {
+    out: O,
+    // these params are used as a template and are cloned for each
+    // message that is actually notified.
+    progress_params: ProgressParams,
+}
+
+impl<O: Output> BuildProgressNotifier<O> {
+    pub fn new(out: O) -> BuildProgressNotifier<O> {
+        BuildProgressNotifier {
+            out,
+            progress_params: new_progress_params("Build".into()),
+        }
+    }
+}
+
+impl<O: Output> ProgressNotifier for BuildProgressNotifier<O> {
+    fn notify_begin_progress(&self) {
+        let params = self.progress_params.clone();
+        self.out.notify(Notification::<Progress>::new(params));
+    }
+    fn notify_progress(&self, update: ProgressUpdate) {
+        let mut params = self.progress_params.clone();
+        match update {
+            ProgressUpdate::Message(s) => params.message = Some(s),
+            ProgressUpdate::Percentage(p) => params.percentage = Some(p),
+        }
+        self.out.notify(Notification::<Progress>::new(params));
+    }
+    fn notify_end_progress(&self) {
+        let mut params = self.progress_params.clone();
+        params.done = Some(true);
+        self.out.notify(Notification::<Progress>::new(params));
+    }
+}
+
+
+/// Notifier of diagnostics after the build has completed.
+pub struct BuildDiagnosticsNotifier<O: Output> {
+    out: O,
+    // these params are used as a template and are cloned for each
+    // message that is actually notified.
+    progress_params: ProgressParams,
+}
+
+impl<O: Output> BuildDiagnosticsNotifier<O> {
+    pub fn new(out: O) -> BuildDiagnosticsNotifier<O> {
+        BuildDiagnosticsNotifier {
+            out,
+            progress_params: new_progress_params("Diagnostics".into()),
+        }
+    }
+}
+
+impl<O: Output> DiagnosticsNotifier for BuildDiagnosticsNotifier<O> {
+    fn notify_begin_diagnostics(&self) {
+        let params = self.progress_params.clone();
+        self.out.notify(Notification::<Progress>::new(params));
+    }
+    fn notify_publish_diagnostics(&self, params: PublishDiagnosticsParams) {
+        self.out.notify(Notification::<PublishDiagnostics>::new(params));
+    }
+    fn notify_end_diagnostics(&self) {
+        let mut params = self.progress_params.clone();
+        params.done = Some(true);
+        self.out.notify(Notification::<Progress>::new(params));
+    }
+    fn notify_error_diagnostics(&self, msg: &str) {
+        self.out.notify(Notification::<ShowMessage>::new(ShowMessageParams {
+            typ: MessageType::Error,
+            message: msg.to_owned(),
+        }));
+    }
+}