From 163d262de280bb22e0bfd5b4791a02a88d7889db Mon Sep 17 00:00:00 2001 From: PiergiorgioZagaria Date: Sat, 2 Jul 2022 16:57:26 +0200 Subject: [PATCH 01/20] Change default formatter for any language --- helix-core/src/syntax.rs | 19 +++++++ helix-term/src/commands/typed.rs | 16 +++--- helix-view/src/document.rs | 90 ++++++++++++++++++++++++++++++-- 3 files changed, 116 insertions(+), 9 deletions(-) diff --git a/helix-core/src/syntax.rs b/helix-core/src/syntax.rs index 8d7520c3d131..10ff320c563d 100644 --- a/helix-core/src/syntax.rs +++ b/helix-core/src/syntax.rs @@ -79,6 +79,9 @@ pub struct LanguageConfiguration { #[serde(default)] pub auto_format: bool, + #[serde(skip_serializing_if = "Option::is_none")] + pub formatter: Option, + #[serde(default)] pub diagnostic_severity: Severity, @@ -126,6 +129,22 @@ pub struct LanguageServerConfiguration { pub language_id: Option, } +#[derive(Debug, Serialize, Deserialize)] +#[serde(rename_all = "kebab-case")] +pub struct FormatterConfiguration { + pub command: String, + #[serde(default)] + #[serde(skip_serializing_if = "Vec::is_empty")] + pub args: Vec, + pub formatter_type: FormatterType, +} + +#[derive(Debug, Clone, Copy, Eq, PartialEq, PartialOrd, Ord, Deserialize, Serialize)] +pub enum FormatterType { + Stdio, + File, +} + #[derive(Debug, PartialEq, Clone, Deserialize, Serialize)] #[serde(rename_all = "kebab-case")] pub struct AdvancedCompletion { diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index 70f5fa9fed82..9991cef571d5 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -203,7 +203,7 @@ fn write_impl( ) -> anyhow::Result<()> { let auto_format = cx.editor.config().auto_format; let jobs = &mut cx.jobs; - let doc = doc_mut!(cx.editor); + let (view, doc) = current!(cx.editor); if let Some(ref path) = path { doc.set_path(Some(path.as_ref().as_ref())) @@ -213,7 +213,7 @@ fn write_impl( bail!("cannot write a buffer without a filename"); } let fmt = if auto_format { - doc.auto_format().map(|fmt| { + doc.auto_format(view.id).map(|fmt| { let shared = fmt.shared(); let callback = make_format_callback( doc.id(), @@ -270,8 +270,8 @@ fn format( _args: &[Cow], _event: PromptEvent, ) -> anyhow::Result<()> { - let doc = doc!(cx.editor); - if let Some(format) = doc.format() { + let (view, doc) = current!(cx.editor); + if let Some(format) = doc.format(view.id) { let callback = make_format_callback(doc.id(), doc.version(), Modified::LeaveModified, format); cx.jobs.callback(callback); @@ -471,7 +471,11 @@ fn write_all_impl( 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() { + // Saves only visible buffers? How do we reload the not visible ones? + for (view, _) in cx.editor.tree.views() { + let id = view.doc; + let doc = cx.editor.documents.get_mut(&id).unwrap(); + if doc.path().is_none() { errors.push_str("cannot write a buffer without a filename\n"); continue; @@ -482,7 +486,7 @@ fn write_all_impl( } let fmt = if auto_format { - doc.auto_format().map(|fmt| { + doc.auto_format(view.id).map(|fmt| { let shared = fmt.shared(); let callback = make_format_callback( doc.id(), diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 00adaa1a71e7..f2c9d7608450 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -1,5 +1,6 @@ use anyhow::{anyhow, bail, Context, Error}; use helix_core::auto_pairs::AutoPairs; +use helix_core::syntax::FormatterType; use helix_core::Range; use serde::de::{self, Deserialize, Deserializer}; use serde::Serialize; @@ -397,9 +398,12 @@ impl Document { /// The same as [`format`], but only returns formatting changes if auto-formatting /// is configured. - pub fn auto_format(&self) -> Option + 'static> { + pub fn auto_format( + &mut self, + view_id: ViewId, + ) -> Option + 'static> { if self.language_config()?.auto_format { - self.format() + self.format(view_id) } else { None } @@ -407,7 +411,87 @@ impl Document { /// If supported, returns the changes that should be applied to this document in order /// to format it nicely. - pub fn format(&self) -> Option + 'static> { + pub fn format( + &mut self, + view_id: ViewId, + ) -> Option + 'static> { + if let Some(language_config) = self.language_config() { + if let Some(formatter) = &language_config.formatter { + use std::process::{Command, Stdio}; + match formatter.formatter_type { + FormatterType::Stdio => { + use std::io::Write; + let mut process = match Command::new(&formatter.command) + .args(&formatter.args) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + { + Ok(process) => process, + Err(e) => { + log::error!("Failed to format with {}. {}", formatter.command, e); + return None; + } + }; + + let mut stdin = process.stdin.take().unwrap(); + stdin + .write_all(&self.text().bytes().collect::>()) + .unwrap(); + let output = process.wait_with_output().ok()?; + + if !output.stderr.is_empty() { + log::error!("Formatter {}", String::from_utf8_lossy(&output.stderr)); + } + + let str = std::str::from_utf8(&output.stdout) + .map_err(|_| anyhow!("Process did not output valid UTF-8")) + .ok()?; + self.apply( + &helix_core::diff::compare_ropes(&self.text(), &Rope::from_str(str)), + view_id, + ); + } + FormatterType::File => { + if let Some(path) = self.path() { + let process = match Command::new(&formatter.command) + .args(&formatter.args) + .arg(path.to_str().unwrap_or("")) + .stderr(Stdio::piped()) + .spawn() + { + Ok(process) => process, + Err(e) => { + log::error!( + "Failed to format with {}. {}", + formatter.command, + e + ); + return None; + } + }; + + let output = process.wait_with_output().ok()?; + + if !output.stderr.is_empty() { + log::error!( + "Formatter {}", + String::from_utf8_lossy(&output.stderr) + ); + } else { + if let Err(e) = self.reload(view_id) { + log::error!("Error reloading after formatting {}", e); + } + } + } else { + log::error!("Cannot format file without filepath"); + } + } + } + return None; + } + } let language_server = self.language_server()?; let text = self.text.clone(); let offset_encoding = language_server.offset_encoding(); From 9965cd4f928547080bb984c78f3a2e47ab6b55bd Mon Sep 17 00:00:00 2001 From: PiergiorgioZagaria Date: Sat, 2 Jul 2022 17:26:17 +0200 Subject: [PATCH 02/20] Fix clippy error --- helix-view/src/document.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index f2c9d7608450..ea451804dbb1 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -449,7 +449,7 @@ impl Document { .map_err(|_| anyhow!("Process did not output valid UTF-8")) .ok()?; self.apply( - &helix_core::diff::compare_ropes(&self.text(), &Rope::from_str(str)), + &helix_core::diff::compare_ropes(self.text(), &Rope::from_str(str)), view_id, ); } @@ -479,10 +479,8 @@ impl Document { "Formatter {}", String::from_utf8_lossy(&output.stderr) ); - } else { - if let Err(e) = self.reload(view_id) { - log::error!("Error reloading after formatting {}", e); - } + } else if let Err(e) = self.reload(view_id) { + log::error!("Error reloading after formatting {}", e); } } else { log::error!("Cannot format file without filepath"); From 1af38a0c8640271dc780da208e7826b2624e6889 Mon Sep 17 00:00:00 2001 From: PiergiorgioZagaria Date: Sat, 2 Jul 2022 18:51:43 +0200 Subject: [PATCH 03/20] Close stdin for Stdio formatters --- helix-view/src/document.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index ea451804dbb1..662d289713b6 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -435,10 +435,15 @@ impl Document { } }; - let mut stdin = process.stdin.take().unwrap(); - stdin - .write_all(&self.text().bytes().collect::>()) - .unwrap(); + { + let mut stdin = process.stdin.take().unwrap(); + // let mut text = self.text().bytes().collect::>(); + // text.push(0); + stdin + .write_all(&self.text().bytes().collect::>()) + .unwrap(); + } + let output = process.wait_with_output().ok()?; if !output.stderr.is_empty() { From 29e24b6eed6263e93caeea33447996ac122e5a3c Mon Sep 17 00:00:00 2001 From: PiergiorgioZagaria Date: Sat, 2 Jul 2022 21:35:52 +0200 Subject: [PATCH 04/20] Better indentation and pattern matching --- helix-core/src/syntax.rs | 2 +- helix-view/src/document.rs | 132 +++++++++++++++++-------------------- 2 files changed, 62 insertions(+), 72 deletions(-) diff --git a/helix-core/src/syntax.rs b/helix-core/src/syntax.rs index 10ff320c563d..fff1f59edff3 100644 --- a/helix-core/src/syntax.rs +++ b/helix-core/src/syntax.rs @@ -136,7 +136,7 @@ pub struct FormatterConfiguration { #[serde(default)] #[serde(skip_serializing_if = "Vec::is_empty")] pub args: Vec, - pub formatter_type: FormatterType, + pub r#type: FormatterType, } #[derive(Debug, Clone, Copy, Eq, PartialEq, PartialOrd, Ord, Deserialize, Serialize)] diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 662d289713b6..c4c82f018b93 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -415,86 +415,76 @@ impl Document { &mut self, view_id: ViewId, ) -> Option + 'static> { - if let Some(language_config) = self.language_config() { - if let Some(formatter) = &language_config.formatter { - use std::process::{Command, Stdio}; - match formatter.formatter_type { - FormatterType::Stdio => { - use std::io::Write; - let mut process = match Command::new(&formatter.command) - .args(&formatter.args) - .stdin(Stdio::piped()) - .stdout(Stdio::piped()) - .stderr(Stdio::piped()) - .spawn() - { - Ok(process) => process, - Err(e) => { - log::error!("Failed to format with {}. {}", formatter.command, e); - return None; - } - }; - - { - let mut stdin = process.stdin.take().unwrap(); - // let mut text = self.text().bytes().collect::>(); - // text.push(0); - stdin - .write_all(&self.text().bytes().collect::>()) - .unwrap(); - } + if let Some(formatter) = self.language_config().and_then(|c| c.formatter.as_ref()) { + use std::process::{Command, Stdio}; + match formatter.r#type { + FormatterType::Stdio => { + use std::io::Write; + let mut process = Command::new(&formatter.command) + .args(&formatter.args) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .map_err(|e| { + log::error!("Failed to format with {}. {}", formatter.command, e); + e + }) + .ok()?; + + { + let mut stdin = process.stdin.take().unwrap(); + stdin + .write_all(&self.text().bytes().collect::>()) + .unwrap(); + } - let output = process.wait_with_output().ok()?; + let output = process.wait_with_output().ok()?; - if !output.stderr.is_empty() { - log::error!("Formatter {}", String::from_utf8_lossy(&output.stderr)); - } - - let str = std::str::from_utf8(&output.stdout) - .map_err(|_| anyhow!("Process did not output valid UTF-8")) - .ok()?; - self.apply( - &helix_core::diff::compare_ropes(self.text(), &Rope::from_str(str)), - view_id, - ); + if !output.stderr.is_empty() { + log::error!("Formatter {}", String::from_utf8_lossy(&output.stderr)); } - FormatterType::File => { - if let Some(path) = self.path() { - let process = match Command::new(&formatter.command) - .args(&formatter.args) - .arg(path.to_str().unwrap_or("")) - .stderr(Stdio::piped()) - .spawn() - { - Ok(process) => process, - Err(e) => { - log::error!( - "Failed to format with {}. {}", - formatter.command, - e - ); - return None; - } - }; - - let output = process.wait_with_output().ok()?; - - if !output.stderr.is_empty() { - log::error!( - "Formatter {}", - String::from_utf8_lossy(&output.stderr) - ); - } else if let Err(e) = self.reload(view_id) { - log::error!("Error reloading after formatting {}", e); - } - } else { + + let str = std::str::from_utf8(&output.stdout) + .map_err(|_| log::error!("Process did not output valid UTF-8")) + .ok()?; + + self.apply( + &helix_core::diff::compare_ropes(self.text(), &Rope::from_str(str)), + view_id, + ); + } + FormatterType::File => { + let path = match self.path() { + Some(path) => path, + None => { log::error!("Cannot format file without filepath"); + return None; } + }; + let process = Command::new(&formatter.command) + .args(&formatter.args) + .arg(path.to_str().unwrap_or("")) + .stderr(Stdio::piped()) + .spawn() + .map_err(|e| { + log::error!("Failed to format with {}. {}", formatter.command, e); + e + }) + .ok()?; + + let output = process.wait_with_output().ok()?; + + if !output.stderr.is_empty() { + log::error!("Formatter {}", String::from_utf8_lossy(&output.stderr)); + } else if let Err(e) = self.reload(view_id) { + log::error!("Error reloading after formatting {}", e); } } - return None; } + return None; } + let language_server = self.language_server()?; let text = self.text.clone(); let offset_encoding = language_server.offset_encoding(); From 124dd5d2b69311605d68befa685698ce4cadc634 Mon Sep 17 00:00:00 2001 From: PiergiorgioZagaria Date: Sat, 2 Jul 2022 23:20:53 +0200 Subject: [PATCH 05/20] Return Result> for fn format instead of Option --- helix-core/src/syntax.rs | 3 +- helix-term/src/commands/typed.rs | 6 ++-- helix-view/src/document.rs | 59 +++++++++++++++++--------------- 3 files changed, 37 insertions(+), 31 deletions(-) diff --git a/helix-core/src/syntax.rs b/helix-core/src/syntax.rs index fff1f59edff3..6c391e66ec5e 100644 --- a/helix-core/src/syntax.rs +++ b/helix-core/src/syntax.rs @@ -136,7 +136,8 @@ pub struct FormatterConfiguration { #[serde(default)] #[serde(skip_serializing_if = "Vec::is_empty")] pub args: Vec, - pub r#type: FormatterType, + #[serde(rename = "type")] + pub type_: FormatterType, } #[derive(Debug, Clone, Copy, Eq, PartialEq, PartialOrd, Ord, Deserialize, Serialize)] diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index 9991cef571d5..7828f7d56ae1 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -213,7 +213,7 @@ fn write_impl( bail!("cannot write a buffer without a filename"); } let fmt = if auto_format { - doc.auto_format(view.id).map(|fmt| { + doc.auto_format(view.id)?.map(|fmt| { let shared = fmt.shared(); let callback = make_format_callback( doc.id(), @@ -271,7 +271,7 @@ fn format( _event: PromptEvent, ) -> anyhow::Result<()> { let (view, doc) = current!(cx.editor); - if let Some(format) = doc.format(view.id) { + if let Some(format) = doc.format(view.id)? { let callback = make_format_callback(doc.id(), doc.version(), Modified::LeaveModified, format); cx.jobs.callback(callback); @@ -486,7 +486,7 @@ fn write_all_impl( } let fmt = if auto_format { - doc.auto_format(view.id).map(|fmt| { + doc.auto_format(view.id)?.map(|fmt| { let shared = fmt.shared(); let callback = make_format_callback( doc.id(), diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index c4c82f018b93..d4b40c0bf977 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -401,11 +401,16 @@ impl Document { pub fn auto_format( &mut self, view_id: ViewId, - ) -> Option + 'static> { - if self.language_config()?.auto_format { + ) -> anyhow::Result + 'static>> { + let config = match self.language_config() { + Some(config) => config, + None => return Ok(None), + }; + + if config.auto_format { self.format(view_id) } else { - None + Ok(None) } } @@ -414,10 +419,10 @@ impl Document { pub fn format( &mut self, view_id: ViewId, - ) -> Option + 'static> { + ) -> anyhow::Result + 'static>> { if let Some(formatter) = self.language_config().and_then(|c| c.formatter.as_ref()) { use std::process::{Command, Stdio}; - match formatter.r#type { + match formatter.type_ { FormatterType::Stdio => { use std::io::Write; let mut process = Command::new(&formatter.command) @@ -427,10 +432,8 @@ impl Document { .stderr(Stdio::piped()) .spawn() .map_err(|e| { - log::error!("Failed to format with {}. {}", formatter.command, e); - e - }) - .ok()?; + anyhow!("Failed to format with {}. {}", formatter.command, e) + })?; { let mut stdin = process.stdin.take().unwrap(); @@ -439,15 +442,14 @@ impl Document { .unwrap(); } - let output = process.wait_with_output().ok()?; + let output = process.wait_with_output()?; if !output.stderr.is_empty() { - log::error!("Formatter {}", String::from_utf8_lossy(&output.stderr)); + bail!("Formatter {}", String::from_utf8_lossy(&output.stderr)); } let str = std::str::from_utf8(&output.stdout) - .map_err(|_| log::error!("Process did not output valid UTF-8")) - .ok()?; + .map_err(|_| anyhow!("Process did not output valid UTF-8"))?; self.apply( &helix_core::diff::compare_ropes(self.text(), &Rope::from_str(str)), @@ -458,8 +460,7 @@ impl Document { let path = match self.path() { Some(path) => path, None => { - log::error!("Cannot format file without filepath"); - return None; + bail!("Cannot format file without filepath"); } }; let process = Command::new(&formatter.command) @@ -468,28 +469,29 @@ impl Document { .stderr(Stdio::piped()) .spawn() .map_err(|e| { - log::error!("Failed to format with {}. {}", formatter.command, e); - e - }) - .ok()?; + anyhow!("Failed to format with {}. {}", formatter.command, e) + })?; - let output = process.wait_with_output().ok()?; + let output = process.wait_with_output()?; if !output.stderr.is_empty() { - log::error!("Formatter {}", String::from_utf8_lossy(&output.stderr)); + bail!("Formatter {}", String::from_utf8_lossy(&output.stderr)); } else if let Err(e) = self.reload(view_id) { - log::error!("Error reloading after formatting {}", e); + bail!("Error reloading after formatting {}", e); } } } - return None; + return Ok(None); } - let language_server = self.language_server()?; + let language_server = match self.language_server() { + Some(server) => server, + None => return Ok(None), + }; let text = self.text.clone(); let offset_encoding = language_server.offset_encoding(); - let request = language_server.text_document_formatting( + let request = match language_server.text_document_formatting( self.identifier(), lsp::FormattingOptions { tab_size: self.tab_width() as u32, @@ -497,7 +499,10 @@ impl Document { ..Default::default() }, None, - )?; + ) { + Some(request) => request, + None => return Ok(None), + }; let fut = async move { let edits = request.await.unwrap_or_else(|e| { @@ -510,7 +515,7 @@ impl Document { offset_encoding, } }; - Some(fut) + Ok(Some(fut)) } pub fn save(&mut self, force: bool) -> impl Future> { From b5dc579908054d37784594c8667fd0f075d92b85 Mon Sep 17 00:00:00 2001 From: PiergiorgioZagaria Date: Sun, 3 Jul 2022 00:36:02 +0200 Subject: [PATCH 06/20] Remove unwrap for stdin --- helix-view/src/document.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index d4b40c0bf977..17dcd068ad02 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -436,10 +436,11 @@ impl Document { })?; { - let mut stdin = process.stdin.take().unwrap(); - stdin - .write_all(&self.text().bytes().collect::>()) - .unwrap(); + let mut stdin = process + .stdin + .take() + .ok_or(anyhow!("Failed to take stdin"))?; + stdin.write_all(&self.text().bytes().collect::>())?; } let output = process.wait_with_output()?; From 2e707709d2912a1032fc2ad6c99c18887430ee8b Mon Sep 17 00:00:00 2001 From: PiergiorgioZagaria Date: Sun, 3 Jul 2022 07:57:50 +0200 Subject: [PATCH 07/20] Handle FormatterErrors instead of Result> --- helix-term/src/commands/typed.rs | 75 ++++++++++++++++++++------------ helix-view/src/document.rs | 35 +++++++++++---- 2 files changed, 74 insertions(+), 36 deletions(-) diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index 7828f7d56ae1..be4213fb8c1c 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -2,7 +2,10 @@ use std::ops::Deref; use super::*; -use helix_view::editor::{Action, ConfigEvent}; +use helix_view::{ + document::FormatterError, + editor::{Action, ConfigEvent}, +}; use ui::completers::{self, Completer}; #[derive(Clone)] @@ -213,17 +216,23 @@ fn write_impl( bail!("cannot write a buffer without a filename"); } let fmt = if auto_format { - doc.auto_format(view.id)?.map(|fmt| { - let shared = fmt.shared(); - let callback = make_format_callback( - doc.id(), - doc.version(), - Modified::SetUnmodified, - shared.clone(), - ); - jobs.callback(callback); - shared - }) + match doc.auto_format(view.id) { + Ok(fmt) => { + let shared = fmt.shared(); + let callback = make_format_callback( + doc.id(), + doc.version(), + Modified::SetUnmodified, + shared.clone(), + ); + jobs.callback(callback); + Some(shared) + } + Err(error) => match error.downcast_ref::() { + Some(_) => None, + None => return Err(error), + }, + } } else { None }; @@ -271,10 +280,16 @@ fn format( _event: PromptEvent, ) -> anyhow::Result<()> { let (view, doc) = current!(cx.editor); - if let Some(format) = doc.format(view.id)? { - let callback = - make_format_callback(doc.id(), doc.version(), Modified::LeaveModified, format); - cx.jobs.callback(callback); + match doc.format(view.id) { + Ok(format) => { + let callback = + make_format_callback(doc.id(), doc.version(), Modified::LeaveModified, format); + cx.jobs.callback(callback); + } + Err(error) => match error.downcast_ref::() { + Some(_) => {} + None => return Err(error), + }, } Ok(()) @@ -486,17 +501,23 @@ fn write_all_impl( } let fmt = if auto_format { - doc.auto_format(view.id)?.map(|fmt| { - let shared = fmt.shared(); - let callback = make_format_callback( - doc.id(), - doc.version(), - Modified::SetUnmodified, - shared.clone(), - ); - jobs.callback(callback); - shared - }) + match doc.auto_format(view.id) { + Ok(fmt) => { + let shared = fmt.shared(); + let callback = make_format_callback( + doc.id(), + doc.version(), + Modified::SetUnmodified, + shared.clone(), + ); + jobs.callback(callback); + Some(shared) + } + Err(error) => match error.downcast_ref::() { + Some(_) => None, + None => return Err(error), + }, + } } else { None }; diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 17dcd068ad02..f912ebd679be 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -401,16 +401,16 @@ impl Document { pub fn auto_format( &mut self, view_id: ViewId, - ) -> anyhow::Result + 'static>> { + ) -> anyhow::Result + 'static> { let config = match self.language_config() { Some(config) => config, - None => return Ok(None), + None => bail!(FormatterError::NoFormattingNeeded), }; if config.auto_format { self.format(view_id) } else { - Ok(None) + bail!(FormatterError::NoFormattingNeeded) } } @@ -419,7 +419,7 @@ impl Document { pub fn format( &mut self, view_id: ViewId, - ) -> anyhow::Result + 'static>> { + ) -> anyhow::Result + 'static> { if let Some(formatter) = self.language_config().and_then(|c| c.formatter.as_ref()) { use std::process::{Command, Stdio}; match formatter.type_ { @@ -439,7 +439,7 @@ impl Document { let mut stdin = process .stdin .take() - .ok_or(anyhow!("Failed to take stdin"))?; + .ok_or_else(|| anyhow!("Failed to take stdin"))?; stdin.write_all(&self.text().bytes().collect::>())?; } @@ -482,12 +482,12 @@ impl Document { } } } - return Ok(None); + bail!(FormatterError::CustomFormatterDefined); } let language_server = match self.language_server() { Some(server) => server, - None => return Ok(None), + None => bail!(FormatterError::NoFormatterDefined), }; let text = self.text.clone(); let offset_encoding = language_server.offset_encoding(); @@ -502,7 +502,7 @@ impl Document { None, ) { Some(request) => request, - None => return Ok(None), + None => bail!(FormatterError::NoFormattingNeeded), }; let fut = async move { @@ -516,7 +516,7 @@ impl Document { offset_encoding, } }; - Ok(Some(fut)) + Ok(fut) } pub fn save(&mut self, force: bool) -> impl Future> { @@ -1117,6 +1117,23 @@ impl Default for Document { } } +#[derive(Debug)] +pub enum FormatterError { + CustomFormatterDefined, + NoFormatterDefined, + NoFormattingNeeded, +} + +impl Display for FormatterError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(match *self { + FormatterError::CustomFormatterDefined => "Custom formatter defined", + FormatterError::NoFormatterDefined => "No lsp or custom formatter defined", + FormatterError::NoFormattingNeeded => "No formatting needed", + }) + } +} + #[cfg(test)] mod test { use super::*; From 06ad531d06f824ed7e17d65ca12eeec788656a86 Mon Sep 17 00:00:00 2001 From: PiergiorgioZagaria Date: Sun, 3 Jul 2022 22:25:17 +0200 Subject: [PATCH 08/20] Use Transaction instead of LspFormatting --- helix-lsp/src/lib.rs | 16 ---------------- helix-term/src/commands.rs | 2 +- helix-view/src/document.rs | 23 ++++++++++++----------- helix-view/src/editor.rs | 3 +-- 4 files changed, 14 insertions(+), 30 deletions(-) diff --git a/helix-lsp/src/lib.rs b/helix-lsp/src/lib.rs index 6a5f9d5ca4f7..e81a4db6ac26 100644 --- a/helix-lsp/src/lib.rs +++ b/helix-lsp/src/lib.rs @@ -205,22 +205,6 @@ pub mod util { }), ) } - - /// The result of asking the language server to format the document. This can be turned into a - /// `Transaction`, but the advantage of not doing that straight away is that this one is - /// `Send` and `Sync`. - #[derive(Clone, Debug)] - pub struct LspFormatting { - pub doc: Rope, - pub edits: Vec, - pub offset_encoding: OffsetEncoding, - } - - impl From for Transaction { - fn from(fmt: LspFormatting) -> Transaction { - generate_transaction_from_edits(&fmt.doc, fmt.edits, fmt.offset_encoding) - } - } } #[derive(Debug, PartialEq, Clone)] diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index df4867fc76c5..9bed66c6c2e4 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -2372,7 +2372,7 @@ async fn make_format_callback( doc_id: DocumentId, doc_version: i32, modified: Modified, - format: impl Future + Send + 'static, + format: impl Future + Send + 'static, ) -> anyhow::Result { let format = format.await; let call: job::Callback = Box::new(move |editor, _compositor| { diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index f912ebd679be..58f3855c530a 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -21,7 +21,6 @@ use helix_core::{ ChangeSet, Diagnostic, LineEnding, Rope, RopeBuilder, Selection, State, Syntax, Transaction, DEFAULT_LINE_ENDING, }; -use helix_lsp::util::LspFormatting; use crate::{DocumentId, Editor, ViewId}; @@ -401,7 +400,7 @@ impl Document { pub fn auto_format( &mut self, view_id: ViewId, - ) -> anyhow::Result + 'static> { + ) -> anyhow::Result + 'static> { let config = match self.language_config() { Some(config) => config, None => bail!(FormatterError::NoFormattingNeeded), @@ -419,7 +418,7 @@ impl Document { pub fn format( &mut self, view_id: ViewId, - ) -> anyhow::Result + 'static> { + ) -> anyhow::Result + 'static> { if let Some(formatter) = self.language_config().and_then(|c| c.formatter.as_ref()) { use std::process::{Command, Stdio}; match formatter.type_ { @@ -452,6 +451,11 @@ impl Document { let str = std::str::from_utf8(&output.stdout) .map_err(|_| anyhow!("Process did not output valid UTF-8"))?; + // Doesn't work, how to fix? + // let fut = async move { + // helix_core::diff::compare_ropes(self.text(), &Rope::from_str(str)) + // } + // return Ok(fut); self.apply( &helix_core::diff::compare_ropes(self.text(), &Rope::from_str(str)), view_id, @@ -510,11 +514,7 @@ impl Document { log::warn!("LSP formatting failed: {}", e); Default::default() }); - LspFormatting { - doc: text, - edits, - offset_encoding, - } + helix_lsp::util::generate_transaction_from_edits(&text, edits, offset_encoding) }; Ok(fut) } @@ -525,7 +525,7 @@ impl Document { pub fn format_and_save( &mut self, - formatting: Option>, + formatting: Option>, force: bool, ) -> impl Future> { self.save_impl(formatting, force) @@ -537,7 +537,7 @@ impl Document { /// at its `path()`. /// /// If `formatting` is present, it supplies some changes that we apply to the text before saving. - fn save_impl>( + fn save_impl>( &mut self, formatting: Option, force: bool, @@ -571,7 +571,8 @@ impl Document { } if let Some(fmt) = formatting { - let success = Transaction::from(fmt.await).changes().apply(&mut text); + let transaction = fmt.await; + 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. diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 1ed27e996eaf..00973aac273e 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -521,7 +521,6 @@ impl Editor { syn_loader: Arc, config: Box>, ) -> Self { - let language_servers = helix_lsp::Registry::new(); let conf = config.load(); let auto_pairs = (&conf.auto_pairs).into(); @@ -537,7 +536,7 @@ impl Editor { macro_recording: None, macro_replaying: Vec::new(), theme: theme_loader.default(), - language_servers, + language_servers: helix_lsp::Registry::new(), diagnostics: BTreeMap::new(), debugger: None, debugger_events: SelectAll::new(), From 7416d6fae9791c661c376d1519252b2f4c73b707 Mon Sep 17 00:00:00 2001 From: Gokul Soumya Date: Sun, 3 Jul 2022 21:55:06 +0530 Subject: [PATCH 09/20] Use Transaction directly in Document::format --- helix-term/src/commands.rs | 2 +- helix-view/src/document.rs | 30 +++++++++++++----------------- 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 9bed66c6c2e4..d14bc7628816 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -2379,7 +2379,7 @@ async fn make_format_callback( let view_id = view!(editor).id; if let Some(doc) = editor.document_mut(doc_id) { if doc.version() == doc_version { - doc.apply(&Transaction::from(format), view_id); + doc.apply(&format, view_id); doc.append_changes_to_history(view_id); doc.detect_indent_and_line_ending(); if let Modified::SetUnmodified = modified { diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 58f3855c530a..1fbbea636a06 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -1,4 +1,6 @@ use anyhow::{anyhow, bail, Context, Error}; +use futures_util::future::BoxFuture; +use futures_util::FutureExt; use helix_core::auto_pairs::AutoPairs; use helix_core::syntax::FormatterType; use helix_core::Range; @@ -400,7 +402,7 @@ impl Document { pub fn auto_format( &mut self, view_id: ViewId, - ) -> anyhow::Result + 'static> { + ) -> anyhow::Result> { let config = match self.language_config() { Some(config) => config, None => bail!(FormatterError::NoFormattingNeeded), @@ -415,10 +417,7 @@ impl Document { /// If supported, returns the changes that should be applied to this document in order /// to format it nicely. - pub fn format( - &mut self, - view_id: ViewId, - ) -> anyhow::Result + 'static> { + pub fn format(&mut self, view_id: ViewId) -> anyhow::Result> { if let Some(formatter) = self.language_config().and_then(|c| c.formatter.as_ref()) { use std::process::{Command, Stdio}; match formatter.type_ { @@ -448,18 +447,13 @@ impl Document { bail!("Formatter {}", String::from_utf8_lossy(&output.stderr)); } - let str = std::str::from_utf8(&output.stdout) + let str = String::from_utf8(output.stdout) .map_err(|_| anyhow!("Process did not output valid UTF-8"))?; - // Doesn't work, how to fix? - // let fut = async move { - // helix_core::diff::compare_ropes(self.text(), &Rope::from_str(str)) - // } - // return Ok(fut); - self.apply( - &helix_core::diff::compare_ropes(self.text(), &Rope::from_str(str)), - view_id, - ); + let text = self.text().clone(); + let fut = + async move { helix_core::diff::compare_ropes(&text, &Rope::from(str)) }; + return Ok(fut.boxed()); } FormatterType::File => { let path = match self.path() { @@ -484,9 +478,11 @@ impl Document { } else if let Err(e) = self.reload(view_id) { bail!("Error reloading after formatting {}", e); } + + let text = self.text().clone(); + return Ok(async move { Transaction::new(&text) }.boxed()); } } - bail!(FormatterError::CustomFormatterDefined); } let language_server = match self.language_server() { @@ -516,7 +512,7 @@ impl Document { }); helix_lsp::util::generate_transaction_from_edits(&text, edits, offset_encoding) }; - Ok(fut) + Ok(fut.boxed()) } pub fn save(&mut self, force: bool) -> impl Future> { From b819c21c29f5ff80e1d2fd6862cd5bd28cc867f0 Mon Sep 17 00:00:00 2001 From: Gokul Soumya Date: Mon, 4 Jul 2022 20:03:56 +0530 Subject: [PATCH 10/20] Perform stdin type formatting asynchronously --- helix-core/src/syntax.rs | 2 +- helix-term/src/commands.rs | 6 +- helix-term/src/commands/typed.rs | 75 +++++------- helix-view/src/document.rs | 190 ++++++++++++++++++------------- 4 files changed, 143 insertions(+), 130 deletions(-) diff --git a/helix-core/src/syntax.rs b/helix-core/src/syntax.rs index 6c391e66ec5e..017c5f293363 100644 --- a/helix-core/src/syntax.rs +++ b/helix-core/src/syntax.rs @@ -129,7 +129,7 @@ pub struct LanguageServerConfiguration { pub language_id: Option, } -#[derive(Debug, Serialize, Deserialize)] +#[derive(Debug, Clone, Serialize, Deserialize)] #[serde(rename_all = "kebab-case")] pub struct FormatterConfiguration { pub command: String, diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index d14bc7628816..23eb9b8e06a4 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -28,7 +28,7 @@ use helix_core::{ }; use helix_view::{ clipboard::ClipboardType, - document::{Mode, SCRATCH_BUFFER_NAME}, + document::{FormatterError, Mode, SCRATCH_BUFFER_NAME}, editor::{Action, Motion}, info::Info, input::KeyEvent, @@ -2372,9 +2372,9 @@ async fn make_format_callback( doc_id: DocumentId, doc_version: i32, modified: Modified, - format: impl Future + Send + 'static, + format: impl Future> + Send + 'static, ) -> anyhow::Result { - let format = format.await; + let format = format.await?; let call: job::Callback = Box::new(move |editor, _compositor| { let view_id = view!(editor).id; if let Some(doc) = editor.document_mut(doc_id) { diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index be4213fb8c1c..9991cef571d5 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -2,10 +2,7 @@ use std::ops::Deref; use super::*; -use helix_view::{ - document::FormatterError, - editor::{Action, ConfigEvent}, -}; +use helix_view::editor::{Action, ConfigEvent}; use ui::completers::{self, Completer}; #[derive(Clone)] @@ -216,23 +213,17 @@ fn write_impl( bail!("cannot write a buffer without a filename"); } let fmt = if auto_format { - match doc.auto_format(view.id) { - Ok(fmt) => { - let shared = fmt.shared(); - let callback = make_format_callback( - doc.id(), - doc.version(), - Modified::SetUnmodified, - shared.clone(), - ); - jobs.callback(callback); - Some(shared) - } - Err(error) => match error.downcast_ref::() { - Some(_) => None, - None => return Err(error), - }, - } + doc.auto_format(view.id).map(|fmt| { + let shared = fmt.shared(); + let callback = make_format_callback( + doc.id(), + doc.version(), + Modified::SetUnmodified, + shared.clone(), + ); + jobs.callback(callback); + shared + }) } else { None }; @@ -280,16 +271,10 @@ fn format( _event: PromptEvent, ) -> anyhow::Result<()> { let (view, doc) = current!(cx.editor); - match doc.format(view.id) { - Ok(format) => { - let callback = - make_format_callback(doc.id(), doc.version(), Modified::LeaveModified, format); - cx.jobs.callback(callback); - } - Err(error) => match error.downcast_ref::() { - Some(_) => {} - None => return Err(error), - }, + if let Some(format) = doc.format(view.id) { + let callback = + make_format_callback(doc.id(), doc.version(), Modified::LeaveModified, format); + cx.jobs.callback(callback); } Ok(()) @@ -501,23 +486,17 @@ fn write_all_impl( } let fmt = if auto_format { - match doc.auto_format(view.id) { - Ok(fmt) => { - let shared = fmt.shared(); - let callback = make_format_callback( - doc.id(), - doc.version(), - Modified::SetUnmodified, - shared.clone(), - ); - jobs.callback(callback); - Some(shared) - } - Err(error) => match error.downcast_ref::() { - Some(_) => None, - None => return Err(error), - }, - } + doc.auto_format(view.id).map(|fmt| { + let shared = fmt.shared(); + let callback = make_format_callback( + doc.id(), + doc.version(), + Modified::SetUnmodified, + shared.clone(), + ); + jobs.callback(callback); + shared + }) } else { None }; diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 1fbbea636a06..056ac41d5049 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -13,6 +13,7 @@ use std::future::Future; use std::path::{Path, PathBuf}; use std::str::FromStr; use std::sync::Arc; +use tokio::io::AsyncWriteExt; use helix_core::{ encoding, @@ -402,97 +403,115 @@ impl Document { pub fn auto_format( &mut self, view_id: ViewId, - ) -> anyhow::Result> { - let config = match self.language_config() { - Some(config) => config, - None => bail!(FormatterError::NoFormattingNeeded), - }; - - if config.auto_format { + ) -> Option>> { + if self.language_config()?.auto_format { self.format(view_id) } else { - bail!(FormatterError::NoFormattingNeeded) + None } } /// If supported, returns the changes that should be applied to this document in order /// to format it nicely. - pub fn format(&mut self, view_id: ViewId) -> anyhow::Result> { - if let Some(formatter) = self.language_config().and_then(|c| c.formatter.as_ref()) { - use std::process::{Command, Stdio}; + // We can't use anyhow::Result here since the output of the future has to be + // clonable to be used as shared future. So use a custom error type. + pub fn format( + &mut self, + view_id: ViewId, + ) -> Option>> { + if let Some(formatter) = self.language_config().and_then(|c| c.formatter.clone()) { + use std::process::Stdio; match formatter.type_ { FormatterType::Stdio => { - use std::io::Write; - let mut process = Command::new(&formatter.command) + let text = self.text().clone(); + let mut process = tokio::process::Command::new(&formatter.command); + process .args(&formatter.args) .stdin(Stdio::piped()) .stdout(Stdio::piped()) - .stderr(Stdio::piped()) - .spawn() - .map_err(|e| { - anyhow!("Failed to format with {}. {}", formatter.command, e) - })?; - - { - let mut stdin = process - .stdin - .take() - .ok_or_else(|| anyhow!("Failed to take stdin"))?; - stdin.write_all(&self.text().bytes().collect::>())?; - } + .stderr(Stdio::piped()); + + let formatting_future = async move { + let mut process = + process + .spawn() + .map_err(|e| FormatterError::SpawningFailed { + command: formatter.command.clone(), + error: e.kind(), + })?; + { + let mut stdin = + process.stdin.take().ok_or(FormatterError::BrokenStdin)?; + stdin + .write_all(&text.bytes().collect::>()) + .await + .map_err(|_| FormatterError::BrokenStdin)?; + } - let output = process.wait_with_output()?; + let output = process + .wait_with_output() + .await + .map_err(|_| FormatterError::WaitForOutputFailed)?; - if !output.stderr.is_empty() { - bail!("Formatter {}", String::from_utf8_lossy(&output.stderr)); - } + if !output.stderr.is_empty() { + return Err(FormatterError::Stderr( + String::from_utf8_lossy(&output.stderr).to_string(), + )); + } - let str = String::from_utf8(output.stdout) - .map_err(|_| anyhow!("Process did not output valid UTF-8"))?; + let str = String::from_utf8(output.stdout) + .map_err(|_| FormatterError::InvalidUtf8Output)?; - let text = self.text().clone(); - let fut = - async move { helix_core::diff::compare_ropes(&text, &Rope::from(str)) }; - return Ok(fut.boxed()); + Ok(helix_core::diff::compare_ropes(&text, &Rope::from(str))) + }; + return Some(formatting_future.boxed()); } FormatterType::File => { - let path = match self.path() { - Some(path) => path, - None => { - bail!("Cannot format file without filepath"); - } - }; - let process = Command::new(&formatter.command) + let path = self.path()?.clone(); + let process = std::process::Command::new(&formatter.command) .args(&formatter.args) .arg(path.to_str().unwrap_or("")) .stderr(Stdio::piped()) - .spawn() - .map_err(|e| { - anyhow!("Failed to format with {}. {}", formatter.command, e) + .spawn(); + + // File type formatters are run synchronously since the doc + // has to be reloaded from disk rather than being resolved + // through a transaction. + let format_and_reload = || -> Result<(), FormatterError> { + let process = process.map_err(|e| FormatterError::SpawningFailed { + command: formatter.command.clone(), + error: e.kind(), })?; + let output = process + .wait_with_output() + .map_err(|_| FormatterError::WaitForOutputFailed)?; + + if !output.stderr.is_empty() { + return Err(FormatterError::Stderr( + String::from_utf8_lossy(&output.stderr).to_string(), + )); + } else if let Err(e) = self.reload(view_id) { + return Err(FormatterError::DiskReloadError(e.to_string())); + } - let output = process.wait_with_output()?; - - if !output.stderr.is_empty() { - bail!("Formatter {}", String::from_utf8_lossy(&output.stderr)); - } else if let Err(e) = self.reload(view_id) { - bail!("Error reloading after formatting {}", e); - } + Ok(()) + }; + let format_result = format_and_reload(); let text = self.text().clone(); - return Ok(async move { Transaction::new(&text) }.boxed()); + + // Generate an empty transaction as a placeholder + let future = async move { format_result.map(|_| Transaction::new(&text)) }; + return Some(future.boxed()); } - } + }; } - let language_server = match self.language_server() { - Some(server) => server, - None => bail!(FormatterError::NoFormatterDefined), - }; + let language_server = self.language_server()?; let text = self.text.clone(); let offset_encoding = language_server.offset_encoding(); - let request = match language_server.text_document_formatting( + let request = language_server.text_document_formatting( self.identifier(), lsp::FormattingOptions { tab_size: self.tab_width() as u32, @@ -500,19 +519,20 @@ impl Document { ..Default::default() }, None, - ) { - Some(request) => request, - None => bail!(FormatterError::NoFormattingNeeded), - }; + )?; let fut = async move { let edits = request.await.unwrap_or_else(|e| { log::warn!("LSP formatting failed: {}", e); Default::default() }); - helix_lsp::util::generate_transaction_from_edits(&text, edits, offset_encoding) + Ok(helix_lsp::util::generate_transaction_from_edits( + &text, + edits, + offset_encoding, + )) }; - Ok(fut.boxed()) + Some(fut.boxed()) } pub fn save(&mut self, force: bool) -> impl Future> { @@ -521,7 +541,7 @@ impl Document { pub fn format_and_save( &mut self, - formatting: Option>, + formatting: Option>>, force: bool, ) -> impl Future> { self.save_impl(formatting, force) @@ -533,7 +553,7 @@ impl Document { /// at its `path()`. /// /// If `formatting` is present, it supplies some changes that we apply to the text before saving. - fn save_impl>( + fn save_impl>>( &mut self, formatting: Option, force: bool, @@ -567,7 +587,7 @@ impl Document { } if let Some(fmt) = formatting { - let transaction = fmt.await; + let transaction = fmt.await?; let success = transaction.changes().apply(&mut text); if !success { // This shouldn't happen, because the transaction changes were generated @@ -1114,20 +1134,34 @@ impl Default for Document { } } -#[derive(Debug)] +#[derive(Clone, Debug)] pub enum FormatterError { - CustomFormatterDefined, - NoFormatterDefined, - NoFormattingNeeded, + SpawningFailed { + command: String, + error: std::io::ErrorKind, + }, + BrokenStdin, + WaitForOutputFailed, + Stderr(String), + InvalidUtf8Output, + DiskReloadError(String), } +impl std::error::Error for FormatterError {} + impl Display for FormatterError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.write_str(match *self { - FormatterError::CustomFormatterDefined => "Custom formatter defined", - FormatterError::NoFormatterDefined => "No lsp or custom formatter defined", - FormatterError::NoFormattingNeeded => "No formatting needed", - }) + let s = match self { + Self::SpawningFailed { command, error } => { + format!("Failed to spawn formatter {}: {}", command, error) + } + Self::BrokenStdin => "Could not write to formatter stdin".to_string(), + Self::WaitForOutputFailed => "Waiting for formatter output failed".to_string(), + Self::Stderr(output) => format!("Formatter error: {}", output), + Self::InvalidUtf8Output => "Invalid UTF-8 formatter output".to_string(), + Self::DiskReloadError(error) => format!("Error reloading file from disk: {}", error), + }; + f.write_str(&s) } } From 48d69b2fda1e930d5e063b425e01639191ba939b Mon Sep 17 00:00:00 2001 From: Gokul Soumya Date: Mon, 4 Jul 2022 21:19:21 +0530 Subject: [PATCH 11/20] Rename formatter.type values to kebab-case --- helix-core/src/syntax.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/helix-core/src/syntax.rs b/helix-core/src/syntax.rs index 017c5f293363..56b7218e55e0 100644 --- a/helix-core/src/syntax.rs +++ b/helix-core/src/syntax.rs @@ -141,6 +141,7 @@ pub struct FormatterConfiguration { } #[derive(Debug, Clone, Copy, Eq, PartialEq, PartialOrd, Ord, Deserialize, Serialize)] +#[serde(rename_all = "kebab-case")] pub enum FormatterType { Stdio, File, From 15685fe8d95cdb80d93ebfb9b986b575fd231dce Mon Sep 17 00:00:00 2001 From: Gokul Soumya Date: Mon, 4 Jul 2022 21:59:12 +0530 Subject: [PATCH 12/20] Debug format for displaying io::ErrorKind (msrv fix) --- helix-view/src/document.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 056ac41d5049..c7c67deafefe 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -1153,7 +1153,7 @@ impl Display for FormatterError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { let s = match self { Self::SpawningFailed { command, error } => { - format!("Failed to spawn formatter {}: {}", command, error) + format!("Failed to spawn formatter {}: {:?}", command, error) } Self::BrokenStdin => "Could not write to formatter stdin".to_string(), Self::WaitForOutputFailed => "Waiting for formatter output failed".to_string(), From 57bb4fcb4950abf2a6304ca2a8745fb30a3d5584 Mon Sep 17 00:00:00 2001 From: PiergiorgioZagaria Date: Tue, 5 Jul 2022 14:34:24 +0200 Subject: [PATCH 13/20] Solve conflict? --- helix-term/src/commands/typed.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index 9991cef571d5..23c5a52e5120 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -268,8 +268,11 @@ fn new_file( fn format( cx: &mut compositor::Context, _args: &[Cow], - _event: PromptEvent, + event: PromptEvent, ) -> anyhow::Result<()> { + if event != PromptEvent::Validate { + return Ok(()); + } let (view, doc) = current!(cx.editor); if let Some(format) = doc.format(view.id) { let callback = From f7ecf9c830e1c92a59bb13a11b08a1bb5a7da39e Mon Sep 17 00:00:00 2001 From: PiergiorgioZagaria Date: Fri, 8 Jul 2022 10:44:52 +0200 Subject: [PATCH 14/20] Use only stdio type formatters --- helix-core/src/syntax.rs | 2 - helix-term/src/commands/typed.rs | 15 ++-- helix-view/src/document.rs | 133 ++++++++++--------------------- languages.toml | 2 + 4 files changed, 49 insertions(+), 103 deletions(-) diff --git a/helix-core/src/syntax.rs b/helix-core/src/syntax.rs index 56b7218e55e0..a890182bf930 100644 --- a/helix-core/src/syntax.rs +++ b/helix-core/src/syntax.rs @@ -136,8 +136,6 @@ pub struct FormatterConfiguration { #[serde(default)] #[serde(skip_serializing_if = "Vec::is_empty")] pub args: Vec, - #[serde(rename = "type")] - pub type_: FormatterType, } #[derive(Debug, Clone, Copy, Eq, PartialEq, PartialOrd, Ord, Deserialize, Serialize)] diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index 2da06340a4f6..66b1d678bde7 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -239,7 +239,7 @@ fn write_impl( ) -> anyhow::Result<()> { let auto_format = cx.editor.config().auto_format; let jobs = &mut cx.jobs; - let (view, doc) = current!(cx.editor); + let doc = doc_mut!(cx.editor); if let Some(ref path) = path { doc.set_path(Some(path.as_ref().as_ref())) @@ -249,7 +249,7 @@ fn write_impl( bail!("cannot write a buffer without a filename"); } let fmt = if auto_format { - doc.auto_format(view.id).map(|fmt| { + doc.auto_format().map(|fmt| { let shared = fmt.shared(); let callback = make_format_callback( doc.id(), @@ -322,8 +322,8 @@ fn format( return Ok(()); } - let (view, doc) = current!(cx.editor); - if let Some(format) = doc.format(view.id) { + let doc = doc!(cx.editor); + if let Some(format) = doc.format() { let callback = make_format_callback(doc.id(), doc.version(), Modified::LeaveModified, format); cx.jobs.callback(callback); @@ -552,10 +552,7 @@ fn write_all_impl( let jobs = &mut cx.jobs; // save all documents // Saves only visible buffers? How do we reload the not visible ones? - for (view, _) in cx.editor.tree.views() { - let id = view.doc; - let doc = cx.editor.documents.get_mut(&id).unwrap(); - + 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; @@ -566,7 +563,7 @@ fn write_all_impl( } let fmt = if auto_format { - doc.auto_format(view.id).map(|fmt| { + doc.auto_format().map(|fmt| { let shared = fmt.shared(); let callback = make_format_callback( doc.id(), diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index c7c67deafefe..198474f00765 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -2,7 +2,6 @@ use anyhow::{anyhow, bail, Context, Error}; use futures_util::future::BoxFuture; use futures_util::FutureExt; use helix_core::auto_pairs::AutoPairs; -use helix_core::syntax::FormatterType; use helix_core::Range; use serde::de::{self, Deserialize, Deserializer}; use serde::Serialize; @@ -400,12 +399,9 @@ impl Document { /// The same as [`format`], but only returns formatting changes if auto-formatting /// is configured. - pub fn auto_format( - &mut self, - view_id: ViewId, - ) -> Option>> { + pub fn auto_format(&self) -> Option>> { if self.language_config()?.auto_format { - self.format(view_id) + self.format() } else { None } @@ -415,97 +411,50 @@ impl Document { /// to format it nicely. // We can't use anyhow::Result here since the output of the future has to be // clonable to be used as shared future. So use a custom error type. - pub fn format( - &mut self, - view_id: ViewId, - ) -> Option>> { + pub fn format(&self) -> Option>> { if let Some(formatter) = self.language_config().and_then(|c| c.formatter.clone()) { use std::process::Stdio; - match formatter.type_ { - FormatterType::Stdio => { - let text = self.text().clone(); - let mut process = tokio::process::Command::new(&formatter.command); - process - .args(&formatter.args) - .stdin(Stdio::piped()) - .stdout(Stdio::piped()) - .stderr(Stdio::piped()); - - let formatting_future = async move { - let mut process = - process - .spawn() - .map_err(|e| FormatterError::SpawningFailed { - command: formatter.command.clone(), - error: e.kind(), - })?; - { - let mut stdin = - process.stdin.take().ok_or(FormatterError::BrokenStdin)?; - stdin - .write_all(&text.bytes().collect::>()) - .await - .map_err(|_| FormatterError::BrokenStdin)?; - } - - let output = process - .wait_with_output() - .await - .map_err(|_| FormatterError::WaitForOutputFailed)?; - - if !output.stderr.is_empty() { - return Err(FormatterError::Stderr( - String::from_utf8_lossy(&output.stderr).to_string(), - )); - } - - let str = String::from_utf8(output.stdout) - .map_err(|_| FormatterError::InvalidUtf8Output)?; - - Ok(helix_core::diff::compare_ropes(&text, &Rope::from(str))) - }; - return Some(formatting_future.boxed()); + let text = self.text().clone(); + let mut process = tokio::process::Command::new(&formatter.command); + process + .args(&formatter.args) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + + let formatting_future = async move { + let mut process = process + .spawn() + .map_err(|e| FormatterError::SpawningFailed { + command: formatter.command.clone(), + error: e.kind(), + })?; + { + let mut stdin = process.stdin.take().ok_or(FormatterError::BrokenStdin)?; + stdin + .write_all(&text.bytes().collect::>()) + .await + .map_err(|_| FormatterError::BrokenStdin)?; } - FormatterType::File => { - let path = self.path()?.clone(); - let process = std::process::Command::new(&formatter.command) - .args(&formatter.args) - .arg(path.to_str().unwrap_or("")) - .stderr(Stdio::piped()) - .spawn(); - - // File type formatters are run synchronously since the doc - // has to be reloaded from disk rather than being resolved - // through a transaction. - let format_and_reload = || -> Result<(), FormatterError> { - let process = process.map_err(|e| FormatterError::SpawningFailed { - command: formatter.command.clone(), - error: e.kind(), - })?; - let output = process - .wait_with_output() - .map_err(|_| FormatterError::WaitForOutputFailed)?; - - if !output.stderr.is_empty() { - return Err(FormatterError::Stderr( - String::from_utf8_lossy(&output.stderr).to_string(), - )); - } else if let Err(e) = self.reload(view_id) { - return Err(FormatterError::DiskReloadError(e.to_string())); - } - - Ok(()) - }; - - let format_result = format_and_reload(); - let text = self.text().clone(); - - // Generate an empty transaction as a placeholder - let future = async move { format_result.map(|_| Transaction::new(&text)) }; - return Some(future.boxed()); + + let output = process + .wait_with_output() + .await + .map_err(|_| FormatterError::WaitForOutputFailed)?; + + if !output.stderr.is_empty() { + return Err(FormatterError::Stderr( + String::from_utf8_lossy(&output.stderr).to_string(), + )); } + + let str = String::from_utf8(output.stdout) + .map_err(|_| FormatterError::InvalidUtf8Output)?; + + Ok(helix_core::diff::compare_ropes(&text, &Rope::from(str))) }; - } + return Some(formatting_future.boxed()); + }; let language_server = self.language_server()?; let text = self.text.clone(); diff --git a/languages.toml b/languages.toml index d7462bb8c583..faa02bcd52f9 100644 --- a/languages.toml +++ b/languages.toml @@ -679,6 +679,8 @@ auto-format = true comment-token = "//" language-server = { command = "zls" } indent = { tab-width = 4, unit = " " } +formatter = { command = "zig" , args = ["fmt", "--stdin"] } + [[grammar]] name = "zig" From acdce30bd49405703fcbc0d1012beceb00fbdee7 Mon Sep 17 00:00:00 2001 From: PiergiorgioZagaria Date: Tue, 12 Jul 2022 07:52:32 +0200 Subject: [PATCH 15/20] Remove FormatterType enum --- helix-core/src/syntax.rs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/helix-core/src/syntax.rs b/helix-core/src/syntax.rs index a890182bf930..9e5bc197eb98 100644 --- a/helix-core/src/syntax.rs +++ b/helix-core/src/syntax.rs @@ -138,13 +138,6 @@ pub struct FormatterConfiguration { pub args: Vec, } -#[derive(Debug, Clone, Copy, Eq, PartialEq, PartialOrd, Ord, Deserialize, Serialize)] -#[serde(rename_all = "kebab-case")] -pub enum FormatterType { - Stdio, - File, -} - #[derive(Debug, PartialEq, Clone, Deserialize, Serialize)] #[serde(rename_all = "kebab-case")] pub struct AdvancedCompletion { From aeb4f8f1e333cde71c43ce6691aa8a138d80ee70 Mon Sep 17 00:00:00 2001 From: PiergiorgioZagaria Date: Fri, 22 Jul 2022 19:44:29 +0200 Subject: [PATCH 16/20] Remove old comment --- helix-term/src/commands/typed.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index 66b1d678bde7..4e1ac0da9a9c 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -551,7 +551,6 @@ fn write_all_impl( let auto_format = cx.editor.config().auto_format; let jobs = &mut cx.jobs; // save all documents - // Saves only visible buffers? How do we reload the not visible ones? 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"); From d711be0693d57028cded34b4910fafdf5ad617fb Mon Sep 17 00:00:00 2001 From: PiergiorgioZagaria Date: Sat, 23 Jul 2022 07:38:49 +0200 Subject: [PATCH 17/20] Check if the formatter exited correctly --- helix-view/src/document.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 198474f00765..cdcc3bbba162 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -448,6 +448,10 @@ impl Document { )); } + if !output.status.success() { + return Err(FormatterError::NonZeroExitStatus); + } + let str = String::from_utf8(output.stdout) .map_err(|_| FormatterError::InvalidUtf8Output)?; @@ -1094,6 +1098,7 @@ pub enum FormatterError { Stderr(String), InvalidUtf8Output, DiskReloadError(String), + NonZeroExitStatus, } impl std::error::Error for FormatterError {} @@ -1109,6 +1114,7 @@ impl Display for FormatterError { Self::Stderr(output) => format!("Formatter error: {}", output), Self::InvalidUtf8Output => "Invalid UTF-8 formatter output".to_string(), Self::DiskReloadError(error) => format!("Error reloading file from disk: {}", error), + Self::NonZeroExitStatus => "Formatter exited with non zero exit status:".to_string(), }; f.write_str(&s) } From 7ec0288a6d7a873943be86e2bc96bbfdc4a7094b Mon Sep 17 00:00:00 2001 From: PiergiorgioZagaria Date: Sat, 23 Jul 2022 08:03:21 +0200 Subject: [PATCH 18/20] Add formatter configuration to the book --- book/src/languages.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/book/src/languages.md b/book/src/languages.md index a9d5bea83436..841b1377d035 100644 --- a/book/src/languages.md +++ b/book/src/languages.md @@ -40,6 +40,7 @@ file-types = ["mylang", "myl"] comment-token = "#" indent = { tab-width = 2, unit = " " } language-server = { command = "mylang-lsp", args = ["--stdio"] } +formatter = { command = "mylang-formatter" , args = ["--stdin"] } ``` These configuration keys are available: @@ -59,6 +60,7 @@ These configuration keys are available: | `language-server` | The Language Server to run. See the Language Server configuration section below. | | `config` | Language Server configuration | | `grammar` | The tree-sitter grammar to use (defaults to the value of `name`) | +| `formatter` | The formatter for the language, it will take precedence over the lsp when defined. The formatter must be able to take the original file as input from stdin and write the formatted file to stdout | ### Language Server configuration From 1ae13bb4da783d8a092f23416005d8bcfce2ad07 Mon Sep 17 00:00:00 2001 From: PiergiorgioZagaria Date: Tue, 2 Aug 2022 06:52:24 +0200 Subject: [PATCH 19/20] Avoid allocations when writing to stdin and formatting errors --- helix-view/src/document.rs | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index cdcc3bbba162..dd4dfc0ca5bb 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -431,8 +431,7 @@ impl Document { })?; { let mut stdin = process.stdin.take().ok_or(FormatterError::BrokenStdin)?; - stdin - .write_all(&text.bytes().collect::>()) + to_writer(&mut stdin, encoding::UTF_8, &text) .await .map_err(|_| FormatterError::BrokenStdin)?; } @@ -1105,18 +1104,17 @@ impl std::error::Error for FormatterError {} impl Display for FormatterError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let s = match self { + match self { Self::SpawningFailed { command, error } => { - format!("Failed to spawn formatter {}: {:?}", command, error) + write!(f, "Failed to spawn formatter {}: {:?}", command, error) } - Self::BrokenStdin => "Could not write to formatter stdin".to_string(), - Self::WaitForOutputFailed => "Waiting for formatter output failed".to_string(), - Self::Stderr(output) => format!("Formatter error: {}", output), - Self::InvalidUtf8Output => "Invalid UTF-8 formatter output".to_string(), - Self::DiskReloadError(error) => format!("Error reloading file from disk: {}", error), - Self::NonZeroExitStatus => "Formatter exited with non zero exit status:".to_string(), - }; - f.write_str(&s) + Self::BrokenStdin => write!(f, "Could not write to formatter stdin"), + Self::WaitForOutputFailed => write!(f, "Waiting for formatter output failed"), + Self::Stderr(output) => write!(f, "Formatter error: {}", output), + Self::InvalidUtf8Output => write!(f, "Invalid UTF-8 formatter output"), + Self::DiskReloadError(error) => write!(f, "Error reloading file from disk: {}", error), + Self::NonZeroExitStatus => write!(f, "Formatter exited with non zero exit status:"), + } } } From fb93d1ec150dab0c1b2b9d35172a26cb8dab877a Mon Sep 17 00:00:00 2001 From: PiergiorgioZagaria Date: Tue, 2 Aug 2022 06:57:52 +0200 Subject: [PATCH 20/20] Remove unused import --- helix-view/src/document.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index dd4dfc0ca5bb..8cac15b2847c 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -12,7 +12,6 @@ use std::future::Future; use std::path::{Path, PathBuf}; use std::str::FromStr; use std::sync::Arc; -use tokio::io::AsyncWriteExt; use helix_core::{ encoding,