diff --git a/assets/settings/default.json b/assets/settings/default.json index 00866af2ca..c52823278f 100644 --- a/assets/settings/default.json +++ b/assets/settings/default.json @@ -51,6 +51,8 @@ // 3. Position the dock full screen over the entire workspace" // "default_dock_anchor": "expanded" "default_dock_anchor": "right", + // Whether or not to remove trailing whitespace from lines before saving. + "remove_trailing_whitespace_on_save": true, // Whether or not to perform a buffer format before saving "format_on_save": "on", // How to perform a buffer format. This setting can take two values: diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 8589bbcc79..253345c93f 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -1,23 +1,23 @@ -use drag_and_drop::DragAndDrop; -use futures::StreamExt; -use indoc::indoc; -use std::{cell::RefCell, rc::Rc, time::Instant}; -use unindent::Unindent; - use super::*; use crate::test::{ assert_text_with_selections, build_editor, editor_lsp_test_context::EditorLspTestContext, editor_test_context::EditorTestContext, select_ranges, }; +use drag_and_drop::DragAndDrop; +use futures::StreamExt; use gpui::{ executor::Deterministic, geometry::{rect::RectF, vector::vec2f}, platform::{WindowBounds, WindowOptions}, serde_json, }; +use indoc::indoc; use language::{BracketPairConfig, FakeLspAdapter, LanguageConfig, LanguageRegistry, Point}; +use parking_lot::Mutex; use project::FakeFs; use settings::EditorSettings; +use std::{cell::RefCell, rc::Rc, time::Instant}; +use unindent::Unindent; use util::{ assert_set_eq, test::{marked_text_ranges, marked_text_ranges_by, sample_text, TextRangeMarker}, @@ -4301,6 +4301,111 @@ async fn test_concurrent_format_requests(cx: &mut gpui::TestAppContext) { "}); } +#[gpui::test] +async fn test_strip_whitespace_and_format_via_lsp(cx: &mut gpui::TestAppContext) { + cx.foreground().forbid_parking(); + + let mut cx = EditorLspTestContext::new_rust( + lsp::ServerCapabilities { + document_formatting_provider: Some(lsp::OneOf::Left(true)), + ..Default::default() + }, + cx, + ) + .await; + + // Set up a buffer white some trailing whitespace. + cx.set_state( + &[ + "one ", // + "twoˇ", // + "three ", // + ] + .join("\n"), + ); + + // Submit a format request. + let format = cx + .update_editor(|editor, cx| editor.format(&Format, cx)) + .unwrap(); + + // Record which buffer changes have been sent to the language server + let buffer_changes = Arc::new(Mutex::new(Vec::new())); + cx.lsp + .handle_notification::({ + let buffer_changes = buffer_changes.clone(); + move |params, _| { + buffer_changes.lock().extend( + params + .content_changes + .into_iter() + .map(|e| (e.range.unwrap(), e.text)), + ); + } + }); + + // Handle formatting requests to the language server. + cx.lsp.handle_request::({ + let buffer_changes = buffer_changes.clone(); + move |_, _| { + // When formatting is requested, trailing whitespace has already been stripped. + assert_eq!( + &buffer_changes.lock()[1..], + &[ + ( + lsp::Range::new(lsp::Position::new(0, 3), lsp::Position::new(0, 4)), + "".into() + ), + ( + lsp::Range::new(lsp::Position::new(2, 5), lsp::Position::new(2, 6)), + "".into() + ), + ] + ); + + // Insert blank lines between each line of the buffer. + async move { + Ok(Some(vec![ + lsp::TextEdit { + range: lsp::Range::new(lsp::Position::new(1, 0), lsp::Position::new(1, 0)), + new_text: "\n".into(), + }, + lsp::TextEdit { + range: lsp::Range::new(lsp::Position::new(2, 0), lsp::Position::new(2, 0)), + new_text: "\n".into(), + }, + ])) + } + } + }); + + // After formatting the buffer, the trailing whitespace is stripped and the + // edits provided by the language server have been applied. + format.await.unwrap(); + cx.assert_editor_state( + &[ + "one", // + "", // + "twoˇ", // + "", // + "three", // + ] + .join("\n"), + ); + + // Undoing the formatting undoes both the trailing whitespace removal and the + // LSP edits. + cx.update_buffer(|buffer, cx| buffer.undo(cx)); + cx.assert_editor_state( + &[ + "one ", // + "twoˇ", // + "three ", // + ] + .join("\n"), + ); +} + #[gpui::test] async fn test_completion(cx: &mut gpui::TestAppContext) { let mut cx = EditorLspTestContext::new_rust( diff --git a/crates/editor/src/test/editor_test_context.rs b/crates/editor/src/test/editor_test_context.rs index 8f8647e88d..9e66fea8df 100644 --- a/crates/editor/src/test/editor_test_context.rs +++ b/crates/editor/src/test/editor_test_context.rs @@ -185,6 +185,7 @@ impl<'a> EditorTestContext<'a> { /// of its selections using a string containing embedded range markers. /// /// See the `util::test::marked_text_ranges` function for more information. + #[track_caller] pub fn assert_editor_state(&mut self, marked_text: &str) { let (unmarked_text, expected_selections) = marked_text_ranges(marked_text, true); let buffer_text = self.buffer_text(); diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index 95bb514dd4..a7e39ff8b6 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -1154,7 +1154,7 @@ impl Buffer { }) } - pub fn normalize_whitespace(&self, cx: &AppContext) -> Task { + pub fn remove_trailing_whitespace(&self, cx: &AppContext) -> Task { let old_text = self.as_rope().clone(); let line_ending = self.line_ending(); let base_version = self.version(); @@ -1189,7 +1189,7 @@ impl Buffer { &mut self, diff: Diff, cx: &mut ModelContext, - ) -> Option<&Transaction> { + ) -> Option { // Check for any edits to the buffer that have occurred since this diff // was computed. let snapshot = self.snapshot(); @@ -1219,12 +1219,10 @@ impl Buffer { Some((start..end, new_text)) }); - self.finalize_last_transaction(); self.start_transaction(); self.text.set_line_ending(diff.line_ending); self.edit(adjusted_edits, None, cx); - self.end_transaction(cx)?; - self.finalize_last_transaction() + self.end_transaction(cx) } pub fn is_dirty(&self) -> bool { diff --git a/crates/language/src/buffer_tests.rs b/crates/language/src/buffer_tests.rs index 6bdff1ea28..9212534959 100644 --- a/crates/language/src/buffer_tests.rs +++ b/crates/language/src/buffer_tests.rs @@ -235,7 +235,7 @@ async fn test_normalize_whitespace(cx: &mut gpui::TestAppContext) { // Spawn a task to format the buffer's whitespace. // Pause so that the foratting task starts running. - let format = buffer.read_with(cx, |buffer, cx| buffer.normalize_whitespace(cx)); + let format = buffer.read_with(cx, |buffer, cx| buffer.remove_trailing_whitespace(cx)); smol::future::yield_now().await; // Edit the buffer while the normalization task is running. diff --git a/crates/lsp/src/lsp.rs b/crates/lsp/src/lsp.rs index 660528daf1..6982445982 100644 --- a/crates/lsp/src/lsp.rs +++ b/crates/lsp/src/lsp.rs @@ -422,6 +422,10 @@ impl LanguageServer { self.notification_handlers.lock().remove(T::METHOD); } + pub fn remove_notification_handler(&self) { + self.notification_handlers.lock().remove(T::METHOD); + } + #[must_use] pub fn on_custom_notification(&self, method: &'static str, mut f: F) -> Subscription where @@ -780,6 +784,26 @@ impl FakeLanguageServer { responded_rx } + pub fn handle_notification( + &self, + mut handler: F, + ) -> futures::channel::mpsc::UnboundedReceiver<()> + where + T: 'static + notification::Notification, + T::Params: 'static + Send, + F: 'static + Send + FnMut(T::Params, gpui::AsyncAppContext), + { + let (handled_tx, handled_rx) = futures::channel::mpsc::unbounded(); + self.server.remove_notification_handler::(); + self.server + .on_notification::(move |params, cx| { + handler(params, cx.clone()); + handled_tx.unbounded_send(()).ok(); + }) + .detach(); + handled_rx + } + pub fn remove_request_handler(&mut self) where T: 'static + request::Request, diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 8ed37a003c..d238009559 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -26,7 +26,7 @@ use language::{ serialize_anchor, serialize_version, }, range_from_lsp, range_to_lsp, Anchor, Bias, Buffer, CachedLspAdapter, CharKind, CodeAction, - CodeLabel, Completion, Diagnostic, DiagnosticEntry, DiagnosticSet, Event as BufferEvent, + CodeLabel, Completion, Diagnostic, DiagnosticEntry, DiagnosticSet, Diff, Event as BufferEvent, File as _, Language, LanguageRegistry, LanguageServerName, LocalFile, OffsetRangeExt, Operation, Patch, PointUtf16, RopeFingerprint, TextBufferSnapshot, ToOffset, ToPointUtf16, Transaction, Unclipped, @@ -2887,38 +2887,68 @@ impl Project { let mut project_transaction = ProjectTransaction::default(); for (buffer, buffer_abs_path, language_server) in &buffers_with_paths_and_servers { - let (format_on_save, formatter, tab_size) = - buffer.read_with(&cx, |buffer, cx| { + let (format_on_save, remove_trailing_whitespace, formatter, tab_size) = buffer + .read_with(&cx, |buffer, cx| { let settings = cx.global::(); let language_name = buffer.language().map(|language| language.name()); ( settings.format_on_save(language_name.as_deref()), + settings + .remove_trailing_whitespace_on_save(language_name.as_deref()), settings.formatter(language_name.as_deref()), settings.tab_size(language_name.as_deref()), ) }); - let transaction = match (formatter, format_on_save) { - (_, FormatOnSave::Off) if trigger == FormatTrigger::Save => continue, + let whitespace_transaction_id = if remove_trailing_whitespace { + let diff = buffer + .read_with(&cx, |buffer, cx| buffer.remove_trailing_whitespace(cx)) + .await; + buffer.update(&mut cx, move |buffer, cx| { + buffer.finalize_last_transaction(); + buffer.apply_diff_force(diff, cx) + }) + } else { + None + }; + + match (formatter, format_on_save) { + (_, FormatOnSave::Off) if trigger == FormatTrigger::Save => {} (Formatter::LanguageServer, FormatOnSave::On | FormatOnSave::Off) - | (_, FormatOnSave::LanguageServer) => Self::format_via_lsp( - &this, - &buffer, - &buffer_abs_path, - &language_server, - tab_size, - &mut cx, - ) - .await - .context("failed to format via language server")?, + | (_, FormatOnSave::LanguageServer) => { + let edits = Self::format_via_lsp( + &this, + &buffer, + &buffer_abs_path, + &language_server, + tab_size, + &mut cx, + ) + .await + .context("failed to format via language server")?; + + buffer.update(&mut cx, |buffer, cx| { + if let Some(tx_id) = whitespace_transaction_id { + if buffer + .peek_undo_stack() + .map_or(false, |e| e.transaction_id() == tx_id) + { + buffer.edit(edits, None, cx); + } + buffer.group_until_transaction(tx_id); + } else { + buffer.edit(edits, None, cx); + } + }); + } ( Formatter::External { command, arguments }, FormatOnSave::On | FormatOnSave::Off, ) | (_, FormatOnSave::External { command, arguments }) => { - Self::format_via_external_command( + let diff = Self::format_via_external_command( &buffer, &buffer_abs_path, &command, @@ -2929,10 +2959,30 @@ impl Project { .context(format!( "failed to format via external command {:?}", command - ))? + ))?; + + if let Some(diff) = diff { + buffer.update(&mut cx, |buffer, cx| { + if let Some(tx_id) = whitespace_transaction_id { + if buffer + .peek_undo_stack() + .map_or(false, |e| e.transaction_id() == tx_id) + { + buffer.apply_diff(diff, cx); + } + buffer.group_until_transaction(tx_id); + } else { + buffer.apply_diff(diff, cx); + } + }); + } } }; + let transaction = buffer.update(&mut cx, |buffer, _| { + buffer.finalize_last_transaction().cloned() + }); + if let Some(transaction) = transaction { if !push_to_history { buffer.update(&mut cx, |buffer, _| { @@ -2981,7 +3031,7 @@ impl Project { language_server: &Arc, tab_size: NonZeroU32, cx: &mut AsyncAppContext, - ) -> Result> { + ) -> Result, String)>> { let text_document = lsp::TextDocumentIdentifier::new(lsp::Url::from_file_path(abs_path).unwrap()); let capabilities = &language_server.capabilities(); @@ -3028,26 +3078,12 @@ impl Project { }; if let Some(lsp_edits) = lsp_edits { - let edits = this - .update(cx, |this, cx| { - this.edits_from_lsp(buffer, lsp_edits, None, cx) - }) - .await?; - buffer.update(cx, |buffer, cx| { - buffer.finalize_last_transaction(); - buffer.start_transaction(); - for (range, text) in edits { - buffer.edit([(range, text)], None, cx); - } - if buffer.end_transaction(cx).is_some() { - let transaction = buffer.finalize_last_transaction().unwrap().clone(); - Ok(Some(transaction)) - } else { - Ok(None) - } + this.update(cx, |this, cx| { + this.edits_from_lsp(buffer, lsp_edits, None, cx) }) + .await } else { - Ok(None) + Ok(Default::default()) } } @@ -3057,7 +3093,7 @@ impl Project { command: &str, arguments: &[String], cx: &mut AsyncAppContext, - ) -> Result> { + ) -> Result> { let working_dir_path = buffer.read_with(cx, |buffer, cx| { let file = File::from_dyn(buffer.file())?; let worktree = file.worktree.read(cx).as_local()?; @@ -3100,10 +3136,11 @@ impl Project { } let stdout = String::from_utf8(output.stdout)?; - let diff = buffer - .read_with(cx, |buffer, cx| buffer.diff(stdout, cx)) - .await; - Ok(buffer.update(cx, |buffer, cx| buffer.apply_diff(diff, cx).cloned())) + Ok(Some( + buffer + .read_with(cx, |buffer, cx| buffer.diff(stdout, cx)) + .await, + )) } else { Ok(None) } diff --git a/crates/settings/src/settings.rs b/crates/settings/src/settings.rs index 21939b26b0..44ef3d774d 100644 --- a/crates/settings/src/settings.rs +++ b/crates/settings/src/settings.rs @@ -94,6 +94,7 @@ pub struct EditorSettings { pub soft_wrap: Option, pub preferred_line_length: Option, pub format_on_save: Option, + pub remove_trailing_whitespace_on_save: Option, pub formatter: Option, pub enable_language_server: Option, } @@ -361,6 +362,9 @@ impl Settings { hard_tabs: required(defaults.editor.hard_tabs), soft_wrap: required(defaults.editor.soft_wrap), preferred_line_length: required(defaults.editor.preferred_line_length), + remove_trailing_whitespace_on_save: required( + defaults.editor.remove_trailing_whitespace_on_save, + ), format_on_save: required(defaults.editor.format_on_save), formatter: required(defaults.editor.formatter), enable_language_server: required(defaults.editor.enable_language_server), @@ -460,6 +464,12 @@ impl Settings { self.language_setting(language, |settings| settings.preferred_line_length) } + pub fn remove_trailing_whitespace_on_save(&self, language: Option<&str>) -> bool { + self.language_setting(language, |settings| { + settings.remove_trailing_whitespace_on_save.clone() + }) + } + pub fn format_on_save(&self, language: Option<&str>) -> FormatOnSave { self.language_setting(language, |settings| settings.format_on_save.clone()) } @@ -558,6 +568,7 @@ impl Settings { hard_tabs: Some(false), soft_wrap: Some(SoftWrap::None), preferred_line_length: Some(80), + remove_trailing_whitespace_on_save: Some(true), format_on_save: Some(FormatOnSave::On), formatter: Some(Formatter::LanguageServer), enable_language_server: Some(true),