From bc463bc205f944d620d5925f01dc59b1adc4084e Mon Sep 17 00:00:00 2001 From: Michael Thomas Date: Wed, 12 Aug 2026 18:14:22 +0000 Subject: [PATCH] Send correct line endings to language servers (#59941) # Objective Zed normalizes all buffer text to `LF` internally, but was sending that `LF`-normalized text to language servers even for `CRLF` files. This caused servers such as ESLint (with a `linebreak-style` rule) to report a false error on every line. Fixes #38453 ## Solution Send the buffer's actual line endings to the language server instead: - `didOpen` and full-document `didChange` now send `text_with_line_endings()`, and incremental changes apply the buffer's line ending to each edit. - Normalize the line endings returning from the LSP before computing changed regions - This effectively incorporates the fix from #59151, which happens to be the reason this change was [originally reverted](https://github.com/zed-industries/zed/commit/1b6cde7032fea1ff38f885419ab4e79c4f8c3231). As such, that PR should likely be integrated first. - Detect when a buffer's line ending differs from what a server was last sent and force a full-document resync, without this the server would keep stale line endings, as the incremental change tracking does not consider line ending differences. - Route the `UpdateLineEnding` operation to `on_buffer_edited` so toggling line endings via the status bar notifies the server immediately rather than waiting for the file to be edited or reopened. ## Testing - Did you test these changes? If so, how? Yes, in addition to new unit test coverage, I used a test project with ESLint configured with the `linebreak-style` rule set to enforce CRLF line endings to verify that the LSP integration worked as expected. - Are there any parts that need more testing? The original reversion seems to have been due to a regression in which LSP formatting would cause the editor to scroll to the bottom. I'm not seeing this in my reproduction, and I believe this was due to a failure to normalize line endings coming back from the LSP, but I don't know the exact circumstances that led to the original reversion, so there might be some additional things to test there. - How can other people (reviewers) test your changes? Is there anything specific they need to know? Not really! As mentioned above, configuring ESLint with the `linebreak-style` rule is probably the easiest way to test. - If relevant, what platforms did you test these changes on, and are there any important ones you can't test? I tested on Linux, but I don't believe it's relevant. ## Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content adheres to Zed's UI standards ([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) and [icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md) guidelines) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable ## Showcase https://github.com/user-attachments/assets/881c5758-a5de-433c-8fd6-3cad7478aa90 --- Release Notes: - Fixed an issue where language servers received incorrect line endings for `CRLF` files, causing linters and formatters to report false errors. --------- Co-authored-by: Kirill Bulatov --- crates/editor/src/editor_tests.rs | 110 ++++++++++++++ crates/project/src/lsp_store.rs | 48 +++++-- .../tests/integration/project_tests.rs | 136 ++++++++++++++++++ crates/text/src/text.rs | 23 +++ 4 files changed, 305 insertions(+), 12 deletions(-) diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index e9e3f61f949..349c8b053b5 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -37583,6 +37583,116 @@ async fn test_paste_image_in_markdown_single_file_worktree_falls_through(cx: &mu ); } +#[gpui::test] +async fn test_format_echoing_received_line_endings_keeps_cursor(cx: &mut TestAppContext) { + init_test(cx, |settings| { + settings.defaults.ensure_final_newline_on_save = Some(false); + }); + + let lf_content = "// one\nstruct Foo {\n bar: Bar,\n}\n\n// two\nstruct Bar {\n foobar:u32,\n}\n\nfn main() {\n let foo = 1;\n}\n"; + let crlf_content = lf_content.replace('\n', "\r\n"); + + let fs = FakeFs::new(cx.executor()); + fs.insert_file(path!("/file.rs"), crlf_content.clone().into()) + .await; + + let project = Project::test(fs, [path!("/file.rs").as_ref()], cx).await; + let language_registry = project.read_with(cx, |project, _| project.languages().clone()); + language_registry.add(rust_lang()); + + let server_side_texts = Arc::new(Mutex::new(Vec::::new())); + let mut fake_servers = language_registry.register_fake_lsp( + "Rust", + FakeLspAdapter { + capabilities: lsp::ServerCapabilities { + document_formatting_provider: Some(lsp::OneOf::Left(true)), + ..Default::default() + }, + initializer: Some({ + let server_side_texts = server_side_texts.clone(); + Box::new(move |fake_server| { + fake_server.handle_notification::({ + let server_side_texts = server_side_texts.clone(); + move |params, _| { + server_side_texts.lock().push(params.text_document.text); + } + }); + }) + }), + ..FakeLspAdapter::default() + }, + ); + + let buffer = project + .update(cx, |project, cx| { + project.open_local_buffer(path!("/file.rs"), cx) + }) + .await + .unwrap(); + + let multi_buffer = cx.new(|cx| MultiBuffer::singleton(buffer.clone(), cx)); + let (editor, cx) = cx.add_window_view(|window, cx| { + build_editor_with_project(project.clone(), multi_buffer, window, cx) + }); + + let fake_server = fake_servers.next().await.unwrap(); + cx.executor().run_until_parked(); + + editor.update_in(cx, |editor, window, cx| { + editor.change_selections(SelectionEffects::default(), window, cx, |s| { + s.select_ranges([Point::new(2, 4)..Point::new(2, 4)]); + }); + }); + + let server_side_text = server_side_texts.lock().last().cloned().unwrap(); + assert_eq!( + server_side_text, crlf_content, + "server should have received the CRLF text on DidOpenTextDocument", + ); + + let formatted_text = server_side_text.replace("foobar:u32", "foobar: u32"); + fake_server.set_request_handler::(move |_, _| { + let formatted_text = formatted_text.clone(); + async move { + Ok(Some(vec![lsp::TextEdit { + range: lsp::Range::new(lsp::Position::new(0, 0), lsp::Position::new(13, 0)), + new_text: formatted_text, + }])) + } + }); + + let save = editor + .update_in(cx, |editor, window, cx| { + editor.save( + SaveOptions { + format: true, + force_format: false, + autosave: false, + }, + project.clone(), + window, + cx, + ) + }) + .unwrap(); + save.await; + + assert_eq!( + editor.update(cx, |editor, cx| editor.text(cx)), + lf_content.replace("foobar:u32", "foobar: u32"), + "only the minimal formatting change should be applied", + ); + editor.update(cx, |editor, cx| { + let snapshot = editor.display_snapshot(cx); + let cursor = editor.selections.newest::(&snapshot).head(); + assert_eq!( + cursor, + Point::new(2, 4), + "whole-document echo of the received line endings must not move the cursor", + ); + }); +} + #[gpui::test] async fn test_race_in_multibuffer_save(cx: &mut TestAppContext) { init_test(cx, |_| {}); diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index 4a78503993e..f5a826ea8fc 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -3051,7 +3051,7 @@ impl LocalLspStore { uri.clone(), adapter.language_id(&language.name()), 0, - initial_snapshot.text(), + initial_snapshot.text_with_line_endings(), ); vec![snapshot] @@ -4844,6 +4844,13 @@ impl LspStore { self.on_buffer_edited(buffer, cx); } + language::BufferEvent::Operation { + operation: language::Operation::UpdateLineEnding { .. }, + .. + } => { + self.on_buffer_edited(buffer, cx); + } + language::BufferEvent::Saved => { self.on_buffer_saved(buffer, cx); } @@ -8582,6 +8589,8 @@ impl LspStore { .with_context(|| format!("Failed to convert path to URI: {}", abs_path.display())) .log_err()?; let next_snapshot = buffer.text_snapshot(); + let line_ending = next_snapshot.line_ending(); + for language_server in language_servers { let language_server = language_server.clone(); @@ -8592,6 +8601,19 @@ impl LspStore { .and_then(|m| m.get_mut(&language_server.server_id()))?; let previous_snapshot = buffer_snapshots.last()?; + // If the line ending differs from what this server was last sent, the LF-normalized + // rope is byte-identical so `edits_since` yields no diffs. We must resync the whole + // document, otherwise the server keeps the stale line endings indefinitely. + let line_ending_changed = line_ending != previous_snapshot.snapshot.line_ending(); + + let build_full_change = || { + vec![lsp::TextDocumentContentChangeEvent { + range: None, + range_length: None, + text: next_snapshot.text_with_line_endings(), + }] + }; + let build_incremental_change = || { buffer .edits_since::>( @@ -8609,7 +8631,7 @@ impl LspStore { point_to_lsp(edit_end), )), range_length: None, - text: new_text, + text: line_ending.apply(new_text), } }) .collect() @@ -8624,19 +8646,21 @@ impl LspStore { lsp::TextDocumentSyncCapability::Options(options) => options.change, }); - let content_changes: Vec<_> = match document_sync_kind { - Some(lsp::TextDocumentSyncKind::FULL) => { - vec![lsp::TextDocumentContentChangeEvent { - range: None, - range_length: None, - text: next_snapshot.text(), - }] + let build_change = || { + if line_ending_changed { + build_full_change() + } else { + build_incremental_change() } - Some(lsp::TextDocumentSyncKind::INCREMENTAL) => build_incremental_change(), + }; + + let content_changes: Vec<_> = match document_sync_kind { + Some(lsp::TextDocumentSyncKind::FULL) => build_full_change(), + Some(lsp::TextDocumentSyncKind::INCREMENTAL) => build_change(), _ => { #[cfg(any(test, feature = "test-support"))] { - build_incremental_change() + build_change() } #[cfg(not(any(test, feature = "test-support")))] @@ -12451,7 +12475,7 @@ impl LspStore { uri, adapter.language_id(&language.name()), version, - initial_snapshot.text(), + initial_snapshot.text_with_line_endings(), ); buffer_paths_registered.push((buffer_id, abs_path)); local diff --git a/crates/project/tests/integration/project_tests.rs b/crates/project/tests/integration/project_tests.rs index 967ed7e2234..fa168b2fa4b 100644 --- a/crates/project/tests/integration/project_tests.rs +++ b/crates/project/tests/integration/project_tests.rs @@ -5194,6 +5194,142 @@ async fn test_edits_from_lsp_with_crlf_line_endings(cx: &mut gpui::TestAppContex }); } +#[gpui::test] +async fn test_lsp_server_document_matches_buffer_line_endings(cx: &mut gpui::TestAppContext) { + init_test(cx); + + fn offset_of(document: &str, position: lsp::Position) -> usize { + let mut offset = 0; + for _ in 0..position.line { + offset += document[offset..] + .find('\n') + .map(|newline_offset| newline_offset + 1) + .unwrap_or_else(|| document.len() - offset); + } + offset + position.character as usize + } + + let crlf_content = "fn main() {\r\n let a = 5;\r\n let b = 6;\r\n}\r\n"; + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree(path!("/dir"), json!({ "a.rs": crlf_content })) + .await; + + let project = Project::test(fs, [path!("/dir").as_ref()], cx).await; + let language_registry = project.read_with(cx, |project, _| project.languages().clone()); + language_registry.add(rust_lang()); + + let server_document = Arc::new(Mutex::new(String::new())); + let mut fake_servers = language_registry.register_fake_lsp( + "Rust", + FakeLspAdapter { + capabilities: lsp::ServerCapabilities { + text_document_sync: Some(lsp::TextDocumentSyncCapability::Kind( + lsp::TextDocumentSyncKind::INCREMENTAL, + )), + ..Default::default() + }, + initializer: Some({ + let server_document = server_document.clone(); + Box::new(move |fake_server| { + fake_server.handle_notification::({ + let server_document = server_document.clone(); + move |params, _| { + *server_document.lock() = params.text_document.text; + } + }); + fake_server.handle_notification::( + { + let server_document = server_document.clone(); + move |params, _| { + let mut document = server_document.lock(); + for change in params.content_changes { + match change.range { + Some(range) => { + let start = offset_of(&document, range.start); + let end = offset_of(&document, range.end); + document.replace_range(start..end, &change.text); + } + None => *document = change.text, + } + } + } + }, + ); + }) + }), + ..FakeLspAdapter::default() + }, + ); + + let (buffer, _lsp_handle) = project + .update(cx, |project, cx| { + project.open_local_buffer_with_lsp(path!("/dir/a.rs"), cx) + }) + .await + .unwrap(); + let _fake_server = fake_servers.next().await.unwrap(); + cx.executor().run_until_parked(); + + assert_eq!( + server_document.lock().as_str(), + crlf_content, + "DidOpenTextDocument should carry the buffer's CRLF line endings", + ); + + buffer.update(cx, |buffer, cx| { + buffer.edit( + [(Point::new(1, 14)..Point::new(1, 14), "\n let a2 = 55;")], + None, + cx, + ); + }); + cx.executor().run_until_parked(); + + assert_eq!( + server_document.lock().as_str(), + "fn main() {\r\n let a = 5;\r\n let a2 = 55;\r\n let b = 6;\r\n}\r\n", + "incremental DidChangeTextDocument should carry the buffer's CRLF line endings", + ); + + buffer.update(cx, |buffer, cx| { + buffer.set_line_ending(LineEnding::Unix, cx); + }); + cx.executor().run_until_parked(); + + assert_eq!( + server_document.lock().as_str(), + "fn main() {\n let a = 5;\n let a2 = 55;\n let b = 6;\n}\n", + "a line ending change should resync the whole document with the new line endings", + ); + + buffer.update(cx, |buffer, cx| { + buffer.edit( + [(Point::new(3, 14)..Point::new(3, 14), "\n let c = 7;")], + None, + cx, + ); + }); + cx.executor().run_until_parked(); + + assert_eq!( + server_document.lock().as_str(), + "fn main() {\n let a = 5;\n let a2 = 55;\n let b = 6;\n let c = 7;\n}\n", + "incremental DidChangeTextDocument should carry the buffer's LF line endings after the change", + ); + + buffer.update(cx, |buffer, cx| { + buffer.set_line_ending(LineEnding::Windows, cx); + }); + cx.executor().run_until_parked(); + + assert_eq!( + server_document.lock().as_str(), + "fn main() {\r\n let a = 5;\r\n let a2 = 55;\r\n let b = 6;\r\n let c = 7;\r\n}\r\n", + "switching back to CRLF should resync the whole document with CRLF line endings", + ); +} + #[gpui::test(iterations = 10)] async fn test_definition(cx: &mut gpui::TestAppContext) { init_test(cx); diff --git a/crates/text/src/text.rs b/crates/text/src/text.rs index e15512f3e2d..280d1bda7d3 100644 --- a/crates/text/src/text.rs +++ b/crates/text/src/text.rs @@ -2194,6 +2194,10 @@ impl BufferSnapshot { self.visible_text.to_string() } + pub fn text_with_line_endings(&self) -> String { + chunks_with_line_ending(&self.visible_text, self.line_ending).collect() + } + pub fn line_ending(&self) -> LineEnding { self.line_ending } @@ -3643,6 +3647,25 @@ impl LineEnding { text } } + + /// Converts `text` to use this line ending. + /// + /// Detects the existing line ending of `text` first; if it already matches + /// `self`, the string is returned unchanged. Mixed line endings are not + /// supported: detection is based on the first newline found. + pub fn apply(&self, text: String) -> String { + match (LineEnding::detect(&text), self) { + (LineEnding::Unix, LineEnding::Unix) | (LineEnding::Windows, LineEnding::Windows) => { + text + } + (LineEnding::Unix, LineEnding::Windows) => text.replace('\n', "\r\n"), + (LineEnding::Windows, LineEnding::Unix) => { + let mut result = text; + LineEnding::normalize(&mut result); + result + } + } + } } pub fn chunks_with_line_ending(rope: &Rope, line_ending: LineEnding) -> impl Iterator {