Skip to content

Commit

Permalink
Change default formatter for any language (#2942)
Browse files Browse the repository at this point in the history
* Change default formatter for any language

* Fix clippy error

* Close stdin for Stdio formatters

* Better indentation and pattern matching

* Return Result<Option<...>> for fn format instead of Option

* Remove unwrap for stdin

* Handle FormatterErrors instead of Result<Option<...>>

* Use Transaction instead of LspFormatting

* Use Transaction directly in Document::format

* Perform stdin type formatting asynchronously

* Rename formatter.type values to kebab-case

* Debug format for displaying io::ErrorKind (msrv fix)

* Solve conflict?

* Use only stdio type formatters

* Remove FormatterType enum

* Remove old comment

* Check if the formatter exited correctly

* Add formatter configuration to the book

* Avoid allocations when writing to stdin and formatting errors

* Remove unused import

Co-authored-by: Gokul Soumya <[email protected]>
  • Loading branch information
PiergiorgioZagaria and sudormrfbin authored Aug 4, 2022
1 parent c5f8a83 commit 219d2c2
Show file tree
Hide file tree
Showing 7 changed files with 114 additions and 32 deletions.
2 changes: 2 additions & 0 deletions book/src/languages.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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

Expand Down
12 changes: 12 additions & 0 deletions helix-core/src/syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,9 @@ pub struct LanguageConfiguration {
#[serde(default)]
pub auto_format: bool,

#[serde(skip_serializing_if = "Option::is_none")]
pub formatter: Option<FormatterConfiguration>,

#[serde(default)]
pub diagnostic_severity: Severity,

Expand Down Expand Up @@ -126,6 +129,15 @@ pub struct LanguageServerConfiguration {
pub language_id: Option<String>,
}

#[derive(Debug, Clone, 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<String>,
}

#[derive(Debug, PartialEq, Clone, Deserialize, Serialize)]
#[serde(rename_all = "kebab-case")]
pub struct AdvancedCompletion {
Expand Down
16 changes: 0 additions & 16 deletions helix-lsp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,22 +213,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<lsp::TextEdit>,
pub offset_encoding: OffsetEncoding,
}

impl From<LspFormatting> for Transaction {
fn from(fmt: LspFormatting) -> Transaction {
generate_transaction_from_edits(&fmt.doc, fmt.edits, fmt.offset_encoding)
}
}
}

#[derive(Debug, PartialEq, Clone)]
Expand Down
8 changes: 4 additions & 4 deletions helix-term/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -2471,14 +2471,14 @@ async fn make_format_callback(
doc_id: DocumentId,
doc_version: i32,
modified: Modified,
format: impl Future<Output = helix_lsp::util::LspFormatting> + Send + 'static,
format: impl Future<Output = Result<Transaction, FormatterError>> + Send + 'static,
) -> anyhow::Result<job::Callback> {
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) {
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 {
Expand Down
103 changes: 93 additions & 10 deletions helix-view/src/document.rs
Original file line number Diff line number Diff line change
@@ -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::Range;
use serde::de::{self, Deserialize, Deserializer};
Expand All @@ -20,7 +22,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};

Expand Down Expand Up @@ -397,7 +398,7 @@ impl Document {

/// The same as [`format`], but only returns formatting changes if auto-formatting
/// is configured.
pub fn auto_format(&self) -> Option<impl Future<Output = LspFormatting> + 'static> {
pub fn auto_format(&self) -> Option<BoxFuture<'static, Result<Transaction, FormatterError>>> {
if self.language_config()?.auto_format {
self.format()
} else {
Expand All @@ -407,7 +408,56 @@ 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<impl Future<Output = LspFormatting> + 'static> {
// 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(&self) -> Option<BoxFuture<'static, Result<Transaction, FormatterError>>> {
if let Some(formatter) = self.language_config().and_then(|c| c.formatter.clone()) {
use std::process::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)?;
to_writer(&mut stdin, encoding::UTF_8, &text)
.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(),
));
}

if !output.status.success() {
return Err(FormatterError::NonZeroExitStatus);
}

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();
let offset_encoding = language_server.offset_encoding();
Expand All @@ -427,13 +477,13 @@ impl Document {
log::warn!("LSP formatting failed: {}", e);
Default::default()
});
LspFormatting {
doc: text,
Ok(helix_lsp::util::generate_transaction_from_edits(
&text,
edits,
offset_encoding,
}
))
};
Some(fut)
Some(fut.boxed())
}

pub fn save(&mut self, force: bool) -> impl Future<Output = Result<(), anyhow::Error>> {
Expand All @@ -442,7 +492,7 @@ impl Document {

pub fn format_and_save(
&mut self,
formatting: Option<impl Future<Output = LspFormatting>>,
formatting: Option<impl Future<Output = Result<Transaction, FormatterError>>>,
force: bool,
) -> impl Future<Output = anyhow::Result<()>> {
self.save_impl(formatting, force)
Expand All @@ -454,7 +504,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<F: Future<Output = LspFormatting>>(
fn save_impl<F: Future<Output = Result<Transaction, FormatterError>>>(
&mut self,
formatting: Option<F>,
force: bool,
Expand Down Expand Up @@ -488,7 +538,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.
Expand Down Expand Up @@ -1034,6 +1085,38 @@ impl Default for Document {
}
}

#[derive(Clone, Debug)]
pub enum FormatterError {
SpawningFailed {
command: String,
error: std::io::ErrorKind,
},
BrokenStdin,
WaitForOutputFailed,
Stderr(String),
InvalidUtf8Output,
DiskReloadError(String),
NonZeroExitStatus,
}

impl std::error::Error for FormatterError {}

impl Display for FormatterError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Self::SpawningFailed { command, error } => {
write!(f, "Failed to spawn formatter {}: {:?}", command, error)
}
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:"),
}
}
}

#[cfg(test)]
mod test {
use super::*;
Expand Down
3 changes: 1 addition & 2 deletions helix-view/src/editor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -613,7 +613,6 @@ impl Editor {
syn_loader: Arc<syntax::Loader>,
config: Box<dyn DynAccess<Config>>,
) -> Self {
let language_servers = helix_lsp::Registry::new();
let conf = config.load();
let auto_pairs = (&conf.auto_pairs).into();

Expand All @@ -629,7 +628,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(),
Expand Down
2 changes: 2 additions & 0 deletions languages.toml
Original file line number Diff line number Diff line change
Expand Up @@ -706,6 +706,8 @@ auto-format = true
comment-token = "//"
language-server = { command = "zls" }
indent = { tab-width = 4, unit = " " }
formatter = { command = "zig" , args = ["fmt", "--stdin"] }


[[grammar]]
name = "zig"
Expand Down

0 comments on commit 219d2c2

Please sign in to comment.