From 116fa92e84f43879a6247557ebbe438ff0bfcae6 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 5 Jul 2022 14:51:28 -0700 Subject: [PATCH 01/28] Change Buffer constructors to construct the History internally --- crates/language/src/buffer.rs | 22 +++++++++------------- crates/text/src/tests.rs | 34 +++++++++++++++++----------------- crates/text/src/text.rs | 7 ++++--- 3 files changed, 30 insertions(+), 33 deletions(-) diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index 4d50affdd508d52363927f5a776548832248739e..b7add947ffededca3702a218d9678fa289aa5f04 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -314,30 +314,30 @@ impl CharKind { } impl Buffer { - pub fn new>>( + pub fn new>( replica_id: ReplicaId, base_text: T, cx: &mut ModelContext, ) -> Self { - let history = History::new(base_text.into()); - let line_ending = LineEnding::detect(&history.base_text); + let base_text = base_text.into(); + let line_ending = LineEnding::detect(&base_text); Self::build( - TextBuffer::new(replica_id, cx.model_id() as u64, history), + TextBuffer::new(replica_id, cx.model_id() as u64, base_text), None, line_ending, ) } - pub fn from_file>>( + pub fn from_file>( replica_id: ReplicaId, base_text: T, file: Arc, cx: &mut ModelContext, ) -> Self { - let history = History::new(base_text.into()); - let line_ending = LineEnding::detect(&history.base_text); + let base_text = base_text.into(); + let line_ending = LineEnding::detect(&base_text); Self::build( - TextBuffer::new(replica_id, cx.model_id() as u64, history), + TextBuffer::new(replica_id, cx.model_id() as u64, base_text), Some(file), line_ending, ) @@ -349,11 +349,7 @@ impl Buffer { file: Option>, cx: &mut ModelContext, ) -> Result { - let buffer = TextBuffer::new( - replica_id, - message.id, - History::new(Arc::from(message.base_text)), - ); + let buffer = TextBuffer::new(replica_id, message.id, message.base_text); let line_ending = proto::LineEnding::from_i32(message.line_ending) .ok_or_else(|| anyhow!("missing line_ending"))?; let mut this = Self::build(buffer, file, LineEnding::from_proto(line_ending)); diff --git a/crates/text/src/tests.rs b/crates/text/src/tests.rs index e66837f21b295a14c5011bc4921f5d43717b386e..216850dbd8b74d5dbbcf6b65c4c758341c133863 100644 --- a/crates/text/src/tests.rs +++ b/crates/text/src/tests.rs @@ -18,7 +18,7 @@ fn init_logger() { #[test] fn test_edit() { - let mut buffer = Buffer::new(0, 0, History::new("abc".into())); + let mut buffer = Buffer::new(0, 0, "abc".into()); assert_eq!(buffer.text(), "abc"); buffer.edit([(3..3, "def")]); assert_eq!(buffer.text(), "abcdef"); @@ -42,7 +42,7 @@ fn test_random_edits(mut rng: StdRng) { let mut reference_string = RandomCharIter::new(&mut rng) .take(reference_string_len) .collect::(); - let mut buffer = Buffer::new(0, 0, History::new(reference_string.clone().into())); + let mut buffer = Buffer::new(0, 0, reference_string.clone().into()); buffer.history.group_interval = Duration::from_millis(rng.gen_range(0..=200)); let mut buffer_versions = Vec::new(); log::info!( @@ -150,7 +150,7 @@ fn test_random_edits(mut rng: StdRng) { #[test] fn test_line_len() { - let mut buffer = Buffer::new(0, 0, History::new("".into())); + let mut buffer = Buffer::new(0, 0, "".into()); buffer.edit([(0..0, "abcd\nefg\nhij")]); buffer.edit([(12..12, "kl\nmno")]); buffer.edit([(18..18, "\npqrs\n")]); @@ -167,7 +167,7 @@ fn test_line_len() { #[test] fn test_common_prefix_at_positionn() { let text = "a = str; b = δα"; - let buffer = Buffer::new(0, 0, History::new(text.into())); + let buffer = Buffer::new(0, 0, text.into()); let offset1 = offset_after(text, "str"); let offset2 = offset_after(text, "δα"); @@ -215,7 +215,7 @@ fn test_common_prefix_at_positionn() { #[test] fn test_text_summary_for_range() { - let buffer = Buffer::new(0, 0, History::new("ab\nefg\nhklm\nnopqrs\ntuvwxyz".into())); + let buffer = Buffer::new(0, 0, "ab\nefg\nhklm\nnopqrs\ntuvwxyz".into()); assert_eq!( buffer.text_summary_for_range::(1..3), TextSummary { @@ -280,7 +280,7 @@ fn test_text_summary_for_range() { #[test] fn test_chars_at() { - let mut buffer = Buffer::new(0, 0, History::new("".into())); + let mut buffer = Buffer::new(0, 0, "".into()); buffer.edit([(0..0, "abcd\nefgh\nij")]); buffer.edit([(12..12, "kl\nmno")]); buffer.edit([(18..18, "\npqrs")]); @@ -302,7 +302,7 @@ fn test_chars_at() { assert_eq!(chars.collect::(), "PQrs"); // Regression test: - let mut buffer = Buffer::new(0, 0, History::new("".into())); + let mut buffer = Buffer::new(0, 0, "".into()); buffer.edit([(0..0, "[workspace]\nmembers = [\n \"xray_core\",\n \"xray_server\",\n \"xray_cli\",\n \"xray_wasm\",\n]\n")]); buffer.edit([(60..60, "\n")]); @@ -312,7 +312,7 @@ fn test_chars_at() { #[test] fn test_anchors() { - let mut buffer = Buffer::new(0, 0, History::new("".into())); + let mut buffer = Buffer::new(0, 0, "".into()); buffer.edit([(0..0, "abc")]); let left_anchor = buffer.anchor_before(2); let right_anchor = buffer.anchor_after(2); @@ -430,7 +430,7 @@ fn test_anchors() { #[test] fn test_anchors_at_start_and_end() { - let mut buffer = Buffer::new(0, 0, History::new("".into())); + let mut buffer = Buffer::new(0, 0, "".into()); let before_start_anchor = buffer.anchor_before(0); let after_end_anchor = buffer.anchor_after(0); @@ -453,7 +453,7 @@ fn test_anchors_at_start_and_end() { #[test] fn test_undo_redo() { - let mut buffer = Buffer::new(0, 0, History::new("1234".into())); + let mut buffer = Buffer::new(0, 0, "1234".into()); // Set group interval to zero so as to not group edits in the undo stack. buffer.history.group_interval = Duration::from_secs(0); @@ -490,7 +490,7 @@ fn test_undo_redo() { #[test] fn test_history() { let mut now = Instant::now(); - let mut buffer = Buffer::new(0, 0, History::new("123456".into())); + let mut buffer = Buffer::new(0, 0, "123456".into()); buffer.start_transaction_at(now); buffer.edit([(2..4, "cd")]); @@ -544,7 +544,7 @@ fn test_history() { #[test] fn test_finalize_last_transaction() { let now = Instant::now(); - let mut buffer = Buffer::new(0, 0, History::new("123456".into())); + let mut buffer = Buffer::new(0, 0, "123456".into()); buffer.start_transaction_at(now); buffer.edit([(2..4, "cd")]); @@ -579,7 +579,7 @@ fn test_finalize_last_transaction() { #[test] fn test_edited_ranges_for_transaction() { let now = Instant::now(); - let mut buffer = Buffer::new(0, 0, History::new("1234567".into())); + let mut buffer = Buffer::new(0, 0, "1234567".into()); buffer.start_transaction_at(now); buffer.edit([(2..4, "cd")]); @@ -618,9 +618,9 @@ fn test_edited_ranges_for_transaction() { fn test_concurrent_edits() { let text = "abcdef"; - let mut buffer1 = Buffer::new(1, 0, History::new(text.into())); - let mut buffer2 = Buffer::new(2, 0, History::new(text.into())); - let mut buffer3 = Buffer::new(3, 0, History::new(text.into())); + let mut buffer1 = Buffer::new(1, 0, text.into()); + let mut buffer2 = Buffer::new(2, 0, text.into()); + let mut buffer3 = Buffer::new(3, 0, text.into()); let buf1_op = buffer1.edit([(1..2, "12")]); assert_eq!(buffer1.text(), "a12cdef"); @@ -659,7 +659,7 @@ fn test_random_concurrent_edits(mut rng: StdRng) { let mut network = Network::new(rng.clone()); for i in 0..peers { - let mut buffer = Buffer::new(i as ReplicaId, 0, History::new(base_text.clone().into())); + let mut buffer = Buffer::new(i as ReplicaId, 0, base_text.clone().into()); buffer.history.group_interval = Duration::from_millis(rng.gen_range(0..=200)); buffers.push(buffer); replica_ids.push(i as u16); diff --git a/crates/text/src/text.rs b/crates/text/src/text.rs index 2c8fc13313c49025faf35aafd7ef14fec1c1558a..4ab5ef094039fd4e2cce6d9937d9678633708e5b 100644 --- a/crates/text/src/text.rs +++ b/crates/text/src/text.rs @@ -148,9 +148,9 @@ impl HistoryEntry { } #[derive(Clone)] -pub struct History { +struct History { // TODO: Turn this into a String or Rope, maybe. - pub base_text: Arc, + base_text: Arc, operations: HashMap, undo_stack: Vec, redo_stack: Vec, @@ -539,7 +539,8 @@ pub struct UndoOperation { } impl Buffer { - pub fn new(replica_id: u16, remote_id: u64, history: History) -> Buffer { + pub fn new(replica_id: u16, remote_id: u64, base_text: String) -> Buffer { + let history = History::new(base_text.into()); let mut fragments = SumTree::new(); let mut insertions = SumTree::new(); From 7e9beaf4bbe8dc8ca966a256eedc5d024741f1a9 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 5 Jul 2022 17:14:12 -0700 Subject: [PATCH 02/28] Strip carriage returns from all text in text::Buffer * Moving the logic from Rope to text::Buffer makes it easier to keep the Rope in sync with the fragment tree. * Removing carriage return characters is lossier, but is much simpler than incrementally maintaining the invariant that there are no carriage returns followed by newlines. We may want to do something smarter in the future. Co-authored-by: Keith Simmons --- crates/language/src/buffer.rs | 94 +++++------------------------ crates/language/src/proto.rs | 14 +++++ crates/language/src/tests.rs | 2 +- crates/project/src/project.rs | 12 ++-- crates/project/src/worktree.rs | 4 +- crates/text/src/random_char_iter.rs | 2 +- crates/text/src/rope.rs | 9 +-- crates/text/src/tests.rs | 18 ++++++ crates/text/src/text.rs | 77 +++++++++++++++++++++-- 9 files changed, 132 insertions(+), 100 deletions(-) diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index b7add947ffededca3702a218d9678fa289aa5f04..1e244ab4c5630f14660c18e028e3c2cd5723392f 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -53,7 +53,6 @@ pub struct Buffer { saved_version: clock::Global, saved_version_fingerprint: String, saved_mtime: SystemTime, - line_ending: LineEnding, transaction_depth: usize, was_dirty_before_starting_transaction: Option, language: Option>, @@ -98,12 +97,6 @@ pub enum IndentKind { Tab, } -#[derive(Copy, Debug, Clone, PartialEq, Eq)] -pub enum LineEnding { - Unix, - Windows, -} - #[derive(Clone, Debug)] struct SelectionSet { line_mode: bool, @@ -319,12 +312,9 @@ impl Buffer { base_text: T, cx: &mut ModelContext, ) -> Self { - let base_text = base_text.into(); - let line_ending = LineEnding::detect(&base_text); Self::build( - TextBuffer::new(replica_id, cx.model_id() as u64, base_text), + TextBuffer::new(replica_id, cx.model_id() as u64, base_text.into()), None, - line_ending, ) } @@ -334,12 +324,9 @@ impl Buffer { file: Arc, cx: &mut ModelContext, ) -> Self { - let base_text = base_text.into(); - let line_ending = LineEnding::detect(&base_text); Self::build( - TextBuffer::new(replica_id, cx.model_id() as u64, base_text), + TextBuffer::new(replica_id, cx.model_id() as u64, base_text.into()), Some(file), - line_ending, ) } @@ -350,9 +337,11 @@ impl Buffer { cx: &mut ModelContext, ) -> Result { let buffer = TextBuffer::new(replica_id, message.id, message.base_text); - let line_ending = proto::LineEnding::from_i32(message.line_ending) - .ok_or_else(|| anyhow!("missing line_ending"))?; - let mut this = Self::build(buffer, file, LineEnding::from_proto(line_ending)); + let mut this = Self::build(buffer, file); + this.text.set_line_ending(proto::deserialize_line_ending( + proto::LineEnding::from_i32(message.line_ending) + .ok_or_else(|| anyhow!("missing line_ending"))?, + )); let ops = message .operations .into_iter() @@ -417,7 +406,7 @@ impl Buffer { diagnostics: proto::serialize_diagnostics(self.diagnostics.iter()), diagnostics_timestamp: self.diagnostics_timestamp.value, completion_triggers: self.completion_triggers.clone(), - line_ending: self.line_ending.to_proto() as i32, + line_ending: proto::serialize_line_ending(self.line_ending()) as i32, } } @@ -426,7 +415,7 @@ impl Buffer { self } - fn build(buffer: TextBuffer, file: Option>, line_ending: LineEnding) -> Self { + fn build(buffer: TextBuffer, file: Option>) -> Self { let saved_mtime; if let Some(file) = file.as_ref() { saved_mtime = file.mtime(); @@ -442,7 +431,6 @@ impl Buffer { was_dirty_before_starting_transaction: None, text: buffer, file, - line_ending, syntax_tree: Mutex::new(None), parsing_in_background: false, parse_count: 0, @@ -503,7 +491,7 @@ impl Buffer { self.remote_id(), text, version, - self.line_ending, + self.line_ending(), cx.as_mut(), ); cx.spawn(|this, mut cx| async move { @@ -559,7 +547,7 @@ impl Buffer { this.did_reload( this.version(), this.as_rope().fingerprint(), - this.line_ending, + this.line_ending(), new_mtime, cx, ); @@ -584,14 +572,14 @@ impl Buffer { ) { self.saved_version = version; self.saved_version_fingerprint = fingerprint; - self.line_ending = line_ending; + self.text.set_line_ending(line_ending); self.saved_mtime = mtime; if let Some(file) = self.file.as_ref().and_then(|f| f.as_local()) { file.buffer_reloaded( self.remote_id(), &self.saved_version, self.saved_version_fingerprint.clone(), - self.line_ending, + self.line_ending(), self.saved_mtime, cx, ); @@ -970,13 +958,13 @@ impl Buffer { } } - pub(crate) fn diff(&self, new_text: String, cx: &AppContext) -> Task { + pub(crate) fn diff(&self, mut new_text: String, cx: &AppContext) -> Task { let old_text = self.as_rope().clone(); let base_version = self.version(); cx.background().spawn(async move { let old_text = old_text.to_string(); let line_ending = LineEnding::detect(&new_text); - let new_text = new_text.replace("\r\n", "\n").replace('\r', "\n"); + LineEnding::strip_carriage_returns(&mut new_text); let changes = TextDiff::from_lines(old_text.as_str(), new_text.as_str()) .iter_all_changes() .map(|c| (c.tag(), c.value().len())) @@ -999,7 +987,7 @@ impl Buffer { if self.version == diff.base_version { self.finalize_last_transaction(); self.start_transaction(); - self.line_ending = diff.line_ending; + self.text.set_line_ending(diff.line_ending); let mut offset = diff.start_offset; for (tag, len) in diff.changes { let range = offset..(offset + len); @@ -1514,10 +1502,6 @@ impl Buffer { pub fn completion_triggers(&self) -> &[String] { &self.completion_triggers } - - pub fn line_ending(&self) -> LineEnding { - self.line_ending - } } #[cfg(any(test, feature = "test-support"))] @@ -2538,52 +2522,6 @@ impl std::ops::SubAssign for IndentSize { } } -impl LineEnding { - pub fn from_proto(style: proto::LineEnding) -> Self { - match style { - proto::LineEnding::Unix => Self::Unix, - proto::LineEnding::Windows => Self::Windows, - } - } - - fn detect(text: &str) -> Self { - let text = &text[..cmp::min(text.len(), 1000)]; - if let Some(ix) = text.find('\n') { - if ix == 0 || text.as_bytes()[ix - 1] != b'\r' { - Self::Unix - } else { - Self::Windows - } - } else { - Default::default() - } - } - - pub fn as_str(self) -> &'static str { - match self { - LineEnding::Unix => "\n", - LineEnding::Windows => "\r\n", - } - } - - pub fn to_proto(self) -> proto::LineEnding { - match self { - LineEnding::Unix => proto::LineEnding::Unix, - LineEnding::Windows => proto::LineEnding::Windows, - } - } -} - -impl Default for LineEnding { - fn default() -> Self { - #[cfg(unix)] - return Self::Unix; - - #[cfg(not(unix))] - return Self::Windows; - } -} - impl Completion { pub fn sort_key(&self) -> (usize, &str) { let kind_key = match self.lsp_completion.kind { diff --git a/crates/language/src/proto.rs b/crates/language/src/proto.rs index 0e876d14dfe63f74203c76bc70831cb83e9ed2ba..7c7ec65fd81065748d95cee4f747610674a37f43 100644 --- a/crates/language/src/proto.rs +++ b/crates/language/src/proto.rs @@ -11,6 +11,20 @@ use text::*; pub use proto::{Buffer, BufferState, LineEnding, SelectionSet}; +pub fn deserialize_line_ending(message: proto::LineEnding) -> text::LineEnding { + match message { + LineEnding::Unix => text::LineEnding::Unix, + LineEnding::Windows => text::LineEnding::Windows, + } +} + +pub fn serialize_line_ending(message: text::LineEnding) -> proto::LineEnding { + match message { + text::LineEnding::Unix => proto::LineEnding::Unix, + text::LineEnding::Windows => proto::LineEnding::Windows, + } +} + pub fn serialize_operation(operation: &Operation) -> proto::Operation { proto::Operation { variant: Some(match operation { diff --git a/crates/language/src/tests.rs b/crates/language/src/tests.rs index 723e57ded435539e6a0516d186a8609464f85592..a645af3c025e5722cf76c17600611bda1ab7d43a 100644 --- a/crates/language/src/tests.rs +++ b/crates/language/src/tests.rs @@ -421,7 +421,7 @@ async fn test_outline(cx: &mut gpui::TestAppContext) { async fn search<'a>( outline: &'a Outline, query: &str, - cx: &gpui::TestAppContext, + cx: &'a gpui::TestAppContext, ) -> Vec<(&'a str, Vec)> { let matches = cx .read(|cx| outline.search(query, cx.background().clone())) diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 677a7afa9a22fcdf7d6d19d78dc7a831687c4fa9..0b9a0569c2e3e2613e916e3249e055bdb9a3a66f 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -20,12 +20,14 @@ use gpui::{ }; use language::{ point_to_lsp, - proto::{deserialize_anchor, deserialize_version, serialize_anchor, serialize_version}, + proto::{ + deserialize_anchor, deserialize_line_ending, deserialize_version, serialize_anchor, + serialize_version, + }, range_from_lsp, range_to_lsp, Anchor, Bias, Buffer, CharKind, CodeAction, CodeLabel, Completion, Diagnostic, DiagnosticEntry, DiagnosticSet, Event as BufferEvent, File as _, - Language, LanguageRegistry, LanguageServerName, LineEnding, LocalFile, LspAdapter, - OffsetRangeExt, Operation, Patch, PointUtf16, TextBufferSnapshot, ToOffset, ToPointUtf16, - Transaction, + Language, LanguageRegistry, LanguageServerName, LocalFile, LspAdapter, OffsetRangeExt, + Operation, Patch, PointUtf16, TextBufferSnapshot, ToOffset, ToPointUtf16, Transaction, }; use lsp::{ DiagnosticSeverity, DiagnosticTag, DocumentHighlightKind, LanguageServer, LanguageString, @@ -5542,7 +5544,7 @@ impl Project { ) -> Result<()> { let payload = envelope.payload; let version = deserialize_version(payload.version); - let line_ending = LineEnding::from_proto( + let line_ending = deserialize_line_ending( proto::LineEnding::from_i32(payload.line_ending) .ok_or_else(|| anyhow!("missing line ending"))?, ); diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 2603b2b70951a16356c7d30322d38166c5cd5717..0735d3e1febb4329a8988fe2a247f666f7b0ccb4 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -23,7 +23,7 @@ use gpui::{ Task, }; use language::{ - proto::{deserialize_version, serialize_version}, + proto::{deserialize_version, serialize_line_ending, serialize_version}, Buffer, DiagnosticEntry, LineEnding, PointUtf16, Rope, }; use lazy_static::lazy_static; @@ -1750,7 +1750,7 @@ impl language::LocalFile for File { version: serialize_version(&version), mtime: Some(mtime.into()), fingerprint, - line_ending: line_ending.to_proto() as i32, + line_ending: serialize_line_ending(line_ending) as i32, }) .log_err(); } diff --git a/crates/text/src/random_char_iter.rs b/crates/text/src/random_char_iter.rs index 1741df8fb7992d92d1e9d6b7613691716f48ec72..04cdcd35243b3d45a49668851230941d13675502 100644 --- a/crates/text/src/random_char_iter.rs +++ b/crates/text/src/random_char_iter.rs @@ -22,7 +22,7 @@ impl Iterator for RandomCharIter { match self.0.gen_range(0..100) { // whitespace - 0..=19 => [' ', '\n', '\t'].choose(&mut self.0).copied(), + 0..=19 => [' ', '\n', '\r', '\t'].choose(&mut self.0).copied(), // two-byte greek letters 20..=32 => char::from_u32(self.0.gen_range(('α' as u32)..('ω' as u32 + 1))), // // three-byte characters diff --git a/crates/text/src/rope.rs b/crates/text/src/rope.rs index 8c5dc260d635906fe914b17e8bf68549a93d2a3b..e8aff3f52fa6648c26c6f7398a57f89a803cf8e0 100644 --- a/crates/text/src/rope.rs +++ b/crates/text/src/rope.rs @@ -58,19 +58,12 @@ impl Rope { pub fn push(&mut self, text: &str) { let mut new_chunks = SmallVec::<[_; 16]>::new(); let mut new_chunk = ArrayString::new(); - let mut chars = text.chars().peekable(); - while let Some(mut ch) = chars.next() { + for ch in text.chars() { if new_chunk.len() + ch.len_utf8() > 2 * CHUNK_BASE { new_chunks.push(Chunk(new_chunk)); new_chunk = ArrayString::new(); } - if ch == '\r' { - ch = '\n'; - if chars.peek().copied() == Some('\n') { - chars.next(); - } - } new_chunk.push(ch); } if !new_chunk.is_empty() { diff --git a/crates/text/src/tests.rs b/crates/text/src/tests.rs index 216850dbd8b74d5dbbcf6b65c4c758341c133863..c534a004b77633e539563ce524df73dd91d8fb9f 100644 --- a/crates/text/src/tests.rs +++ b/crates/text/src/tests.rs @@ -43,6 +43,8 @@ fn test_random_edits(mut rng: StdRng) { .take(reference_string_len) .collect::(); let mut buffer = Buffer::new(0, 0, reference_string.clone().into()); + reference_string = reference_string.replace("\r", ""); + buffer.history.group_interval = Duration::from_millis(rng.gen_range(0..=200)); let mut buffer_versions = Vec::new(); log::info!( @@ -56,6 +58,8 @@ fn test_random_edits(mut rng: StdRng) { for (old_range, new_text) in edits.iter().rev() { reference_string.replace_range(old_range.clone(), &new_text); } + reference_string = reference_string.replace("\r", ""); + assert_eq!(buffer.text(), reference_string); log::info!( "buffer text {:?}, version: {:?}", @@ -148,6 +152,20 @@ fn test_random_edits(mut rng: StdRng) { } } +#[test] +fn test_line_endings() { + let mut buffer = Buffer::new(0, 0, "one\r\ntwo".into()); + assert_eq!(buffer.text(), "one\ntwo"); + assert_eq!(buffer.line_ending(), LineEnding::Windows); + buffer.check_invariants(); + + buffer.edit([(buffer.len()..buffer.len(), "\r\nthree")]); + buffer.edit([(0..0, "zero\r\n")]); + assert_eq!(buffer.text(), "zero\none\ntwo\nthree"); + assert_eq!(buffer.line_ending(), LineEnding::Windows); + buffer.check_invariants(); +} + #[test] fn test_line_len() { let mut buffer = Buffer::new(0, 0, "".into()); diff --git a/crates/text/src/text.rs b/crates/text/src/text.rs index 4ab5ef094039fd4e2cce6d9937d9678633708e5b..7e564d3eca79cb3698cf905dcb51a1b9e3450feb 100644 --- a/crates/text/src/text.rs +++ b/crates/text/src/text.rs @@ -63,6 +63,7 @@ pub struct BufferSnapshot { remote_id: u64, visible_text: Rope, deleted_text: Rope, + line_ending: LineEnding, undo_map: UndoMap, fragments: SumTree, insertions: SumTree, @@ -86,6 +87,12 @@ pub struct Transaction { pub ranges: Vec>, } +#[derive(Clone, Copy, Debug, PartialEq)] +pub enum LineEnding { + Unix, + Windows, +} + impl HistoryEntry { pub fn transaction_id(&self) -> TransactionId { self.transaction.id @@ -539,7 +546,10 @@ pub struct UndoOperation { } impl Buffer { - pub fn new(replica_id: u16, remote_id: u64, base_text: String) -> Buffer { + pub fn new(replica_id: u16, remote_id: u64, mut base_text: String) -> Buffer { + let line_ending = LineEnding::detect(&base_text); + LineEnding::strip_carriage_returns(&mut base_text); + let history = History::new(base_text.into()); let mut fragments = SumTree::new(); let mut insertions = SumTree::new(); @@ -547,6 +557,7 @@ impl Buffer { let mut local_clock = clock::Local::new(replica_id); let mut lamport_clock = clock::Lamport::new(replica_id); let mut version = clock::Global::new(); + let visible_text = Rope::from(history.base_text.as_ref()); if visible_text.len() > 0 { let insertion_timestamp = InsertionTimestamp { @@ -577,6 +588,7 @@ impl Buffer { remote_id, visible_text, deleted_text: Rope::new(), + line_ending, fragments, insertions, version, @@ -659,7 +671,7 @@ impl Buffer { let mut new_insertions = Vec::new(); let mut insertion_offset = 0; - let mut ranges = edits + let mut edits = edits .map(|(range, new_text)| (range.to_offset(&*self), new_text)) .peekable(); @@ -667,12 +679,12 @@ impl Buffer { RopeBuilder::new(self.visible_text.cursor(0), self.deleted_text.cursor(0)); let mut old_fragments = self.fragments.cursor::(); let mut new_fragments = - old_fragments.slice(&ranges.peek().unwrap().0.start, Bias::Right, &None); + old_fragments.slice(&edits.peek().unwrap().0.start, Bias::Right, &None); new_ropes.push_tree(new_fragments.summary().text); let mut fragment_start = old_fragments.start().visible; - for (range, new_text) in ranges { - let new_text = new_text.into(); + for (range, new_text) in edits { + let new_text = LineEnding::strip_carriage_returns_from_arc(new_text.into()); let fragment_end = old_fragments.end(&None).visible; // If the current fragment ends before this range, then jump ahead to the first fragment @@ -715,6 +727,7 @@ impl Buffer { // Insert the new text before any existing fragments within the range. if !new_text.is_empty() { let new_start = new_fragments.summary().text.visible; + edits_patch.push(Edit { old: fragment_start..fragment_start, new: new_start..new_start + new_text.len(), @@ -806,6 +819,10 @@ impl Buffer { edit_op } + pub fn set_line_ending(&mut self, line_ending: LineEnding) { + self.snapshot.line_ending = line_ending; + } + pub fn apply_ops>(&mut self, ops: I) -> Result<()> { let mut deferred_ops = Vec::new(); for op in ops { @@ -1413,6 +1430,8 @@ impl Buffer { fragment_summary.text.deleted, self.snapshot.deleted_text.len() ); + + assert!(!self.text().contains("\r\n")); } pub fn set_group_interval(&mut self, group_interval: Duration) { @@ -1550,6 +1569,10 @@ impl BufferSnapshot { self.visible_text.to_string() } + pub fn line_ending(&self) -> LineEnding { + self.line_ending + } + pub fn deleted_text(&self) -> String { self.deleted_text.to_string() } @@ -2311,6 +2334,50 @@ impl operation_queue::Operation for Operation { } } +impl Default for LineEnding { + fn default() -> Self { + #[cfg(unix)] + return Self::Unix; + + #[cfg(not(unix))] + return Self::CRLF; + } +} + +impl LineEnding { + pub fn as_str(&self) -> &'static str { + match self { + LineEnding::Unix => "\n", + LineEnding::Windows => "\r\n", + } + } + + pub fn detect(text: &str) -> Self { + if let Some(ix) = text[..cmp::min(text.len(), 1000)].find(&['\n']) { + let text = text.as_bytes(); + if ix > 0 && text[ix - 1] == b'\r' { + Self::Windows + } else { + Self::Unix + } + } else { + Self::default() + } + } + + pub fn strip_carriage_returns(text: &mut String) { + text.retain(|c| c != '\r') + } + + fn strip_carriage_returns_from_arc(text: Arc) -> Arc { + if text.contains('\r') { + text.replace('\r', "").into() + } else { + text + } + } +} + pub trait ToOffset { fn to_offset<'a>(&self, snapshot: &BufferSnapshot) -> usize; } From 5e2306d0e0e9e3965c2b89e639e81a096d5455a8 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 5 Jul 2022 17:36:57 -0700 Subject: [PATCH 03/28] 0.44 --- Cargo.lock | 2 +- crates/zed/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f5dbc470f597c8e3f17c9bfc3052115d51886227..bfdb62936e283fd6670f591ef2cce7bc34a98fbf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6158,7 +6158,7 @@ dependencies = [ [[package]] name = "zed" -version = "0.43.0" +version = "0.44.0" dependencies = [ "activity_indicator", "anyhow", diff --git a/crates/zed/Cargo.toml b/crates/zed/Cargo.toml index 8d131d17a85478229a912ed6d6c949087c3dd402..60116f06c2a8d3ce609ded19724f47bfd2621bb5 100644 --- a/crates/zed/Cargo.toml +++ b/crates/zed/Cargo.toml @@ -3,7 +3,7 @@ authors = ["Nathan Sobo "] description = "The fast, collaborative code editor." edition = "2021" name = "zed" -version = "0.43.0" +version = "0.44.0" [lib] name = "zed" From 113eb9b94fb79a5145d4135e772cb2f4366fc9d2 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 6 Jul 2022 10:21:30 +0200 Subject: [PATCH 04/28] Don't slice midway through multi-byte char when detecting line ending --- crates/text/src/tests.rs | 11 +++++++++++ crates/text/src/text.rs | 10 +++++++--- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/crates/text/src/tests.rs b/crates/text/src/tests.rs index c534a004b77633e539563ce524df73dd91d8fb9f..2d57b6579272a5c57c991e73af35efe2cfce0c4c 100644 --- a/crates/text/src/tests.rs +++ b/crates/text/src/tests.rs @@ -154,6 +154,17 @@ fn test_random_edits(mut rng: StdRng) { #[test] fn test_line_endings() { + assert_eq!(LineEnding::detect(&"🍐✅\n".repeat(1000)), LineEnding::Unix); + assert_eq!(LineEnding::detect(&"abcd\n".repeat(1000)), LineEnding::Unix); + assert_eq!( + LineEnding::detect(&"🍐✅\r\n".repeat(1000)), + LineEnding::Windows + ); + assert_eq!( + LineEnding::detect(&"abcd\r\n".repeat(1000)), + LineEnding::Windows + ); + let mut buffer = Buffer::new(0, 0, "one\r\ntwo".into()); assert_eq!(buffer.text(), "one\ntwo"); assert_eq!(buffer.line_ending(), LineEnding::Windows); diff --git a/crates/text/src/text.rs b/crates/text/src/text.rs index 7e564d3eca79cb3698cf905dcb51a1b9e3450feb..4d1e50b2749c5ebc291cee824c1e31cdc2c540e8 100644 --- a/crates/text/src/text.rs +++ b/crates/text/src/text.rs @@ -2353,9 +2353,13 @@ impl LineEnding { } pub fn detect(text: &str) -> Self { - if let Some(ix) = text[..cmp::min(text.len(), 1000)].find(&['\n']) { - let text = text.as_bytes(); - if ix > 0 && text[ix - 1] == b'\r' { + let mut max_ix = cmp::min(text.len(), 1000); + while !text.is_char_boundary(max_ix) { + max_ix -= 1; + } + + if let Some(ix) = text[..max_ix].find(&['\n']) { + if ix > 0 && text.as_bytes()[ix - 1] == b'\r' { Self::Windows } else { Self::Unix From 13c9b1778b935785bfee7a3e56e6660a19609852 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 6 Jul 2022 11:00:56 +0200 Subject: [PATCH 05/28] Replace lone carriage returns with newlines --- Cargo.lock | 1 + crates/text/Cargo.toml | 1 + crates/text/src/tests.rs | 11 +++++------ crates/text/src/text.rs | 24 +++++++++++++++++++++--- 4 files changed, 28 insertions(+), 9 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bfdb62936e283fd6670f591ef2cce7bc34a98fbf..7647e3ccd30e49f440f7a5fe947762f9f705cc7c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4910,6 +4910,7 @@ dependencies = [ "parking_lot 0.11.2", "postage", "rand 0.8.5", + "regex", "smallvec", "sum_tree", "util", diff --git a/crates/text/Cargo.toml b/crates/text/Cargo.toml index cbbd65027e6a5112a779d5b8b742e38977e1c22f..4fc09eff46a43291ea16e61304933ecc6c2a5e4f 100644 --- a/crates/text/Cargo.toml +++ b/crates/text/Cargo.toml @@ -23,6 +23,7 @@ log = { version = "0.4.16", features = ["kv_unstable_serde"] } parking_lot = "0.11" postage = { version = "0.4.1", features = ["futures-traits"] } rand = { version = "0.8.3", optional = true } +regex = "1.5" smallvec = { version = "1.6", features = ["union"] } [dev-dependencies] diff --git a/crates/text/src/tests.rs b/crates/text/src/tests.rs index 2d57b6579272a5c57c991e73af35efe2cfce0c4c..c0cc0556d56fd6c50e27256ffbf304dc86b836c2 100644 --- a/crates/text/src/tests.rs +++ b/crates/text/src/tests.rs @@ -43,7 +43,7 @@ fn test_random_edits(mut rng: StdRng) { .take(reference_string_len) .collect::(); let mut buffer = Buffer::new(0, 0, reference_string.clone().into()); - reference_string = reference_string.replace("\r", ""); + LineEnding::strip_carriage_returns(&mut reference_string); buffer.history.group_interval = Duration::from_millis(rng.gen_range(0..=200)); let mut buffer_versions = Vec::new(); @@ -58,7 +58,6 @@ fn test_random_edits(mut rng: StdRng) { for (old_range, new_text) in edits.iter().rev() { reference_string.replace_range(old_range.clone(), &new_text); } - reference_string = reference_string.replace("\r", ""); assert_eq!(buffer.text(), reference_string); log::info!( @@ -165,14 +164,14 @@ fn test_line_endings() { LineEnding::Windows ); - let mut buffer = Buffer::new(0, 0, "one\r\ntwo".into()); - assert_eq!(buffer.text(), "one\ntwo"); + let mut buffer = Buffer::new(0, 0, "one\r\ntwo\rthree".into()); + assert_eq!(buffer.text(), "one\ntwo\nthree"); assert_eq!(buffer.line_ending(), LineEnding::Windows); buffer.check_invariants(); - buffer.edit([(buffer.len()..buffer.len(), "\r\nthree")]); + buffer.edit([(buffer.len()..buffer.len(), "\r\nfour")]); buffer.edit([(0..0, "zero\r\n")]); - assert_eq!(buffer.text(), "zero\none\ntwo\nthree"); + assert_eq!(buffer.text(), "zero\none\ntwo\nthree\nfour"); assert_eq!(buffer.line_ending(), LineEnding::Windows); buffer.check_invariants(); } diff --git a/crates/text/src/text.rs b/crates/text/src/text.rs index 4d1e50b2749c5ebc291cee824c1e31cdc2c540e8..f73163438b13b0b6931af508d011ee385c11d68c 100644 --- a/crates/text/src/text.rs +++ b/crates/text/src/text.rs @@ -18,6 +18,7 @@ pub use anchor::*; use anyhow::Result; use clock::ReplicaId; use collections::{HashMap, HashSet}; +use lazy_static::lazy_static; use locator::Locator; use operation_queue::OperationQueue; pub use patch::Patch; @@ -26,10 +27,12 @@ pub use point_utf16::*; use postage::{barrier, oneshot, prelude::*}; #[cfg(any(test, feature = "test-support"))] pub use random_char_iter::*; +use regex::Regex; use rope::TextDimension; pub use rope::{Chunks, Rope, TextSummary}; pub use selection::*; use std::{ + borrow::Cow, cmp::{self, Ordering}, future::Future, iter::Iterator, @@ -42,6 +45,10 @@ pub use subscription::*; pub use sum_tree::Bias; use sum_tree::{FilterCursor, SumTree}; +lazy_static! { + static ref CARRIAGE_RETURNS_REGEX: Regex = Regex::new("\r\n|\r").unwrap(); +} + pub type TransactionId = clock::Local; pub struct Buffer { @@ -1472,6 +1479,15 @@ impl Buffer { log::info!("mutating buffer {} with {:?}", self.replica_id, edits); let op = self.edit(edits.iter().cloned()); + if let Operation::Edit(edit) = &op { + assert_eq!(edits.len(), edit.new_text.len()); + for (edit, new_text) in edits.iter_mut().zip(&edit.new_text) { + edit.1 = new_text.clone(); + } + } else { + unreachable!() + } + (edits, op) } @@ -2370,12 +2386,14 @@ impl LineEnding { } pub fn strip_carriage_returns(text: &mut String) { - text.retain(|c| c != '\r') + if let Cow::Owned(replaced) = CARRIAGE_RETURNS_REGEX.replace_all(text, "\n") { + *text = replaced; + } } fn strip_carriage_returns_from_arc(text: Arc) -> Arc { - if text.contains('\r') { - text.replace('\r', "").into() + if let Cow::Owned(replaced) = CARRIAGE_RETURNS_REGEX.replace_all(&text, "\n") { + replaced.into() } else { text } From 573dd29882ffd43eb4767026cce41fa97cec6662 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 6 Jul 2022 12:42:41 +0200 Subject: [PATCH 06/28] v0.44.1 --- Cargo.lock | 2 +- crates/zed/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7647e3ccd30e49f440f7a5fe947762f9f705cc7c..7dcecd009390c92a0602ae3fc70c421be27c6f1d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6159,7 +6159,7 @@ dependencies = [ [[package]] name = "zed" -version = "0.44.0" +version = "0.44.1" dependencies = [ "activity_indicator", "anyhow", diff --git a/crates/zed/Cargo.toml b/crates/zed/Cargo.toml index 60116f06c2a8d3ce609ded19724f47bfd2621bb5..6d488e83e8ce64729791d96b2a6b9c28119a69e9 100644 --- a/crates/zed/Cargo.toml +++ b/crates/zed/Cargo.toml @@ -3,7 +3,7 @@ authors = ["Nathan Sobo "] description = "The fast, collaborative code editor." edition = "2021" name = "zed" -version = "0.44.0" +version = "0.44.1" [lib] name = "zed" From 980730a4e17adc5765734cdf77090cda2c9b2964 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 6 Jul 2022 15:53:40 +0200 Subject: [PATCH 07/28] Report whether a view was focused or blurred when observing focus --- crates/gpui/src/app.rs | 126 +++++++++++++++++++--------- crates/search/src/project_search.rs | 12 ++- 2 files changed, 95 insertions(+), 43 deletions(-) diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index 505f609f5709f57b0c037ec5be2032729f736266..fd447e2469fc24c538bc7a05ab77fe64460a0a36 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -811,7 +811,7 @@ type GlobalActionCallback = dyn FnMut(&dyn Action, &mut MutableAppContext); type SubscriptionCallback = Box bool>; type GlobalSubscriptionCallback = Box; type ObservationCallback = Box bool>; -type FocusObservationCallback = Box bool>; +type FocusObservationCallback = Box bool>; type GlobalObservationCallback = Box; type ReleaseObservationCallback = Box; type ActionObservationCallback = Box; @@ -1305,7 +1305,7 @@ impl MutableAppContext { fn observe_focus(&mut self, handle: &ViewHandle, mut callback: F) -> Subscription where - F: 'static + FnMut(ViewHandle, &mut MutableAppContext) -> bool, + F: 'static + FnMut(ViewHandle, bool, &mut MutableAppContext) -> bool, V: View, { let subscription_id = post_inc(&mut self.next_subscription_id); @@ -1314,9 +1314,9 @@ impl MutableAppContext { self.pending_effects.push_back(Effect::FocusObservation { view_id, subscription_id, - callback: Box::new(move |cx| { + callback: Box::new(move |focused, cx| { if let Some(observed) = observed.upgrade(cx) { - callback(observed, cx) + callback(observed, focused, cx) } else { false } @@ -2525,6 +2525,31 @@ impl MutableAppContext { if let Some(mut blurred_view) = this.cx.views.remove(&(window_id, blurred_id)) { blurred_view.on_blur(this, window_id, blurred_id); this.cx.views.insert((window_id, blurred_id), blurred_view); + + let callbacks = this.focus_observations.lock().remove(&blurred_id); + if let Some(callbacks) = callbacks { + for (id, callback) in callbacks { + if let Some(mut callback) = callback { + let alive = callback(false, this); + if alive { + match this + .focus_observations + .lock() + .entry(blurred_id) + .or_default() + .entry(id) + { + btree_map::Entry::Vacant(entry) => { + entry.insert(Some(callback)); + } + btree_map::Entry::Occupied(entry) => { + entry.remove(); + } + } + } + } + } + } } } @@ -2537,7 +2562,7 @@ impl MutableAppContext { if let Some(callbacks) = callbacks { for (id, callback) in callbacks { if let Some(mut callback) = callback { - let alive = callback(this); + let alive = callback(true, this); if alive { match this .focus_observations @@ -3598,20 +3623,21 @@ impl<'a, T: View> ViewContext<'a, T> { pub fn observe_focus(&mut self, handle: &ViewHandle, mut callback: F) -> Subscription where - F: 'static + FnMut(&mut T, ViewHandle, &mut ViewContext), + F: 'static + FnMut(&mut T, ViewHandle, bool, &mut ViewContext), V: View, { let observer = self.weak_handle(); - self.app.observe_focus(handle, move |observed, cx| { - if let Some(observer) = observer.upgrade(cx) { - observer.update(cx, |observer, cx| { - callback(observer, observed, cx); - }); - true - } else { - false - } - }) + self.app + .observe_focus(handle, move |observed, focused, cx| { + if let Some(observer) = observer.upgrade(cx) { + observer.update(cx, |observer, cx| { + callback(observer, observed, focused, cx); + }); + true + } else { + false + } + }) } pub fn observe_release(&mut self, handle: &H, mut callback: F) -> Subscription @@ -6448,11 +6474,13 @@ mod tests { view_1.update(cx, |_, cx| { cx.observe_focus(&view_2, { let observed_events = observed_events.clone(); - move |this, view, cx| { + move |this, view, focused, cx| { + let label = if focused { "focus" } else { "blur" }; observed_events.lock().push(format!( - "{} observed {}'s focus", + "{} observed {}'s {}", this.name, - view.read(cx).name + view.read(cx).name, + label )) } }) @@ -6461,16 +6489,20 @@ mod tests { view_2.update(cx, |_, cx| { cx.observe_focus(&view_1, { let observed_events = observed_events.clone(); - move |this, view, cx| { + move |this, view, focused, cx| { + let label = if focused { "focus" } else { "blur" }; observed_events.lock().push(format!( - "{} observed {}'s focus", + "{} observed {}'s {}", this.name, - view.read(cx).name + view.read(cx).name, + label )) } }) .detach(); }); + assert_eq!(mem::take(&mut *view_events.lock()), ["view 1 focused"]); + assert_eq!(mem::take(&mut *observed_events.lock()), Vec::<&str>::new()); view_1.update(cx, |_, cx| { // Ensure only the latest focus is honored. @@ -6478,31 +6510,47 @@ mod tests { cx.focus(&view_1); cx.focus(&view_2); }); - view_1.update(cx, |_, cx| cx.focus(&view_1)); - view_1.update(cx, |_, cx| cx.focus(&view_2)); - view_1.update(cx, |_, _| drop(view_2)); + assert_eq!( + mem::take(&mut *view_events.lock()), + ["view 1 blurred", "view 2 focused"], + ); + assert_eq!( + mem::take(&mut *observed_events.lock()), + [ + "view 2 observed view 1's blur", + "view 1 observed view 2's focus" + ] + ); + view_1.update(cx, |_, cx| cx.focus(&view_1)); + assert_eq!( + mem::take(&mut *view_events.lock()), + ["view 2 blurred", "view 1 focused"], + ); assert_eq!( - *view_events.lock(), + mem::take(&mut *observed_events.lock()), [ - "view 1 focused".to_string(), - "view 1 blurred".to_string(), - "view 2 focused".to_string(), - "view 2 blurred".to_string(), - "view 1 focused".to_string(), - "view 1 blurred".to_string(), - "view 2 focused".to_string(), - "view 1 focused".to_string(), - ], + "view 1 observed view 2's blur", + "view 2 observed view 1's focus" + ] ); + + view_1.update(cx, |_, cx| cx.focus(&view_2)); assert_eq!( - *observed_events.lock(), + mem::take(&mut *view_events.lock()), + ["view 1 blurred", "view 2 focused"], + ); + assert_eq!( + mem::take(&mut *observed_events.lock()), [ - "view 1 observed view 2's focus".to_string(), - "view 2 observed view 1's focus".to_string(), - "view 1 observed view 2's focus".to_string(), + "view 2 observed view 1's blur", + "view 1 observed view 2's focus" ] ); + + view_1.update(cx, |_, _| drop(view_2)); + assert_eq!(mem::take(&mut *view_events.lock()), ["view 1 focused"]); + assert_eq!(mem::take(&mut *observed_events.lock()), Vec::<&str>::new()); } #[crate::test(self)] diff --git a/crates/search/src/project_search.rs b/crates/search/src/project_search.rs index bf862f0d9d0433c08b316ba34951af1eb1d4ac36..2aa89932850700973e1512c545842ad4e9493425 100644 --- a/crates/search/src/project_search.rs +++ b/crates/search/src/project_search.rs @@ -365,8 +365,10 @@ impl ProjectSearchView { cx.emit(ViewEvent::EditorEvent(event.clone())) }) .detach(); - cx.observe_focus(&query_editor, |this, _, _| { - this.results_editor_was_focused = false; + cx.observe_focus(&query_editor, |this, _, focused, _| { + if focused { + this.results_editor_was_focused = false; + } }) .detach(); @@ -377,8 +379,10 @@ impl ProjectSearchView { }); cx.observe(&results_editor, |_, _, cx| cx.emit(ViewEvent::UpdateTab)) .detach(); - cx.observe_focus(&results_editor, |this, _, _| { - this.results_editor_was_focused = true; + cx.observe_focus(&results_editor, |this, _, focused, _| { + if focused { + this.results_editor_was_focused = true; + } }) .detach(); cx.subscribe(&results_editor, |this, _, event, cx| { From b937c1acec6f858ed0b9abd2f383ca52f0877596 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 6 Jul 2022 16:11:06 +0200 Subject: [PATCH 08/28] Move autosave logic up into `Workspace` and `Pane` --- crates/diagnostics/src/diagnostics.rs | 9 +- crates/editor/src/editor.rs | 131 +------------------------- crates/editor/src/items.rs | 4 + crates/search/src/project_search.rs | 8 ++ crates/workspace/src/pane.rs | 12 +++ crates/workspace/src/workspace.rs | 59 +++++++++++- crates/zed/src/zed.rs | 75 +++++++++++++++ 7 files changed, 166 insertions(+), 132 deletions(-) diff --git a/crates/diagnostics/src/diagnostics.rs b/crates/diagnostics/src/diagnostics.rs index 94eda67c391b8f7ecc4cd9ddddf5210b24e278b8..ecc1b2df681414d322d0420ea2e67632e6b5cebc 100644 --- a/crates/diagnostics/src/diagnostics.rs +++ b/crates/diagnostics/src/diagnostics.rs @@ -568,10 +568,11 @@ impl workspace::Item for ProjectDiagnosticsEditor { } fn should_update_tab_on_event(event: &Event) -> bool { - matches!( - event, - Event::Saved | Event::DirtyChanged | Event::TitleChanged - ) + Editor::should_update_tab_on_event(event) + } + + fn is_edit_event(event: &Self::Event) -> bool { + Editor::is_edit_event(event) } fn set_nav_history(&mut self, nav_history: ItemNavHistory, cx: &mut ViewContext) { diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 31a636fd6163591e66227240647b2a8bf43c4d03..9dd40413fd254e7ef852643acfdb5cbe464ee415 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -18,7 +18,6 @@ use collections::{BTreeMap, Bound, HashMap, HashSet, VecDeque}; pub use display_map::DisplayPoint; use display_map::*; pub use element::*; -use futures::{channel::oneshot, FutureExt}; use fuzzy::{StringMatch, StringMatchCandidate}; use gpui::{ actions, @@ -51,7 +50,7 @@ use ordered_float::OrderedFloat; use project::{LocationLink, Project, ProjectPath, ProjectTransaction}; use selections_collection::{resolve_multiple, MutableSelectionsCollection, SelectionsCollection}; use serde::{Deserialize, Serialize}; -use settings::{Autosave, Settings}; +use settings::Settings; use smallvec::SmallVec; use smol::Timer; use snippet::Snippet; @@ -439,8 +438,6 @@ pub struct Editor { leader_replica_id: Option, hover_state: HoverState, link_go_to_definition_state: LinkGoToDefinitionState, - pending_autosave: Option>>, - cancel_pending_autosave: Option>, _subscriptions: Vec, } @@ -1028,13 +1025,10 @@ impl Editor { leader_replica_id: None, hover_state: Default::default(), link_go_to_definition_state: Default::default(), - pending_autosave: Default::default(), - cancel_pending_autosave: Default::default(), _subscriptions: vec![ cx.observe(&buffer, Self::on_buffer_changed), cx.subscribe(&buffer, Self::on_buffer_event), cx.observe(&display_map, Self::on_display_map_changed), - cx.observe_window_activation(Self::on_window_activation_changed), ], }; this.end_selection(cx); @@ -5584,33 +5578,6 @@ impl Editor { self.refresh_active_diagnostics(cx); self.refresh_code_actions(cx); cx.emit(Event::BufferEdited); - if let Autosave::AfterDelay { milliseconds } = cx.global::().autosave { - let pending_autosave = - self.pending_autosave.take().unwrap_or(Task::ready(None)); - if let Some(cancel_pending_autosave) = self.cancel_pending_autosave.take() { - let _ = cancel_pending_autosave.send(()); - } - - let (cancel_tx, mut cancel_rx) = oneshot::channel(); - self.cancel_pending_autosave = Some(cancel_tx); - self.pending_autosave = Some(cx.spawn_weak(|this, mut cx| async move { - let mut timer = cx - .background() - .timer(Duration::from_millis(milliseconds)) - .fuse(); - pending_autosave.await; - futures::select_biased! { - _ = cancel_rx => return None, - _ = timer => {} - } - - this.upgrade(&cx)? - .update(&mut cx, |this, cx| this.autosave(cx)) - .await - .log_err(); - None - })); - } } language::Event::Reparsed => cx.emit(Event::Reparsed), language::Event::DirtyChanged => cx.emit(Event::DirtyChanged), @@ -5629,25 +5596,6 @@ impl Editor { cx.notify(); } - fn on_window_activation_changed(&mut self, active: bool, cx: &mut ViewContext) { - if !active && cx.global::().autosave == Autosave::OnWindowChange { - self.autosave(cx).detach_and_log_err(cx); - } - } - - fn autosave(&mut self, cx: &mut ViewContext) -> Task> { - if let Some(project) = self.project.clone() { - if self.buffer.read(cx).is_dirty(cx) - && !self.buffer.read(cx).has_conflict(cx) - && workspace::Item::can_save(self, cx) - { - return workspace::Item::save(self, project, cx); - } - } - - Task::ready(Ok(())) - } - pub fn set_searchable(&mut self, searchable: bool) { self.searchable = searchable; } @@ -5865,10 +5813,6 @@ impl View for Editor { hide_hover(self, cx); cx.emit(Event::Blurred); cx.notify(); - - if cx.global::().autosave == Autosave::OnFocusChange { - self.autosave(cx).detach_and_log_err(cx); - } } fn keymap_context(&self, _: &AppContext) -> gpui::keymap::Context { @@ -6282,23 +6226,22 @@ mod tests { use super::*; use futures::StreamExt; use gpui::{ - executor::Deterministic, geometry::rect::RectF, platform::{WindowBounds, WindowOptions}, }; use indoc::indoc; use language::{FakeLspAdapter, LanguageConfig}; use lsp::FakeLanguageServer; - use project::{FakeFs, Fs}; + use project::FakeFs; use settings::LanguageSettings; - use std::{cell::RefCell, path::Path, rc::Rc, time::Instant}; + use std::{cell::RefCell, rc::Rc, time::Instant}; use text::Point; use unindent::Unindent; use util::{ assert_set_eq, test::{marked_text_by, marked_text_ranges, marked_text_ranges_by, sample_text}, }; - use workspace::{FollowableItem, Item, ItemHandle}; + use workspace::{FollowableItem, ItemHandle}; #[gpui::test] fn test_edit_events(cx: &mut MutableAppContext) { @@ -9562,72 +9505,6 @@ mod tests { save.await.unwrap(); } - #[gpui::test] - async fn test_autosave(deterministic: Arc, cx: &mut gpui::TestAppContext) { - deterministic.forbid_parking(); - - let fs = FakeFs::new(cx.background().clone()); - fs.insert_file("/file.rs", Default::default()).await; - - let project = Project::test(fs.clone(), ["/file.rs".as_ref()], cx).await; - let buffer = project - .update(cx, |project, cx| project.open_local_buffer("/file.rs", cx)) - .await - .unwrap(); - - let (_, editor) = cx.add_window(|cx| Editor::for_buffer(buffer, Some(project), cx)); - - // Autosave on window change. - editor.update(cx, |editor, cx| { - cx.update_global(|settings: &mut Settings, _| { - settings.autosave = Autosave::OnWindowChange; - }); - editor.insert("X", cx); - assert!(editor.is_dirty(cx)) - }); - - // Deactivating the window saves the file. - cx.simulate_window_activation(None); - deterministic.run_until_parked(); - assert_eq!(fs.load(Path::new("/file.rs")).await.unwrap(), "X"); - editor.read_with(cx, |editor, cx| assert!(!editor.is_dirty(cx))); - - // Autosave on focus change. - editor.update(cx, |editor, cx| { - cx.focus_self(); - cx.update_global(|settings: &mut Settings, _| { - settings.autosave = Autosave::OnFocusChange; - }); - editor.insert("X", cx); - assert!(editor.is_dirty(cx)) - }); - - // Blurring the editor saves the file. - editor.update(cx, |_, cx| cx.blur()); - deterministic.run_until_parked(); - assert_eq!(fs.load(Path::new("/file.rs")).await.unwrap(), "XX"); - editor.read_with(cx, |editor, cx| assert!(!editor.is_dirty(cx))); - - // Autosave after delay. - editor.update(cx, |editor, cx| { - cx.update_global(|settings: &mut Settings, _| { - settings.autosave = Autosave::AfterDelay { milliseconds: 500 }; - }); - editor.insert("X", cx); - assert!(editor.is_dirty(cx)) - }); - - // Delay hasn't fully expired, so the file is still dirty and unsaved. - deterministic.advance_clock(Duration::from_millis(250)); - assert_eq!(fs.load(Path::new("/file.rs")).await.unwrap(), "XX"); - editor.read_with(cx, |editor, cx| assert!(editor.is_dirty(cx))); - - // After delay expires, the file is saved. - deterministic.advance_clock(Duration::from_millis(250)); - assert_eq!(fs.load(Path::new("/file.rs")).await.unwrap(), "XXX"); - editor.read_with(cx, |editor, cx| assert!(!editor.is_dirty(cx))); - } - #[gpui::test] async fn test_completion(cx: &mut gpui::TestAppContext) { let mut language = Language::new( diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index 8e15dce83c517ca0672df8ca72722bbc8bf9076a..f7aa80beaa6aa9219d42e091d258306781aef62c 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -445,6 +445,10 @@ impl Item for Editor { Event::Saved | Event::DirtyChanged | Event::TitleChanged ) } + + fn is_edit_event(event: &Self::Event) -> bool { + matches!(event, Event::BufferEdited) + } } impl ProjectItem for Editor { diff --git a/crates/search/src/project_search.rs b/crates/search/src/project_search.rs index 2aa89932850700973e1512c545842ad4e9493425..5ee2dcbb27cd99f24c925b3b686999d86df9140d 100644 --- a/crates/search/src/project_search.rs +++ b/crates/search/src/project_search.rs @@ -329,6 +329,14 @@ impl Item for ProjectSearchView { fn should_update_tab_on_event(event: &ViewEvent) -> bool { matches!(event, ViewEvent::UpdateTab) } + + fn is_edit_event(event: &Self::Event) -> bool { + if let ViewEvent::EditorEvent(editor_event) = event { + Editor::is_edit_event(editor_event) + } else { + false + } + } } impl ProjectSearchView { diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 5e039b8cd0eadbea3ce0860140482e060bb68910..f8474e41b10c4f94a40937c83be253f48be7f707 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -718,6 +718,18 @@ impl Pane { Ok(true) } + pub fn autosave_item( + item: &dyn ItemHandle, + project: ModelHandle, + cx: &mut MutableAppContext, + ) -> Task> { + if item.is_dirty(cx) && !item.has_conflict(cx) && item.can_save(cx) { + item.save(project, cx) + } else { + Task::ready(Ok(())) + } + } + pub fn focus_active_item(&mut self, cx: &mut ViewContext) { if let Some(active_item) = self.active_item() { cx.focus(active_item); diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 419998e7303679851796806af1850c8700b676eb..cf1092662f9e0f9f025ca3356d20311ff0433b20 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -11,6 +11,7 @@ use client::{ }; use clock::ReplicaId; use collections::{hash_map, HashMap, HashSet}; +use futures::{channel::oneshot, FutureExt}; use gpui::{ actions, color::Color, @@ -30,7 +31,7 @@ pub use pane_group::*; use postage::prelude::Stream; use project::{fs, Fs, Project, ProjectEntryId, ProjectPath, ProjectStore, Worktree, WorktreeId}; use serde::Deserialize; -use settings::Settings; +use settings::{Autosave, Settings}; use sidebar::{Side, Sidebar, SidebarButtons, ToggleSidebarItem}; use smallvec::SmallVec; use status_bar::StatusBar; @@ -41,12 +42,14 @@ use std::{ cell::RefCell, fmt, future::Future, + mem, path::{Path, PathBuf}, rc::Rc, sync::{ atomic::{AtomicBool, Ordering::SeqCst}, Arc, }, + time::Duration, }; use theme::{Theme, ThemeRegistry}; pub use toolbar::{ToolbarItemLocation, ToolbarItemView}; @@ -296,6 +299,9 @@ pub trait Item: View { fn should_update_tab_on_event(_: &Self::Event) -> bool { false } + fn is_edit_event(_: &Self::Event) -> bool { + false + } fn act_as_type( &self, type_id: TypeId, @@ -510,6 +516,8 @@ impl ItemHandle for ViewHandle { } } + let mut pending_autosave = None; + let mut cancel_pending_autosave = oneshot::channel::<()>().0; let pending_update = Rc::new(RefCell::new(None)); let pending_update_scheduled = Rc::new(AtomicBool::new(false)); let pane = pane.downgrade(); @@ -570,6 +578,40 @@ impl ItemHandle for ViewHandle { cx.notify(); }); } + + if T::is_edit_event(event) { + if let Autosave::AfterDelay { milliseconds } = cx.global::().autosave { + let prev_autosave = pending_autosave.take().unwrap_or(Task::ready(Some(()))); + let (cancel_tx, mut cancel_rx) = oneshot::channel::<()>(); + let prev_cancel_tx = mem::replace(&mut cancel_pending_autosave, cancel_tx); + let project = workspace.project.downgrade(); + let _ = prev_cancel_tx.send(()); + pending_autosave = Some(cx.spawn_weak(|_, mut cx| async move { + let mut timer = cx + .background() + .timer(Duration::from_millis(milliseconds)) + .fuse(); + prev_autosave.await; + futures::select_biased! { + _ = cancel_rx => return None, + _ = timer => {} + } + + let project = project.upgrade(&cx)?; + cx.update(|cx| Pane::autosave_item(&item, project, cx)) + .await + .log_err(); + None + })); + } + } + }) + .detach(); + + cx.observe_focus(self, move |workspace, item, focused, cx| { + if !focused && cx.global::().autosave == Autosave::OnFocusChange { + Pane::autosave_item(&item, workspace.project.clone(), cx).detach_and_log_err(cx); + } }) .detach(); } @@ -774,6 +816,8 @@ impl Workspace { cx.notify() }) .detach(); + cx.observe_window_activation(Self::on_window_activation_changed) + .detach(); cx.subscribe(&project, move |this, project, event, cx| { match event { @@ -2314,6 +2358,19 @@ impl Workspace { } None } + + fn on_window_activation_changed(&mut self, active: bool, cx: &mut ViewContext) { + if !active && cx.global::().autosave == Autosave::OnWindowChange { + for pane in &self.panes { + pane.update(cx, |pane, cx| { + for item in pane.items() { + Pane::autosave_item(item.as_ref(), self.project.clone(), cx) + .detach_and_log_err(cx); + } + }); + } + } + } } impl Entity for Workspace { diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index f88aee3d7c6c06265e94e8c541e0c6ec44261e02..4fa2e238bf4d20ef59465c8ced1b7feb1bc68747 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -396,9 +396,11 @@ mod tests { }; use project::{Project, ProjectPath}; use serde_json::json; + use settings::Autosave; use std::{ collections::HashSet, path::{Path, PathBuf}, + time::Duration, }; use theme::{Theme, ThemeRegistry, DEFAULT_THEME_NAME}; use workspace::{ @@ -977,6 +979,79 @@ mod tests { }) } + #[gpui::test] + async fn test_autosave(deterministic: Arc, cx: &mut gpui::TestAppContext) { + let app_state = init(cx); + let fs = app_state.fs.clone(); + fs.as_fake() + .insert_tree("/root", json!({ "a.txt": "" })) + .await; + + let project = Project::test(fs.clone(), ["/root".as_ref()], cx).await; + let (_, workspace) = cx.add_window(|cx| Workspace::new(project, cx)); + cx.update(|cx| { + workspace.update(cx, |view, cx| { + view.open_paths(vec![PathBuf::from("/root/a.txt")], true, cx) + }) + }) + .await; + let editor = cx.read(|cx| { + let pane = workspace.read(cx).active_pane().read(cx); + let item = pane.active_item().unwrap(); + item.downcast::().unwrap() + }); + + // Autosave on window change. + editor.update(cx, |editor, cx| { + cx.update_global(|settings: &mut Settings, _| { + settings.autosave = Autosave::OnWindowChange; + }); + editor.insert("X", cx); + assert!(editor.is_dirty(cx)) + }); + + // Deactivating the window saves the file. + cx.simulate_window_activation(None); + deterministic.run_until_parked(); + assert_eq!(fs.load(Path::new("/root/a.txt")).await.unwrap(), "X"); + editor.read_with(cx, |editor, cx| assert!(!editor.is_dirty(cx))); + + // Autosave on focus change. + editor.update(cx, |editor, cx| { + cx.focus_self(); + cx.update_global(|settings: &mut Settings, _| { + settings.autosave = Autosave::OnFocusChange; + }); + editor.insert("X", cx); + assert!(editor.is_dirty(cx)) + }); + + // Blurring the editor saves the file. + editor.update(cx, |_, cx| cx.blur()); + deterministic.run_until_parked(); + assert_eq!(fs.load(Path::new("/root/a.txt")).await.unwrap(), "XX"); + editor.read_with(cx, |editor, cx| assert!(!editor.is_dirty(cx))); + + // Autosave after delay. + editor.update(cx, |editor, cx| { + cx.update_global(|settings: &mut Settings, _| { + settings.autosave = Autosave::AfterDelay { milliseconds: 500 }; + }); + editor.insert("X", cx); + assert!(editor.is_dirty(cx)) + }); + + // Delay hasn't fully expired, so the file is still dirty and unsaved. + deterministic.advance_clock(Duration::from_millis(250)); + assert_eq!(fs.load(Path::new("/root/a.txt")).await.unwrap(), "XX"); + editor.read_with(cx, |editor, cx| assert!(editor.is_dirty(cx))); + + // After delay expires, the file is saved. + deterministic.advance_clock(Duration::from_millis(250)); + assert_eq!(fs.load(Path::new("/root/a.txt")).await.unwrap(), "XXX"); + editor.read_with(cx, |editor, cx| assert!(!editor.is_dirty(cx))); + } + #[gpui::test] async fn test_setting_language_when_saving_as_single_file_worktree(cx: &mut TestAppContext) { let app_state = init(cx); From 5e00df62672b96baf73812eb8d5d4a4cb7491b70 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 6 Jul 2022 16:55:05 +0200 Subject: [PATCH 09/28] Move autosave tests down into `Workspace` --- crates/workspace/src/workspace.rs | 92 ++++++++++++++++++++++++++++++- crates/zed/src/zed.rs | 75 ------------------------- 2 files changed, 90 insertions(+), 77 deletions(-) diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index cf1092662f9e0f9f025ca3356d20311ff0433b20..21206957605f938c178c89276918c6f27dfc7f50 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -2688,7 +2688,7 @@ fn open_new(app_state: &Arc, cx: &mut MutableAppContext) { #[cfg(test)] mod tests { use super::*; - use gpui::{ModelHandle, TestAppContext, ViewContext}; + use gpui::{executor::Deterministic, ModelHandle, TestAppContext, ViewContext}; use project::{FakeFs, Project, ProjectEntryId}; use serde_json::json; @@ -3026,6 +3026,86 @@ mod tests { }); } + #[gpui::test] + async fn test_autosave(deterministic: Arc, cx: &mut gpui::TestAppContext) { + deterministic.forbid_parking(); + + Settings::test_async(cx); + let fs = FakeFs::new(cx.background()); + + let project = Project::test(fs, [], cx).await; + let (window_id, workspace) = cx.add_window(|cx| Workspace::new(project, cx)); + + let item = cx.add_view(window_id, |_| { + let mut item = TestItem::new(); + item.project_entry_ids = vec![ProjectEntryId::from_proto(1)]; + item.is_dirty = true; + item + }); + let item_id = item.id(); + workspace.update(cx, |workspace, cx| { + workspace.add_item(Box::new(item.clone()), cx); + }); + + // Autosave on window change. + item.update(cx, |_, cx| { + cx.update_global(|settings: &mut Settings, _| { + settings.autosave = Autosave::OnWindowChange; + }); + }); + + // Deactivating the window saves the file. + cx.simulate_window_activation(None); + deterministic.run_until_parked(); + item.read_with(cx, |item, _| assert_eq!(item.save_count, 1)); + + // Autosave on focus change. + item.update(cx, |_, cx| { + cx.focus_self(); + cx.update_global(|settings: &mut Settings, _| { + settings.autosave = Autosave::OnFocusChange; + }); + }); + + // Blurring the item saves the file. + item.update(cx, |_, cx| cx.blur()); + deterministic.run_until_parked(); + item.read_with(cx, |item, _| assert_eq!(item.save_count, 2)); + + // Autosave after delay. + item.update(cx, |_, cx| { + cx.update_global(|settings: &mut Settings, _| { + settings.autosave = Autosave::AfterDelay { milliseconds: 500 }; + }); + cx.emit(TestItemEvent::Edit); + }); + + // Delay hasn't fully expired, so the file is still dirty and unsaved. + deterministic.advance_clock(Duration::from_millis(250)); + item.read_with(cx, |item, _| assert_eq!(item.save_count, 2)); + + // After delay expires, the file is saved. + deterministic.advance_clock(Duration::from_millis(250)); + item.read_with(cx, |item, _| assert_eq!(item.save_count, 3)); + + // Autosave on focus change, ensuring closing the tab counts as such. + item.update(cx, |_, cx| { + cx.update_global(|settings: &mut Settings, _| { + settings.autosave = Autosave::OnFocusChange; + }); + }); + + workspace + .update(cx, |workspace, cx| { + let pane = workspace.active_pane().clone(); + Pane::close_items(workspace, pane, cx, move |id| id == item_id) + }) + .await + .unwrap(); + assert!(!cx.has_pending_prompt(window_id)); + item.read_with(cx, |item, _| assert_eq!(item.save_count, 4)); + } + #[derive(Clone)] struct TestItem { save_count: usize, @@ -3038,6 +3118,10 @@ mod tests { is_singleton: bool, } + enum TestItemEvent { + Edit, + } + impl TestItem { fn new() -> Self { Self { @@ -3054,7 +3138,7 @@ mod tests { } impl Entity for TestItem { - type Event = (); + type Event = TestItemEvent; } impl View for TestItem { @@ -3136,5 +3220,9 @@ mod tests { fn should_update_tab_on_event(_: &Self::Event) -> bool { true } + + fn is_edit_event(event: &Self::Event) -> bool { + matches!(event, TestItemEvent::Edit) + } } } diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index 4fa2e238bf4d20ef59465c8ced1b7feb1bc68747..f88aee3d7c6c06265e94e8c541e0c6ec44261e02 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -396,11 +396,9 @@ mod tests { }; use project::{Project, ProjectPath}; use serde_json::json; - use settings::Autosave; use std::{ collections::HashSet, path::{Path, PathBuf}, - time::Duration, }; use theme::{Theme, ThemeRegistry, DEFAULT_THEME_NAME}; use workspace::{ @@ -979,79 +977,6 @@ mod tests { }) } - #[gpui::test] - async fn test_autosave(deterministic: Arc, cx: &mut gpui::TestAppContext) { - let app_state = init(cx); - let fs = app_state.fs.clone(); - fs.as_fake() - .insert_tree("/root", json!({ "a.txt": "" })) - .await; - - let project = Project::test(fs.clone(), ["/root".as_ref()], cx).await; - let (_, workspace) = cx.add_window(|cx| Workspace::new(project, cx)); - cx.update(|cx| { - workspace.update(cx, |view, cx| { - view.open_paths(vec![PathBuf::from("/root/a.txt")], true, cx) - }) - }) - .await; - let editor = cx.read(|cx| { - let pane = workspace.read(cx).active_pane().read(cx); - let item = pane.active_item().unwrap(); - item.downcast::().unwrap() - }); - - // Autosave on window change. - editor.update(cx, |editor, cx| { - cx.update_global(|settings: &mut Settings, _| { - settings.autosave = Autosave::OnWindowChange; - }); - editor.insert("X", cx); - assert!(editor.is_dirty(cx)) - }); - - // Deactivating the window saves the file. - cx.simulate_window_activation(None); - deterministic.run_until_parked(); - assert_eq!(fs.load(Path::new("/root/a.txt")).await.unwrap(), "X"); - editor.read_with(cx, |editor, cx| assert!(!editor.is_dirty(cx))); - - // Autosave on focus change. - editor.update(cx, |editor, cx| { - cx.focus_self(); - cx.update_global(|settings: &mut Settings, _| { - settings.autosave = Autosave::OnFocusChange; - }); - editor.insert("X", cx); - assert!(editor.is_dirty(cx)) - }); - - // Blurring the editor saves the file. - editor.update(cx, |_, cx| cx.blur()); - deterministic.run_until_parked(); - assert_eq!(fs.load(Path::new("/root/a.txt")).await.unwrap(), "XX"); - editor.read_with(cx, |editor, cx| assert!(!editor.is_dirty(cx))); - - // Autosave after delay. - editor.update(cx, |editor, cx| { - cx.update_global(|settings: &mut Settings, _| { - settings.autosave = Autosave::AfterDelay { milliseconds: 500 }; - }); - editor.insert("X", cx); - assert!(editor.is_dirty(cx)) - }); - - // Delay hasn't fully expired, so the file is still dirty and unsaved. - deterministic.advance_clock(Duration::from_millis(250)); - assert_eq!(fs.load(Path::new("/root/a.txt")).await.unwrap(), "XX"); - editor.read_with(cx, |editor, cx| assert!(editor.is_dirty(cx))); - - // After delay expires, the file is saved. - deterministic.advance_clock(Duration::from_millis(250)); - assert_eq!(fs.load(Path::new("/root/a.txt")).await.unwrap(), "XXX"); - editor.read_with(cx, |editor, cx| assert!(!editor.is_dirty(cx))); - } - #[gpui::test] async fn test_setting_language_when_saving_as_single_file_worktree(cx: &mut TestAppContext) { let app_state = init(cx); From 92868931776a1ea1ff2d0843eebf74fb9fdd421b Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 6 Jul 2022 16:55:25 +0200 Subject: [PATCH 10/28] Save item when closing it if autosave on focus change is enabled --- crates/workspace/src/pane.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index f8474e41b10c4f94a40937c83be253f48be7f707..b98ce49cd9d0cbc0f4bf91653155cdc046d2276b 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -14,7 +14,7 @@ use gpui::{ }; use project::{Project, ProjectEntryId, ProjectPath}; use serde::Deserialize; -use settings::Settings; +use settings::{Autosave, Settings}; use std::{any::Any, cell::RefCell, mem, path::Path, rc::Rc}; use util::ResultExt; @@ -677,7 +677,13 @@ impl Pane { _ => return Ok(false), } } else if is_dirty && (can_save || is_singleton) { - let should_save = if should_prompt_for_save { + let autosave_enabled = cx.read(|cx| { + matches!( + cx.global::().autosave, + Autosave::OnFocusChange | Autosave::OnWindowChange + ) + }); + let should_save = if should_prompt_for_save && !autosave_enabled { let mut answer = pane.update(cx, |pane, cx| { pane.activate_item(item_ix, true, true, cx); cx.prompt( From ab4931da6565842727a13d7fa41a43df819660f0 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 6 Jul 2022 17:25:33 +0200 Subject: [PATCH 11/28] Prevent autosave for deleted files --- crates/workspace/src/pane.rs | 13 +++++++---- crates/workspace/src/workspace.rs | 37 ++++++++++++++++++++++++++----- 2 files changed, 41 insertions(+), 9 deletions(-) diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index b98ce49cd9d0cbc0f4bf91653155cdc046d2276b..bbc086395b12a4fd09d7c8ce07e6c52c6491cb53 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -677,13 +677,13 @@ impl Pane { _ => return Ok(false), } } else if is_dirty && (can_save || is_singleton) { - let autosave_enabled = cx.read(|cx| { + let will_autosave = cx.read(|cx| { matches!( cx.global::().autosave, Autosave::OnFocusChange | Autosave::OnWindowChange - ) + ) && Self::can_autosave_item(item.as_ref(), cx) }); - let should_save = if should_prompt_for_save && !autosave_enabled { + let should_save = if should_prompt_for_save && !will_autosave { let mut answer = pane.update(cx, |pane, cx| { pane.activate_item(item_ix, true, true, cx); cx.prompt( @@ -724,12 +724,17 @@ impl Pane { Ok(true) } + fn can_autosave_item(item: &dyn ItemHandle, cx: &AppContext) -> bool { + let is_deleted = item.project_entry_ids(cx).is_empty(); + item.is_dirty(cx) && !item.has_conflict(cx) && item.can_save(cx) && !is_deleted + } + pub fn autosave_item( item: &dyn ItemHandle, project: ModelHandle, cx: &mut MutableAppContext, ) -> Task> { - if item.is_dirty(cx) && !item.has_conflict(cx) && item.can_save(cx) { + if Self::can_autosave_item(item, cx) { item.save(project, cx) } else { Task::ready(Ok(())) diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 21206957605f938c178c89276918c6f27dfc7f50..53318d943fe1b5854823e88b4bcbd1edd474692a 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -3039,7 +3039,6 @@ mod tests { let item = cx.add_view(window_id, |_| { let mut item = TestItem::new(); item.project_entry_ids = vec![ProjectEntryId::from_proto(1)]; - item.is_dirty = true; item }); let item_id = item.id(); @@ -3048,10 +3047,11 @@ mod tests { }); // Autosave on window change. - item.update(cx, |_, cx| { + item.update(cx, |item, cx| { cx.update_global(|settings: &mut Settings, _| { settings.autosave = Autosave::OnWindowChange; }); + item.is_dirty = true; }); // Deactivating the window saves the file. @@ -3060,11 +3060,12 @@ mod tests { item.read_with(cx, |item, _| assert_eq!(item.save_count, 1)); // Autosave on focus change. - item.update(cx, |_, cx| { + item.update(cx, |item, cx| { cx.focus_self(); cx.update_global(|settings: &mut Settings, _| { settings.autosave = Autosave::OnFocusChange; }); + item.is_dirty = true; }); // Blurring the item saves the file. @@ -3073,10 +3074,11 @@ mod tests { item.read_with(cx, |item, _| assert_eq!(item.save_count, 2)); // Autosave after delay. - item.update(cx, |_, cx| { + item.update(cx, |item, cx| { cx.update_global(|settings: &mut Settings, _| { settings.autosave = Autosave::AfterDelay { milliseconds: 500 }; }); + item.is_dirty = true; cx.emit(TestItemEvent::Edit); }); @@ -3089,10 +3091,11 @@ mod tests { item.read_with(cx, |item, _| assert_eq!(item.save_count, 3)); // Autosave on focus change, ensuring closing the tab counts as such. - item.update(cx, |_, cx| { + item.update(cx, |item, cx| { cx.update_global(|settings: &mut Settings, _| { settings.autosave = Autosave::OnFocusChange; }); + item.is_dirty = true; }); workspace @@ -3104,6 +3107,27 @@ mod tests { .unwrap(); assert!(!cx.has_pending_prompt(window_id)); item.read_with(cx, |item, _| assert_eq!(item.save_count, 4)); + + // Add the item again, ensuring autosave is prevented if the underlying file has been deleted. + workspace.update(cx, |workspace, cx| { + workspace.add_item(Box::new(item.clone()), cx); + }); + item.update(cx, |item, cx| { + item.project_entry_ids = Default::default(); + item.is_dirty = true; + cx.blur(); + }); + deterministic.run_until_parked(); + item.read_with(cx, |item, _| assert_eq!(item.save_count, 4)); + + // Ensure autosave is prevented for deleted files also when closing the buffer. + let _close_items = workspace.update(cx, |workspace, cx| { + let pane = workspace.active_pane().clone(); + Pane::close_items(workspace, pane, cx, move |id| id == item_id) + }); + deterministic.run_until_parked(); + assert!(cx.has_pending_prompt(window_id)); + item.read_with(cx, |item, _| assert_eq!(item.save_count, 4)); } #[derive(Clone)] @@ -3195,6 +3219,7 @@ mod tests { _: &mut ViewContext, ) -> Task> { self.save_count += 1; + self.is_dirty = false; Task::ready(Ok(())) } @@ -3205,6 +3230,7 @@ mod tests { _: &mut ViewContext, ) -> Task> { self.save_as_count += 1; + self.is_dirty = false; Task::ready(Ok(())) } @@ -3214,6 +3240,7 @@ mod tests { _: &mut ViewContext, ) -> Task> { self.reload_count += 1; + self.is_dirty = false; Task::ready(Ok(())) } From d3db700db421d2a505bb3a2d17863573608cc04d Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 6 Jul 2022 19:00:11 +0200 Subject: [PATCH 12/28] Fix panic on paste when editing with auto-indent Instead of accepting text as it's input by the user, we will read it out of the edit operation after it gets sanitized by the buffer. --- crates/language/src/buffer.rs | 3 ++- crates/language/src/tests.rs | 23 +++++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index 1e244ab4c5630f14660c18e028e3c2cd5723392f..42cca419008b987e13d3338b5983cfb8be0682df 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -1233,7 +1233,8 @@ impl Buffer { let inserted_ranges = edits .into_iter() - .filter_map(|(range, new_text)| { + .zip(&edit_operation.as_edit().unwrap().new_text) + .filter_map(|((range, _), new_text)| { let first_newline_ix = new_text.find('\n')?; let new_text_len = new_text.len(); let start = (delta + range.start as isize) as usize + first_newline_ix + 1; diff --git a/crates/language/src/tests.rs b/crates/language/src/tests.rs index a645af3c025e5722cf76c17600611bda1ab7d43a..ba8744624db8a222cf7a3825b15c1605e3903f7d 100644 --- a/crates/language/src/tests.rs +++ b/crates/language/src/tests.rs @@ -22,6 +22,29 @@ fn init_logger() { } } +#[gpui::test] +fn test_line_endings(cx: &mut gpui::MutableAppContext) { + cx.add_model(|cx| { + let mut buffer = + Buffer::new(0, "one\r\ntwo\rthree", cx).with_language(Arc::new(rust_lang()), cx); + assert_eq!(buffer.text(), "one\ntwo\nthree"); + assert_eq!(buffer.line_ending(), LineEnding::Windows); + + buffer.check_invariants(); + buffer.edit_with_autoindent( + [(buffer.len()..buffer.len(), "\r\nfour")], + IndentSize::spaces(2), + cx, + ); + buffer.edit([(0..0, "zero\r\n")], cx); + assert_eq!(buffer.text(), "zero\none\ntwo\nthree\nfour"); + assert_eq!(buffer.line_ending(), LineEnding::Windows); + buffer.check_invariants(); + + buffer + }); +} + #[gpui::test] fn test_select_language() { let registry = LanguageRegistry::test(); From 2c1906d710fce949b5f2c4a96018b05824299cbf Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 6 Jul 2022 19:32:45 +0200 Subject: [PATCH 13/28] Normalize line endings when parsing completions Co-Authored-By: Max Brunsfeld --- crates/language/src/buffer.rs | 2 +- crates/project/src/project.rs | 9 +++-- crates/project/src/project_tests.rs | 53 +++++++++++++++++++++++++++++ crates/text/src/tests.rs | 2 +- crates/text/src/text.rs | 8 ++--- 5 files changed, 65 insertions(+), 9 deletions(-) diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index 42cca419008b987e13d3338b5983cfb8be0682df..cebf5f504ea8c634a38155e5c5654362fb345b12 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -964,7 +964,7 @@ impl Buffer { cx.background().spawn(async move { let old_text = old_text.to_string(); let line_ending = LineEnding::detect(&new_text); - LineEnding::strip_carriage_returns(&mut new_text); + LineEnding::normalize(&mut new_text); let changes = TextDiff::from_lines(old_text.as_str(), new_text.as_str()) .iter_all_changes() .map(|c| (c.tag(), c.value().len())) diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 0b9a0569c2e3e2613e916e3249e055bdb9a3a66f..e4425e341476dfb4a316bfdc9df5b010e97e0964 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -26,8 +26,9 @@ use language::{ }, range_from_lsp, range_to_lsp, Anchor, Bias, Buffer, CharKind, CodeAction, CodeLabel, Completion, Diagnostic, DiagnosticEntry, DiagnosticSet, Event as BufferEvent, File as _, - Language, LanguageRegistry, LanguageServerName, LocalFile, LspAdapter, OffsetRangeExt, - Operation, Patch, PointUtf16, TextBufferSnapshot, ToOffset, ToPointUtf16, Transaction, + Language, LanguageRegistry, LanguageServerName, LineEnding, LocalFile, LspAdapter, + OffsetRangeExt, Operation, Patch, PointUtf16, TextBufferSnapshot, ToOffset, ToPointUtf16, + Transaction, }; use lsp::{ DiagnosticSeverity, DiagnosticTag, DocumentHighlightKind, LanguageServer, LanguageString, @@ -3381,7 +3382,8 @@ impl Project { return None; } - let (old_range, new_text) = match lsp_completion.text_edit.as_ref() { + let (old_range, mut new_text) = match lsp_completion.text_edit.as_ref() + { // If the language server provides a range to overwrite, then // check that the range is valid. Some(lsp::CompletionTextEdit::Edit(edit)) => { @@ -3431,6 +3433,7 @@ impl Project { } }; + LineEnding::normalize(&mut new_text); Some(Completion { old_range, new_text, diff --git a/crates/project/src/project_tests.rs b/crates/project/src/project_tests.rs index 9d80dadb840a79db455e53e25c38c379317b6061..00f7bb8c9463c32079f6c7057cb0c57f1c2ea46e 100644 --- a/crates/project/src/project_tests.rs +++ b/crates/project/src/project_tests.rs @@ -1850,6 +1850,59 @@ async fn test_completions_without_edit_ranges(cx: &mut gpui::TestAppContext) { ); } +#[gpui::test] +async fn test_completions_with_carriage_returns(cx: &mut gpui::TestAppContext) { + let mut language = Language::new( + LanguageConfig { + name: "TypeScript".into(), + path_suffixes: vec!["ts".to_string()], + ..Default::default() + }, + Some(tree_sitter_typescript::language_typescript()), + ); + let mut fake_language_servers = language.set_fake_lsp_adapter(Default::default()); + + let fs = FakeFs::new(cx.background()); + fs.insert_tree( + "/dir", + json!({ + "a.ts": "", + }), + ) + .await; + + let project = Project::test(fs, ["/dir".as_ref()], cx).await; + project.update(cx, |project, _| project.languages.add(Arc::new(language))); + let buffer = project + .update(cx, |p, cx| p.open_local_buffer("/dir/a.ts", cx)) + .await + .unwrap(); + + let fake_server = fake_language_servers.next().await.unwrap(); + + let text = "let a = b.fqn"; + buffer.update(cx, |buffer, cx| buffer.set_text(text, cx)); + let completions = project.update(cx, |project, cx| { + project.completions(&buffer, text.len(), cx) + }); + + fake_server + .handle_request::(|_, _| async move { + Ok(Some(lsp::CompletionResponse::Array(vec![ + lsp::CompletionItem { + label: "fullyQualifiedName?".into(), + insert_text: Some("fully\rQualified\r\nName".into()), + ..Default::default() + }, + ]))) + }) + .next() + .await; + let completions = completions.await.unwrap(); + assert_eq!(completions.len(), 1); + assert_eq!(completions[0].new_text, "fully\nQualified\nName"); +} + #[gpui::test(iterations = 10)] async fn test_apply_code_actions_with_commands(cx: &mut gpui::TestAppContext) { let mut language = Language::new( diff --git a/crates/text/src/tests.rs b/crates/text/src/tests.rs index c0cc0556d56fd6c50e27256ffbf304dc86b836c2..4da9edd7351d3c67bf128d5d4b94a656217c04a6 100644 --- a/crates/text/src/tests.rs +++ b/crates/text/src/tests.rs @@ -43,7 +43,7 @@ fn test_random_edits(mut rng: StdRng) { .take(reference_string_len) .collect::(); let mut buffer = Buffer::new(0, 0, reference_string.clone().into()); - LineEnding::strip_carriage_returns(&mut reference_string); + LineEnding::normalize(&mut reference_string); buffer.history.group_interval = Duration::from_millis(rng.gen_range(0..=200)); let mut buffer_versions = Vec::new(); diff --git a/crates/text/src/text.rs b/crates/text/src/text.rs index f73163438b13b0b6931af508d011ee385c11d68c..536b8329420628693f84dad0b9398de2577164f0 100644 --- a/crates/text/src/text.rs +++ b/crates/text/src/text.rs @@ -555,7 +555,7 @@ pub struct UndoOperation { impl Buffer { pub fn new(replica_id: u16, remote_id: u64, mut base_text: String) -> Buffer { let line_ending = LineEnding::detect(&base_text); - LineEnding::strip_carriage_returns(&mut base_text); + LineEnding::normalize(&mut base_text); let history = History::new(base_text.into()); let mut fragments = SumTree::new(); @@ -691,7 +691,7 @@ impl Buffer { let mut fragment_start = old_fragments.start().visible; for (range, new_text) in edits { - let new_text = LineEnding::strip_carriage_returns_from_arc(new_text.into()); + let new_text = LineEnding::normalize_arc(new_text.into()); let fragment_end = old_fragments.end(&None).visible; // If the current fragment ends before this range, then jump ahead to the first fragment @@ -2385,13 +2385,13 @@ impl LineEnding { } } - pub fn strip_carriage_returns(text: &mut String) { + pub fn normalize(text: &mut String) { if let Cow::Owned(replaced) = CARRIAGE_RETURNS_REGEX.replace_all(text, "\n") { *text = replaced; } } - fn strip_carriage_returns_from_arc(text: Arc) -> Arc { + fn normalize_arc(text: Arc) -> Arc { if let Cow::Owned(replaced) = CARRIAGE_RETURNS_REGEX.replace_all(&text, "\n") { replaced.into() } else { From a858b3fda9134361c46b6dcbbb590b4710cbfbf7 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Wed, 6 Jul 2022 11:20:29 -0700 Subject: [PATCH 14/28] Treat window deactivation as a focus change for the purpose of autosave --- crates/workspace/src/workspace.rs | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 53318d943fe1b5854823e88b4bcbd1edd474692a..f7d3032eabd33b327523af15603f642d8f3234d0 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -2360,7 +2360,12 @@ impl Workspace { } fn on_window_activation_changed(&mut self, active: bool, cx: &mut ViewContext) { - if !active && cx.global::().autosave == Autosave::OnWindowChange { + if !active + && matches!( + cx.global::().autosave, + Autosave::OnWindowChange | Autosave::OnFocusChange + ) + { for pane in &self.panes { pane.update(cx, |pane, cx| { for item in pane.items() { @@ -3073,6 +3078,17 @@ mod tests { deterministic.run_until_parked(); item.read_with(cx, |item, _| assert_eq!(item.save_count, 2)); + // Deactivating the window still saves the file. + cx.simulate_window_activation(Some(window_id)); + item.update(cx, |item, cx| { + cx.focus_self(); + item.is_dirty = true; + }); + cx.simulate_window_activation(None); + + deterministic.run_until_parked(); + item.read_with(cx, |item, _| assert_eq!(item.save_count, 3)); + // Autosave after delay. item.update(cx, |item, cx| { cx.update_global(|settings: &mut Settings, _| { @@ -3084,11 +3100,11 @@ mod tests { // Delay hasn't fully expired, so the file is still dirty and unsaved. deterministic.advance_clock(Duration::from_millis(250)); - item.read_with(cx, |item, _| assert_eq!(item.save_count, 2)); + item.read_with(cx, |item, _| assert_eq!(item.save_count, 3)); // After delay expires, the file is saved. deterministic.advance_clock(Duration::from_millis(250)); - item.read_with(cx, |item, _| assert_eq!(item.save_count, 3)); + item.read_with(cx, |item, _| assert_eq!(item.save_count, 4)); // Autosave on focus change, ensuring closing the tab counts as such. item.update(cx, |item, cx| { @@ -3106,7 +3122,7 @@ mod tests { .await .unwrap(); assert!(!cx.has_pending_prompt(window_id)); - item.read_with(cx, |item, _| assert_eq!(item.save_count, 4)); + item.read_with(cx, |item, _| assert_eq!(item.save_count, 5)); // Add the item again, ensuring autosave is prevented if the underlying file has been deleted. workspace.update(cx, |workspace, cx| { @@ -3118,7 +3134,7 @@ mod tests { cx.blur(); }); deterministic.run_until_parked(); - item.read_with(cx, |item, _| assert_eq!(item.save_count, 4)); + item.read_with(cx, |item, _| assert_eq!(item.save_count, 5)); // Ensure autosave is prevented for deleted files also when closing the buffer. let _close_items = workspace.update(cx, |workspace, cx| { @@ -3127,7 +3143,7 @@ mod tests { }); deterministic.run_until_parked(); assert!(cx.has_pending_prompt(window_id)); - item.read_with(cx, |item, _| assert_eq!(item.save_count, 4)); + item.read_with(cx, |item, _| assert_eq!(item.save_count, 5)); } #[derive(Clone)] From bbe325930f78e478775022c9df86be75ca9d847d Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Wed, 6 Jul 2022 11:32:21 -0700 Subject: [PATCH 15/28] 0.45 --- Cargo.lock | 2 +- crates/zed/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7dcecd009390c92a0602ae3fc70c421be27c6f1d..8624921d976cae526e121a843be3eeec68e5c740 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6159,7 +6159,7 @@ dependencies = [ [[package]] name = "zed" -version = "0.44.1" +version = "0.45.0" dependencies = [ "activity_indicator", "anyhow", diff --git a/crates/zed/Cargo.toml b/crates/zed/Cargo.toml index 6d488e83e8ce64729791d96b2a6b9c28119a69e9..88acfdb14dd2f3796c5715abfe56a4c8da8fd829 100644 --- a/crates/zed/Cargo.toml +++ b/crates/zed/Cargo.toml @@ -3,7 +3,7 @@ authors = ["Nathan Sobo "] description = "The fast, collaborative code editor." edition = "2021" name = "zed" -version = "0.44.1" +version = "0.45.0" [lib] name = "zed" From 7e5cf6669ff985c96f1824a58e3e7cb2cd237021 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Wed, 6 Jul 2022 14:05:24 -0700 Subject: [PATCH 16/28] Add forward and backward navigation buttons to toolbar --- assets/icons/arrow-left.svg | 3 ++ assets/icons/arrow-right.svg | 3 ++ crates/theme/src/theme.rs | 1 + crates/workspace/src/pane.rs | 3 +- crates/workspace/src/toolbar.rs | 70 +++++++++++++++++++++++++++---- styles/src/styleTree/workspace.ts | 9 ++++ 6 files changed, 80 insertions(+), 9 deletions(-) create mode 100644 assets/icons/arrow-left.svg create mode 100644 assets/icons/arrow-right.svg diff --git a/assets/icons/arrow-left.svg b/assets/icons/arrow-left.svg new file mode 100644 index 0000000000000000000000000000000000000000..904fdaa1a73221efda6946614db7c707dc74c2ed --- /dev/null +++ b/assets/icons/arrow-left.svg @@ -0,0 +1,3 @@ + + + diff --git a/assets/icons/arrow-right.svg b/assets/icons/arrow-right.svg new file mode 100644 index 0000000000000000000000000000000000000000..b7e1bec6d8a0830b8ebfcf677cd1a5b3480ce5b0 --- /dev/null +++ b/assets/icons/arrow-right.svg @@ -0,0 +1,3 @@ + + + diff --git a/crates/theme/src/theme.rs b/crates/theme/src/theme.rs index 184b1880f0c0a05cc581059b80766b3d282cc9cc..c3052ff6c5afc0a7a5acde8135f555e7113ab6c1 100644 --- a/crates/theme/src/theme.rs +++ b/crates/theme/src/theme.rs @@ -108,6 +108,7 @@ pub struct Toolbar { pub container: ContainerStyle, pub height: f32, pub item_spacing: f32, + pub nav_button: Interactive, } #[derive(Clone, Deserialize, Default)] diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index bbc086395b12a4fd09d7c8ce07e6c52c6491cb53..0fb5225cc533266753c4b84acf225373ce0c6cf0 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -168,12 +168,13 @@ pub struct NavigationEntry { impl Pane { pub fn new(cx: &mut ViewContext) -> Self { + let handle = cx.weak_handle(); Self { items: Vec::new(), active_item_index: 0, autoscroll: false, nav_history: Default::default(), - toolbar: cx.add_view(|_| Toolbar::new()), + toolbar: cx.add_view(|_| Toolbar::new(handle)), } } diff --git a/crates/workspace/src/toolbar.rs b/crates/workspace/src/toolbar.rs index e9b20bf3a04e5876a7a6e202f06b3b78c8727a68..9d8274288be5b6e5a4d8a8a26287da028f347768 100644 --- a/crates/workspace/src/toolbar.rs +++ b/crates/workspace/src/toolbar.rs @@ -1,7 +1,7 @@ -use crate::ItemHandle; +use crate::{ItemHandle, Pane}; use gpui::{ - elements::*, AnyViewHandle, AppContext, ElementBox, Entity, MutableAppContext, RenderContext, - View, ViewContext, ViewHandle, + elements::*, platform::CursorStyle, Action, AnyViewHandle, AppContext, ElementBox, Entity, + MutableAppContext, RenderContext, View, ViewContext, ViewHandle, WeakViewHandle, }; use settings::Settings; @@ -42,6 +42,7 @@ pub enum ToolbarItemLocation { pub struct Toolbar { active_pane_item: Option>, + pane: WeakViewHandle, items: Vec<(Box, ToolbarItemLocation)>, } @@ -60,6 +61,7 @@ impl View for Toolbar { let mut primary_left_items = Vec::new(); let mut primary_right_items = Vec::new(); let mut secondary_item = None; + let spacing = theme.item_spacing; for (item, position) in &self.items { match *position { @@ -68,7 +70,7 @@ impl View for Toolbar { let left_item = ChildView::new(item.as_ref()) .aligned() .contained() - .with_margin_right(theme.item_spacing); + .with_margin_right(spacing); if let Some((flex, expanded)) = flex { primary_left_items.push(left_item.flex(flex, expanded).boxed()); } else { @@ -79,7 +81,7 @@ impl View for Toolbar { let right_item = ChildView::new(item.as_ref()) .aligned() .contained() - .with_margin_left(theme.item_spacing) + .with_margin_left(spacing) .flex_float(); if let Some((flex, expanded)) = flex { primary_right_items.push(right_item.flex(flex, expanded).boxed()); @@ -98,26 +100,78 @@ impl View for Toolbar { } } + let pane = self.pane.clone(); + let container_style = theme.container; + let height = theme.height; + let button_style = theme.nav_button; + Flex::column() .with_child( Flex::row() + .with_child(nav_button( + "icons/arrow-left.svg", + button_style, + spacing, + super::GoBack { + pane: Some(pane.clone()), + }, + cx, + )) + .with_child(nav_button( + "icons/arrow-right.svg", + button_style, + spacing, + super::GoForward { + pane: Some(pane.clone()), + }, + cx, + )) .with_children(primary_left_items) .with_children(primary_right_items) .constrained() - .with_height(theme.height) + .with_height(height) .boxed(), ) .with_children(secondary_item) .contained() - .with_style(theme.container) + .with_style(container_style) .boxed() } } +fn nav_button( + svg_path: &'static str, + style: theme::Interactive, + spacing: f32, + action: A, + cx: &mut RenderContext, +) -> ElementBox { + MouseEventHandler::new::(0, cx, |state, _| { + let style = style.style_for(state, false); + Svg::new(svg_path) + .with_color(style.color) + .constrained() + .with_width(style.icon_width) + .aligned() + .contained() + .with_style(style.container) + .constrained() + .with_width(style.button_width) + .with_height(style.button_width) + .boxed() + }) + .with_cursor_style(CursorStyle::PointingHand) + .on_mouse_down(move |_, cx| cx.dispatch_action(action.clone())) + .contained() + .with_margin_right(spacing) + .boxed() +} + impl Toolbar { - pub fn new() -> Self { + pub fn new(pane: WeakViewHandle) -> Self { Self { active_pane_item: None, + pane, items: Default::default(), } } diff --git a/styles/src/styleTree/workspace.ts b/styles/src/styleTree/workspace.ts index 2deadc02e7730041c2801d5fb87cf95c7bfb3ec8..ba3978fc74e1ef93d956cd30beb8d64c9f89df8b 100644 --- a/styles/src/styleTree/workspace.ts +++ b/styles/src/styleTree/workspace.ts @@ -139,6 +139,15 @@ export default function workspace(theme: Theme) { background: backgroundColor(theme, 500), border: border(theme, "secondary", { bottom: true }), itemSpacing: 8, + navButton: { + color: iconColor(theme, "secondary"), + iconWidth: 8, + buttonWidth: 12, + margin: { left: 8, right: 8 }, + hover: { + color: iconColor(theme, "active"), + }, + }, padding: { left: 16, right: 8, top: 4, bottom: 4 }, }, breadcrumbs: { From a378ec49ec8e4bd0a8ed08f32a35d4f830f16730 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Wed, 6 Jul 2022 15:43:42 -0700 Subject: [PATCH 17/28] Enable and disable nav buttons based on pane's navigation stack Also, make the `NavHistory` type private to the `workspace` crate. Expose only the `ItemNavHistory` type, via a method on Pane called `nav_history_for_item`. --- crates/editor/src/editor.rs | 57 +++++++------ crates/theme/src/theme.rs | 29 +++---- crates/workspace/src/pane.rs | 128 ++++++++++++++++++++---------- crates/workspace/src/toolbar.rs | 23 +++++- crates/workspace/src/workspace.rs | 10 +-- styles/src/styleTree/workspace.ts | 6 +- 6 files changed, 158 insertions(+), 95 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 9dd40413fd254e7ef852643acfdb5cbe464ee415..c1e25575557bef78921841f1c2a9eb8e589085a2 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -4065,13 +4065,16 @@ impl Editor { } } - nav_history.push(Some(NavigationData { - cursor_anchor: position, - cursor_position: point, - scroll_position: self.scroll_position, - scroll_top_anchor: self.scroll_top_anchor.clone(), - scroll_top_row, - })); + nav_history.push( + Some(NavigationData { + cursor_anchor: position, + cursor_position: point, + scroll_position: self.scroll_position, + scroll_top_anchor: self.scroll_top_anchor.clone(), + scroll_top_row, + }), + cx, + ); } } @@ -4669,7 +4672,7 @@ impl Editor { definitions: Vec, cx: &mut ViewContext, ) { - let nav_history = workspace.active_pane().read(cx).nav_history().clone(); + let pane = workspace.active_pane().clone(); for definition in definitions { let range = definition .target @@ -4681,13 +4684,13 @@ impl Editor { // When selecting a definition in a different buffer, disable the nav history // to avoid creating a history entry at the previous cursor location. if editor_handle != target_editor_handle { - nav_history.borrow_mut().disable(); + pane.update(cx, |pane, _| pane.disable_history()); } target_editor.change_selections(Some(Autoscroll::Center), cx, |s| { s.select_ranges([range]); }); - nav_history.borrow_mut().enable(); + pane.update(cx, |pane, _| pane.enable_history()); }); } } @@ -5641,8 +5644,8 @@ impl Editor { editor_handle.update(cx, |editor, cx| { editor.push_to_nav_history(editor.selections.newest_anchor().head(), None, cx); }); - let nav_history = workspace.active_pane().read(cx).nav_history().clone(); - nav_history.borrow_mut().disable(); + let pane = workspace.active_pane().clone(); + pane.update(cx, |pane, _| pane.disable_history()); // We defer the pane interaction because we ourselves are a workspace item // and activating a new item causes the pane to call a method on us reentrantly, @@ -5657,7 +5660,7 @@ impl Editor { }); } - nav_history.borrow_mut().enable(); + pane.update(cx, |pane, _| pane.enable_history()); }); } @@ -6241,7 +6244,7 @@ mod tests { assert_set_eq, test::{marked_text_by, marked_text_ranges, marked_text_ranges_by, sample_text}, }; - use workspace::{FollowableItem, ItemHandle}; + use workspace::{FollowableItem, ItemHandle, NavigationEntry, Pane}; #[gpui::test] fn test_edit_events(cx: &mut MutableAppContext) { @@ -6589,12 +6592,20 @@ mod tests { fn test_navigation_history(cx: &mut gpui::MutableAppContext) { cx.set_global(Settings::test(cx)); use workspace::Item; - let nav_history = Rc::new(RefCell::new(workspace::NavHistory::default())); + let pane = cx.add_view(Default::default(), |cx| Pane::new(cx)); let buffer = MultiBuffer::build_simple(&sample_text(300, 5, 'a'), cx); cx.add_window(Default::default(), |cx| { let mut editor = build_editor(buffer.clone(), cx); - editor.nav_history = Some(ItemNavHistory::new(nav_history.clone(), &cx.handle())); + let handle = cx.handle(); + editor.set_nav_history(Some(pane.read(cx).nav_history_for_item(&handle))); + + fn pop_history( + editor: &mut Editor, + cx: &mut MutableAppContext, + ) -> Option { + editor.nav_history.as_mut().unwrap().pop_backward(cx) + } // Move the cursor a small distance. // Nothing is added to the navigation history. @@ -6604,21 +6615,21 @@ mod tests { editor.change_selections(None, cx, |s| { s.select_display_ranges([DisplayPoint::new(3, 0)..DisplayPoint::new(3, 0)]) }); - assert!(nav_history.borrow_mut().pop_backward().is_none()); + assert!(pop_history(&mut editor, cx).is_none()); // Move the cursor a large distance. // The history can jump back to the previous position. editor.change_selections(None, cx, |s| { s.select_display_ranges([DisplayPoint::new(13, 0)..DisplayPoint::new(13, 3)]) }); - let nav_entry = nav_history.borrow_mut().pop_backward().unwrap(); + let nav_entry = pop_history(&mut editor, cx).unwrap(); editor.navigate(nav_entry.data.unwrap(), cx); assert_eq!(nav_entry.item.id(), cx.view_id()); assert_eq!( editor.selections.display_ranges(cx), &[DisplayPoint::new(3, 0)..DisplayPoint::new(3, 0)] ); - assert!(nav_history.borrow_mut().pop_backward().is_none()); + assert!(pop_history(&mut editor, cx).is_none()); // Move the cursor a small distance via the mouse. // Nothing is added to the navigation history. @@ -6628,7 +6639,7 @@ mod tests { editor.selections.display_ranges(cx), &[DisplayPoint::new(5, 0)..DisplayPoint::new(5, 0)] ); - assert!(nav_history.borrow_mut().pop_backward().is_none()); + assert!(pop_history(&mut editor, cx).is_none()); // Move the cursor a large distance via the mouse. // The history can jump back to the previous position. @@ -6638,14 +6649,14 @@ mod tests { editor.selections.display_ranges(cx), &[DisplayPoint::new(15, 0)..DisplayPoint::new(15, 0)] ); - let nav_entry = nav_history.borrow_mut().pop_backward().unwrap(); + let nav_entry = pop_history(&mut editor, cx).unwrap(); editor.navigate(nav_entry.data.unwrap(), cx); assert_eq!(nav_entry.item.id(), cx.view_id()); assert_eq!( editor.selections.display_ranges(cx), &[DisplayPoint::new(5, 0)..DisplayPoint::new(5, 0)] ); - assert!(nav_history.borrow_mut().pop_backward().is_none()); + assert!(pop_history(&mut editor, cx).is_none()); // Set scroll position to check later editor.set_scroll_position(Vector2F::new(5.5, 5.5), cx); @@ -6658,7 +6669,7 @@ mod tests { assert_ne!(editor.scroll_position, original_scroll_position); assert_ne!(editor.scroll_top_anchor, original_scroll_top_anchor); - let nav_entry = nav_history.borrow_mut().pop_backward().unwrap(); + let nav_entry = pop_history(&mut editor, cx).unwrap(); editor.navigate(nav_entry.data.unwrap(), cx); assert_eq!(editor.scroll_position, original_scroll_position); assert_eq!(editor.scroll_top_anchor, original_scroll_top_anchor); diff --git a/crates/theme/src/theme.rs b/crates/theme/src/theme.rs index c3052ff6c5afc0a7a5acde8135f555e7113ab6c1..058dd5a331729c2ab84dcdd5d41f7349768ce48c 100644 --- a/crates/theme/src/theme.rs +++ b/crates/theme/src/theme.rs @@ -510,28 +510,23 @@ pub struct Interactive { pub default: T, pub hover: Option, pub active: Option, - pub active_hover: Option, + pub disabled: Option, } impl Interactive { pub fn style_for(&self, state: MouseState, active: bool) -> &T { if active { - if state.hovered { - self.active_hover - .as_ref() - .or(self.active.as_ref()) - .unwrap_or(&self.default) - } else { - self.active.as_ref().unwrap_or(&self.default) - } + self.active.as_ref().unwrap_or(&self.default) + } else if state.hovered { + self.hover.as_ref().unwrap_or(&self.default) } else { - if state.hovered { - self.hover.as_ref().unwrap_or(&self.default) - } else { - &self.default - } + &self.default } } + + pub fn disabled_style(&self) -> &T { + self.disabled.as_ref().unwrap_or(&self.default) + } } impl<'de, T: DeserializeOwned> Deserialize<'de> for Interactive { @@ -545,7 +540,7 @@ impl<'de, T: DeserializeOwned> Deserialize<'de> for Interactive { default: Value, hover: Option, active: Option, - active_hover: Option, + disabled: Option, } let json = Helper::deserialize(deserializer)?; @@ -571,14 +566,14 @@ impl<'de, T: DeserializeOwned> Deserialize<'de> for Interactive { let hover = deserialize_state(json.hover)?; let active = deserialize_state(json.active)?; - let active_hover = deserialize_state(json.active_hover)?; + let disabled = deserialize_state(json.disabled)?; let default = serde_json::from_value(json.default).map_err(serde::de::Error::custom)?; Ok(Interactive { default, hover, active, - active_hover, + disabled, }) } } diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 0fb5225cc533266753c4b84acf225373ce0c6cf0..391ddfc1f7a6d0c9e6a727622d925b3d40ead78b 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -136,13 +136,13 @@ pub struct ItemNavHistory { item: Rc, } -#[derive(Default)] -pub struct NavHistory { +struct NavHistory { mode: NavigationMode, backward_stack: VecDeque, forward_stack: VecDeque, closed_stack: VecDeque, paths_by_item: HashMap, + pane: WeakViewHandle, } #[derive(Copy, Clone)] @@ -173,13 +173,23 @@ impl Pane { items: Vec::new(), active_item_index: 0, autoscroll: false, - nav_history: Default::default(), + nav_history: Rc::new(RefCell::new(NavHistory { + mode: NavigationMode::Normal, + backward_stack: Default::default(), + forward_stack: Default::default(), + closed_stack: Default::default(), + paths_by_item: Default::default(), + pane: handle.clone(), + })), toolbar: cx.add_view(|_| Toolbar::new(handle)), } } - pub fn nav_history(&self) -> &Rc> { - &self.nav_history + pub fn nav_history_for_item(&self, item: &ViewHandle) -> ItemNavHistory { + ItemNavHistory { + history: self.nav_history.clone(), + item: Rc::new(item.downgrade()), + } } pub fn activate(&self, cx: &mut ViewContext) { @@ -224,6 +234,26 @@ impl Pane { ) } + pub fn disable_history(&mut self) { + self.nav_history.borrow_mut().disable(); + } + + pub fn enable_history(&mut self) { + self.nav_history.borrow_mut().enable(); + } + + pub fn can_navigate_backward(&self) -> bool { + !self.nav_history.borrow().backward_stack.is_empty() + } + + pub fn can_navigate_forward(&self) -> bool { + !self.nav_history.borrow().forward_stack.is_empty() + } + + fn history_updated(&mut self, cx: &mut ViewContext) { + self.toolbar.update(cx, |_, cx| cx.notify()); + } + fn navigate_history( workspace: &mut Workspace, pane: ViewHandle, @@ -235,7 +265,7 @@ impl Pane { let to_load = pane.update(cx, |pane, cx| { loop { // Retrieve the weak item handle from the history. - let entry = pane.nav_history.borrow_mut().pop(mode)?; + let entry = pane.nav_history.borrow_mut().pop(mode, cx)?; // If the item is still present in this pane, then activate it. if let Some(index) = entry @@ -368,7 +398,6 @@ impl Pane { return; } - item.set_nav_history(pane.read(cx).nav_history.clone(), cx); item.added_to_pane(workspace, pane.clone(), cx); pane.update(cx, |pane, cx| { // If there is already an active item, then insert the new item @@ -626,11 +655,16 @@ impl Pane { .borrow_mut() .set_mode(NavigationMode::Normal); - let mut nav_history = pane.nav_history().borrow_mut(); if let Some(path) = item.project_path(cx) { - nav_history.paths_by_item.insert(item.id(), path); + pane.nav_history + .borrow_mut() + .paths_by_item + .insert(item.id(), path); } else { - nav_history.paths_by_item.remove(&item.id()); + pane.nav_history + .borrow_mut() + .paths_by_item + .remove(&item.id()); } } }); @@ -954,57 +988,56 @@ impl View for Pane { } impl ItemNavHistory { - pub fn new(history: Rc>, item: &ViewHandle) -> Self { - Self { - history, - item: Rc::new(item.downgrade()), - } + pub fn push(&self, data: Option, cx: &mut MutableAppContext) { + self.history.borrow_mut().push(data, self.item.clone(), cx); } - pub fn history(&self) -> Rc> { - self.history.clone() + pub fn pop_backward(&self, cx: &mut MutableAppContext) -> Option { + self.history.borrow_mut().pop(NavigationMode::GoingBack, cx) } - pub fn push(&self, data: Option) { - self.history.borrow_mut().push(data, self.item.clone()); + pub fn pop_forward(&self, cx: &mut MutableAppContext) -> Option { + self.history + .borrow_mut() + .pop(NavigationMode::GoingForward, cx) } } impl NavHistory { - pub fn disable(&mut self) { - self.mode = NavigationMode::Disabled; - } - - pub fn enable(&mut self) { - self.mode = NavigationMode::Normal; - } - - pub fn pop_backward(&mut self) -> Option { - self.backward_stack.pop_back() + fn set_mode(&mut self, mode: NavigationMode) { + self.mode = mode; } - pub fn pop_forward(&mut self) -> Option { - self.forward_stack.pop_back() + fn disable(&mut self) { + self.mode = NavigationMode::Disabled; } - pub fn pop_closed(&mut self) -> Option { - self.closed_stack.pop_back() + fn enable(&mut self) { + self.mode = NavigationMode::Normal; } - fn pop(&mut self, mode: NavigationMode) -> Option { - match mode { - NavigationMode::Normal | NavigationMode::Disabled | NavigationMode::ClosingItem => None, - NavigationMode::GoingBack => self.pop_backward(), - NavigationMode::GoingForward => self.pop_forward(), - NavigationMode::ReopeningClosedItem => self.pop_closed(), + fn pop(&mut self, mode: NavigationMode, cx: &mut MutableAppContext) -> Option { + let entry = match mode { + NavigationMode::Normal | NavigationMode::Disabled | NavigationMode::ClosingItem => { + return None + } + NavigationMode::GoingBack => &mut self.backward_stack, + NavigationMode::GoingForward => &mut self.forward_stack, + NavigationMode::ReopeningClosedItem => &mut self.closed_stack, } + .pop_back(); + if entry.is_some() { + self.did_update(cx); + } + entry } - fn set_mode(&mut self, mode: NavigationMode) { - self.mode = mode; - } - - pub fn push(&mut self, data: Option, item: Rc) { + fn push( + &mut self, + data: Option, + item: Rc, + cx: &mut MutableAppContext, + ) { match self.mode { NavigationMode::Disabled => {} NavigationMode::Normal | NavigationMode::ReopeningClosedItem => { @@ -1045,5 +1078,12 @@ impl NavHistory { }); } } + self.did_update(cx); + } + + fn did_update(&self, cx: &mut MutableAppContext) { + if let Some(pane) = self.pane.upgrade(cx) { + cx.defer(move |cx| pane.update(cx, |pane, cx| pane.history_updated(cx))); + } } } diff --git a/crates/workspace/src/toolbar.rs b/crates/workspace/src/toolbar.rs index 9d8274288be5b6e5a4d8a8a26287da028f347768..7525c0413d7097d7b4e2e1f338835737ba2eab25 100644 --- a/crates/workspace/src/toolbar.rs +++ b/crates/workspace/src/toolbar.rs @@ -101,6 +101,14 @@ impl View for Toolbar { } let pane = self.pane.clone(); + let mut enable_go_backward = false; + let mut enable_go_forward = false; + if let Some(pane) = pane.upgrade(cx) { + let pane = pane.read(cx); + enable_go_backward = pane.can_navigate_backward(); + enable_go_forward = pane.can_navigate_forward(); + } + let container_style = theme.container; let height = theme.height; let button_style = theme.nav_button; @@ -111,6 +119,7 @@ impl View for Toolbar { .with_child(nav_button( "icons/arrow-left.svg", button_style, + enable_go_backward, spacing, super::GoBack { pane: Some(pane.clone()), @@ -120,6 +129,7 @@ impl View for Toolbar { .with_child(nav_button( "icons/arrow-right.svg", button_style, + enable_go_forward, spacing, super::GoForward { pane: Some(pane.clone()), @@ -142,12 +152,17 @@ impl View for Toolbar { fn nav_button( svg_path: &'static str, style: theme::Interactive, + enabled: bool, spacing: f32, action: A, cx: &mut RenderContext, ) -> ElementBox { MouseEventHandler::new::(0, cx, |state, _| { - let style = style.style_for(state, false); + let style = if enabled { + style.style_for(state, false) + } else { + style.disabled_style() + }; Svg::new(svg_path) .with_color(style.color) .constrained() @@ -160,7 +175,11 @@ fn nav_button( .with_height(style.button_width) .boxed() }) - .with_cursor_style(CursorStyle::PointingHand) + .with_cursor_style(if enabled { + CursorStyle::PointingHand + } else { + CursorStyle::default() + }) .on_mouse_down(move |_, cx| cx.dispatch_action(action.clone())) .contained() .with_margin_right(spacing) diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index f7d3032eabd33b327523af15603f642d8f3234d0..2ba1b4d008400d2429afac0874c164e296c673ef 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -414,7 +414,6 @@ pub trait ItemHandle: 'static + fmt::Debug { fn project_entry_ids(&self, cx: &AppContext) -> SmallVec<[ProjectEntryId; 3]>; fn is_singleton(&self, cx: &AppContext) -> bool; fn boxed_clone(&self) -> Box; - fn set_nav_history(&self, nav_history: Rc>, cx: &mut MutableAppContext); fn clone_on_split(&self, cx: &mut MutableAppContext) -> Option>; fn added_to_pane( &self, @@ -484,12 +483,6 @@ impl ItemHandle for ViewHandle { Box::new(self.clone()) } - fn set_nav_history(&self, nav_history: Rc>, cx: &mut MutableAppContext) { - self.update(cx, |item, cx| { - item.set_nav_history(ItemNavHistory::new(nav_history, &cx.handle()), cx); - }) - } - fn clone_on_split(&self, cx: &mut MutableAppContext) -> Option> { self.update(cx, |item, cx| { cx.add_option_view(|cx| item.clone_on_split(cx)) @@ -503,6 +496,9 @@ impl ItemHandle for ViewHandle { pane: ViewHandle, cx: &mut ViewContext, ) { + let history = pane.read(cx).nav_history_for_item(self); + self.update(cx, |this, cx| this.set_nav_history(history, cx)); + if let Some(followed_item) = self.to_followable_item_handle(cx) { if let Some(message) = followed_item.to_state_proto(cx) { workspace.update_followers( diff --git a/styles/src/styleTree/workspace.ts b/styles/src/styleTree/workspace.ts index ba3978fc74e1ef93d956cd30beb8d64c9f89df8b..0b71453e7148c71a4664c135c0b0f6926759d1db 100644 --- a/styles/src/styleTree/workspace.ts +++ b/styles/src/styleTree/workspace.ts @@ -140,13 +140,15 @@ export default function workspace(theme: Theme) { border: border(theme, "secondary", { bottom: true }), itemSpacing: 8, navButton: { - color: iconColor(theme, "secondary"), + color: iconColor(theme, "primary"), iconWidth: 8, buttonWidth: 12, - margin: { left: 8, right: 8 }, hover: { color: iconColor(theme, "active"), }, + disabled: { + color: iconColor(theme, "muted") + } }, padding: { left: 16, right: 8, top: 4, bottom: 4 }, }, From 4e8dbbfd4b637fa2ad114f42298584643cc31637 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Wed, 6 Jul 2022 16:29:11 -0700 Subject: [PATCH 18/28] Add test for pane nav history covering notification of pane's toolbar --- crates/workspace/src/workspace.rs | 113 +++++++++++++++++++++++++++++- 1 file changed, 110 insertions(+), 3 deletions(-) diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 2ba1b4d008400d2429afac0874c164e296c673ef..fdfa640718fdd066f801a81142c54e7a99c9c9b3 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -3142,25 +3142,104 @@ mod tests { item.read_with(cx, |item, _| assert_eq!(item.save_count, 5)); } - #[derive(Clone)] + #[gpui::test] + async fn test_pane_navigation( + deterministic: Arc, + cx: &mut gpui::TestAppContext, + ) { + deterministic.forbid_parking(); + Settings::test_async(cx); + let fs = FakeFs::new(cx.background()); + + let project = Project::test(fs, [], cx).await; + let (window_id, workspace) = cx.add_window(|cx| Workspace::new(project, cx)); + + let item = cx.add_view(window_id, |_| { + let mut item = TestItem::new(); + item.project_entry_ids = vec![ProjectEntryId::from_proto(1)]; + item + }); + let pane = workspace.read_with(cx, |workspace, _| workspace.active_pane().clone()); + let toolbar = pane.read_with(cx, |pane, _| pane.toolbar().clone()); + let toolbar_notify_count = Rc::new(RefCell::new(0)); + + workspace.update(cx, |workspace, cx| { + workspace.add_item(Box::new(item.clone()), cx); + let toolbar_notification_count = toolbar_notify_count.clone(); + cx.observe(&toolbar, move |_, _, _| { + *toolbar_notification_count.borrow_mut() += 1 + }) + .detach(); + }); + + pane.read_with(cx, |pane, _| { + assert!(!pane.can_navigate_backward()); + assert!(!pane.can_navigate_forward()); + }); + + item.update(cx, |item, cx| { + item.set_state("one".to_string(), cx); + }); + + // Toolbar must be notified to re-render the navigation buttons + assert_eq!(*toolbar_notify_count.borrow(), 1); + + pane.read_with(cx, |pane, _| { + assert!(pane.can_navigate_backward()); + assert!(!pane.can_navigate_forward()); + }); + + workspace + .update(cx, |workspace, cx| { + Pane::go_back(workspace, Some(pane.clone()), cx) + }) + .await; + + assert_eq!(*toolbar_notify_count.borrow(), 3); + pane.read_with(cx, |pane, _| { + assert!(!pane.can_navigate_backward()); + assert!(pane.can_navigate_forward()); + }); + } + struct TestItem { + state: String, save_count: usize, save_as_count: usize, reload_count: usize, is_dirty: bool, + is_singleton: bool, has_conflict: bool, project_entry_ids: Vec, project_path: Option, - is_singleton: bool, + nav_history: Option, } enum TestItemEvent { Edit, } + impl Clone for TestItem { + fn clone(&self) -> Self { + Self { + state: self.state.clone(), + save_count: self.save_count, + save_as_count: self.save_as_count, + reload_count: self.reload_count, + is_dirty: self.is_dirty, + is_singleton: self.is_singleton, + has_conflict: self.has_conflict, + project_entry_ids: self.project_entry_ids.clone(), + project_path: self.project_path.clone(), + nav_history: None, + } + } + } + impl TestItem { fn new() -> Self { Self { + state: String::new(), save_count: 0, save_as_count: 0, reload_count: 0, @@ -3169,6 +3248,18 @@ mod tests { project_entry_ids: Vec::new(), project_path: None, is_singleton: true, + nav_history: None, + } + } + + fn set_state(&mut self, state: String, cx: &mut ViewContext) { + self.push_to_nav_history(cx); + self.state = state; + } + + fn push_to_nav_history(&mut self, cx: &mut ViewContext) { + if let Some(history) = &mut self.nav_history { + history.push(Some(Box::new(self.state.clone())), cx); } } } @@ -3204,7 +3295,23 @@ mod tests { self.is_singleton } - fn set_nav_history(&mut self, _: ItemNavHistory, _: &mut ViewContext) {} + fn set_nav_history(&mut self, history: ItemNavHistory, _: &mut ViewContext) { + self.nav_history = Some(history); + } + + fn navigate(&mut self, state: Box, _: &mut ViewContext) -> bool { + let state = *state.downcast::().unwrap_or_default(); + if state != self.state { + self.state = state; + true + } else { + false + } + } + + fn deactivated(&mut self, cx: &mut ViewContext) { + self.push_to_nav_history(cx); + } fn clone_on_split(&self, _: &mut ViewContext) -> Option where From 70cf6b4041f979bf3a5e0056f3bd0337ff89acc4 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Wed, 6 Jul 2022 16:33:44 -0700 Subject: [PATCH 19/28] Give nav buttons a background on hover --- crates/workspace/src/toolbar.rs | 1 + styles/src/styleTree/workspace.ts | 6 ++++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/crates/workspace/src/toolbar.rs b/crates/workspace/src/toolbar.rs index 7525c0413d7097d7b4e2e1f338835737ba2eab25..9e0c085b1f45dbfc85816b7dae3591d6f21909c6 100644 --- a/crates/workspace/src/toolbar.rs +++ b/crates/workspace/src/toolbar.rs @@ -173,6 +173,7 @@ fn nav_button( .constrained() .with_width(style.button_width) .with_height(style.button_width) + .aligned() .boxed() }) .with_cursor_style(if enabled { diff --git a/styles/src/styleTree/workspace.ts b/styles/src/styleTree/workspace.ts index 0b71453e7148c71a4664c135c0b0f6926759d1db..e23e047eda29cc5240b420728297f5bf1100309d 100644 --- a/styles/src/styleTree/workspace.ts +++ b/styles/src/styleTree/workspace.ts @@ -142,13 +142,15 @@ export default function workspace(theme: Theme) { navButton: { color: iconColor(theme, "primary"), iconWidth: 8, - buttonWidth: 12, + buttonWidth: 18, + cornerRadius: 6, hover: { color: iconColor(theme, "active"), + background: backgroundColor(theme, 300), }, disabled: { color: iconColor(theme, "muted") - } + }, }, padding: { left: 16, right: 8, top: 4, bottom: 4 }, }, From 4ec2d6e50d9542ce86248b2c67b37249cc255fda Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Wed, 6 Jul 2022 16:45:38 -0700 Subject: [PATCH 20/28] Tweak navigation bar colors in theme I meant to include this in #1297 --- styles/src/styleTree/workspace.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/styles/src/styleTree/workspace.ts b/styles/src/styleTree/workspace.ts index e23e047eda29cc5240b420728297f5bf1100309d..fbd7b05a224f8272126631ad5107e1a36215c623 100644 --- a/styles/src/styleTree/workspace.ts +++ b/styles/src/styleTree/workspace.ts @@ -140,7 +140,7 @@ export default function workspace(theme: Theme) { border: border(theme, "secondary", { bottom: true }), itemSpacing: 8, navButton: { - color: iconColor(theme, "primary"), + color: iconColor(theme, "secondary"), iconWidth: 8, buttonWidth: 18, cornerRadius: 6, @@ -149,7 +149,7 @@ export default function workspace(theme: Theme) { background: backgroundColor(theme, 300), }, disabled: { - color: iconColor(theme, "muted") + color: withOpacity(iconColor(theme, "muted"), 0.6), }, }, padding: { left: 16, right: 8, top: 4, bottom: 4 }, From c6254247c3c5ddbf5b0bba84732312ec1a108b7b Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Thu, 7 Jul 2022 11:03:37 +0200 Subject: [PATCH 21/28] Allow providing an external format in `format_on_save` setting --- crates/editor/src/items.rs | 7 +- crates/language/src/buffer.rs | 10 +- crates/project/src/project.rs | 247 +++++++++++++++++++++++--------- crates/settings/src/settings.rs | 25 +++- 4 files changed, 200 insertions(+), 89 deletions(-) diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index f7aa80beaa6aa9219d42e091d258306781aef62c..0e3aca1447043aaba3354fb925babcbaa324b35f 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -352,13 +352,8 @@ impl Item for Editor { project: ModelHandle, cx: &mut ViewContext, ) -> Task> { - let settings = cx.global::(); let buffer = self.buffer().clone(); - let mut buffers = buffer.read(cx).all_buffers(); - buffers.retain(|buffer| { - let language_name = buffer.read(cx).language().map(|l| l.name()); - settings.format_on_save(language_name.as_deref()) - }); + let buffers = buffer.read(cx).all_buffers(); let mut timeout = cx.background().timer(FORMAT_TIMEOUT).fuse(); let format = project.update(cx, |project, cx| project.format(buffers, true, cx)); cx.spawn(|this, mut cx| async move { diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index cebf5f504ea8c634a38155e5c5654362fb345b12..d5ed1c1620da0b514ba22f4c04e050f5043c67fb 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -273,7 +273,7 @@ pub struct Chunk<'a> { pub is_unnecessary: bool, } -pub(crate) struct Diff { +pub struct Diff { base_version: clock::Global, new_text: Arc, changes: Vec<(ChangeTag, usize)>, @@ -958,7 +958,7 @@ impl Buffer { } } - pub(crate) fn diff(&self, mut new_text: String, cx: &AppContext) -> Task { + pub fn diff(&self, mut new_text: String, cx: &AppContext) -> Task { let old_text = self.as_rope().clone(); let base_version = self.version(); cx.background().spawn(async move { @@ -979,11 +979,7 @@ impl Buffer { }) } - pub(crate) fn apply_diff( - &mut self, - diff: Diff, - cx: &mut ModelContext, - ) -> Option<&Transaction> { + 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(); diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index e4425e341476dfb4a316bfdc9df5b010e97e0964..0ac3064e56b33840130d0e43a98d12d8908f4476 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -12,7 +12,7 @@ use anyhow::{anyhow, Context, Result}; use client::{proto, Client, PeerId, TypedEnvelope, User, UserStore}; use clock::ReplicaId; use collections::{hash_map, BTreeMap, HashMap, HashSet}; -use futures::{future::Shared, Future, FutureExt, StreamExt, TryFutureExt}; +use futures::{future::Shared, AsyncWriteExt, Future, FutureExt, StreamExt, TryFutureExt}; use fuzzy::{PathMatch, PathMatchCandidate, PathMatchCandidateSet}; use gpui::{ AnyModelHandle, AppContext, AsyncAppContext, Entity, ModelContext, ModelHandle, @@ -51,10 +51,12 @@ use std::{ ffi::OsString, hash::Hash, mem, + num::NonZeroU32, ops::Range, os::unix::{ffi::OsStrExt, prelude::OsStringExt}, path::{Component, Path, PathBuf}, rc::Rc, + str, sync::{ atomic::{AtomicBool, AtomicUsize, Ordering::SeqCst}, Arc, @@ -3025,78 +3027,50 @@ impl Project { } for (buffer, buffer_abs_path, language_server) in local_buffers { - let text_document = lsp::TextDocumentIdentifier::new( - lsp::Url::from_file_path(&buffer_abs_path).unwrap(), - ); - let capabilities = &language_server.capabilities(); - let tab_size = cx.update(|cx| { - let language_name = buffer.read(cx).language().map(|language| language.name()); - cx.global::().tab_size(language_name.as_deref()) + let (format_on_save, 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.tab_size(language_name.as_deref()), + ) }); - let lsp_edits = if capabilities - .document_formatting_provider - .as_ref() - .map_or(false, |provider| *provider != lsp::OneOf::Left(false)) - { - language_server - .request::(lsp::DocumentFormattingParams { - text_document, - options: lsp::FormattingOptions { - tab_size: tab_size.into(), - insert_spaces: true, - insert_final_newline: Some(true), - ..Default::default() - }, - work_done_progress_params: Default::default(), - }) - .await? - } else if capabilities - .document_range_formatting_provider - .as_ref() - .map_or(false, |provider| *provider != lsp::OneOf::Left(false)) - { - let buffer_start = lsp::Position::new(0, 0); - let buffer_end = - buffer.read_with(&cx, |buffer, _| point_to_lsp(buffer.max_point_utf16())); - language_server - .request::( - lsp::DocumentRangeFormattingParams { - text_document, - range: lsp::Range::new(buffer_start, buffer_end), - options: lsp::FormattingOptions { - tab_size: tab_size.into(), - insert_spaces: true, - insert_final_newline: Some(true), - ..Default::default() - }, - work_done_progress_params: Default::default(), - }, + + let transaction = match format_on_save { + settings::FormatOnSave::Off => continue, + settings::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")?, + settings::FormatOnSave::External { command, arguments } => { + Self::format_via_external_command( + &buffer, + &buffer_abs_path, + &command, + &arguments, + &mut cx, ) - .await? - } else { - continue; + .await + .context(format!( + "failed to format via external command {:?}", + command + ))? + } }; - if let Some(lsp_edits) = lsp_edits { - let edits = this - .update(&mut cx, |this, cx| { - this.edits_from_lsp(&buffer, lsp_edits, None, cx) - }) - .await?; - buffer.update(&mut cx, |buffer, cx| { - buffer.finalize_last_transaction(); - buffer.start_transaction(); - for (range, text) in edits { - buffer.edit([(range, text)], cx); - } - if buffer.end_transaction(cx).is_some() { - let transaction = buffer.finalize_last_transaction().unwrap().clone(); - if !push_to_history { - buffer.forget_transaction(transaction.id); - } - project_transaction.0.insert(cx.handle(), transaction); - } - }); + if let Some(transaction) = transaction { + if !push_to_history { + buffer.update(&mut cx, |buffer, _| { + buffer.forget_transaction(transaction.id) + }); + } + project_transaction.0.insert(buffer, transaction); } } @@ -3104,6 +3078,141 @@ impl Project { }) } + async fn format_via_lsp( + this: &ModelHandle, + buffer: &ModelHandle, + abs_path: &Path, + language_server: &Arc, + tab_size: NonZeroU32, + cx: &mut AsyncAppContext, + ) -> Result> { + let text_document = + lsp::TextDocumentIdentifier::new(lsp::Url::from_file_path(abs_path).unwrap()); + let capabilities = &language_server.capabilities(); + let lsp_edits = if capabilities + .document_formatting_provider + .as_ref() + .map_or(false, |provider| *provider != lsp::OneOf::Left(false)) + { + language_server + .request::(lsp::DocumentFormattingParams { + text_document, + options: lsp::FormattingOptions { + tab_size: tab_size.into(), + insert_spaces: true, + insert_final_newline: Some(true), + ..Default::default() + }, + work_done_progress_params: Default::default(), + }) + .await? + } else if capabilities + .document_range_formatting_provider + .as_ref() + .map_or(false, |provider| *provider != lsp::OneOf::Left(false)) + { + let buffer_start = lsp::Position::new(0, 0); + let buffer_end = + buffer.read_with(cx, |buffer, _| point_to_lsp(buffer.max_point_utf16())); + language_server + .request::(lsp::DocumentRangeFormattingParams { + text_document, + range: lsp::Range::new(buffer_start, buffer_end), + options: lsp::FormattingOptions { + tab_size: tab_size.into(), + insert_spaces: true, + insert_final_newline: Some(true), + ..Default::default() + }, + work_done_progress_params: Default::default(), + }) + .await? + } else { + None + }; + + 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)], cx); + } + if buffer.end_transaction(cx).is_some() { + let transaction = buffer.finalize_last_transaction().unwrap().clone(); + Ok(Some(transaction)) + } else { + Ok(None) + } + }) + } else { + Ok(None) + } + } + + async fn format_via_external_command( + buffer: &ModelHandle, + buffer_abs_path: &Path, + command: &str, + arguments: &[String], + cx: &mut AsyncAppContext, + ) -> 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()?; + let mut worktree_path = worktree.abs_path().to_path_buf(); + if worktree.root_entry()?.is_file() { + worktree_path.pop(); + } + Some(worktree_path) + }); + + if let Some(working_dir_path) = working_dir_path { + let mut child = + smol::process::Command::new(command) + .args(arguments.iter().map(|arg| { + arg.replace("{buffer_path}", &buffer_abs_path.to_string_lossy()) + })) + .current_dir(&working_dir_path) + .stdin(smol::process::Stdio::piped()) + .stdout(smol::process::Stdio::piped()) + .stderr(smol::process::Stdio::piped()) + .spawn()?; + let stdin = child + .stdin + .as_mut() + .ok_or_else(|| anyhow!("failed to acquire stdin"))?; + let text = buffer.read_with(cx, |buffer, _| buffer.as_rope().clone()); + for chunk in text.chunks() { + stdin.write_all(chunk.as_bytes()).await?; + } + stdin.flush().await?; + + let output = child.output().await?; + if !output.status.success() { + return Err(anyhow!( + "command failed with exit code {:?}:\nstdout: {}\nstderr: {}", + output.status.code(), + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr), + )); + } + + 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())) + } else { + Ok(None) + } + } + pub fn definition( &self, buffer: &ModelHandle, diff --git a/crates/settings/src/settings.rs b/crates/settings/src/settings.rs index 0a59d28cc9e845adb1ba4d3fd434245b101a5fbf..98df5e2f1f6be1d664fd4e63278ecac48b084454 100644 --- a/crates/settings/src/settings.rs +++ b/crates/settings/src/settings.rs @@ -38,7 +38,7 @@ pub struct LanguageSettings { pub hard_tabs: Option, pub soft_wrap: Option, pub preferred_line_length: Option, - pub format_on_save: Option, + pub format_on_save: Option, pub enable_language_server: Option, } @@ -50,6 +50,17 @@ pub enum SoftWrap { PreferredLineLength, } +#[derive(Clone, Debug, Deserialize, PartialEq, Eq, JsonSchema)] +#[serde(rename_all = "snake_case")] +pub enum FormatOnSave { + Off, + LanguageServer, + External { + command: String, + arguments: Vec, + }, +} + #[derive(Copy, Clone, Debug, Deserialize, PartialEq, Eq, JsonSchema)] #[serde(rename_all = "snake_case")] pub enum Autosave { @@ -72,7 +83,7 @@ pub struct SettingsFileContent { #[serde(default)] pub vim_mode: Option, #[serde(default)] - pub format_on_save: Option, + pub format_on_save: Option, #[serde(default)] pub autosave: Option, #[serde(default)] @@ -136,9 +147,9 @@ impl Settings { .unwrap_or(80) } - pub fn format_on_save(&self, language: Option<&str>) -> bool { - self.language_setting(language, |settings| settings.format_on_save) - .unwrap_or(true) + pub fn format_on_save(&self, language: Option<&str>) -> FormatOnSave { + self.language_setting(language, |settings| settings.format_on_save.clone()) + .unwrap_or(FormatOnSave::LanguageServer) } pub fn enable_language_server(&self, language: Option<&str>) -> bool { @@ -215,7 +226,7 @@ impl Settings { merge(&mut self.autosave, data.autosave); merge_option( &mut self.language_settings.format_on_save, - data.format_on_save, + data.format_on_save.clone(), ); merge_option( &mut self.language_settings.enable_language_server, @@ -339,7 +350,7 @@ fn merge(target: &mut T, value: Option) { } } -fn merge_option(target: &mut Option, value: Option) { +fn merge_option(target: &mut Option, value: Option) { if value.is_some() { *target = value; } From b91d44b448119562a7c88152848297c7494e6d09 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Thu, 7 Jul 2022 11:52:56 +0200 Subject: [PATCH 22/28] Respond with a debug version of the error in rpc `Client` --- crates/client/src/client.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/client/src/client.rs b/crates/client/src/client.rs index 538b0fa4b0101be73c64d366faf09238e5d8da16..0e9ec4076ad43754a53e11f833935c9b807f025c 100644 --- a/crates/client/src/client.rs +++ b/crates/client/src/client.rs @@ -549,7 +549,7 @@ impl Client { client.respond_with_error( receipt, proto::Error { - message: error.to_string(), + message: format!("{:?}", error), }, )?; Err(error) From 52b8efca1b5ccc77c063ea25972ebf00b4c70232 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Thu, 7 Jul 2022 11:53:32 +0200 Subject: [PATCH 23/28] Add integration test to exercise formatting via external command --- crates/collab/src/integration_tests.rs | 34 ++++++++++++++++++++++---- crates/project/src/fs.rs | 26 ++------------------ crates/zed/src/zed.rs | 33 ++++++++++++++++--------- 3 files changed, 53 insertions(+), 40 deletions(-) diff --git a/crates/collab/src/integration_tests.rs b/crates/collab/src/integration_tests.rs index a4c8386b13fb58a88fd2a4a0f1d5664ed9226852..7767b361c1bbbd327ae765ab62a8308ca3cb6b61 100644 --- a/crates/collab/src/integration_tests.rs +++ b/crates/collab/src/integration_tests.rs @@ -35,7 +35,7 @@ use project::{ use rand::prelude::*; use rpc::PeerId; use serde_json::json; -use settings::Settings; +use settings::{FormatOnSave, Settings}; use sqlx::types::time::OffsetDateTime; use std::{ cell::RefCell, @@ -1912,7 +1912,6 @@ async fn test_reloading_buffer_manually(cx_a: &mut TestAppContext, cx_b: &mut Te #[gpui::test(iterations = 10)] async fn test_formatting_buffer(cx_a: &mut TestAppContext, cx_b: &mut TestAppContext) { - cx_a.foreground().forbid_parking(); let mut server = TestServer::start(cx_a.foreground(), cx_a.background()).await; let client_a = server.create_client(cx_a, "user_a").await; let client_b = server.create_client(cx_b, "user_b").await; @@ -1932,11 +1931,15 @@ async fn test_formatting_buffer(cx_a: &mut TestAppContext, cx_b: &mut TestAppCon let mut fake_language_servers = language.set_fake_lsp_adapter(Default::default()); client_a.language_registry.add(Arc::new(language)); + // Here we insert a fake tree with a directory that exists on disk. This is needed + // because later we'll invoke a command, which requires passing a working directory + // that points to a valid location on disk. + let directory = env::current_dir().unwrap(); client_a .fs - .insert_tree("/a", json!({ "a.rs": "let one = two" })) + .insert_tree(&directory, json!({ "a.rs": "let one = \"two\"" })) .await; - let (project_a, worktree_id) = client_a.build_local_project("/a", cx_a).await; + let (project_a, worktree_id) = client_a.build_local_project(&directory, cx_a).await; let project_b = client_b.build_remote_project(&project_a, cx_a, cx_b).await; let buffer_b = cx_b @@ -1967,7 +1970,28 @@ async fn test_formatting_buffer(cx_a: &mut TestAppContext, cx_b: &mut TestAppCon .unwrap(); assert_eq!( buffer_b.read_with(cx_b, |buffer, _| buffer.text()), - "let honey = two" + "let honey = \"two\"" + ); + + // Ensure buffer can be formatted using an external command. Notice how the + // host's configuration is honored as opposed to using the guest's settings. + cx_a.update(|cx| { + cx.update_global(|settings: &mut Settings, _| { + settings.language_settings.format_on_save = Some(FormatOnSave::External { + command: "awk".to_string(), + arguments: vec!["{sub(/two/,\"{buffer_path}\")}1".to_string()], + }); + }); + }); + project_b + .update(cx_b, |project, cx| { + project.format(HashSet::from_iter([buffer_b.clone()]), true, cx) + }) + .await + .unwrap(); + assert_eq!( + buffer_b.read_with(cx_b, |buffer, _| buffer.text()), + format!("let honey = \"{}/a.rs\"\n", directory.to_str().unwrap()) ); } diff --git a/crates/project/src/fs.rs b/crates/project/src/fs.rs index 17d7264f1d80853a02707ccdff6308feeff178e9..5c528016118fdde2126f512170e5c46e6be9e171 100644 --- a/crates/project/src/fs.rs +++ b/crates/project/src/fs.rs @@ -334,28 +334,6 @@ impl FakeFs { }) } - pub async fn insert_dir(&self, path: impl AsRef) { - let mut state = self.state.lock().await; - let path = path.as_ref(); - state.validate_path(path).unwrap(); - - let inode = state.next_inode; - state.next_inode += 1; - state.entries.insert( - path.to_path_buf(), - FakeFsEntry { - metadata: Metadata { - inode, - mtime: SystemTime::now(), - is_dir: true, - is_symlink: false, - }, - content: None, - }, - ); - state.emit_event(&[path]).await; - } - pub async fn insert_file(&self, path: impl AsRef, content: String) { let mut state = self.state.lock().await; let path = path.as_ref(); @@ -392,7 +370,7 @@ impl FakeFs { match tree { Object(map) => { - self.insert_dir(path).await; + self.create_dir(path).await.unwrap(); for (name, contents) in map { let mut path = PathBuf::from(path); path.push(name); @@ -400,7 +378,7 @@ impl FakeFs { } } Null => { - self.insert_dir(&path).await; + self.create_dir(&path).await.unwrap(); } String(contents) => { self.insert_file(&path, contents).await; diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index f88aee3d7c6c06265e94e8c541e0c6ec44261e02..56ccfaf9fed86fbfddd5544d75565c808bdce24a 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -554,7 +554,7 @@ mod tests { }); let save_task = workspace.update(cx, |workspace, cx| workspace.save_active_item(false, cx)); - app_state.fs.as_fake().insert_dir("/root").await; + app_state.fs.create_dir(Path::new("/root")).await.unwrap(); cx.simulate_new_path_selection(|_| Some(PathBuf::from("/root/the-new-name"))); save_task.await.unwrap(); editor.read_with(cx, |editor, cx| { @@ -680,14 +680,25 @@ mod tests { async fn test_open_paths(cx: &mut TestAppContext) { let app_state = init(cx); - let fs = app_state.fs.as_fake(); - fs.insert_dir("/dir1").await; - fs.insert_dir("/dir2").await; - fs.insert_dir("/dir3").await; - fs.insert_file("/dir1/a.txt", "".into()).await; - fs.insert_file("/dir2/b.txt", "".into()).await; - fs.insert_file("/dir3/c.txt", "".into()).await; - fs.insert_file("/d.txt", "".into()).await; + app_state + .fs + .as_fake() + .insert_tree( + "/", + json!({ + "dir1": { + "a.txt": "" + }, + "dir2": { + "b.txt": "" + }, + "dir3": { + "c.txt": "" + }, + "d.txt": "" + }), + ) + .await; let project = Project::test(app_state.fs.clone(), ["/dir1".as_ref()], cx).await; let (_, workspace) = cx.add_window(|cx| Workspace::new(project, cx)); @@ -891,7 +902,7 @@ mod tests { #[gpui::test] async fn test_open_and_save_new_file(cx: &mut TestAppContext) { let app_state = init(cx); - app_state.fs.as_fake().insert_dir("/root").await; + app_state.fs.create_dir(Path::new("/root")).await.unwrap(); let project = Project::test(app_state.fs.clone(), ["/root".as_ref()], cx).await; project.update(cx, |project, _| project.languages().add(rust_lang())); @@ -980,7 +991,7 @@ mod tests { #[gpui::test] async fn test_setting_language_when_saving_as_single_file_worktree(cx: &mut TestAppContext) { let app_state = init(cx); - app_state.fs.as_fake().insert_dir("/root").await; + app_state.fs.create_dir(Path::new("/root")).await.unwrap(); let project = Project::test(app_state.fs.clone(), [], cx).await; project.update(cx, |project, _| project.languages().add(rust_lang())); From 9c518085aeed7b388035c494bc8c1b27421fa253 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Thu, 7 Jul 2022 11:01:26 -0700 Subject: [PATCH 24/28] Fixed working directory issues, added tests. Working on regression --- Cargo.lock | 2 + crates/terminal/Cargo.toml | 4 ++ crates/terminal/src/terminal.rs | 99 ++++++++++++++++++++++++++++++++- 3 files changed, 102 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f5dbc470f597c8e3f17c9bfc3052115d51886227..634797507be3b71e9942be0cdee5e43f98924dc1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4878,6 +4878,8 @@ name = "terminal" version = "0.1.0" dependencies = [ "alacritty_terminal", + "client", + "dirs 4.0.0", "editor", "futures", "gpui", diff --git a/crates/terminal/Cargo.toml b/crates/terminal/Cargo.toml index 0bbc0569227198a45538016b63a24f35d079abd0..b44b93e745ccabe06da58d540ded3eec17f3a145 100644 --- a/crates/terminal/Cargo.toml +++ b/crates/terminal/Cargo.toml @@ -21,7 +21,11 @@ mio-extras = "2.0.6" futures = "0.3" ordered-float = "2.1.1" itertools = "0.10" +dirs = "4.0.0" [dev-dependencies] gpui = { path = "../gpui", features = ["test-support"] } +client = { path = "../client", features = ["test-support"]} +project = { path = "../project", features = ["test-support"]} + diff --git a/crates/terminal/src/terminal.rs b/crates/terminal/src/terminal.rs index eb7f2a0a90b0d6d4b1204bdc13396634145c4371..846ed2680665264c0dfa4d4d0c0d6af74000fc52 100644 --- a/crates/terminal/src/terminal.rs +++ b/crates/terminal/src/terminal.rs @@ -9,6 +9,7 @@ use alacritty_terminal::{ Term, }; +use dirs::home_dir; use futures::{ channel::mpsc::{unbounded, UnboundedSender}, StreamExt, @@ -17,7 +18,7 @@ use gpui::{ actions, color::Color, elements::*, impl_internal_actions, platform::CursorStyle, ClipboardItem, Entity, MutableAppContext, View, ViewContext, }; -use project::{Project, ProjectPath}; +use project::{LocalWorktree, Project, ProjectPath}; use settings::Settings; use smallvec::SmallVec; use std::{collections::HashMap, path::PathBuf, sync::Arc}; @@ -268,11 +269,12 @@ impl Terminal { ///Create a new Terminal in the current working directory or the user's home directory fn deploy(workspace: &mut Workspace, _: &Deploy, cx: &mut ViewContext) { let project = workspace.project().read(cx); + let abs_path = project .active_entry() .and_then(|entry_id| project.worktree_for_entry(entry_id, cx)) .and_then(|worktree_handle| worktree_handle.read(cx).as_local()) - .map(|wt| wt.abs_path().to_path_buf()); + .and_then(get_working_directory); workspace.add_item(Box::new(cx.add_view(|cx| Terminal::new(cx, abs_path))), cx); } @@ -477,19 +479,28 @@ fn to_alac_rgb(color: Color) -> AlacRgb { } } +fn get_working_directory(wt: &LocalWorktree) -> Option { + Some(wt.abs_path().to_path_buf()) + .filter(|path| path.is_dir()) + .or_else(|| home_dir()) +} + #[cfg(test)] mod tests { + + use std::{path::Path, sync::atomic::AtomicUsize}; + use super::*; use alacritty_terminal::{grid::GridIterator, term::cell::Cell}; use gpui::TestAppContext; use itertools::Itertools; + use project::{FakeFs, Fs, RealFs, RemoveOptions, Worktree}; ///Basic integration test, can we get the terminal to show up, execute a command, //and produce noticable output? #[gpui::test] async fn test_terminal(cx: &mut TestAppContext) { let terminal = cx.add_view(Default::default(), |cx| Terminal::new(cx, None)); - terminal.update(cx, |terminal, cx| { terminal.write_to_pty(&Input(("expr 3 + 4".to_string()).to_string()), cx); terminal.carriage_return(&Return, cx); @@ -499,6 +510,7 @@ mod tests { .condition(cx, |terminal, _cx| { let term = terminal.term.clone(); let content = grid_as_str(term.lock().renderable_content().display_iter); + dbg!(&content); content.contains("7") }) .await; @@ -512,4 +524,85 @@ mod tests { .collect::>() .join("\n") } + + #[gpui::test] + async fn single_file_worktree(cx: &mut TestAppContext) { + let mut async_cx = cx.to_async(); + let http_client = client::test::FakeHttpClient::with_404_response(); + let client = client::Client::new(http_client.clone()); + let fake_fs = FakeFs::new(cx.background().clone()); + + let path = Path::new("/file/"); + fake_fs.insert_file(path, "a".to_string()).await; + + let worktree_handle = Worktree::local( + client, + path, + true, + fake_fs, + Arc::new(AtomicUsize::new(0)), + &mut async_cx, + ) + .await + .ok() + .unwrap(); + + async_cx.update(|cx| { + let wt = worktree_handle.read(cx).as_local().unwrap(); + let wd = get_working_directory(wt); + assert!(wd.is_some()); + let path = wd.unwrap(); + //This should be the system's working directory, so querying the real file system is probably ok. + assert!(path.is_dir()); + assert_eq!(path, home_dir().unwrap()); + }); + } + + #[gpui::test] + async fn test_worktree_directory(cx: &mut TestAppContext) { + let mut async_cx = cx.to_async(); + let http_client = client::test::FakeHttpClient::with_404_response(); + let client = client::Client::new(http_client.clone()); + + let fs = RealFs; + let mut test_wd = home_dir().unwrap(); + test_wd.push("dir"); + + fs.create_dir(test_wd.as_path()) + .await + .expect("File could not be created"); + + let worktree_handle = Worktree::local( + client, + test_wd.clone(), + true, + Arc::new(RealFs), + Arc::new(AtomicUsize::new(0)), + &mut async_cx, + ) + .await + .ok() + .unwrap(); + + async_cx.update(|cx| { + let wt = worktree_handle.read(cx).as_local().unwrap(); + let wd = get_working_directory(wt); + assert!(wd.is_some()); + let path = wd.unwrap(); + assert!(path.is_dir()); + assert_eq!(path, test_wd); + }); + + //Clean up after ourselves. + fs.remove_dir( + test_wd.as_path(), + RemoveOptions { + recursive: false, + ignore_if_not_exists: true, + }, + ) + .await + .ok() + .expect("Could not remove test directory"); + } } From 02525c5bbefe9ed28b228398042863eb7a20636f Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Thu, 7 Jul 2022 12:04:17 -0700 Subject: [PATCH 25/28] Added a way to change the timeout with state --- crates/gpui/src/app.rs | 23 +++++++++++++++++------ crates/terminal/src/terminal.rs | 5 +++-- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index 505f609f5709f57b0c037ec5be2032729f736266..bcb8908629500c0916ea4110f28119b7e0189869 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -151,6 +151,7 @@ pub struct AsyncAppContext(Rc>); pub struct TestAppContext { cx: Rc>, foreground_platform: Rc, + condition_duration: Option, } impl App { @@ -337,6 +338,7 @@ impl TestAppContext { let cx = TestAppContext { cx: Rc::new(RefCell::new(cx)), foreground_platform, + condition_duration: None, }; cx.cx.borrow_mut().weak_self = Some(Rc::downgrade(&cx.cx)); cx @@ -612,6 +614,19 @@ impl TestAppContext { test_window }) } + + pub fn set_condition_duration(&mut self, duration: Duration) { + self.condition_duration = Some(duration); + } + pub fn condition_duration(&self) -> Duration { + self.condition_duration.unwrap_or_else(|| { + if std::env::var("CI").is_ok() { + Duration::from_secs(2) + } else { + Duration::from_millis(500) + } + }) + } } impl AsyncAppContext { @@ -4398,6 +4413,7 @@ impl ViewHandle { use postage::prelude::{Sink as _, Stream as _}; let (tx, mut rx) = postage::mpsc::channel(1024); + let timeout_duration = cx.condition_duration(); let mut cx = cx.cx.borrow_mut(); let subscriptions = self.update(&mut *cx, |_, cx| { @@ -4419,14 +4435,9 @@ impl ViewHandle { let cx = cx.weak_self.as_ref().unwrap().upgrade().unwrap(); let handle = self.downgrade(); - let duration = if std::env::var("CI").is_ok() { - Duration::from_secs(2) - } else { - Duration::from_millis(500) - }; async move { - crate::util::timeout(duration, async move { + crate::util::timeout(timeout_duration, async move { loop { { let cx = cx.borrow(); diff --git a/crates/terminal/src/terminal.rs b/crates/terminal/src/terminal.rs index 846ed2680665264c0dfa4d4d0c0d6af74000fc52..a99e211dca24eb8f9de3d182b48abdc443462c26 100644 --- a/crates/terminal/src/terminal.rs +++ b/crates/terminal/src/terminal.rs @@ -488,7 +488,7 @@ fn get_working_directory(wt: &LocalWorktree) -> Option { #[cfg(test)] mod tests { - use std::{path::Path, sync::atomic::AtomicUsize}; + use std::{path::Path, sync::atomic::AtomicUsize, time::Duration}; use super::*; use alacritty_terminal::{grid::GridIterator, term::cell::Cell}; @@ -501,6 +501,8 @@ mod tests { #[gpui::test] async fn test_terminal(cx: &mut TestAppContext) { let terminal = cx.add_view(Default::default(), |cx| Terminal::new(cx, None)); + cx.set_condition_duration(Duration::from_secs(2)); + terminal.update(cx, |terminal, cx| { terminal.write_to_pty(&Input(("expr 3 + 4".to_string()).to_string()), cx); terminal.carriage_return(&Return, cx); @@ -510,7 +512,6 @@ mod tests { .condition(cx, |terminal, _cx| { let term = terminal.term.clone(); let content = grid_as_str(term.lock().renderable_content().display_iter); - dbg!(&content); content.contains("7") }) .await; From ec4082695b6f442745b3d0f3f80638bbdac10b3d Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Thu, 7 Jul 2022 12:31:21 -0700 Subject: [PATCH 26/28] Now defaults to using user's shell --- crates/terminal/src/terminal.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/terminal/src/terminal.rs b/crates/terminal/src/terminal.rs index a99e211dca24eb8f9de3d182b48abdc443462c26..57a78fbe78db8a1b3b62c1b3506d3ef8449088cd 100644 --- a/crates/terminal/src/terminal.rs +++ b/crates/terminal/src/terminal.rs @@ -1,5 +1,5 @@ use alacritty_terminal::{ - config::{Config, Program, PtyConfig}, + config::{Config, PtyConfig}, event::{Event as AlacTermEvent, EventListener, Notify}, event_loop::{EventLoop, Msg, Notifier}, grid::Scroll, @@ -125,7 +125,7 @@ impl Terminal { .detach(); let pty_config = PtyConfig { - shell: Some(Program::Just("zsh".to_string())), + shell: None, working_directory, hold: false, }; From 98f6dccd43f80ca79306efd4cf5409f5ebd31c87 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Thu, 7 Jul 2022 13:01:16 -0700 Subject: [PATCH 27/28] Fixed terminal clone on split --- crates/terminal/src/terminal.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/crates/terminal/src/terminal.rs b/crates/terminal/src/terminal.rs index 57a78fbe78db8a1b3b62c1b3506d3ef8449088cd..4542e283ee2fd5ee5c0db34bace104fdada1f861 100644 --- a/crates/terminal/src/terminal.rs +++ b/crates/terminal/src/terminal.rs @@ -91,6 +91,7 @@ pub struct Terminal { has_new_content: bool, has_bell: bool, //Currently using iTerm bell, show bell emoji in tab until input is received cur_size: SizeInfo, + associated_directory: Option, } ///Upward flowing events, for changing the title and such @@ -126,7 +127,7 @@ impl Terminal { let pty_config = PtyConfig { shell: None, - working_directory, + working_directory: working_directory.clone(), hold: false, }; @@ -173,6 +174,7 @@ impl Terminal { has_new_content: false, has_bell: false, cur_size: size_info, + associated_directory: working_directory, } } @@ -410,6 +412,13 @@ impl Item for Terminal { .boxed() } + fn clone_on_split(&self, cx: &mut ViewContext) -> Option { + //From what I can tell, there's no way to tell the current working + //Directory of the terminal from outside the terminal. There might be + //solutions to this, but they are non-trivial and require more IPC + Some(Terminal::new(cx, self.associated_directory.clone())) + } + fn project_path(&self, _cx: &gpui::AppContext) -> Option { None } From 6642b78331bd05f36d3c1fd69daa634e4ee38db5 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Thu, 7 Jul 2022 13:36:08 -0700 Subject: [PATCH 28/28] Add tooltips to pane nav buttons and make them trigger on click --- crates/workspace/src/toolbar.rs | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/crates/workspace/src/toolbar.rs b/crates/workspace/src/toolbar.rs index 9e0c085b1f45dbfc85816b7dae3591d6f21909c6..636df9a03955ff2ebc7c98c63f3b8d9963e3ea6c 100644 --- a/crates/workspace/src/toolbar.rs +++ b/crates/workspace/src/toolbar.rs @@ -112,6 +112,7 @@ impl View for Toolbar { let container_style = theme.container; let height = theme.height; let button_style = theme.nav_button; + let tooltip_style = cx.global::().theme.tooltip.clone(); Flex::column() .with_child( @@ -119,21 +120,27 @@ impl View for Toolbar { .with_child(nav_button( "icons/arrow-left.svg", button_style, + tooltip_style.clone(), enable_go_backward, spacing, super::GoBack { pane: Some(pane.clone()), }, + super::GoBack { pane: None }, + "Go Back", cx, )) .with_child(nav_button( "icons/arrow-right.svg", button_style, + tooltip_style.clone(), enable_go_forward, spacing, super::GoForward { pane: Some(pane.clone()), }, + super::GoForward { pane: None }, + "Go Forward", cx, )) .with_children(primary_left_items) @@ -152,9 +159,12 @@ impl View for Toolbar { fn nav_button( svg_path: &'static str, style: theme::Interactive, + tooltip_style: TooltipStyle, enabled: bool, spacing: f32, action: A, + tooltip_action: A, + action_name: &str, cx: &mut RenderContext, ) -> ElementBox { MouseEventHandler::new::(0, cx, |state, _| { @@ -181,7 +191,14 @@ fn nav_button( } else { CursorStyle::default() }) - .on_mouse_down(move |_, cx| cx.dispatch_action(action.clone())) + .on_click(move |_, _, cx| cx.dispatch_action(action.clone())) + .with_tooltip::( + 0, + action_name.to_string(), + Some(Box::new(tooltip_action)), + tooltip_style, + cx, + ) .contained() .with_margin_right(spacing) .boxed()