From 77df7e56f7852aaa121cafb0bc0596c9b43d3680 Mon Sep 17 00:00:00 2001
From: Tom Wieczorek <twz123@users.noreply.github.com>
Date: Mon, 30 Sep 2024 08:34:41 +0200
Subject: [PATCH] settings: Make external formatter arguments optional (#18340)

If specifying a formatter in the settings like this:

    "languages": {
      "foo": {
        "formatter": {
          "external": {
            "command": "/path/to/foo-formatter"
          }
        }
      }
    }

Zed will show an error like this:

    Invalid user settings file
    data did not match any variant of untagged enum SingleOrVec

This is because the arguments are not optional. The error is hard to
understand, so let's make the arguments actually optional, which makes
the above settings snippet valid.

Release Notes:

- Make external formatter arguments optional
---
 crates/collab/src/tests/integration_tests.rs |  2 +-
 crates/language/src/language_settings.rs     |  2 +-
 crates/project/src/lsp_store.rs              | 31 +++++++++++++-------
 3 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs
index afc3e7cfb8..615ad52e2e 100644
--- a/crates/collab/src/tests/integration_tests.rs
+++ b/crates/collab/src/tests/integration_tests.rs
@@ -4409,7 +4409,7 @@ async fn test_formatting_buffer(
                 file.defaults.formatter = Some(SelectedFormatter::List(FormatterList(
                     vec![Formatter::External {
                         command: "awk".into(),
-                        arguments: vec!["{sub(/two/,\"{buffer_path}\")}1".to_string()].into(),
+                        arguments: Some(vec!["{sub(/two/,\"{buffer_path}\")}1".to_string()].into()),
                     }]
                     .into(),
                 )));
diff --git a/crates/language/src/language_settings.rs b/crates/language/src/language_settings.rs
index 2f1a7be2bf..d610ab0986 100644
--- a/crates/language/src/language_settings.rs
+++ b/crates/language/src/language_settings.rs
@@ -661,7 +661,7 @@ pub enum Formatter {
         /// The external program to run.
         command: Arc<str>,
         /// The arguments to pass to the program.
-        arguments: Arc<[String]>,
+        arguments: Option<Arc<[String]>>,
     },
     /// Files should be formatted using code actions executed by language servers.
     CodeActions(HashMap<String, bool>),
diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs
index 37922b7c2e..a3763810e1 100644
--- a/crates/project/src/lsp_store.rs
+++ b/crates/project/src/lsp_store.rs
@@ -539,13 +539,19 @@ impl LocalLspStore {
             }
             Formatter::External { command, arguments } => {
                 let buffer_abs_path = buffer_abs_path.as_ref().map(|path| path.as_path());
-                Self::format_via_external_command(buffer, buffer_abs_path, command, arguments, cx)
-                    .await
-                    .context(format!(
-                        "failed to format via external command {:?}",
-                        command
-                    ))?
-                    .map(FormatOperation::External)
+                Self::format_via_external_command(
+                    buffer,
+                    buffer_abs_path,
+                    command,
+                    arguments.as_deref(),
+                    cx,
+                )
+                .await
+                .context(format!(
+                    "failed to format via external command {:?}",
+                    command
+                ))?
+                .map(FormatOperation::External)
             }
             Formatter::CodeActions(code_actions) => {
                 let code_actions = deserialize_code_actions(code_actions);
@@ -571,7 +577,7 @@ impl LocalLspStore {
         buffer: &Model<Buffer>,
         buffer_abs_path: Option<&Path>,
         command: &str,
-        arguments: &[String],
+        arguments: Option<&[String]>,
         cx: &mut AsyncAppContext,
     ) -> Result<Option<Diff>> {
         let working_dir_path = buffer.update(cx, |buffer, cx| {
@@ -595,14 +601,17 @@ impl LocalLspStore {
             child.current_dir(working_dir_path);
         }
 
-        let mut child = child
-            .args(arguments.iter().map(|arg| {
+        if let Some(arguments) = arguments {
+            child.args(arguments.iter().map(|arg| {
                 if let Some(buffer_abs_path) = buffer_abs_path {
                     arg.replace("{buffer_path}", &buffer_abs_path.to_string_lossy())
                 } else {
                     arg.replace("{buffer_path}", "Untitled")
                 }
-            }))
+            }));
+        }
+
+        let mut child = child
             .stdin(smol::process::Stdio::piped())
             .stdout(smol::process::Stdio::piped())
             .stderr(smol::process::Stdio::piped())