From bd903bf132dfc188e2f4c634a8b457ab4d7d4852 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Fri, 26 Jun 2020 16:17:22 +0200 Subject: Be more precise about flycheck status --- crates/flycheck/src/lib.rs | 92 ++++++++++++----------------------- crates/rust-analyzer/src/main_loop.rs | 13 +++-- 2 files changed, 38 insertions(+), 67 deletions(-) (limited to 'crates') diff --git a/crates/flycheck/src/lib.rs b/crates/flycheck/src/lib.rs index 4dcab7a61..92ec4f92e 100644 --- a/crates/flycheck/src/lib.rs +++ b/crates/flycheck/src/lib.rs @@ -7,7 +7,7 @@ use std::{ io::{self, BufReader}, path::PathBuf, process::{Command, Stdio}, - time::Instant, + time::Duration, }; use crossbeam_channel::{never, select, unbounded, Receiver, Sender}; @@ -74,9 +74,6 @@ impl FlycheckHandle { #[derive(Debug)] pub enum Message { - /// Request a clearing of all cached diagnostics from the check watcher - ClearDiagnostics, - /// Request adding a diagnostic with fixes included to a file AddDiagnostic { workspace_root: PathBuf, diagnostic: Diagnostic }, @@ -86,9 +83,10 @@ pub enum Message { #[derive(Debug)] pub enum Progress { - Being, + DidStart, DidCheckCrate(String), - End, + DidFinish, + DidCancel, } struct Restart; @@ -97,19 +95,18 @@ struct FlycheckActor { sender: Box, config: FlycheckConfig, workspace_root: PathBuf, - last_update_req: Option, /// WatchThread exists to wrap around the communication needed to be able to /// run `cargo check` without blocking. Currently the Rust standard library /// doesn't provide a way to read sub-process output without blocking, so we /// have to wrap sub-processes output handling in a thread and pass messages /// back over a channel. // XXX: drop order is significant - check_process: Option<(Receiver, jod_thread::JoinHandle)>, + check_process: Option<(Receiver, jod_thread::JoinHandle)>, } enum Event { Restart(Restart), - CheckEvent(Option), + CheckEvent(Option), } impl FlycheckActor { @@ -118,7 +115,7 @@ impl FlycheckActor { config: FlycheckConfig, workspace_root: PathBuf, ) -> FlycheckActor { - FlycheckActor { sender, config, workspace_root, last_update_req: None, check_process: None } + FlycheckActor { sender, config, workspace_root, check_process: None } } fn next_event(&self, inbox: &Receiver) -> Option { let check_chan = self.check_process.as_ref().map(|(chan, _thread)| chan); @@ -128,65 +125,48 @@ impl FlycheckActor { } } fn run(&mut self, inbox: Receiver) { - // If we rerun the thread, we need to discard the previous check results first - self.send(Message::ClearDiagnostics); - self.send(Message::Progress(Progress::End)); - while let Some(event) = self.next_event(&inbox) { match event { - Event::Restart(Restart) => self.last_update_req = Some(Instant::now()), + Event::Restart(Restart) => { + while let Ok(Restart) = inbox.recv_timeout(Duration::from_millis(50)) {} + self.cancel_check_process(); + self.check_process = Some(self.start_check_process()); + self.send(Message::Progress(Progress::DidStart)); + } Event::CheckEvent(None) => { // Watcher finished, replace it with a never channel to // avoid busy-waiting. - self.check_process = None; + assert!(self.check_process.take().is_some()); + self.send(Message::Progress(Progress::DidFinish)); } - Event::CheckEvent(Some(event)) => match event { - CheckEvent::Begin => { - self.send(Message::Progress(Progress::Being)); - } - - CheckEvent::End => { - self.send(Message::Progress(Progress::End)); - } - - CheckEvent::Msg(cargo_metadata::Message::CompilerArtifact(msg)) => { + Event::CheckEvent(Some(message)) => match message { + cargo_metadata::Message::CompilerArtifact(msg) => { self.send(Message::Progress(Progress::DidCheckCrate(msg.target.name))); } - CheckEvent::Msg(cargo_metadata::Message::CompilerMessage(msg)) => { + cargo_metadata::Message::CompilerMessage(msg) => { self.send(Message::AddDiagnostic { workspace_root: self.workspace_root.clone(), diagnostic: msg.message, }); } - CheckEvent::Msg(cargo_metadata::Message::BuildScriptExecuted(_)) - | CheckEvent::Msg(cargo_metadata::Message::BuildFinished(_)) - | CheckEvent::Msg(cargo_metadata::Message::TextLine(_)) - | CheckEvent::Msg(cargo_metadata::Message::Unknown) => {} + cargo_metadata::Message::BuildScriptExecuted(_) + | cargo_metadata::Message::BuildFinished(_) + | cargo_metadata::Message::TextLine(_) + | cargo_metadata::Message::Unknown => {} }, } - if self.should_recheck() { - self.last_update_req = None; - self.send(Message::ClearDiagnostics); - self.restart_check_process(); - } } + // If we rerun the thread, we need to discard the previous check results first + self.cancel_check_process(); } - fn should_recheck(&mut self) -> bool { - if let Some(_last_update_req) = &self.last_update_req { - // We currently only request an update on save, as we need up to - // date source on disk for cargo check to do it's magic, so we - // don't really need to debounce the requests at this point. - return true; + fn cancel_check_process(&mut self) { + if self.check_process.take().is_some() { + self.send(Message::Progress(Progress::DidCancel)); } - false } - - fn restart_check_process(&mut self) { - // First, clear and cancel the old thread - self.check_process = None; - + fn start_check_process(&self) -> (Receiver, jod_thread::JoinHandle) { let mut cmd = match &self.config { FlycheckConfig::CargoCommand { command, @@ -223,8 +203,6 @@ impl FlycheckActor { let thread = jod_thread::spawn(move || { // If we trigger an error here, we will do so in the loop instead, // which will break out of the loop, and continue the shutdown - let _ = message_send.send(CheckEvent::Begin); - let res = run_cargo(cmd, &mut |message| { // Skip certain kinds of messages to only spend time on what's useful match &message { @@ -237,7 +215,7 @@ impl FlycheckActor { } // if the send channel was closed, we want to shutdown - message_send.send(CheckEvent::Msg(message)).is_ok() + message_send.send(message).is_ok() }); if let Err(err) = res { @@ -245,12 +223,8 @@ impl FlycheckActor { // to display user-caused misconfiguration errors instead of just logging them here log::error!("Cargo watcher failed {:?}", err); } - - // We can ignore any error here, as we are already in the progress - // of shutting down. - let _ = message_send.send(CheckEvent::End); }); - self.check_process = Some((message_recv, thread)) + (message_recv, thread) } fn send(&self, check_task: Message) { @@ -258,12 +232,6 @@ impl FlycheckActor { } } -enum CheckEvent { - Begin, - Msg(cargo_metadata::Message), - End, -} - fn run_cargo( mut command: Command, on_message: &mut dyn FnMut(cargo_metadata::Message) -> bool, diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs index 8fc816cbd..ed3d27750 100644 --- a/crates/rust-analyzer/src/main_loop.rs +++ b/crates/rust-analyzer/src/main_loop.rs @@ -191,8 +191,6 @@ impl GlobalState { } }, Event::Flycheck(task) => match task { - flycheck::Message::ClearDiagnostics => self.diagnostics.clear_check(), - flycheck::Message::AddDiagnostic { workspace_root, diagnostic } => { let diagnostics = crate::diagnostics::to_proto::map_rust_diagnostic_to_lsp( &self.config.diagnostics, @@ -215,11 +213,16 @@ impl GlobalState { flycheck::Message::Progress(status) => { let (state, message) = match status { - flycheck::Progress::Being => (Progress::Begin, None), + flycheck::Progress::DidStart => { + self.diagnostics.clear_check(); + (Progress::Begin, None) + }, flycheck::Progress::DidCheckCrate(target) => { (Progress::Report, Some(target)) } - flycheck::Progress::End => (Progress::End, None), + flycheck::Progress::DidFinish | flycheck::Progress::DidCancel => { + (Progress::End, None) + } }; report_progress(self, "cargo check", state, message, None); @@ -466,7 +469,7 @@ impl GlobalState { } } -#[derive(Eq, PartialEq)] +#[derive(Debug, Eq, PartialEq)] enum Progress { Begin, Report, -- cgit v1.2.3 From 1893289e5c7cebeeb9705c031c996fc29d8c5b54 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Fri, 26 Jun 2020 16:33:57 +0200 Subject: Move progress reporting to utils --- crates/rust-analyzer/src/global_state.rs | 4 -- crates/rust-analyzer/src/lib.rs | 2 +- crates/rust-analyzer/src/lsp_utils.rs | 85 ++++++++++++++++++++++++++------ crates/rust-analyzer/src/main_loop.rs | 68 ++----------------------- crates/rust-analyzer/src/reload.rs | 24 +++++---- 5 files changed, 91 insertions(+), 92 deletions(-) (limited to 'crates') diff --git a/crates/rust-analyzer/src/global_state.rs b/crates/rust-analyzer/src/global_state.rs index 4da094083..41659f99d 100644 --- a/crates/rust-analyzer/src/global_state.rs +++ b/crates/rust-analyzer/src/global_state.rs @@ -21,7 +21,6 @@ use crate::{ main_loop::Task, reload::SourceRootConfig, request_metrics::{LatestRequests, RequestMetrics}, - show_message, thread_pool::TaskPool, to_proto::url_from_abs_path, Result, @@ -182,9 +181,6 @@ impl GlobalState { self.send(response.into()); } } - pub(crate) fn show_message(&self, typ: lsp_types::MessageType, message: String) { - show_message(typ, message, &self.sender) - } } impl Drop for GlobalState { diff --git a/crates/rust-analyzer/src/lib.rs b/crates/rust-analyzer/src/lib.rs index a24dfe58c..407944d85 100644 --- a/crates/rust-analyzer/src/lib.rs +++ b/crates/rust-analyzer/src/lib.rs @@ -39,7 +39,7 @@ pub mod config; use serde::de::DeserializeOwned; pub type Result> = std::result::Result; -pub use crate::{caps::server_capabilities, lsp_utils::show_message, main_loop::main_loop}; +pub use crate::{caps::server_capabilities, main_loop::main_loop}; use std::fmt; pub fn from_json(what: &'static str, json: serde_json::Value) -> Result { diff --git a/crates/rust-analyzer/src/lsp_utils.rs b/crates/rust-analyzer/src/lsp_utils.rs index 35917030c..fd793a17c 100644 --- a/crates/rust-analyzer/src/lsp_utils.rs +++ b/crates/rust-analyzer/src/lsp_utils.rs @@ -1,24 +1,13 @@ //! Utilities for LSP-related boilerplate code. use std::{error::Error, ops::Range}; -use crossbeam_channel::Sender; -use lsp_server::{Message, Notification}; +use lsp_server::Notification; +use lsp_types::request::Request; use ra_db::Canceled; use ra_ide::LineIndex; use serde::Serialize; -use crate::from_proto; - -pub fn show_message( - typ: lsp_types::MessageType, - message: impl Into, - sender: &Sender, -) { - let message = message.into(); - let params = lsp_types::ShowMessageParams { typ, message }; - let not = notification_new::(params); - sender.send(not.into()).unwrap(); -} +use crate::{from_proto, global_state::GlobalState}; pub(crate) fn is_canceled(e: &(dyn Error + 'static)) -> bool { e.downcast_ref::().is_some() @@ -38,6 +27,74 @@ where Notification::new(N::METHOD.to_string(), params) } +#[derive(Debug, Eq, PartialEq)] +pub(crate) enum Progress { + Begin, + Report, + End, +} + +impl Progress { + pub(crate) fn percentage(done: usize, total: usize) -> f64 { + (done as f64 / total.max(1) as f64) * 100.0 + } +} + +impl GlobalState { + pub(crate) fn show_message(&mut self, typ: lsp_types::MessageType, message: String) { + let message = message.into(); + let params = lsp_types::ShowMessageParams { typ, message }; + let not = notification_new::(params); + self.send(not.into()); + } + + pub(crate) fn report_progress( + &mut self, + title: &str, + state: Progress, + message: Option, + percentage: Option, + ) { + if !self.config.client_caps.work_done_progress { + return; + } + let token = lsp_types::ProgressToken::String(format!("rustAnalyzer/{}", title)); + let work_done_progress = match state { + Progress::Begin => { + let work_done_progress_create = self.req_queue.outgoing.register( + lsp_types::request::WorkDoneProgressCreate::METHOD.to_string(), + lsp_types::WorkDoneProgressCreateParams { token: token.clone() }, + |_, _| (), + ); + self.send(work_done_progress_create.into()); + + lsp_types::WorkDoneProgress::Begin(lsp_types::WorkDoneProgressBegin { + title: title.into(), + cancellable: None, + message, + percentage, + }) + } + Progress::Report => { + lsp_types::WorkDoneProgress::Report(lsp_types::WorkDoneProgressReport { + cancellable: None, + message, + percentage, + }) + } + Progress::End => { + lsp_types::WorkDoneProgress::End(lsp_types::WorkDoneProgressEnd { message }) + } + }; + let notification = + notification_new::(lsp_types::ProgressParams { + token, + value: lsp_types::ProgressParamsValue::WorkDone(work_done_progress), + }); + self.send(notification.into()); + } +} + pub(crate) fn apply_document_changes( old_text: &mut String, content_changes: Vec, diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs index ed3d27750..ae3c7e30e 100644 --- a/crates/rust-analyzer/src/main_loop.rs +++ b/crates/rust-analyzer/src/main_loop.rs @@ -18,7 +18,7 @@ use crate::{ from_proto, global_state::{file_id_to_url, url_to_file_id, GlobalState, Status}, handlers, lsp_ext, - lsp_utils::{apply_document_changes, is_canceled, notification_is, notification_new}, + lsp_utils::{apply_document_changes, is_canceled, notification_is, notification_new, Progress}, Result, }; @@ -181,12 +181,11 @@ impl GlobalState { became_ready = true; Progress::End }; - report_progress( - self, + self.report_progress( "roots scanned", state, Some(format!("{}/{}", n_done, n_total)), - Some(percentage(n_done, n_total)), + Some(Progress::percentage(n_done, n_total)), ) } }, @@ -216,7 +215,7 @@ impl GlobalState { flycheck::Progress::DidStart => { self.diagnostics.clear_check(); (Progress::Begin, None) - }, + } flycheck::Progress::DidCheckCrate(target) => { (Progress::Report, Some(target)) } @@ -225,7 +224,7 @@ impl GlobalState { } }; - report_progress(self, "cargo check", state, message, None); + self.report_progress("cargo check", state, message, None); } }, } @@ -468,60 +467,3 @@ impl GlobalState { }); } } - -#[derive(Debug, Eq, PartialEq)] -enum Progress { - Begin, - Report, - End, -} - -fn percentage(done: usize, total: usize) -> f64 { - (done as f64 / total.max(1) as f64) * 100.0 -} - -fn report_progress( - global_state: &mut GlobalState, - title: &str, - state: Progress, - message: Option, - percentage: Option, -) { - if !global_state.config.client_caps.work_done_progress { - return; - } - let token = lsp_types::ProgressToken::String(format!("rustAnalyzer/{}", title)); - let work_done_progress = match state { - Progress::Begin => { - let work_done_progress_create = global_state.req_queue.outgoing.register( - lsp_types::request::WorkDoneProgressCreate::METHOD.to_string(), - lsp_types::WorkDoneProgressCreateParams { token: token.clone() }, - |_, _| (), - ); - global_state.send(work_done_progress_create.into()); - - lsp_types::WorkDoneProgress::Begin(lsp_types::WorkDoneProgressBegin { - title: title.into(), - cancellable: None, - message, - percentage, - }) - } - Progress::Report => { - lsp_types::WorkDoneProgress::Report(lsp_types::WorkDoneProgressReport { - cancellable: None, - message, - percentage, - }) - } - Progress::End => { - lsp_types::WorkDoneProgress::End(lsp_types::WorkDoneProgressEnd { message }) - } - }; - let notification = - notification_new::(lsp_types::ProgressParams { - token, - value: lsp_types::ProgressParamsValue::WorkDone(work_done_progress), - }); - global_state.send(notification.into()); -} diff --git a/crates/rust-analyzer/src/reload.rs b/crates/rust-analyzer/src/reload.rs index a22d3e262..fece6176e 100644 --- a/crates/rust-analyzer/src/reload.rs +++ b/crates/rust-analyzer/src/reload.rs @@ -36,27 +36,31 @@ impl GlobalState { self.config .linked_projects .iter() - .filter_map(|project| match project { + .map(|project| match project { LinkedProject::ProjectManifest(manifest) => { ra_project_model::ProjectWorkspace::load( manifest.clone(), &self.config.cargo, self.config.with_sysroot, ) - .map_err(|err| { - log::error!("failed to load workspace: {:#}", err); - self.show_message( - lsp_types::MessageType::Error, - format!("rust-analyzer failed to load workspace: {:#}", err), - ); - }) - .ok() } LinkedProject::InlineJsonProject(it) => { - Some(ra_project_model::ProjectWorkspace::Json { project: it.clone() }) + Ok(ra_project_model::ProjectWorkspace::Json { project: it.clone() }) } }) .collect::>() + .into_iter() + .filter_map(|res| { + res.map_err(|err| { + log::error!("failed to load workspace: {:#}", err); + self.show_message( + lsp_types::MessageType::Error, + format!("rust-analyzer failed to load workspace: {:#}", err), + ); + }) + .ok() + }) + .collect::>() }; if let FilesWatcher::Client = self.config.files.watcher { -- cgit v1.2.3