From 116fa92e84f43879a6247557ebbe438ff0bfcae6 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 5 Jul 2022 14:51:28 -0700 Subject: [PATCH 01/25] 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 4d50affdd5..b7add947ff 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 e66837f21b..216850dbd8 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 2c8fc13313..4ab5ef0940 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/25] 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 b7add947ff..1e244ab4c5 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 0e876d14df..7c7ec65fd8 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 723e57ded4..a645af3c02 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 677a7afa9a..0b9a0569c2 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 2603b2b709..0735d3e1fe 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 1741df8fb7..04cdcd3524 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 8c5dc260d6..e8aff3f52f 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 216850dbd8..c534a004b7 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 4ab5ef0940..7e564d3eca 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/25] 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 f5dbc470f5..bfdb62936e 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 8d131d17a8..60116f06c2 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/25] 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 c534a004b7..2d57b65792 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 7e564d3eca..4d1e50b274 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/25] 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 bfdb62936e..7647e3ccd3 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 cbbd65027e..4fc09eff46 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 2d57b65792..c0cc0556d5 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 4d1e50b274..f73163438b 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/25] 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 7647e3ccd3..7dcecd0093 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 60116f06c2..6d488e83e8 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/25] Report whether a view was focused or blurred when observing focus --- crates/gpui/src/app.rs | 130 +++++++++++++++++++--------- crates/search/src/project_search.rs | 12 ++- 2 files changed, 97 insertions(+), 45 deletions(-) diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index 505f609f57..fd447e2469 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!( - *view_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(), - ], + mem::take(&mut *view_events.lock()), + ["view 1 blurred", "view 2 focused"], ); assert_eq!( - *observed_events.lock(), + 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, |_, cx| cx.focus(&view_1)); + assert_eq!( + mem::take(&mut *view_events.lock()), + ["view 2 blurred", "view 1 focused"], + ); + assert_eq!( + mem::take(&mut *observed_events.lock()), + [ + "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!( + 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, |_, _| 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 bf862f0d9d..2aa8993285 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/25] 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 94eda67c39..ecc1b2df68 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 31a636fd61..9dd40413fd 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 8e15dce83c..f7aa80beaa 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 2aa8993285..5ee2dcbb27 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 5e039b8cd0..f8474e41b1 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 419998e730..cf1092662f 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 f88aee3d7c..4fa2e238bf 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/25] 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 cf1092662f..2120695760 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 4fa2e238bf..f88aee3d7c 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/25] 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 f8474e41b1..b98ce49cd9 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/25] 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 b98ce49cd9..bbc086395b 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 2120695760..53318d943f 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/25] 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 1e244ab4c5..42cca41900 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 a645af3c02..ba8744624d 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/25] 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 42cca41900..cebf5f504e 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 0b9a0569c2..e4425e3414 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 9d80dadb84..00f7bb8c94 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 c0cc0556d5..4da9edd735 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 f73163438b..536b832942 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/25] 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 53318d943f..f7d3032eab 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/25] 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 7dcecd0093..8624921d97 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 6d488e83e8..88acfdb14d 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/25] 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 0000000000..904fdaa1a7 --- /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 0000000000..b7e1bec6d8 --- /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 184b1880f0..c3052ff6c5 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 bbc086395b..0fb5225cc5 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 e9b20bf3a0..9d8274288b 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 2deadc02e7..ba3978fc74 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/25] 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 | 138 +++++++++++++++++++----------- crates/workspace/src/toolbar.rs | 23 ++++- crates/workspace/src/workspace.rs | 10 +-- styles/src/styleTree/workspace.ts | 6 +- 6 files changed, 163 insertions(+), 100 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 9dd40413fd..c1e2557555 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 c3052ff6c5..058dd5a331 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 0fb5225cc5..391ddfc1f7 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() - } - - pub fn pop_forward(&mut self) -> Option { - self.forward_stack.pop_back() - } - - pub fn pop_closed(&mut self) -> Option { - self.closed_stack.pop_back() - } - - 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 set_mode(&mut self, mode: NavigationMode) { self.mode = mode; } - pub fn push(&mut self, data: Option, item: Rc) { + fn disable(&mut self) { + self.mode = NavigationMode::Disabled; + } + + fn enable(&mut self) { + self.mode = NavigationMode::Normal; + } + + 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 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 9d8274288b..7525c0413d 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 f7d3032eab..2ba1b4d008 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 ba3978fc74..0b71453e71 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/25] 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 2ba1b4d008..fdfa640718 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/25] 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 7525c0413d..9e0c085b1f 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 0b71453e71..e23e047eda 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/25] 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 e23e047eda..fbd7b05a22 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/25] 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 f7aa80beaa..0e3aca1447 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 cebf5f504e..d5ed1c1620 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 e4425e3414..0ac3064e56 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 0a59d28cc9..98df5e2f1f 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/25] 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 538b0fa4b0..0e9ec4076a 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/25] 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 a4c8386b13..7767b361c1 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 17d7264f1d..5c52801611 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 f88aee3d7c..56ccfaf9fe 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/25] 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 f5dbc470f5..634797507b 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 0bbc056922..b44b93e745 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 eb7f2a0a90..846ed26806 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/25] 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 505f609f57..bcb8908629 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 846ed26806..a99e211dca 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;