From 76c1160ffa626fc5f07b309420e6666eb79a3311 Mon Sep 17 00:00:00 2001 From: veetaha Date: Sun, 10 May 2020 18:35:33 +0300 Subject: Migrate flycheck to fully-lsp-compatible progress reports (introduce ra_progress crate) --- crates/ra_flycheck/Cargo.toml | 1 + crates/ra_flycheck/src/lib.rs | 62 +++++++++++++++++++++++++++---------------- 2 files changed, 40 insertions(+), 23 deletions(-) (limited to 'crates/ra_flycheck') diff --git a/crates/ra_flycheck/Cargo.toml b/crates/ra_flycheck/Cargo.toml index 1aa39bade..838973963 100644 --- a/crates/ra_flycheck/Cargo.toml +++ b/crates/ra_flycheck/Cargo.toml @@ -14,3 +14,4 @@ cargo_metadata = "0.10.0" serde_json = "1.0.48" jod-thread = "0.1.1" ra_toolchain = { path = "../ra_toolchain" } +ra_progress = { path = "../ra_progress" } diff --git a/crates/ra_flycheck/src/lib.rs b/crates/ra_flycheck/src/lib.rs index 6c4170529..7b9f48eb0 100644 --- a/crates/ra_flycheck/src/lib.rs +++ b/crates/ra_flycheck/src/lib.rs @@ -3,6 +3,7 @@ //! LSP diagnostics based on the output of the command. use std::{ + fmt, io::{self, BufReader}, path::PathBuf, process::{Command, Stdio}, @@ -16,6 +17,9 @@ pub use cargo_metadata::diagnostic::{ Applicability, Diagnostic, DiagnosticLevel, DiagnosticSpan, DiagnosticSpanMacroExpansion, }; +type Progress = ra_progress::Progress<(), String>; +type ProgressSource = ra_progress::ProgressSource<(), String>; + #[derive(Clone, Debug, PartialEq, Eq)] pub enum FlycheckConfig { CargoCommand { @@ -31,6 +35,17 @@ pub enum FlycheckConfig { }, } +impl fmt::Display for FlycheckConfig { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + FlycheckConfig::CargoCommand { command, .. } => write!(f, "cargo {}", command), + FlycheckConfig::CustomCommand { command, args } => { + write!(f, "{} {}", command, args.join(" ")) + } + } + } +} + /// Flycheck wraps the shared state and communication machinery used for /// running `cargo check` (or other compatible command) and providing /// diagnostics based on the output. @@ -44,11 +59,15 @@ pub struct Flycheck { } impl Flycheck { - pub fn new(config: FlycheckConfig, workspace_root: PathBuf) -> Flycheck { + pub fn new( + config: FlycheckConfig, + workspace_root: PathBuf, + progress_src: ProgressSource, + ) -> Flycheck { let (task_send, task_recv) = unbounded::(); let (cmd_send, cmd_recv) = unbounded::(); let handle = jod_thread::spawn(move || { - FlycheckThread::new(config, workspace_root).run(&task_send, &cmd_recv); + FlycheckThread::new(config, workspace_root, progress_src).run(&task_send, &cmd_recv); }); Flycheck { task_recv, cmd_send, handle } } @@ -66,16 +85,6 @@ pub enum CheckTask { /// Request adding a diagnostic with fixes included to a file AddDiagnostic { workspace_root: PathBuf, diagnostic: Diagnostic }, - - /// Request check progress notification to client - Status(Status), -} - -#[derive(Debug)] -pub enum Status { - Being, - Progress(String), - End, } pub enum CheckCommand { @@ -87,6 +96,8 @@ struct FlycheckThread { config: FlycheckConfig, workspace_root: PathBuf, last_update_req: Option, + progress_src: ProgressSource, + progress: Option, // XXX: drop order is significant message_recv: Receiver, /// WatchThread exists to wrap around the communication needed to be able to @@ -98,11 +109,17 @@ struct FlycheckThread { } impl FlycheckThread { - fn new(config: FlycheckConfig, workspace_root: PathBuf) -> FlycheckThread { + fn new( + config: FlycheckConfig, + workspace_root: PathBuf, + progress_src: ProgressSource, + ) -> FlycheckThread { FlycheckThread { config, workspace_root, + progress_src, last_update_req: None, + progress: None, message_recv: never(), check_process: None, } @@ -140,9 +157,9 @@ impl FlycheckThread { } } - fn clean_previous_results(&self, task_send: &Sender) { + fn clean_previous_results(&mut self, task_send: &Sender) { task_send.send(CheckTask::ClearDiagnostics).unwrap(); - task_send.send(CheckTask::Status(Status::End)).unwrap(); + self.progress = None; } fn should_recheck(&mut self) -> bool { @@ -161,18 +178,17 @@ impl FlycheckThread { } } - fn handle_message(&self, msg: CheckEvent, task_send: &Sender) { + fn handle_message(&mut self, msg: CheckEvent, task_send: &Sender) { match msg { CheckEvent::Begin => { - task_send.send(CheckTask::Status(Status::Being)).unwrap(); + self.progress = Some(self.progress_src.begin(())); } - - CheckEvent::End => { - task_send.send(CheckTask::Status(Status::End)).unwrap(); - } - + CheckEvent::End => self.progress = None, CheckEvent::Msg(Message::CompilerArtifact(msg)) => { - task_send.send(CheckTask::Status(Status::Progress(msg.target.name))).unwrap(); + self.progress + .as_mut() + .expect("check process reported progress without the 'Begin' notification") + .report(msg.target.name); } CheckEvent::Msg(Message::CompilerMessage(msg)) => { -- cgit v1.2.3 From 874a5f80c74851aa142a196be49b73f55bd1c619 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Thu, 25 Jun 2020 08:01:03 +0200 Subject: Scale progress down There are two reasons why we don't want a generic ra_progress crate just yet: *First*, it introduces a common interface between separate components, and that is usually undesirable (b/c components start to fit the interface, rather than doing what makes most sense for each particular component). *Second*, it introduces a separate async channel for progress, which makes it harder to correlate progress reports with the work done. Ie, when we see 100% progress, it's not blindly obvious that the work has actually finished, we might have some pending messages still. --- crates/ra_flycheck/Cargo.toml | 1 - crates/ra_flycheck/src/lib.rs | 50 ++++++++++++++++++++----------------------- 2 files changed, 23 insertions(+), 28 deletions(-) (limited to 'crates/ra_flycheck') diff --git a/crates/ra_flycheck/Cargo.toml b/crates/ra_flycheck/Cargo.toml index 838973963..1aa39bade 100644 --- a/crates/ra_flycheck/Cargo.toml +++ b/crates/ra_flycheck/Cargo.toml @@ -14,4 +14,3 @@ cargo_metadata = "0.10.0" serde_json = "1.0.48" jod-thread = "0.1.1" ra_toolchain = { path = "../ra_toolchain" } -ra_progress = { path = "../ra_progress" } diff --git a/crates/ra_flycheck/src/lib.rs b/crates/ra_flycheck/src/lib.rs index 7b9f48eb0..0e2ee8698 100644 --- a/crates/ra_flycheck/src/lib.rs +++ b/crates/ra_flycheck/src/lib.rs @@ -17,9 +17,6 @@ pub use cargo_metadata::diagnostic::{ Applicability, Diagnostic, DiagnosticLevel, DiagnosticSpan, DiagnosticSpanMacroExpansion, }; -type Progress = ra_progress::Progress<(), String>; -type ProgressSource = ra_progress::ProgressSource<(), String>; - #[derive(Clone, Debug, PartialEq, Eq)] pub enum FlycheckConfig { CargoCommand { @@ -59,15 +56,11 @@ pub struct Flycheck { } impl Flycheck { - pub fn new( - config: FlycheckConfig, - workspace_root: PathBuf, - progress_src: ProgressSource, - ) -> Flycheck { + pub fn new(config: FlycheckConfig, workspace_root: PathBuf) -> Flycheck { let (task_send, task_recv) = unbounded::(); let (cmd_send, cmd_recv) = unbounded::(); let handle = jod_thread::spawn(move || { - FlycheckThread::new(config, workspace_root, progress_src).run(&task_send, &cmd_recv); + FlycheckThread::new(config, workspace_root).run(&task_send, &cmd_recv); }); Flycheck { task_recv, cmd_send, handle } } @@ -85,6 +78,16 @@ pub enum CheckTask { /// Request adding a diagnostic with fixes included to a file AddDiagnostic { workspace_root: PathBuf, diagnostic: Diagnostic }, + + /// Request check progress notification to client + Status(Status), +} + +#[derive(Debug)] +pub enum Status { + Being, + Progress(String), + End, } pub enum CheckCommand { @@ -96,8 +99,6 @@ struct FlycheckThread { config: FlycheckConfig, workspace_root: PathBuf, last_update_req: Option, - progress_src: ProgressSource, - progress: Option, // XXX: drop order is significant message_recv: Receiver, /// WatchThread exists to wrap around the communication needed to be able to @@ -109,17 +110,11 @@ struct FlycheckThread { } impl FlycheckThread { - fn new( - config: FlycheckConfig, - workspace_root: PathBuf, - progress_src: ProgressSource, - ) -> FlycheckThread { + fn new(config: FlycheckConfig, workspace_root: PathBuf) -> FlycheckThread { FlycheckThread { config, workspace_root, - progress_src, last_update_req: None, - progress: None, message_recv: never(), check_process: None, } @@ -157,9 +152,9 @@ impl FlycheckThread { } } - fn clean_previous_results(&mut self, task_send: &Sender) { + fn clean_previous_results(&self, task_send: &Sender) { task_send.send(CheckTask::ClearDiagnostics).unwrap(); - self.progress = None; + task_send.send(CheckTask::Status(Status::End)).unwrap(); } fn should_recheck(&mut self) -> bool { @@ -178,17 +173,18 @@ impl FlycheckThread { } } - fn handle_message(&mut self, msg: CheckEvent, task_send: &Sender) { + fn handle_message(&self, msg: CheckEvent, task_send: &Sender) { match msg { CheckEvent::Begin => { - self.progress = Some(self.progress_src.begin(())); + task_send.send(CheckTask::Status(Status::Being)).unwrap(); + } + + CheckEvent::End => { + task_send.send(CheckTask::Status(Status::End)).unwrap(); } - CheckEvent::End => self.progress = None, + CheckEvent::Msg(Message::CompilerArtifact(msg)) => { - self.progress - .as_mut() - .expect("check process reported progress without the 'Begin' notification") - .report(msg.target.name); + task_send.send(CheckTask::Status(Status::Progress(msg.target.name))).unwrap(); } CheckEvent::Msg(Message::CompilerMessage(msg)) => { -- cgit v1.2.3