From 1db5b3ae06cba5423eb517b7d9aac1ba8a8d8101 Mon Sep 17 00:00:00 2001 From: saberoueslati Date: Thu, 23 Jul 2026 22:21:38 +0100 Subject: [PATCH] project: Prevent empty external formatter output from clearing buffers (#61276) ## Context Zed runs external formatters as stdin/stdout filters: it writes the current buffer to stdin and replaces the buffer with the formatter's stdout. Commands such as `cargo fmt` instead rewrite files on disk and exit successfully without producing stdout, causing Zed to interpret the empty output as the formatted contents and clear a non-empty buffer. The fix treats empty stdout from a successful external formatter as no output when the original buffer is non-empty. Zed leaves the buffer unchanged and displays a notification explaining that the formatter did not return formatted contents. The formatter documentation now clarifies the stdin/stdout contract and recommends using the Rust language server or invoking `rustfmt` directly. Closes #56344 Behavior before the fix : [Screencast from 2026-07-19 02-58-29.webm](https://github.com/user-attachments/assets/61fc5bf9-4420-43f1-94a3-394bbbc4f2a0) Behavior after the fix : [Screencast from 2026-07-19 02-55-35.webm](https://github.com/user-attachments/assets/21062065-6b95-4cd4-8200-506f5681d98e) ## How to Review **crates/project/src/lsp_store.rs** Start with `format_via_external_command`, which now checks whether a successful formatter returned empty stdout while the input buffer was non-empty. In that case, it returns `None` before constructing a diff, preventing the buffer contents from being replaced with an empty string. Then review the external formatter branch in `apply_formatter`: when it receives `None`, it skips extending the formatting transaction, logs the condition, and emits an `LspStoreEvent::Notification` explaining why the buffer was left unchanged. **crates/editor/src/editor_tests.rs** Adds a GPUI regression test that configures an external command to consume stdin and return no stdout. It uses platform-specific commands for Windows and Unix, invokes manual formatting on a non-empty Rust buffer, and verifies that the buffer remains unchanged, the operation is not recorded as a formatter failure, and a notification is emitted. **docs/src/reference/all-settings.md** Extends the external formatter documentation to state that formatters must return the formatted buffer through stdout. It calls out file-rewriting tools such as `cargo fmt` as incompatible and recommends using the Rust language server or `rustfmt --emit stdout`. ## Self-Review Checklist - [x] I've reviewed my own diff for quality, security, and reliability - [ ] Unsafe blocks (if any) have justifying comments - [x] The content is consistent with the UI/UX checklist - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Release Notes: - Fixed external formatters that produce no output clearing non-empty buffers --- crates/editor/src/editor_tests.rs | 74 ++++++++++++++++++++++++++++++ crates/project/src/lsp_store.rs | 10 ++++ docs/src/reference/all-settings.md | 2 + 3 files changed, 86 insertions(+) diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 830613be16..dbf3e48cf3 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -18093,6 +18093,80 @@ async fn test_auto_formatter_failure_is_silent(cx: &mut TestAppContext) { ); } +#[gpui::test] +async fn test_external_formatter_with_no_output_leaves_buffer_unchanged(cx: &mut TestAppContext) { + init_test(cx, |settings| { + #[cfg(windows)] + let (command, arguments) = ("cmd", vec!["/C".to_string(), "more > nul".to_string()]); + #[cfg(not(windows))] + let (command, arguments) = ("sh", vec!["-c".to_string(), "cat >/dev/null".to_string()]); + + // Consume stdin without printing anything, mirroring tools like `cargo fmt` + // that format files in place instead of writing to stdout. + settings.defaults.formatter = Some(FormatterList::Single(Formatter::External { + command: command.into(), + arguments: Some(arguments), + })); + }); + + let fs = FakeFs::new(cx.executor()); + fs.insert_file(path!("/file.rs"), "fn main() {}\n".into()) + .await; + + let project = Project::test(fs, [path!("/").as_ref()], cx).await; + let language_registry = project.read_with(cx, |project, _| project.languages().clone()); + language_registry.add(rust_lang()); + + let buffer = project + .update(cx, |project, cx| { + project.open_local_buffer(path!("/file.rs"), cx) + }) + .await + .unwrap(); + + let buffer = cx.new(|cx| MultiBuffer::singleton(buffer, cx)); + let (editor, cx) = cx.add_window_view(|window, cx| { + build_editor_with_project(project.clone(), buffer, window, cx) + }); + + // Formatting via an external command spawns a real subprocess, whose I/O + // happens off the deterministic test scheduler. + cx.executor().allow_parking(); + + editor + .update_in(cx, |editor, window, cx| { + editor.perform_format( + project.clone(), + FormatTrigger::Manual, + FormatTarget::Buffers(editor.buffer().read(cx).all_buffers()), + window, + cx, + ) + }) + .unwrap() + .await; + + editor.update(cx, |editor, cx| { + assert_eq!( + editor.text(cx), + "fn main() {}\n", + "buffer should be left unchanged when the external formatter produces no output" + ); + }); + + let last_failure = project.read_with(cx, |project, cx| { + project + .lsp_store() + .read(cx) + .last_formatting_failure() + .map(str::to_string) + }); + assert_eq!( + last_failure, None, + "producing no output is not a formatter failure" + ); +} + #[gpui::test] async fn test_concurrent_format_requests(cx: &mut TestAppContext) { init_test(cx, |_| {}); diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index 8a0d37adda..e8bb95ac0d 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -2601,6 +2601,16 @@ impl LocalLspStore { ); let stdout = String::from_utf8(output.stdout)?; + if stdout.is_empty() && !text.is_empty() { + // Some formatters (e.g. `cargo fmt`) rewrite files on disk instead of + // printing the formatted contents to stdout. Treating empty stdout as the + // new buffer contents would erase the buffer, so leave it untouched. + log::warn!( + "External formatter `{command}` produced no output on stdout; leaving the buffer unchanged" + ); + return Ok(None); + } + Ok(Some( buffer .handle diff --git a/docs/src/reference/all-settings.md b/docs/src/reference/all-settings.md index 25024fc908..d785045fdb 100644 --- a/docs/src/reference/all-settings.md +++ b/docs/src/reference/all-settings.md @@ -2004,6 +2004,8 @@ Similar to `modifications`, but behaves like `on` when range formatting cannot b } ``` +Tools that rewrite files on disk instead of printing the formatted contents to stdout (such as `cargo fmt`) are not compatible with `"external"`: since Zed reads the formatted buffer from stdout, a formatter that emits nothing there will not update the buffer. For Rust, use `"formatter": "language_server"` or invoke `rustfmt` directly (which supports stdin/stdout via `--emit stdout`) instead of `cargo fmt`. + 3. External formatters may optionally include a `{buffer_path}` placeholder which at runtime will include the path of the buffer being formatted. Formatters operate by receiving file content via standard input, reformatting it and then outputting it to standard output and so normally don't know the filename of what they are formatting. Tools like Prettier support receiving the file path via a command line argument which can then be used to impact formatting decisions. WARNING: `{buffer_path}` should not be used to direct your formatter to read from a filename. Your formatter should only read from standard input and should not read or write files directly.