From b00e467ede67d8205707f0a9aff64b3818d1763b Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 23 Feb 2023 19:45:48 -0800 Subject: [PATCH 01/13] Add APIs for stripping trailing whitespace from a buffer --- crates/language/src/buffer.rs | 122 +++++++++++++++++++++++++--- crates/language/src/buffer_tests.rs | 120 +++++++++++++++++++++++++++ 2 files changed, 229 insertions(+), 13 deletions(-) diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index 03d2802591..95bb514dd4 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -305,7 +305,7 @@ pub struct Chunk<'a> { } pub struct Diff { - base_version: clock::Global, + pub(crate) base_version: clock::Global, line_ending: LineEnding, edits: Vec<(Range, Arc)>, } @@ -1154,20 +1154,77 @@ impl Buffer { }) } - pub fn apply_diff(&mut self, diff: Diff, cx: &mut ModelContext) -> Option<&Transaction> { - if self.version == diff.base_version { - self.finalize_last_transaction(); - self.start_transaction(); - self.text.set_line_ending(diff.line_ending); - self.edit(diff.edits, None, cx); - if self.end_transaction(cx).is_some() { - self.finalize_last_transaction() - } else { - None + pub fn normalize_whitespace(&self, cx: &AppContext) -> Task { + let old_text = self.as_rope().clone(); + let line_ending = self.line_ending(); + let base_version = self.version(); + cx.background().spawn(async move { + let ranges = trailing_whitespace_ranges(&old_text); + let empty = Arc::::from(""); + Diff { + base_version, + line_ending, + edits: ranges + .into_iter() + .map(|range| (range, empty.clone())) + .collect(), } - } else { - None + }) + } + + pub fn apply_diff(&mut self, diff: Diff, cx: &mut ModelContext) -> Option<&Transaction> { + if self.version != diff.base_version { + return None; } + + self.finalize_last_transaction(); + self.start_transaction(); + self.text.set_line_ending(diff.line_ending); + self.edit(diff.edits, None, cx); + self.end_transaction(cx)?; + self.finalize_last_transaction() + } + + pub fn apply_diff_force( + &mut self, + diff: Diff, + cx: &mut ModelContext, + ) -> Option<&Transaction> { + // Check for any edits to the buffer that have occurred since this diff + // was computed. + let snapshot = self.snapshot(); + let mut edits_since = snapshot.edits_since::(&diff.base_version).peekable(); + let mut delta = 0; + let adjusted_edits = diff.edits.into_iter().filter_map(|(range, new_text)| { + while let Some(edit_since) = edits_since.peek() { + // If the edit occurs after a diff hunk, then it can does not + // affect that hunk. + if edit_since.old.start > range.end { + break; + } + // If the edit precedes the diff hunk, then adjust the hunk + // to reflect the edit. + else if edit_since.old.end < range.start { + delta += edit_since.new_len() as i64 - edit_since.old_len() as i64; + edits_since.next(); + } + // If the edit intersects a diff hunk, then discard that hunk. + else { + return None; + } + } + + let start = (range.start as i64 + delta) as usize; + let end = (range.end as i64 + delta) as usize; + 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() } pub fn is_dirty(&self) -> bool { @@ -2840,3 +2897,42 @@ pub fn char_kind(c: char) -> CharKind { CharKind::Punctuation } } + +/// Find all of the ranges of whitespace that occur at the ends of lines +/// in the given rope. +/// +/// This could also be done with a regex search, but this implementation +/// avoids copying text. +pub fn trailing_whitespace_ranges(rope: &Rope) -> Vec> { + let mut ranges = Vec::new(); + + let mut offset = 0; + let mut prev_chunk_trailing_whitespace_range = 0..0; + for chunk in rope.chunks() { + let mut prev_line_trailing_whitespace_range = 0..0; + for (i, line) in chunk.split('\n').enumerate() { + let line_end_offset = offset + line.len(); + let trimmed_line_len = line.trim_end_matches(|c| matches!(c, ' ' | '\t')).len(); + let mut trailing_whitespace_range = (offset + trimmed_line_len)..line_end_offset; + + if i == 0 && trimmed_line_len == 0 { + trailing_whitespace_range.start = prev_chunk_trailing_whitespace_range.start; + } + if !prev_line_trailing_whitespace_range.is_empty() { + ranges.push(prev_line_trailing_whitespace_range); + } + + offset = line_end_offset + 1; + prev_line_trailing_whitespace_range = trailing_whitespace_range; + } + + offset -= 1; + prev_chunk_trailing_whitespace_range = prev_line_trailing_whitespace_range; + } + + if !prev_chunk_trailing_whitespace_range.is_empty() { + ranges.push(prev_chunk_trailing_whitespace_range); + } + + ranges +} diff --git a/crates/language/src/buffer_tests.rs b/crates/language/src/buffer_tests.rs index 36add5f1f3..6bdff1ea28 100644 --- a/crates/language/src/buffer_tests.rs +++ b/crates/language/src/buffer_tests.rs @@ -6,6 +6,7 @@ use gpui::{ModelHandle, MutableAppContext}; use indoc::indoc; use proto::deserialize_operation; use rand::prelude::*; +use regex::RegexBuilder; use settings::Settings; use std::{ cell::RefCell, @@ -18,6 +19,13 @@ use text::network::Network; use unindent::Unindent as _; use util::{assert_set_eq, post_inc, test::marked_text_ranges, RandomCharIter}; +lazy_static! { + static ref TRAILING_WHITESPACE_REGEX: Regex = RegexBuilder::new("[ \t]+$") + .multi_line(true) + .build() + .unwrap(); +} + #[cfg(test)] #[ctor::ctor] fn init_logger() { @@ -211,6 +219,79 @@ async fn test_apply_diff(cx: &mut gpui::TestAppContext) { }); } +#[gpui::test(iterations = 10)] +async fn test_normalize_whitespace(cx: &mut gpui::TestAppContext) { + let text = [ + "zero", // + "one ", // 2 trailing spaces + "two", // + "three ", // 3 trailing spaces + "four", // + "five ", // 4 trailing spaces + ] + .join("\n"); + + let buffer = cx.add_model(|cx| Buffer::new(0, text, cx)); + + // 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)); + smol::future::yield_now().await; + + // Edit the buffer while the normalization task is running. + let version_before_edit = buffer.read_with(cx, |buffer, _| buffer.version()); + buffer.update(cx, |buffer, cx| { + buffer.edit( + [ + (Point::new(0, 1)..Point::new(0, 1), "EE"), + (Point::new(3, 5)..Point::new(3, 5), "EEE"), + ], + None, + cx, + ); + }); + + let format_diff = format.await; + buffer.update(cx, |buffer, cx| { + let version_before_format = format_diff.base_version.clone(); + buffer.apply_diff_force(format_diff, cx); + + // The outcome depends on the order of concurrent taks. + // + // If the edit occurred while searching for trailing whitespace ranges, + // then the trailing whitespace region touched by the edit is left intact. + if version_before_format == version_before_edit { + assert_eq!( + buffer.text(), + [ + "zEEero", // + "one", // + "two", // + "threeEEE ", // + "four", // + "five", // + ] + .join("\n") + ); + } + // Otherwise, all trailing whitespace is removed. + else { + assert_eq!( + buffer.text(), + [ + "zEEero", // + "one", // + "two", // + "threeEEE", // + "four", // + "five", // + ] + .join("\n") + ); + } + }); +} + #[gpui::test] async fn test_reparse(cx: &mut gpui::TestAppContext) { let text = "fn a() {}"; @@ -1943,6 +2024,45 @@ fn test_contiguous_ranges() { ); } +#[gpui::test(iterations = 500)] +fn test_trailing_whitespace_ranges(mut rng: StdRng) { + // Generate a random multi-line string containing + // some lines with trailing whitespace. + let mut text = String::new(); + for _ in 0..rng.gen_range(0..16) { + for _ in 0..rng.gen_range(0..36) { + text.push(match rng.gen_range(0..10) { + 0..=1 => ' ', + 3 => '\t', + _ => rng.gen_range('a'..'z'), + }); + } + text.push('\n'); + } + + match rng.gen_range(0..10) { + // sometimes remove the last newline + 0..=1 => drop(text.pop()), // + + // sometimes add extra newlines + 2..=3 => text.push_str(&"\n".repeat(rng.gen_range(1..5))), + _ => {} + } + + let rope = Rope::from(text.as_str()); + let actual_ranges = trailing_whitespace_ranges(&rope); + let expected_ranges = TRAILING_WHITESPACE_REGEX + .find_iter(&text) + .map(|m| m.range()) + .collect::>(); + assert_eq!( + actual_ranges, + expected_ranges, + "wrong ranges for text lines:\n{:?}", + text.split("\n").collect::>() + ); +} + fn ruby_lang() -> Language { Language::new( LanguageConfig { From ff85bc6d424e274c72fd1bc4081c3f12544ea2c7 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Mon, 27 Feb 2023 12:14:18 -0800 Subject: [PATCH 02/13] Add setting for removing trailing whitespace on save --- assets/settings/default.json | 2 + crates/editor/src/editor_tests.rs | 117 ++++++++++++++++- crates/editor/src/test/editor_test_context.rs | 1 + crates/language/src/buffer.rs | 8 +- crates/language/src/buffer_tests.rs | 2 +- crates/lsp/src/lsp.rs | 24 ++++ crates/project/src/project.rs | 119 ++++++++++++------ crates/settings/src/settings.rs | 11 ++ 8 files changed, 231 insertions(+), 53 deletions(-) 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), From 7faa0da5c7ad8f5fc6faf8666103988573bb8709 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Mon, 27 Feb 2023 13:33:20 -0800 Subject: [PATCH 03/13] Avoid finalizing transactions inside Buffer::apply_diff --- crates/language/src/buffer.rs | 19 ++++++++----------- crates/language/src/buffer_tests.rs | 2 +- crates/project/src/project.rs | 2 +- 3 files changed, 10 insertions(+), 13 deletions(-) diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index a7e39ff8b6..ed09ffbff2 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -569,7 +569,9 @@ impl Buffer { .read_with(&cx, |this, cx| this.diff(new_text, cx)) .await; this.update(&mut cx, |this, cx| { - if let Some(transaction) = this.apply_diff(diff, cx).cloned() { + this.finalize_last_transaction(); + this.apply_diff(diff, cx); + if let Some(transaction) = this.finalize_last_transaction().cloned() { this.did_reload( this.version(), this.as_rope().fingerprint(), @@ -1172,20 +1174,15 @@ impl Buffer { }) } - pub fn apply_diff(&mut self, diff: Diff, cx: &mut ModelContext) -> Option<&Transaction> { - if self.version != diff.base_version { + pub fn apply_diff(&mut self, diff: Diff, cx: &mut ModelContext) -> Option { + if self.version == diff.base_version { + self.apply_non_conflicting_portion_of_diff(diff, cx) + } else { return None; } - - self.finalize_last_transaction(); - self.start_transaction(); - self.text.set_line_ending(diff.line_ending); - self.edit(diff.edits, None, cx); - self.end_transaction(cx)?; - self.finalize_last_transaction() } - pub fn apply_diff_force( + pub fn apply_non_conflicting_portion_of_diff( &mut self, diff: Diff, cx: &mut ModelContext, diff --git a/crates/language/src/buffer_tests.rs b/crates/language/src/buffer_tests.rs index 9212534959..c7fc25d793 100644 --- a/crates/language/src/buffer_tests.rs +++ b/crates/language/src/buffer_tests.rs @@ -254,7 +254,7 @@ async fn test_normalize_whitespace(cx: &mut gpui::TestAppContext) { let format_diff = format.await; buffer.update(cx, |buffer, cx| { let version_before_format = format_diff.base_version.clone(); - buffer.apply_diff_force(format_diff, cx); + buffer.apply_non_conflicting_portion_of_diff(format_diff, cx); // The outcome depends on the order of concurrent taks. // diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index d238009559..9ad29691c5 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -2906,7 +2906,7 @@ impl Project { .await; buffer.update(&mut cx, move |buffer, cx| { buffer.finalize_last_transaction(); - buffer.apply_diff_force(diff, cx) + buffer.apply_non_conflicting_portion_of_diff(diff, cx) }) } else { None From a890b8f3b7bba2d28497d032115dcbb3f0288416 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Mon, 27 Feb 2023 14:32:08 -0800 Subject: [PATCH 04/13] Add a setting for ensuring a single final newline on save --- assets/settings/default.json | 6 ++++- crates/editor/src/editor_tests.rs | 22 +++++++++++----- crates/language/src/buffer.rs | 26 +++++++++++++++++++ crates/project/src/project.rs | 43 +++++++++++++++++++++---------- crates/settings/src/settings.rs | 11 ++++++++ 5 files changed, 88 insertions(+), 20 deletions(-) diff --git a/assets/settings/default.json b/assets/settings/default.json index c52823278f..2a5e05b401 100644 --- a/assets/settings/default.json +++ b/assets/settings/default.json @@ -51,8 +51,12 @@ // 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. + // Whether or not to remove any trailing whitespace from lines of a buffer + // before saving it. "remove_trailing_whitespace_on_save": true, + // Whether or not to ensure there's a single newline at the end of a buffer + // when saving it. + "ensure_final_newline_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 253345c93f..269390f72f 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -4314,12 +4314,13 @@ async fn test_strip_whitespace_and_format_via_lsp(cx: &mut gpui::TestAppContext) ) .await; - // Set up a buffer white some trailing whitespace. + // Set up a buffer white some trailing whitespace and no trailing newline. cx.set_state( &[ "one ", // "twoˇ", // "three ", // + "four", // ] .join("\n"), ); @@ -4348,7 +4349,8 @@ async fn test_strip_whitespace_and_format_via_lsp(cx: &mut gpui::TestAppContext) cx.lsp.handle_request::({ let buffer_changes = buffer_changes.clone(); move |_, _| { - // When formatting is requested, trailing whitespace has already been stripped. + // When formatting is requested, trailing whitespace has already been stripped, + // and the trailing newline has already been added. assert_eq!( &buffer_changes.lock()[1..], &[ @@ -4360,6 +4362,10 @@ async fn test_strip_whitespace_and_format_via_lsp(cx: &mut gpui::TestAppContext) lsp::Range::new(lsp::Position::new(2, 5), lsp::Position::new(2, 6)), "".into() ), + ( + lsp::Range::new(lsp::Position::new(3, 4), lsp::Position::new(3, 4)), + "\n".into() + ), ] ); @@ -4379,8 +4385,9 @@ async fn test_strip_whitespace_and_format_via_lsp(cx: &mut gpui::TestAppContext) } }); - // After formatting the buffer, the trailing whitespace is stripped and the - // edits provided by the language server have been applied. + // After formatting the buffer, the trailing whitespace is stripped, + // a newline is appended, and the edits provided by the language server + // have been applied. format.await.unwrap(); cx.assert_editor_state( &[ @@ -4389,18 +4396,21 @@ async fn test_strip_whitespace_and_format_via_lsp(cx: &mut gpui::TestAppContext) "twoˇ", // "", // "three", // + "four", // + "", // ] .join("\n"), ); - // Undoing the formatting undoes both the trailing whitespace removal and the - // LSP edits. + // Undoing the formatting undoes the trailing whitespace removal, the + // trailing newline, and the LSP edits. cx.update_buffer(|buffer, cx| buffer.undo(cx)); cx.assert_editor_state( &[ "one ", // "twoˇ", // "three ", // + "four", // ] .join("\n"), ); diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index ed09ffbff2..5402709124 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -1156,6 +1156,8 @@ impl Buffer { }) } + /// Spawn a background task that searches the buffer for any whitespace + /// at the ends of a lines, and returns a `Diff` that removes that whitespace. pub fn remove_trailing_whitespace(&self, cx: &AppContext) -> Task { let old_text = self.as_rope().clone(); let line_ending = self.line_ending(); @@ -1174,6 +1176,27 @@ impl Buffer { }) } + /// Ensure that the buffer ends with a single newline character, and + /// no other whitespace. + pub fn ensure_final_newline(&mut self, cx: &mut ModelContext) { + let len = self.len(); + let mut offset = len; + for chunk in self.as_rope().reversed_chunks_in_range(0..len) { + let non_whitespace_len = chunk + .trim_end_matches(|c: char| c.is_ascii_whitespace()) + .len(); + offset -= chunk.len(); + offset += non_whitespace_len; + if non_whitespace_len != 0 { + if offset == len - 1 && chunk.get(non_whitespace_len..) == Some("\n") { + return; + } + break; + } + } + self.edit([(offset..len, "\n")], None, cx); + } + pub fn apply_diff(&mut self, diff: Diff, cx: &mut ModelContext) -> Option { if self.version == diff.base_version { self.apply_non_conflicting_portion_of_diff(diff, cx) @@ -1182,6 +1205,9 @@ impl Buffer { } } + /// Apply a diff to the buffer. If the buffer has changed since the given diff was + /// calculated, then adjust the diff to account for those changes, and discard any + /// parts of the diff that conflict with those changes. pub fn apply_non_conflicting_portion_of_diff( &mut self, diff: Diff, diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 9ad29691c5..8682614e64 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -2887,18 +2887,23 @@ 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, 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 ( + format_on_save, + remove_trailing_whitespace, + ensure_final_newline, + 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.ensure_final_newline_on_save(language_name.as_deref()), + settings.formatter(language_name.as_deref()), + settings.tab_size(language_name.as_deref()), + ) + }); let whitespace_transaction_id = if remove_trailing_whitespace { let diff = buffer @@ -2906,7 +2911,19 @@ impl Project { .await; buffer.update(&mut cx, move |buffer, cx| { buffer.finalize_last_transaction(); - buffer.apply_non_conflicting_portion_of_diff(diff, cx) + buffer.start_transaction(); + buffer.apply_non_conflicting_portion_of_diff(diff, cx); + if ensure_final_newline { + buffer.ensure_final_newline(cx); + } + buffer.end_transaction(cx) + }) + } else if ensure_final_newline { + buffer.update(&mut cx, move |buffer, cx| { + buffer.finalize_last_transaction(); + buffer.start_transaction(); + buffer.ensure_final_newline(cx); + buffer.end_transaction(cx) }) } else { None diff --git a/crates/settings/src/settings.rs b/crates/settings/src/settings.rs index 44ef3d774d..6b51b06c9c 100644 --- a/crates/settings/src/settings.rs +++ b/crates/settings/src/settings.rs @@ -95,6 +95,7 @@ pub struct EditorSettings { pub preferred_line_length: Option, pub format_on_save: Option, pub remove_trailing_whitespace_on_save: Option, + pub ensure_final_newline_on_save: Option, pub formatter: Option, pub enable_language_server: Option, } @@ -365,6 +366,9 @@ impl Settings { remove_trailing_whitespace_on_save: required( defaults.editor.remove_trailing_whitespace_on_save, ), + ensure_final_newline_on_save: required( + defaults.editor.ensure_final_newline_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), @@ -470,6 +474,12 @@ impl Settings { }) } + pub fn ensure_final_newline_on_save(&self, language: Option<&str>) -> bool { + self.language_setting(language, |settings| { + settings.ensure_final_newline_on_save.clone() + }) + } + pub fn format_on_save(&self, language: Option<&str>) -> FormatOnSave { self.language_setting(language, |settings| settings.format_on_save.clone()) } @@ -569,6 +579,7 @@ impl Settings { soft_wrap: Some(SoftWrap::None), preferred_line_length: Some(80), remove_trailing_whitespace_on_save: Some(true), + ensure_final_newline_on_save: Some(true), format_on_save: Some(FormatOnSave::On), formatter: Some(Formatter::LanguageServer), enable_language_server: Some(true), From 1deff43639f012b2400d6f2a8cc3f16aa057da26 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 28 Feb 2023 08:27:15 -0800 Subject: [PATCH 05/13] Avoid calling edits_since in apply_diff --- crates/language/src/buffer.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index 5402709124..5d7e4325d1 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -1199,7 +1199,10 @@ impl Buffer { pub fn apply_diff(&mut self, diff: Diff, cx: &mut ModelContext) -> Option { if self.version == diff.base_version { - self.apply_non_conflicting_portion_of_diff(diff, cx) + self.start_transaction(); + self.text.set_line_ending(diff.line_ending); + self.edit(diff.edits, None, cx); + self.end_transaction(cx) } else { return None; } From e7b56f6342efc58614c2444dc6f886a542ba45c9 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 28 Feb 2023 08:34:38 -0800 Subject: [PATCH 06/13] adjust buffer-formatting assertion to reflect final newline addition --- crates/collab/src/tests/integration_tests.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs index 4c8f978164..1ab78ac310 100644 --- a/crates/collab/src/tests/integration_tests.rs +++ b/crates/collab/src/tests/integration_tests.rs @@ -3892,9 +3892,11 @@ async fn test_formatting_buffer( }) .await .unwrap(); + + // The edits from the LSP are applied, and a final newline is added. assert_eq!( buffer_b.read_with(cx_b, |buffer, _| buffer.text()), - "let honey = \"two\"" + "let honey = \"two\"\n" ); // Ensure buffer can be formatted using an external command. Notice how the From 368d2a73ea4c515dffbf54600b5e2a1491b564a2 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 28 Feb 2023 11:01:42 -0800 Subject: [PATCH 07/13] Perform whitespace formatting regardless of whether buffer has a language server or path --- crates/project/src/project.rs | 187 ++++++++++++++++++---------------- 1 file changed, 100 insertions(+), 87 deletions(-) diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 8682614e64..cadbde0bac 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -2858,9 +2858,11 @@ impl Project { .filter_map(|buffer_handle| { let buffer = buffer_handle.read(cx); let file = File::from_dyn(buffer.file())?; - let buffer_abs_path = file.as_local()?.abs_path(cx); - let (_, server) = self.language_server_for_buffer(buffer, cx)?; - Some((buffer_handle, buffer_abs_path, server.clone())) + let buffer_abs_path = file.as_local().map(|f| f.abs_path(cx)); + let server = self + .language_server_for_buffer(buffer, cx) + .map(|s| s.1.clone()); + Some((buffer_handle, buffer_abs_path, server)) }) .collect::>(); @@ -2875,10 +2877,10 @@ impl Project { let _cleanup = defer({ let this = this.clone(); let mut cx = cx.clone(); - let local_buffers = &buffers_with_paths_and_servers; + let buffers = &buffers_with_paths_and_servers; move || { this.update(&mut cx, |this, _| { - for (buffer, _, _) in local_buffers { + for (buffer, _, _) in buffers { this.buffers_being_formatted.remove(&buffer.id()); } }); @@ -2905,59 +2907,59 @@ impl Project { ) }); - 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.start_transaction(); - buffer.apply_non_conflicting_portion_of_diff(diff, cx); - if ensure_final_newline { - buffer.ensure_final_newline(cx); - } - buffer.end_transaction(cx) - }) - } else if ensure_final_newline { - buffer.update(&mut cx, move |buffer, cx| { - buffer.finalize_last_transaction(); - buffer.start_transaction(); - buffer.ensure_final_newline(cx); - buffer.end_transaction(cx) - }) + // First, format buffer's whitespace according to the settings. + let trailing_whitespace_diff = if remove_trailing_whitespace { + Some( + buffer + .read_with(&cx, |b, cx| b.remove_trailing_whitespace(cx)) + .await, + ) } else { None }; + let whitespace_transaction_id = buffer.update(&mut cx, |buffer, cx| { + buffer.finalize_last_transaction(); + buffer.start_transaction(); + if let Some(diff) = trailing_whitespace_diff { + buffer.apply_non_conflicting_portion_of_diff(diff, cx); + } + if ensure_final_newline { + buffer.ensure_final_newline(cx); + } + buffer.end_transaction(cx) + }); + // Currently, formatting operations are represented differently depending on + // whether they come from a language server or an external command. + enum FormatOperation { + Lsp(Vec<(Range, String)>), + External(Diff), + } + + // Apply language-specific formatting using either a language server + // or external command. + let mut format_operation = None; match (formatter, format_on_save) { (_, FormatOnSave::Off) if trigger == FormatTrigger::Save => {} (Formatter::LanguageServer, FormatOnSave::On | FormatOnSave::Off) | (_, 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); - } - }); + if let Some((language_server, buffer_abs_path)) = + language_server.as_ref().zip(buffer_abs_path.as_ref()) + { + format_operation = Some(FormatOperation::Lsp( + Self::format_via_lsp( + &this, + &buffer, + buffer_abs_path, + &language_server, + tab_size, + &mut cx, + ) + .await + .context("failed to format via language server")?, + )); + } } ( @@ -2965,49 +2967,60 @@ impl Project { FormatOnSave::On | FormatOnSave::Off, ) | (_, FormatOnSave::External { command, arguments }) => { - let diff = Self::format_via_external_command( - &buffer, - &buffer_abs_path, - &command, - &arguments, - &mut cx, - ) - .await - .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); - } - }); + if let Some(buffer_abs_path) = buffer_abs_path { + format_operation = Self::format_via_external_command( + &buffer, + &buffer_abs_path, + &command, + &arguments, + &mut cx, + ) + .await + .context(format!( + "failed to format via external command {:?}", + command + ))? + .map(FormatOperation::External); } } }; - 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, _| { - buffer.forget_transaction(transaction.id) - }); + buffer.update(&mut cx, |b, cx| { + // If the buffer had its whitespace formatted and was edited while the language-specific + // formatting was being computed, avoid applying the language-specific formatting, because + // it can't be grouped with the whitespace formatting in the undo history. + if let Some(transaction_id) = whitespace_transaction_id { + if b.peek_undo_stack() + .map_or(true, |e| e.transaction_id() != transaction_id) + { + format_operation.take(); + } } - project_transaction.0.insert(buffer.clone(), transaction); - } + + // Apply any language-specific formatting, and group the two formatting operations + // in the buffer's undo history. + if let Some(operation) = format_operation { + match operation { + FormatOperation::Lsp(edits) => { + b.edit(edits, None, cx); + } + FormatOperation::External(diff) => { + b.apply_diff(diff, cx); + } + } + + if let Some(transaction_id) = whitespace_transaction_id { + b.group_until_transaction(transaction_id); + } + } + + if let Some(transaction) = b.finalize_last_transaction().cloned() { + if !push_to_history { + b.forget_transaction(transaction.id); + } + project_transaction.0.insert(buffer.clone(), transaction); + } + }); } Ok(project_transaction) From 70cb2fa8d73ab0b2ccdcd5ba5b01f1c45aafd8ec Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Wed, 1 Mar 2023 10:17:04 -0800 Subject: [PATCH 08/13] Apply external command formatting if buffer has changed while computing it --- crates/language/src/buffer.rs | 46 ++++++++++------------------- crates/language/src/buffer_tests.rs | 2 +- crates/project/src/project.rs | 2 +- 3 files changed, 18 insertions(+), 32 deletions(-) diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index 5d7e4325d1..acccb553d3 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -569,20 +569,21 @@ impl Buffer { .read_with(&cx, |this, cx| this.diff(new_text, cx)) .await; this.update(&mut cx, |this, cx| { - this.finalize_last_transaction(); - this.apply_diff(diff, cx); - if let Some(transaction) = this.finalize_last_transaction().cloned() { - this.did_reload( - this.version(), - this.as_rope().fingerprint(), - this.line_ending(), - new_mtime, - cx, - ); - Ok(Some(transaction)) - } else { - Ok(None) + if this.version() == diff.base_version { + this.finalize_last_transaction(); + this.apply_diff(diff, cx); + if let Some(transaction) = this.finalize_last_transaction().cloned() { + this.did_reload( + this.version(), + this.as_rope().fingerprint(), + this.line_ending(), + new_mtime, + cx, + ); + return Ok(Some(transaction)); + } } + Ok(None) }) } else { Ok(None) @@ -1197,25 +1198,10 @@ impl Buffer { self.edit([(offset..len, "\n")], None, cx); } - pub fn apply_diff(&mut self, diff: Diff, cx: &mut ModelContext) -> Option { - if self.version == diff.base_version { - self.start_transaction(); - self.text.set_line_ending(diff.line_ending); - self.edit(diff.edits, None, cx); - self.end_transaction(cx) - } else { - return None; - } - } - /// Apply a diff to the buffer. If the buffer has changed since the given diff was /// calculated, then adjust the diff to account for those changes, and discard any /// parts of the diff that conflict with those changes. - pub fn apply_non_conflicting_portion_of_diff( - &mut self, - diff: Diff, - cx: &mut ModelContext, - ) -> Option { + pub fn apply_diff(&mut self, diff: Diff, cx: &mut ModelContext) -> Option { // Check for any edits to the buffer that have occurred since this diff // was computed. let snapshot = self.snapshot(); @@ -1223,7 +1209,7 @@ impl Buffer { let mut delta = 0; let adjusted_edits = diff.edits.into_iter().filter_map(|(range, new_text)| { while let Some(edit_since) = edits_since.peek() { - // If the edit occurs after a diff hunk, then it can does not + // If the edit occurs after a diff hunk, then it does not // affect that hunk. if edit_since.old.start > range.end { break; diff --git a/crates/language/src/buffer_tests.rs b/crates/language/src/buffer_tests.rs index c7fc25d793..0c375f0f4e 100644 --- a/crates/language/src/buffer_tests.rs +++ b/crates/language/src/buffer_tests.rs @@ -254,7 +254,7 @@ async fn test_normalize_whitespace(cx: &mut gpui::TestAppContext) { let format_diff = format.await; buffer.update(cx, |buffer, cx| { let version_before_format = format_diff.base_version.clone(); - buffer.apply_non_conflicting_portion_of_diff(format_diff, cx); + buffer.apply_diff(format_diff, cx); // The outcome depends on the order of concurrent taks. // diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index cadbde0bac..0325d07ce6 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -2921,7 +2921,7 @@ impl Project { buffer.finalize_last_transaction(); buffer.start_transaction(); if let Some(diff) = trailing_whitespace_diff { - buffer.apply_non_conflicting_portion_of_diff(diff, cx); + buffer.apply_diff(diff, cx); } if ensure_final_newline { buffer.ensure_final_newline(cx); From a366ba19af58ff527e5d21a8ed1d5a618deb8efe Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Wed, 1 Mar 2023 14:38:35 -0800 Subject: [PATCH 09/13] Fix range relativization when combined injections occur inside of other injections For example, ERB template inside of a markdown code block Co-authored-by: Antonio Scandurra --- crates/language/src/syntax_map.rs | 63 +++++++++++++++++++++++++------ 1 file changed, 51 insertions(+), 12 deletions(-) diff --git a/crates/language/src/syntax_map.rs b/crates/language/src/syntax_map.rs index 670f479f10..5923241955 100644 --- a/crates/language/src/syntax_map.rs +++ b/crates/language/src/syntax_map.rs @@ -165,6 +165,7 @@ struct ParseStep { mode: ParseMode, } +#[derive(Debug)] enum ParseStepLanguage { Loaded { language: Arc }, Pending { name: Arc }, @@ -514,15 +515,32 @@ impl SyntaxSnapshot { let Some(grammar) = language.grammar() else { continue }; let tree; let changed_ranges; + let mut included_ranges = step.included_ranges; + for range in &mut included_ranges { + range.start_byte -= step_start_byte; + range.end_byte -= step_start_byte; + range.start_point = (Point::from_ts_point(range.start_point) + - step_start_point) + .to_ts_point(); + range.end_point = (Point::from_ts_point(range.end_point) + - step_start_point) + .to_ts_point(); + } + if let Some(SyntaxLayerContent::Parsed { tree: old_tree, .. }) = old_layer.map(|layer| &layer.content) { if let ParseMode::Combined { - parent_layer_changed_ranges, + mut parent_layer_changed_ranges, .. } = step.mode { + for range in &mut parent_layer_changed_ranges { + range.start -= step_start_byte; + range.end -= step_start_byte; + } + included_ranges = splice_included_ranges( old_tree.included_ranges(), &parent_layer_changed_ranges, @@ -534,7 +552,6 @@ impl SyntaxSnapshot { grammar, text.as_rope(), step_start_byte, - step_start_point, included_ranges, Some(old_tree.clone()), ); @@ -551,7 +568,6 @@ impl SyntaxSnapshot { grammar, text.as_rope(), step_start_byte, - step_start_point, included_ranges, None, ); @@ -1060,17 +1076,9 @@ fn parse_text( grammar: &Grammar, text: &Rope, start_byte: usize, - start_point: Point, - mut ranges: Vec, + ranges: Vec, old_tree: Option, ) -> Tree { - for range in &mut ranges { - range.start_byte -= start_byte; - range.end_byte -= start_byte; - range.start_point = (Point::from_ts_point(range.start_point) - start_point).to_ts_point(); - range.end_point = (Point::from_ts_point(range.end_point) - start_point).to_ts_point(); - } - PARSER.with(|parser| { let mut parser = parser.borrow_mut(); let mut chunks = text.chunks_in_range(start_byte..text.len()); @@ -2208,6 +2216,37 @@ mod tests { ); } + #[gpui::test] + fn test_combined_injections_inside_injections() { + let (_buffer, _syntax_map) = test_edit_sequence( + "Markdown", + &[ + r#" + here is some ERB code: + + ```erb +
    + <% people.each do |person| %> +
  • <%= person.name %>
    • + <% end %> +
+ ``` + "#, + r#" + here is some ERB code: + + ```erb +
    + <% people«2».each do |person| %> +
  • <%= person.name %>
    • + <% end %> +
+ ``` + "#, + ], + ); + } + #[gpui::test(iterations = 50)] fn test_random_syntax_map_edits(mut rng: StdRng) { let operations = env::var("OPERATIONS") From a598f0b13cd7c9beb1b63f7e97b9b71bfd7f8dde Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 2 Mar 2023 20:51:29 -0800 Subject: [PATCH 10/13] Avoid clobbering panic files when they happen at the same time Co-authored-by: Antonio Scandurra --- crates/zed/src/main.rs | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/crates/zed/src/main.rs b/crates/zed/src/main.rs index 78a435be70..57345b4f69 100644 --- a/crates/zed/src/main.rs +++ b/crates/zed/src/main.rs @@ -13,7 +13,6 @@ use client::{ http::{self, HttpClient}, UserStore, ZED_APP_VERSION, ZED_SECRET_CLIENT_TOKEN, }; - use futures::{ channel::{mpsc, oneshot}, FutureExt, SinkExt, StreamExt, @@ -31,8 +30,10 @@ use settings::{ }; use simplelog::ConfigBuilder; use smol::process::Command; -use std::{env, ffi::OsStr, panic, path::PathBuf, sync::Arc, thread, time::Duration}; -use std::{fs::OpenOptions, os::unix::prelude::OsStrExt}; +use std::{ + env, ffi::OsStr, fs::OpenOptions, io::Write as _, os::unix::prelude::OsStrExt, panic, + path::PathBuf, sync::Arc, thread, time::Duration, +}; use terminal_view::{get_working_directory, TerminalView}; use fs::RealFs; @@ -330,13 +331,18 @@ fn init_panic_hook(app_version: String) { ), }; - let panic_filename = chrono::Utc::now().format("%Y_%m_%d %H_%M_%S").to_string(); - std::fs::write( - paths::LOGS_DIR.join(format!("zed-{}-{}.panic", app_version, panic_filename)), - &message, - ) - .context("error writing panic to disk") - .log_err(); + let timestamp = chrono::Utc::now().format("%Y_%m_%d %H_%M_%S").to_string(); + let panic_file_path = + paths::LOGS_DIR.join(format!("zed-{}-{}.panic", app_version, timestamp)); + let panic_file = std::fs::OpenOptions::new() + .append(true) + .create(true) + .open(&panic_file_path) + .log_err(); + if let Some(mut panic_file) = panic_file { + write!(&mut panic_file, "{}", message).log_err(); + panic_file.flush().log_err(); + } if is_pty { eprintln!("{}", message); From 6854063d0be705455ae39f9c9e69871d2ba280b7 Mon Sep 17 00:00:00 2001 From: Nate Butler Date: Fri, 3 Mar 2023 09:47:58 -0800 Subject: [PATCH 11/13] Revert "Avoid tab bar background activating an item at the end of a tab drag" --- crates/workspace/src/pane.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 3c9ad1f6b0..22df4de7c4 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -1438,7 +1438,7 @@ impl View for Pane { .with_style(theme.workspace.tab_bar.container) .boxed() }) - .on_down(MouseButton::Left, move |_, cx| { + .on_click(MouseButton::Left, move |_, cx| { cx.dispatch_action(ActivateItem(active_item_index)); }) .boxed(), From 9311e012718cf163b88d5535f0698b824a166083 Mon Sep 17 00:00:00 2001 From: Julia Date: Fri, 3 Mar 2023 10:04:44 -0800 Subject: [PATCH 12/13] Avoid wrapping to the 0th glyph variant when the 4th should be used Co-Authored-By: Antonio Scandurra --- crates/gpui/src/platform/mac/sprite_cache.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/gpui/src/platform/mac/sprite_cache.rs b/crates/gpui/src/platform/mac/sprite_cache.rs index ceeb06698c..2127d12960 100644 --- a/crates/gpui/src/platform/mac/sprite_cache.rs +++ b/crates/gpui/src/platform/mac/sprite_cache.rs @@ -90,11 +90,12 @@ impl SpriteCache { let fonts = &self.fonts; let atlases = &mut self.atlases; let subpixel_variant = ( - (target_position.x().fract() * SUBPIXEL_VARIANTS as f32).round() as u8 + (target_position.x().fract() * SUBPIXEL_VARIANTS as f32).floor() as u8 % SUBPIXEL_VARIANTS, - (target_position.y().fract() * SUBPIXEL_VARIANTS as f32).round() as u8 + (target_position.y().fract() * SUBPIXEL_VARIANTS as f32).floor() as u8 % SUBPIXEL_VARIANTS, ); + self.glyphs .entry(GlyphDescriptor { font_id, From 6194c5df1609213f4ccc3da11713d3be5ee28a04 Mon Sep 17 00:00:00 2001 From: Petros Amoiridis Date: Fri, 3 Mar 2023 11:36:26 -0800 Subject: [PATCH 13/13] Add missing steps to the setup instructions --- README.md | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index b9c12abea2..d23744aac0 100644 --- a/README.md +++ b/README.md @@ -23,10 +23,18 @@ Welcome to Zed, a lightning-fast, collaborative code editor that makes your drea git clone https://github.com/zed-industries/zed.dev ``` -* Set up a local `zed` database and seed it with some initial users: +* Initialize submodules ``` - script/bootstrap + git submodule update --init --recursive + ``` + +* Set up a local `zed` database and seed it with some initial users: + + Create a personal GitHub token to run `script/bootstrap` once successfully. Then delete that token. + + ``` + GITHUB_TOKEN=<$token> script/bootstrap ``` ### Testing against locally-running servers