Prefer language server formatting

book/src/languages.md

@@ -66,7 +66,7 @@ These configuration keys are available:
 | `indent`              | The indent to use. Has sub keys `unit` (the text inserted into the document when indenting; usually set to N spaces or `"\t"` for tabs) and `tab-width` (the number of spaces rendered for a tab) |
 | `language-servers`    | The Language Servers used for this language. See below for more information in the section [Configuring Language Servers for a language](#configuring-language-servers-for-a-language)   |
 | `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. The filename of the current buffer can be passed as argument by using the `%{buffer_name}` expansion variable. See below for more information in the [Configuring the formatter command](#configuring-the-formatter-command) |
+| `formatter`           | The external formatter for the language. It is used when no available language server supports formatting. The formatter must be able to take the original file as input from stdin and write the formatted file to stdout. The filename of the current buffer can be passed as argument by using the `%{buffer_name}` expansion variable. See below for more information in the [Configuring the formatter command](#configuring-the-formatter-command) |
 | `soft-wrap`           | [editor.softwrap](./editor.md#editorsoft-wrap-section)
 | `text-width`          |  Maximum line length. Used for the `:reflow` command and soft-wrapping if `soft-wrap.wrap-at-text-width` is set, defaults to `editor.text-width`   |
 | `rulers`              | Overrides the `editor.rulers` config key for the language. |

helix-term/tests/test/commands/write.rs

@@ -1,14 +1,78 @@
 use std::{
     io::{Read, Seek, Write},
     ops::RangeInclusive,
+    time::Duration,
 };
 
-use helix_core::diagnostic::Severity;
+use helix_core::{diagnostic::Severity, syntax::config::LanguageServerFeature};
 use helix_stdx::path;
 use helix_view::doc;
 
 use super::*;
 
+fn language_server_script(
+    supports_formatting: bool,
+    formatted_text: &str,
+) -> anyhow::Result<tempfile::NamedTempFile> {
+    let mut server = tempfile::NamedTempFile::new()?;
+    let script = indoc! {r#"
+            send() {
+              printf 'Content-Length: %d\r\n\r\n%s' "${#1}" "$1"
+            }
+
+            while IFS= read -r header; do
+              header=${header%$'\r'}
+              case "$header" in
+                'Content-Length: '*) length=${header#Content-Length: } ;;
+                '')
+                  IFS= read -r -N "$length" payload || true
+                  id=$(printf '%s' "$payload" | sed -n 's/.*"id":\([0-9][0-9]*\).*/\1/p')
+                  case "$payload" in
+                    *'"method":"initialize"'*)
+                      send "{\"jsonrpc\":\"2.0\",\"id\":$id,\"result\":{\"capabilities\":{\"documentFormattingProvider\":FORMAT_SUPPORT,\"textDocumentSync\":1}}}"
+                      ;;
+                    *'"method":"textDocument/formatting"'*)
+                      send "{\"jsonrpc\":\"2.0\",\"id\":$id,\"result\":[{\"range\":{\"start\":{\"line\":0,\"character\":0},\"end\":{\"line\":1,\"character\":0}},\"newText\":\"FORMAT_OUTPUT\\n\"}]}"
+                      ;;
+                    *'"method":"shutdown"'*)
+                      send "{\"jsonrpc\":\"2.0\",\"id\":$id,\"result\":null}"
+                      ;;
+                  esac
+                  ;;
+              esac
+            done
+        "#}
+    .replace(
+        "FORMAT_SUPPORT",
+        if supports_formatting {
+            "true"
+        } else {
+            "false"
+        },
+    )
+    .replace("FORMAT_OUTPUT", formatted_text);
+    server.write_all(script.as_bytes())?;
+    server.flush()?;
+    Ok(server)
+}
+
+async fn wait_for_language_servers(
+    app: &mut Application,
+    expected_count: usize,
+) -> anyhow::Result<()> {
+    tokio::time::timeout(Duration::from_secs(5), async {
+        loop {
+            helpers::run_event_loop_until_idle(app).await;
+            if doc!(app.editor).language_servers().count() == expected_count {
+                break;
+            }
+            tokio::task::yield_now().await;
+        }
+    })
+    .await?;
+    Ok(())
+}
+
 #[tokio::test(flavor = "multi_thread")]
 async fn test_exit_w_buffer_w_path() -> anyhow::Result<()> {
     let mut file = tempfile::NamedTempFile::new()?;
@@ -475,6 +539,107 @@ async fn test_write_quit_auto_format_exits_after_format() -> anyhow::Result<()>
     Ok(())
 }
 
+#[tokio::test(flavor = "multi_thread")]
+async fn test_format_prefers_language_server_to_external_formatter() -> anyhow::Result<()> {
+    let unsupported_server = language_server_script(false, "unsupported formatter")?;
+    let server = language_server_script(true, "lsp formatter")?;
+
+    let file = tempfile::Builder::new().suffix(".rs").tempfile()?;
+    let unsupported_server_path = format!(
+        "{:?}",
+        unsupported_server
+            .path()
+            .to_string_lossy()
+            .replace('\\', "/")
+    );
+    let server_path = format!("{:?}", server.path().to_string_lossy().replace('\\', "/"));
+    let lang_conf = format!(
+        indoc! {r#"
+            [language-server.unsupported-formatter]
+            command = "bash"
+            args = [{}]
+
+            [language-server.test-formatter]
+            command = "bash"
+            args = [{}]
+
+            [[language]]
+            name = "rust"
+            language-servers = ["unsupported-formatter", "test-formatter"]
+            formatter = {{ command = "bash", args = [ "-c", "echo external formatter" ] }}
+        "#},
+        unsupported_server_path, server_path
+    );
+    let mut config = helpers::test_config();
+    config.editor.lsp.enable = true;
+    let mut app = helpers::AppBuilder::new()
+        .with_file(file.path(), None)
+        .with_input_text("#[external source|]#\n")
+        .with_config(config)
+        .with_lang_loader(helpers::test_syntax_loader(Some(lang_conf)))
+        .build()?;
+
+    wait_for_language_servers(&mut app, 2).await?;
+    assert!(doc!(app.editor).has_language_server_with_feature(LanguageServerFeature::Format));
+
+    let assert_lsp_formatter = |app: &Application| {
+        assert_eq!(doc!(app.editor).text(), "lsp formatter\n");
+    };
+    test_key_sequences(
+        &mut app,
+        vec![(Some(":format<ret>"), Some(&assert_lsp_formatter))],
+        false,
+    )
+    .await?;
+
+    Ok(())
+}
+
+#[tokio::test(flavor = "multi_thread")]
+async fn test_format_uses_external_formatter_when_lsp_formatting_is_disabled() -> anyhow::Result<()>
+{
+    let server = language_server_script(true, "lsp formatter")?;
+
+    let file = tempfile::Builder::new().suffix(".rs").tempfile()?;
+    let server_path = format!("{:?}", server.path().to_string_lossy().replace('\\', "/"));
+    let lang_conf = format!(
+        indoc! {r#"
+            [language-server.test-formatter]
+            command = "bash"
+            args = [{}]
+
+            [[language]]
+            name = "rust"
+            language-servers = [{{ name = "test-formatter", except-features = ["format"] }}]
+            formatter = {{ command = "bash", args = [ "-c", "echo external formatter" ] }}
+        "#},
+        server_path
+    );
+    let mut config = helpers::test_config();
+    config.editor.lsp.enable = true;
+    let mut app = helpers::AppBuilder::new()
+        .with_file(file.path(), None)
+        .with_input_text("#[external source|]#\n")
+        .with_config(config)
+        .with_lang_loader(helpers::test_syntax_loader(Some(lang_conf)))
+        .build()?;
+
+    wait_for_language_servers(&mut app, 1).await?;
+    assert!(!doc!(app.editor).has_language_server_with_feature(LanguageServerFeature::Format));
+
+    let assert_external_formatter = |app: &Application| {
+        assert_eq!(doc!(app.editor).text(), "external formatter\n");
+    };
+    test_key_sequences(
+        &mut app,
+        vec![(Some(":format<ret>"), Some(&assert_external_formatter))],
+        false,
+    )
+    .await?;
+
+    Ok(())
+}
+
 #[tokio::test(flavor = "multi_thread")]
 async fn test_write_code_actions_on_save_without_server_still_saves() -> anyhow::Result<()> {
     let mut file = tempfile::Builder::new().suffix(".rs").tempfile()?;

helix-view/src/document.rs

@@ -858,6 +858,39 @@ impl Document {
         &self,
         editor: &Editor,
     ) -> Option<BoxFuture<'static, Result<Transaction, FormatterError>>> {
+        if let Some(language_server) = self
+            .language_servers_with_feature(LanguageServerFeature::Format)
+            .next()
+        {
+            let text = self.text.clone();
+            let offset_encoding = language_server.offset_encoding();
+            let request = language_server.text_document_formatting(
+                self.identifier(),
+                lsp::FormattingOptions {
+                    tab_size: self.tab_width() as u32,
+                    insert_spaces: matches!(self.indent_style, IndentStyle::Spaces(_)),
+                    ..Default::default()
+                },
+                None,
+            )?;
+
+            let fut = async move {
+                let edits = request
+                    .await
+                    .unwrap_or_else(|e| {
+                        log::warn!("LSP formatting failed: {}", e);
+                        Default::default()
+                    })
+                    .unwrap_or_default();
+                Ok(helix_lsp::util::generate_transaction_from_edits(
+                    &text,
+                    edits,
+                    offset_encoding,
+                ))
+            };
+            return Some(fut.boxed());
+        }
+
         if let Some((fmt_cmd, fmt_args)) = self
             .language_config()
             .and_then(|c| c.formatter.as_ref())
@@ -944,39 +977,9 @@ impl Document {
                 Ok(helix_core::diff::compare_ropes(&text, &Rope::from(str)))
             };
             return Some(formatting_future.boxed());
-        };
-
-        let text = self.text.clone();
-        // finds first language server that supports formatting and then formats
-        let language_server = self
-            .language_servers_with_feature(LanguageServerFeature::Format)
-            .next()?;
-        let offset_encoding = language_server.offset_encoding();
-        let request = language_server.text_document_formatting(
-            self.identifier(),
-            lsp::FormattingOptions {
-                tab_size: self.tab_width() as u32,
-                insert_spaces: matches!(self.indent_style, IndentStyle::Spaces(_)),
-                ..Default::default()
-            },
-            None,
-        )?;
+        }
 
-        let fut = async move {
-            let edits = request
-                .await
-                .unwrap_or_else(|e| {
-                    log::warn!("LSP formatting failed: {}", e);
-                    Default::default()
-                })
-                .unwrap_or_default();
-            Ok(helix_lsp::util::generate_transaction_from_edits(
-                &text,
-                edits,
-                offset_encoding,
-            ))
-        };
-        Some(fut.boxed())
+        None
     }
 
     pub fn save<P: Into<PathBuf>>(