From 3eea2fb5f8605c0a86c3b5559890ce38b87d96ad Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Wed, 10 May 2023 13:35:47 +0300 Subject: [PATCH 01/13] Parse file find queries with extra data --- crates/file_finder/src/file_finder.rs | 112 ++++++++++++++++++++++---- 1 file changed, 95 insertions(+), 17 deletions(-) diff --git a/crates/file_finder/src/file_finder.rs b/crates/file_finder/src/file_finder.rs index f00430feb7e16d02eebdbcf3a9f8c142243675a9..62eae72cc59c62441c6c479a9177228ddc5fe6b4 100644 --- a/crates/file_finder/src/file_finder.rs +++ b/crates/file_finder/src/file_finder.rs @@ -23,7 +23,7 @@ pub struct FileFinderDelegate { search_count: usize, latest_search_id: usize, latest_search_did_cancel: bool, - latest_search_query: String, + latest_search_query: Option, relative_to: Option>, matches: Vec, selected: Option<(usize, Arc)>, @@ -60,6 +60,62 @@ pub enum Event { Dismissed, } +#[derive(Debug, Clone, PartialEq, Eq)] +struct FileSearchQuery { + raw_query: String, + file_path_end: Option, + file_row: Option, + file_column: Option, +} + +impl FileSearchQuery { + fn new(raw_query: String) -> Self { + let fallback_query = Self { + raw_query: raw_query.clone(), + file_path_end: None, + file_row: None, + file_column: None, + }; + + let mut possible_path_and_coordinates = raw_query.as_str().rsplitn(3, ':').fuse(); + match ( + possible_path_and_coordinates.next(), + possible_path_and_coordinates.next(), + possible_path_and_coordinates.next(), + ) { + (Some(column_number_str), Some(row_number_str), Some(file_path_part)) => Self { + file_path_end: Some(file_path_part.len()), + file_row: match row_number_str.parse().ok() { + None => return fallback_query, + row => row, + }, + file_column: match column_number_str.parse().ok() { + None => return fallback_query, + column => column, + }, + raw_query, + }, + (Some(row_number_str), Some(file_path_part), None) => Self { + file_path_end: Some(file_path_part.len()), + file_row: match row_number_str.parse().ok() { + None => return fallback_query, + row => row, + }, + file_column: None, + raw_query, + }, + _no_colons_query => fallback_query, + } + } + + fn path_query(&self) -> &str { + match self.file_path_end { + Some(file_path_end) => &self.raw_query[..file_path_end], + None => &self.raw_query, + } + } +} + impl FileFinderDelegate { fn labels_for_match(&self, path_match: &PathMatch) -> (String, Vec, String, Vec) { let path = &path_match.path; @@ -103,7 +159,7 @@ impl FileFinderDelegate { search_count: 0, latest_search_id: 0, latest_search_did_cancel: false, - latest_search_query: String::new(), + latest_search_query: None, relative_to, matches: Vec::new(), selected: None, @@ -111,7 +167,11 @@ impl FileFinderDelegate { } } - fn spawn_search(&mut self, query: String, cx: &mut ViewContext) -> Task<()> { + fn spawn_search( + &mut self, + query: FileSearchQuery, + cx: &mut ViewContext, + ) -> Task<()> { let relative_to = self.relative_to.clone(); let worktrees = self .project @@ -140,7 +200,7 @@ impl FileFinderDelegate { cx.spawn(|picker, mut cx| async move { let matches = fuzzy::match_path_sets( candidate_sets.as_slice(), - &query, + query.path_query(), relative_to, false, 100, @@ -163,18 +223,18 @@ impl FileFinderDelegate { &mut self, search_id: usize, did_cancel: bool, - query: String, + query: FileSearchQuery, matches: Vec, cx: &mut ViewContext, ) { if search_id >= self.latest_search_id { self.latest_search_id = search_id; - if self.latest_search_did_cancel && query == self.latest_search_query { + if self.latest_search_did_cancel && Some(&query) == self.latest_search_query.as_ref() { util::extend_sorted(&mut self.matches, matches.into_iter(), 100, |a, b| b.cmp(a)); } else { self.matches = matches; } - self.latest_search_query = query; + self.latest_search_query = Some(query); self.latest_search_did_cancel = did_cancel; cx.notify(); } @@ -209,14 +269,14 @@ impl PickerDelegate for FileFinderDelegate { cx.notify(); } - fn update_matches(&mut self, query: String, cx: &mut ViewContext) -> Task<()> { - if query.is_empty() { + fn update_matches(&mut self, raw_query: String, cx: &mut ViewContext) -> Task<()> { + if raw_query.is_empty() { self.latest_search_id = post_inc(&mut self.search_count); self.matches.clear(); cx.notify(); Task::ready(()) } else { - self.spawn_search(query, cx) + self.spawn_search(FileSearchQuery::new(raw_query), cx) } } @@ -230,6 +290,8 @@ impl PickerDelegate for FileFinderDelegate { workspace.update(cx, |workspace, cx| { workspace + // TODO kb need to pass row and column here + // use self.latest_search_query .open_path(project_path.clone(), None, true, cx) .detach_and_log_err(cx); workspace.dismiss_modal(cx); @@ -371,7 +433,7 @@ mod tests { ) }); - let query = "hi".to_string(); + let query = FileSearchQuery::new("hi".to_string()); finder .update(cx, |f, cx| f.delegate_mut().spawn_search(query.clone(), cx)) .await; @@ -455,7 +517,10 @@ mod tests { ) }); finder - .update(cx, |f, cx| f.delegate_mut().spawn_search("hi".into(), cx)) + .update(cx, |f, cx| { + f.delegate_mut() + .spawn_search(FileSearchQuery::new("hi".to_string()), cx) + }) .await; finder.read_with(cx, |f, _| assert_eq!(f.delegate().matches.len(), 7)); } @@ -491,7 +556,10 @@ mod tests { // Even though there is only one worktree, that worktree's filename // is included in the matching, because the worktree is a single file. finder - .update(cx, |f, cx| f.delegate_mut().spawn_search("thf".into(), cx)) + .update(cx, |f, cx| { + f.delegate_mut() + .spawn_search(FileSearchQuery::new("thf".to_string()), cx) + }) .await; cx.read(|cx| { let finder = finder.read(cx); @@ -509,7 +577,10 @@ mod tests { // Since the worktree root is a file, searching for its name followed by a slash does // not match anything. finder - .update(cx, |f, cx| f.delegate_mut().spawn_search("thf/".into(), cx)) + .update(cx, |f, cx| { + f.delegate_mut() + .spawn_search(FileSearchQuery::new("thf/".to_string()), cx) + }) .await; finder.read_with(cx, |f, _| assert_eq!(f.delegate().matches.len(), 0)); } @@ -553,7 +624,10 @@ mod tests { // Run a search that matches two files with the same relative path. finder - .update(cx, |f, cx| f.delegate_mut().spawn_search("a.t".into(), cx)) + .update(cx, |f, cx| { + f.delegate_mut() + .spawn_search(FileSearchQuery::new("a.t".to_string()), cx) + }) .await; // Can switch between different matches with the same relative path. @@ -609,7 +683,8 @@ mod tests { finder .update(cx, |f, cx| { - f.delegate_mut().spawn_search("a.txt".into(), cx) + f.delegate_mut() + .spawn_search(FileSearchQuery::new("a.txt".to_string()), cx) }) .await; @@ -651,7 +726,10 @@ mod tests { ) }); finder - .update(cx, |f, cx| f.delegate_mut().spawn_search("dir".into(), cx)) + .update(cx, |f, cx| { + f.delegate_mut() + .spawn_search(FileSearchQuery::new("dir".to_string()), cx) + }) .await; cx.read(|cx| { let finder = finder.read(cx); From 54c1e77aff03eeb7f203256dd760269404faae4f Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Wed, 10 May 2023 15:03:51 +0300 Subject: [PATCH 02/13] Move the caret to the opened file --- crates/file_finder/src/file_finder.rs | 83 +++++++++++++++++++++------ 1 file changed, 65 insertions(+), 18 deletions(-) diff --git a/crates/file_finder/src/file_finder.rs b/crates/file_finder/src/file_finder.rs index 62eae72cc59c62441c6c479a9177228ddc5fe6b4..8181d6a1f0b8d2cb02b9fc16da594b6de10ffffd 100644 --- a/crates/file_finder/src/file_finder.rs +++ b/crates/file_finder/src/file_finder.rs @@ -1,3 +1,4 @@ +use editor::{scroll::autoscroll::Autoscroll, DisplayPoint, Editor}; use fuzzy::PathMatch; use gpui::{ actions, elements::*, AppContext, ModelHandle, MouseState, Task, ViewContext, WeakViewHandle, @@ -60,12 +61,12 @@ pub enum Event { Dismissed, } -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone)] struct FileSearchQuery { raw_query: String, file_path_end: Option, - file_row: Option, - file_column: Option, + file_row: Option, + file_column: Option, } impl FileSearchQuery { @@ -77,30 +78,43 @@ impl FileSearchQuery { file_column: None, }; - let mut possible_path_and_coordinates = raw_query.as_str().rsplitn(3, ':').fuse(); + let mut possible_path_and_coordinates = + // TODO kb go_to_line.rs uses ',' as a separator?? + raw_query.as_str().splitn(3, ':').map(str::trim).fuse(); match ( possible_path_and_coordinates.next(), possible_path_and_coordinates.next(), possible_path_and_coordinates.next(), ) { - (Some(column_number_str), Some(row_number_str), Some(file_path_part)) => Self { + (Some(file_path_part), Some(row_number_str), Some(column_number_str)) + if !row_number_str.is_empty() && !column_number_str.is_empty() => + { + Self { + file_path_end: Some(file_path_part.len()), + file_row: match row_number_str.parse().ok() { + None => return fallback_query, + row => row, + }, + file_column: match column_number_str.parse().ok() { + None => return fallback_query, + column => column, + }, + raw_query, + } + } + (Some(file_path_part), Some(row_number_str), _) if !row_number_str.is_empty() => Self { file_path_end: Some(file_path_part.len()), file_row: match row_number_str.parse().ok() { None => return fallback_query, row => row, }, - file_column: match column_number_str.parse().ok() { - None => return fallback_query, - column => column, - }, + file_column: None, raw_query, }, - (Some(row_number_str), Some(file_path_part), None) => Self { + // Covers inputs like `foo.rs:` trimming all extra colons + (Some(file_path_part), _, _) => Self { file_path_end: Some(file_path_part.len()), - file_row: match row_number_str.parse().ok() { - None => return fallback_query, - row => row, - }, + file_row: None, file_column: None, raw_query, }, @@ -229,7 +243,13 @@ impl FileFinderDelegate { ) { if search_id >= self.latest_search_id { self.latest_search_id = search_id; - if self.latest_search_did_cancel && Some(&query) == self.latest_search_query.as_ref() { + if self.latest_search_did_cancel + && Some(query.path_query()) + == self + .latest_search_query + .as_ref() + .map(|query| query.path_query()) + { util::extend_sorted(&mut self.matches, matches.into_iter(), 100, |a, b| b.cmp(a)); } else { self.matches = matches; @@ -290,12 +310,39 @@ impl PickerDelegate for FileFinderDelegate { workspace.update(cx, |workspace, cx| { workspace - // TODO kb need to pass row and column here - // use self.latest_search_query .open_path(project_path.clone(), None, true, cx) .detach_and_log_err(cx); + }); + + workspace.update(cx, |workspace, cx| { + if let Some(query) = &self.latest_search_query { + let row = query.file_row; + let column = query.file_column; + if let Some(row) = row { + if let Some(active_editor) = workspace + .active_item(cx) + .and_then(|active_item| active_item.downcast::()) + { + // TODO kb does not open proper lines for the first time + active_editor.update(cx, |active_editor, cx| { + let snapshot = active_editor.snapshot(cx).display_snapshot; + let point = DisplayPoint::new( + row.saturating_sub(1), + column.map(|column| column.saturating_sub(1)).unwrap_or(0), + ) + .to_point(&snapshot); + active_editor.change_selections( + Some(Autoscroll::center()), + cx, + |s| s.select_ranges([point..point]), + ); + }) + } + } + } + workspace.dismiss_modal(cx); - }) + }); } } } From 0db7f4202ab242584e147c4e8720df6d2dca5d74 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Wed, 10 May 2023 21:37:59 +0300 Subject: [PATCH 03/13] Properly place the caret into the window of the file opened co-authored-by: Mikayla Maki --- crates/file_finder/src/file_finder.rs | 65 ++++++++++++++++----------- 1 file changed, 39 insertions(+), 26 deletions(-) diff --git a/crates/file_finder/src/file_finder.rs b/crates/file_finder/src/file_finder.rs index 8181d6a1f0b8d2cb02b9fc16da594b6de10ffffd..3ad777b7fa0a482258455caa657decbad28d5d97 100644 --- a/crates/file_finder/src/file_finder.rs +++ b/crates/file_finder/src/file_finder.rs @@ -1,4 +1,4 @@ -use editor::{scroll::autoscroll::Autoscroll, DisplayPoint, Editor}; +use editor::{scroll::autoscroll::Autoscroll, Bias, DisplayPoint, Editor}; use fuzzy::PathMatch; use gpui::{ actions, elements::*, AppContext, ModelHandle, MouseState, Task, ViewContext, WeakViewHandle, @@ -308,41 +308,54 @@ impl PickerDelegate for FileFinderDelegate { path: m.path.clone(), }; - workspace.update(cx, |workspace, cx| { - workspace - .open_path(project_path.clone(), None, true, cx) - .detach_and_log_err(cx); + let open_task = workspace.update(cx, |workspace, cx| { + workspace.open_path(project_path.clone(), None, true, cx) }); - workspace.update(cx, |workspace, cx| { - if let Some(query) = &self.latest_search_query { - let row = query.file_row; - let column = query.file_column; - if let Some(row) = row { - if let Some(active_editor) = workspace - .active_item(cx) - .and_then(|active_item| active_item.downcast::()) - { - // TODO kb does not open proper lines for the first time - active_editor.update(cx, |active_editor, cx| { - let snapshot = active_editor.snapshot(cx).display_snapshot; + let workspace = workspace.downgrade(); + + cx.spawn(|file_finder, mut cx| async move { + let item = open_task.await.log_err()?; + + let (row, col) = file_finder + .read_with(&cx, |file_finder, _| { + file_finder + .delegate() + .latest_search_query + .as_ref() + .map(|query| (query.file_row, query.file_column)) + }) + .log_err() + .flatten()?; + + if let Some(row) = row { + if let Some(active_editor) = item.downcast::() { + active_editor + .downgrade() + .update(&mut cx, |editor, cx| { + let snapshot = editor.snapshot(cx).display_snapshot; let point = DisplayPoint::new( row.saturating_sub(1), - column.map(|column| column.saturating_sub(1)).unwrap_or(0), + col.map(|column| column.saturating_sub(1)).unwrap_or(0), ) .to_point(&snapshot); - active_editor.change_selections( - Some(Autoscroll::center()), - cx, - |s| s.select_ranges([point..point]), - ); + let point = + snapshot.buffer_snapshot.clip_point(point, Bias::Left); + editor.change_selections(Some(Autoscroll::center()), cx, |s| { + s.select_ranges([point..point]) + }); }) - } + .log_err(); } } - workspace.dismiss_modal(cx); - }); + workspace + .update(&mut cx, |workspace, cx| workspace.dismiss_modal(cx)) + .log_err(); + + Some(()) + }) + .detach(); } } } From e9606982e6ed2ecec14b085d2a017cbae8a37cba Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Thu, 11 May 2023 11:32:41 +0300 Subject: [PATCH 04/13] Use ':' instead of ',' to separate files, rows and columns --- crates/editor/src/editor.rs | 1 + crates/editor/src/items.rs | 7 ++++++- crates/go_to_line/src/go_to_line.rs | 17 ++++++++++------- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index b6d44397a9c4933a0a7e6a7f990c9fdc2515b0a2..915bd14786d2026cc143ffc46368114d9194c3af 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -98,6 +98,7 @@ const MAX_SELECTION_HISTORY_LEN: usize = 1024; const COPILOT_DEBOUNCE_TIMEOUT: Duration = Duration::from_millis(75); pub const FORMAT_TIMEOUT: Duration = Duration::from_secs(2); +pub const FILE_ROW_COLUMN_DELIMITER: char = ':'; #[derive(Clone, Deserialize, PartialEq, Default)] pub struct SelectNext { diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index d2b9c20803e9ee211953302a36613e579b3b9ba2..a99f9c3d08eb9294e094bbe456281d574a5cda3e 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -2,6 +2,7 @@ use crate::{ display_map::ToDisplayPoint, link_go_to_definition::hide_link_definition, movement::surrounding_word, persistence::DB, scroll::ScrollAnchor, Anchor, Autoscroll, Editor, Event, ExcerptId, ExcerptRange, MultiBuffer, MultiBufferSnapshot, NavigationData, ToPoint as _, + FILE_ROW_COLUMN_DELIMITER, }; use anyhow::{Context, Result}; use collections::HashSet; @@ -1112,7 +1113,11 @@ impl View for CursorPosition { fn render(&mut self, cx: &mut ViewContext) -> AnyElement { if let Some(position) = self.position { let theme = &cx.global::().theme.workspace.status_bar; - let mut text = format!("{},{}", position.row + 1, position.column + 1); + let mut text = format!( + "{}{FILE_ROW_COLUMN_DELIMITER}{}", + position.row + 1, + position.column + 1 + ); if self.selected_count > 0 { write!(text, " ({} selected)", self.selected_count).unwrap(); } diff --git a/crates/go_to_line/src/go_to_line.rs b/crates/go_to_line/src/go_to_line.rs index 90287e927072779b0f2eb73eb14ef60792eb54e5..d6db685906009c6280c2aa3b27d5568bc69a8276 100644 --- a/crates/go_to_line/src/go_to_line.rs +++ b/crates/go_to_line/src/go_to_line.rs @@ -1,6 +1,9 @@ use std::sync::Arc; -use editor::{display_map::ToDisplayPoint, scroll::autoscroll::Autoscroll, DisplayPoint, Editor}; +use editor::{ + display_map::ToDisplayPoint, scroll::autoscroll::Autoscroll, DisplayPoint, Editor, + FILE_ROW_COLUMN_DELIMITER, +}; use gpui::{ actions, elements::*, geometry::vector::Vector2F, AnyViewHandle, AppContext, Axis, Entity, View, ViewContext, ViewHandle, @@ -97,14 +100,14 @@ impl GoToLine { editor::Event::Blurred => cx.emit(Event::Dismissed), editor::Event::BufferEdited { .. } => { let line_editor = self.line_editor.read(cx).text(cx); - let mut components = line_editor.trim().split(&[',', ':'][..]); + let mut components = line_editor + .splitn(2, FILE_ROW_COLUMN_DELIMITER) + .map(str::trim) + .fuse(); let row = components.next().and_then(|row| row.parse::().ok()); let column = components.next().and_then(|row| row.parse::().ok()); if let Some(point) = row.map(|row| { - Point::new( - row.saturating_sub(1), - column.map(|column| column.saturating_sub(1)).unwrap_or(0), - ) + Point::new(row.saturating_sub(1), column.unwrap_or(0).saturating_sub(1)) }) { self.active_editor.update(cx, |active_editor, cx| { let snapshot = active_editor.snapshot(cx).display_snapshot; @@ -147,7 +150,7 @@ impl View for GoToLine { let theme = &cx.global::().theme.picker; let label = format!( - "{},{} of {} lines", + "{}{FILE_ROW_COLUMN_DELIMITER}{} of {} lines", self.cursor_point.row + 1, self.cursor_point.column + 1, self.max_point.row + 1 From e5bca9c8710ecf70377bc1ca61277fadb0005907 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Thu, 11 May 2023 12:17:47 +0300 Subject: [PATCH 05/13] Simplify file-row-column parsing --- crates/file_finder/src/file_finder.rs | 91 +++++++++------------------ 1 file changed, 29 insertions(+), 62 deletions(-) diff --git a/crates/file_finder/src/file_finder.rs b/crates/file_finder/src/file_finder.rs index 3ad777b7fa0a482258455caa657decbad28d5d97..3d1a9a0c996f276dd727e20b581f31056e468d24 100644 --- a/crates/file_finder/src/file_finder.rs +++ b/crates/file_finder/src/file_finder.rs @@ -1,4 +1,4 @@ -use editor::{scroll::autoscroll::Autoscroll, Bias, DisplayPoint, Editor}; +use editor::{scroll::autoscroll::Autoscroll, Bias, DisplayPoint, Editor, FILE_ROW_COLUMN_DELIMITER}; use fuzzy::PathMatch; use gpui::{ actions, elements::*, AppContext, ModelHandle, MouseState, Task, ViewContext, WeakViewHandle, @@ -71,54 +71,20 @@ struct FileSearchQuery { impl FileSearchQuery { fn new(raw_query: String) -> Self { - let fallback_query = Self { - raw_query: raw_query.clone(), - file_path_end: None, - file_row: None, - file_column: None, - }; - - let mut possible_path_and_coordinates = - // TODO kb go_to_line.rs uses ',' as a separator?? - raw_query.as_str().splitn(3, ':').map(str::trim).fuse(); - match ( - possible_path_and_coordinates.next(), - possible_path_and_coordinates.next(), - possible_path_and_coordinates.next(), - ) { - (Some(file_path_part), Some(row_number_str), Some(column_number_str)) - if !row_number_str.is_empty() && !column_number_str.is_empty() => - { - Self { - file_path_end: Some(file_path_part.len()), - file_row: match row_number_str.parse().ok() { - None => return fallback_query, - row => row, - }, - file_column: match column_number_str.parse().ok() { - None => return fallback_query, - column => column, - }, - raw_query, - } - } - (Some(file_path_part), Some(row_number_str), _) if !row_number_str.is_empty() => Self { - file_path_end: Some(file_path_part.len()), - file_row: match row_number_str.parse().ok() { - None => return fallback_query, - row => row, - }, - file_column: None, - raw_query, - }, - // Covers inputs like `foo.rs:` trimming all extra colons - (Some(file_path_part), _, _) => Self { - file_path_end: Some(file_path_part.len()), - file_row: None, - file_column: None, - raw_query, - }, - _no_colons_query => fallback_query, + let mut components = raw_query + .as_str() + .splitn(3, FILE_ROW_COLUMN_DELIMITER) + .map(str::trim) + .fuse(); + let file_query = components.next().filter(|str| !str.is_empty()); + let file_row = components.next().and_then(|row| row.parse::().ok()); + let file_column = components.next().and_then(|col| col.parse::().ok()); + + Self { + file_path_end: file_query.map(|query| query.len()), + file_row, + file_column, + raw_query, } } @@ -314,19 +280,18 @@ impl PickerDelegate for FileFinderDelegate { let workspace = workspace.downgrade(); - cx.spawn(|file_finder, mut cx| async move { - let item = open_task.await.log_err()?; - - let (row, col) = file_finder - .read_with(&cx, |file_finder, _| { - file_finder - .delegate() - .latest_search_query - .as_ref() - .map(|query| (query.file_row, query.file_column)) - }) - .log_err() - .flatten()?; + if let Some(row) = self + .latest_search_query + .as_ref() + .and_then(|query| query.file_row) + .map(|row| row.saturating_sub(1)) + { + let col = self + .latest_search_query + .as_ref() + .and_then(|query| query.file_column) + .unwrap_or(0) + .saturating_sub(1); if let Some(row) = row { if let Some(active_editor) = item.downcast::() { @@ -339,6 +304,8 @@ impl PickerDelegate for FileFinderDelegate { col.map(|column| column.saturating_sub(1)).unwrap_or(0), ) .to_point(&snapshot); + let point = + snapshot.buffer_snapshot.clip_point(point, Bias::Left); let point = snapshot.buffer_snapshot.clip_point(point, Bias::Left); editor.change_selections(Some(Autoscroll::center()), cx, |s| { From 477bc8da05c5771bce4189c6393cfb3f01d155c3 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Thu, 11 May 2023 12:23:05 +0300 Subject: [PATCH 06/13] Make Go To Line to respect column numbers --- crates/go_to_line/src/go_to_line.rs | 41 ++++++++++++++++------------- 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/crates/go_to_line/src/go_to_line.rs b/crates/go_to_line/src/go_to_line.rs index d6db685906009c6280c2aa3b27d5568bc69a8276..072ba2f1992eccf6039d4544ccf30cfdc2e9091b 100644 --- a/crates/go_to_line/src/go_to_line.rs +++ b/crates/go_to_line/src/go_to_line.rs @@ -1,8 +1,7 @@ use std::sync::Arc; use editor::{ - display_map::ToDisplayPoint, scroll::autoscroll::Autoscroll, DisplayPoint, Editor, - FILE_ROW_COLUMN_DELIMITER, + display_map::ToDisplayPoint, scroll::autoscroll::Autoscroll, Editor, FILE_ROW_COLUMN_DELIMITER, }; use gpui::{ actions, elements::*, geometry::vector::Vector2F, AnyViewHandle, AppContext, Axis, Entity, @@ -78,15 +77,16 @@ impl GoToLine { fn confirm(&mut self, _: &Confirm, cx: &mut ViewContext) { self.prev_scroll_position.take(); - self.active_editor.update(cx, |active_editor, cx| { - if let Some(rows) = active_editor.highlighted_rows() { + if let Some(point) = self.point_from_query(cx) { + self.active_editor.update(cx, |active_editor, cx| { let snapshot = active_editor.snapshot(cx).display_snapshot; - let position = DisplayPoint::new(rows.start, 0).to_point(&snapshot); + let point = snapshot.buffer_snapshot.clip_point(point, Bias::Left); active_editor.change_selections(Some(Autoscroll::center()), cx, |s| { - s.select_ranges([position..position]) + s.select_ranges([point..point]) }); - } - }); + }); + } + cx.emit(Event::Dismissed); } @@ -99,16 +99,7 @@ impl GoToLine { match event { editor::Event::Blurred => cx.emit(Event::Dismissed), editor::Event::BufferEdited { .. } => { - let line_editor = self.line_editor.read(cx).text(cx); - let mut components = line_editor - .splitn(2, FILE_ROW_COLUMN_DELIMITER) - .map(str::trim) - .fuse(); - let row = components.next().and_then(|row| row.parse::().ok()); - let column = components.next().and_then(|row| row.parse::().ok()); - if let Some(point) = row.map(|row| { - Point::new(row.saturating_sub(1), column.unwrap_or(0).saturating_sub(1)) - }) { + if let Some(point) = self.point_from_query(cx) { self.active_editor.update(cx, |active_editor, cx| { let snapshot = active_editor.snapshot(cx).display_snapshot; let point = snapshot.buffer_snapshot.clip_point(point, Bias::Left); @@ -123,6 +114,20 @@ impl GoToLine { _ => {} } } + + fn point_from_query(&self, cx: &ViewContext) -> Option { + let line_editor = self.line_editor.read(cx).text(cx); + let mut components = line_editor + .splitn(2, FILE_ROW_COLUMN_DELIMITER) + .map(str::trim) + .fuse(); + let row = components.next().and_then(|row| row.parse::().ok())?; + let column = components.next().and_then(|col| col.parse::().ok()); + Some(Point::new( + row.saturating_sub(1), + column.unwrap_or(0).saturating_sub(1), + )) + } } impl Entity for GoToLine { From 89fe5c6b0990f484032c0ff22f3b6059664326d8 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Thu, 11 May 2023 11:45:29 +0300 Subject: [PATCH 07/13] Test caret selection in file finder co-authored-by: Max --- Cargo.lock | 1 + crates/editor/src/hover_popover.rs | 3 +- crates/file_finder/Cargo.toml | 1 + crates/file_finder/src/file_finder.rs | 216 +++++++++++++++++++++++--- 4 files changed, 198 insertions(+), 23 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e009cfd34267a9b341254fc029e2d21a1a8298b2..871a95c1903af01caa36c9d96097e750575248dc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2185,6 +2185,7 @@ dependencies = [ "project", "serde_json", "settings", + "text", "theme", "util", "workspace", diff --git a/crates/editor/src/hover_popover.rs b/crates/editor/src/hover_popover.rs index 438c662ed1125209640b6481097f9b34fd0787c6..85edb97da46cba1837727244f36e83b9dd90d28f 100644 --- a/crates/editor/src/hover_popover.rs +++ b/crates/editor/src/hover_popover.rs @@ -1006,8 +1006,7 @@ mod tests { .zip(expected_styles.iter().cloned()) .collect::>(); assert_eq!( - rendered.text, - dbg!(expected_text), + rendered.text, expected_text, "wrong text for input {blocks:?}" ); assert_eq!( diff --git a/crates/file_finder/Cargo.toml b/crates/file_finder/Cargo.toml index 30a4650ad74d033221c43139631b6dc44d5bcafb..8b99cc58560b858642300899b1fd7f6077da5437 100644 --- a/crates/file_finder/Cargo.toml +++ b/crates/file_finder/Cargo.toml @@ -16,6 +16,7 @@ menu = { path = "../menu" } picker = { path = "../picker" } project = { path = "../project" } settings = { path = "../settings" } +text = { path = "../text" } util = { path = "../util" } theme = { path = "../theme" } workspace = { path = "../workspace" } diff --git a/crates/file_finder/src/file_finder.rs b/crates/file_finder/src/file_finder.rs index 3d1a9a0c996f276dd727e20b581f31056e468d24..6447706358ff55aec82c5a3201c13807f31ee557 100644 --- a/crates/file_finder/src/file_finder.rs +++ b/crates/file_finder/src/file_finder.rs @@ -1,4 +1,4 @@ -use editor::{scroll::autoscroll::Autoscroll, Bias, DisplayPoint, Editor, FILE_ROW_COLUMN_DELIMITER}; +use editor::{scroll::autoscroll::Autoscroll, Bias, Editor, FILE_ROW_COLUMN_DELIMITER}; use fuzzy::PathMatch; use gpui::{ actions, elements::*, AppContext, ModelHandle, MouseState, Task, ViewContext, WeakViewHandle, @@ -13,6 +13,7 @@ use std::{ Arc, }, }; +use text::Point; use util::{post_inc, ResultExt}; use workspace::Workspace; @@ -64,7 +65,7 @@ pub enum Event { #[derive(Debug, Clone)] struct FileSearchQuery { raw_query: String, - file_path_end: Option, + file_query_end: Option, file_row: Option, file_column: Option, } @@ -81,7 +82,9 @@ impl FileSearchQuery { let file_column = components.next().and_then(|col| col.parse::().ok()); Self { - file_path_end: file_query.map(|query| query.len()), + file_query_end: file_query + .filter(|_| file_row.is_some()) + .map(|query| query.len()), file_row, file_column, raw_query, @@ -89,7 +92,7 @@ impl FileSearchQuery { } fn path_query(&self) -> &str { - match self.file_path_end { + match self.file_query_end { Some(file_path_end) => &self.raw_query[..file_path_end], None => &self.raw_query, } @@ -280,32 +283,28 @@ impl PickerDelegate for FileFinderDelegate { let workspace = workspace.downgrade(); - if let Some(row) = self + let row = self .latest_search_query .as_ref() .and_then(|query| query.file_row) - .map(|row| row.saturating_sub(1)) - { - let col = self - .latest_search_query - .as_ref() - .and_then(|query| query.file_column) - .unwrap_or(0) - .saturating_sub(1); - + .map(|row| row.saturating_sub(1)); + let col = self + .latest_search_query + .as_ref() + .and_then(|query| query.file_column) + .unwrap_or(0) + .saturating_sub(1); + cx.spawn(|_, mut cx| async move { + let item = open_task.await.log_err()?; if let Some(row) = row { if let Some(active_editor) = item.downcast::() { active_editor .downgrade() .update(&mut cx, |editor, cx| { let snapshot = editor.snapshot(cx).display_snapshot; - let point = DisplayPoint::new( - row.saturating_sub(1), - col.map(|column| column.saturating_sub(1)).unwrap_or(0), - ) - .to_point(&snapshot); - let point = - snapshot.buffer_snapshot.clip_point(point, Bias::Left); + let point = snapshot + .buffer_snapshot + .clip_point(Point::new(row, col), Bias::Left); let point = snapshot.buffer_snapshot.clip_point(point, Bias::Left); editor.change_selections(Some(Autoscroll::center()), cx, |s| { @@ -359,6 +358,7 @@ impl PickerDelegate for FileFinderDelegate { mod tests { use super::*; use editor::Editor; + use gpui::executor::Deterministic; use menu::{Confirm, SelectNext}; use serde_json::json; use workspace::{AppState, Workspace}; @@ -426,6 +426,180 @@ mod tests { }); } + #[gpui::test] + async fn test_row_column_numbers_query_inside_file( + deterministic: Arc, + cx: &mut gpui::TestAppContext, + ) { + let app_state = cx.update(|cx| { + super::init(cx); + editor::init(cx); + AppState::test(cx) + }); + + let first_file_name = "first.rs"; + let first_file_contents = "// First Rust file"; + app_state + .fs + .as_fake() + .insert_tree( + "/src", + json!({ + "test": { + first_file_name: first_file_contents, + "second.rs": "// Second Rust file", + } + }), + ) + .await; + + let project = Project::test(app_state.fs.clone(), ["/src".as_ref()], cx).await; + let (window_id, workspace) = cx.add_window(|cx| Workspace::test_new(project, cx)); + cx.dispatch_action(window_id, Toggle); + let finder = cx.read(|cx| workspace.read(cx).modal::().unwrap()); + + let file_query = &first_file_name[..3]; + let file_row = 1; + let file_column = 3; + assert!(file_column <= first_file_contents.len()); + let query_inside_file = format!("{file_query}:{file_row}:{file_column}"); + finder + .update(cx, |finder, cx| { + finder + .delegate_mut() + .update_matches(query_inside_file.to_string(), cx) + }) + .await; + finder.read_with(cx, |finder, _| { + let finder = finder.delegate(); + assert_eq!(finder.matches.len(), 1); + let latest_search_query = finder + .latest_search_query + .as_ref() + .expect("Finder should have a query after the update_matches call"); + assert_eq!(latest_search_query.raw_query, query_inside_file); + assert_eq!(latest_search_query.file_row, Some(file_row)); + assert_eq!(latest_search_query.file_column, Some(file_column as u32)); + assert_eq!(latest_search_query.file_query_end, Some(file_query.len())); + }); + + let active_pane = cx.read(|cx| workspace.read(cx).active_pane().clone()); + cx.dispatch_action(window_id, SelectNext); + cx.dispatch_action(window_id, Confirm); + active_pane + .condition(cx, |pane, _| pane.active_item().is_some()) + .await; + let editor = cx.update(|cx| { + let active_item = active_pane.read(cx).active_item().unwrap(); + active_item.downcast::().unwrap() + }); + deterministic.advance_clock(std::time::Duration::from_secs(2)); + deterministic.start_waiting(); + deterministic.finish_waiting(); + editor.update(cx, |editor, cx| { + let all_selections = editor.selections.all_adjusted(cx); + assert_eq!( + all_selections.len(), + 1, + "Expected to have 1 selection (caret) after file finder confirm, but got: {all_selections:?}" + ); + let caret_selection = all_selections.into_iter().next().unwrap(); + assert_eq!(caret_selection.start, caret_selection.end, + "Caret selection should have its start and end at the same position"); + assert_eq!(file_row, caret_selection.start.row + 1, + "Query inside file should get caret with the same focus row"); + assert_eq!(file_column, caret_selection.start.column as usize + 1, + "Query inside file should get caret with the same focus column"); + }); + } + + #[gpui::test] + async fn test_row_column_numbers_query_outside_file( + deterministic: Arc, + cx: &mut gpui::TestAppContext, + ) { + let app_state = cx.update(|cx| { + super::init(cx); + editor::init(cx); + AppState::test(cx) + }); + + let first_file_name = "first.rs"; + let first_file_contents = "// First Rust file"; + app_state + .fs + .as_fake() + .insert_tree( + "/src", + json!({ + "test": { + first_file_name: first_file_contents, + "second.rs": "// Second Rust file", + } + }), + ) + .await; + + let project = Project::test(app_state.fs.clone(), ["/src".as_ref()], cx).await; + let (window_id, workspace) = cx.add_window(|cx| Workspace::test_new(project, cx)); + cx.dispatch_action(window_id, Toggle); + let finder = cx.read(|cx| workspace.read(cx).modal::().unwrap()); + + let file_query = &first_file_name[..3]; + let file_row = 200; + let file_column = 300; + assert!(file_column > first_file_contents.len()); + let query_outside_file = format!("{file_query}:{file_row}:{file_column}"); + finder + .update(cx, |finder, cx| { + finder + .delegate_mut() + .update_matches(query_outside_file.to_string(), cx) + }) + .await; + finder.read_with(cx, |finder, _| { + let finder = finder.delegate(); + assert_eq!(finder.matches.len(), 1); + let latest_search_query = finder + .latest_search_query + .as_ref() + .expect("Finder should have a query after the update_matches call"); + assert_eq!(latest_search_query.raw_query, query_outside_file); + assert_eq!(latest_search_query.file_row, Some(file_row)); + assert_eq!(latest_search_query.file_column, Some(file_column as u32)); + assert_eq!(latest_search_query.file_query_end, Some(file_query.len())); + }); + + let active_pane = cx.read(|cx| workspace.read(cx).active_pane().clone()); + cx.dispatch_action(window_id, SelectNext); + cx.dispatch_action(window_id, Confirm); + active_pane + .condition(cx, |pane, _| pane.active_item().is_some()) + .await; + let editor = cx.update(|cx| { + let active_item = active_pane.read(cx).active_item().unwrap(); + active_item.downcast::().unwrap() + }); + deterministic.advance_clock(std::time::Duration::from_secs(2)); + deterministic.start_waiting(); + deterministic.finish_waiting(); + editor.update(cx, |editor, cx| { + let all_selections = editor.selections.all_adjusted(cx); + assert_eq!( + all_selections.len(), + 1, + "Expected to have 1 selection (caret) after file finder confirm, but got: {all_selections:?}" + ); + let caret_selection = all_selections.into_iter().next().unwrap(); + assert_eq!(caret_selection.start, caret_selection.end, + "Caret selection should have its start and end at the same position"); + assert_eq!(0, caret_selection.start.row, + "Excessive rows (as in query outside file borders) should get trimmed to last file row"); + assert_eq!(first_file_contents.len(), caret_selection.start.column as usize, + "Excessive columns (as in query outside file borders) should get trimmed to selected row's last column"); + }); + } + #[gpui::test] async fn test_matching_cancellation(cx: &mut gpui::TestAppContext) { let app_state = cx.update(AppState::test); From d7193521520d08b3f166434d952fc67d0b990c91 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Fri, 12 May 2023 16:52:07 +0300 Subject: [PATCH 08/13] Unify path:row:column parsing, use it in CLI --- Cargo.lock | 2 + crates/cli/Cargo.toml | 1 + crates/cli/src/main.rs | 30 +++++-- crates/editor/src/editor.rs | 1 - crates/editor/src/items.rs | 3 +- crates/file_finder/src/file_finder.rs | 112 +++++++++++++------------- crates/go_to_line/Cargo.toml | 1 + crates/go_to_line/src/go_to_line.rs | 5 +- crates/util/src/paths.rs | 37 +++++++++ 9 files changed, 127 insertions(+), 65 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 871a95c1903af01caa36c9d96097e750575248dc..41c3eab8219a03f5c08bf27d3e6f4cfd512b89ae 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1097,6 +1097,7 @@ dependencies = [ "plist", "serde", "serde_derive", + "util", ] [[package]] @@ -2675,6 +2676,7 @@ dependencies = [ "postage", "settings", "text", + "util", "workspace", ] diff --git a/crates/cli/Cargo.toml b/crates/cli/Cargo.toml index 9b8009dd696e51aab3090a22f5a97434b0acf3d9..2b4a375a5bf9a2e58ffc889f9f7786bb4c35a324 100644 --- a/crates/cli/Cargo.toml +++ b/crates/cli/Cargo.toml @@ -19,6 +19,7 @@ dirs = "3.0" ipc-channel = "0.16" serde.workspace = true serde_derive.workspace = true +util = { path = "../util" } [target.'cfg(target_os = "macos")'.dependencies] core-foundation = "0.9" diff --git a/crates/cli/src/main.rs b/crates/cli/src/main.rs index 0ae4d2477e846c18b8031013cf65ba71d4109bf0..80ec2bbf99e644aaa87a08acc6fa983af3453603 100644 --- a/crates/cli/src/main.rs +++ b/crates/cli/src/main.rs @@ -16,6 +16,7 @@ use std::{ path::{Path, PathBuf}, ptr, }; +use util::paths::PathLikeWithPosition; #[derive(Parser)] #[clap(name = "zed", global_setting(clap::AppSettings::NoAutoVersion))] @@ -24,8 +25,11 @@ struct Args { #[clap(short, long)] wait: bool, /// A sequence of space-separated paths that you want to open. - #[clap()] - paths: Vec, + /// + /// Use `path:line:row` syntax to open a file at a specific location. + /// Non-existing paths and directories will ignore `:line:row` suffix. + #[clap(value_parser = parse_path_with_position)] + paths_with_position: Vec>, /// Print Zed's version and the app path. #[clap(short, long)] version: bool, @@ -34,6 +38,14 @@ struct Args { bundle_path: Option, } +fn parse_path_with_position( + argument_str: &str, +) -> Result, std::convert::Infallible> { + PathLikeWithPosition::parse_str(argument_str, |path_str| { + Ok(Path::new(path_str).to_path_buf()) + }) +} + #[derive(Debug, Deserialize)] struct InfoPlist { #[serde(rename = "CFBundleShortVersionString")] @@ -50,7 +62,11 @@ fn main() -> Result<()> { return Ok(()); } - for path in args.paths.iter() { + for path in args + .paths_with_position + .iter() + .map(|path_with_position| &path_with_position.path_like) + { if !path.exists() { touch(path.as_path())?; } @@ -60,9 +76,13 @@ fn main() -> Result<()> { tx.send(CliRequest::Open { paths: args - .paths + .paths_with_position .into_iter() - .map(|path| fs::canonicalize(path).map_err(|error| anyhow!(error))) + // TODO kb continue sendint path with the position further + .map(|path_with_position| path_with_position.path_like) + .map(|path| { + fs::canonicalize(&path).with_context(|| format!("path {path:?} canonicalization")) + }) .collect::>>()?, wait: args.wait, })?; diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 915bd14786d2026cc143ffc46368114d9194c3af..b6d44397a9c4933a0a7e6a7f990c9fdc2515b0a2 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -98,7 +98,6 @@ const MAX_SELECTION_HISTORY_LEN: usize = 1024; const COPILOT_DEBOUNCE_TIMEOUT: Duration = Duration::from_millis(75); pub const FORMAT_TIMEOUT: Duration = Duration::from_secs(2); -pub const FILE_ROW_COLUMN_DELIMITER: char = ':'; #[derive(Clone, Deserialize, PartialEq, Default)] pub struct SelectNext { diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index a99f9c3d08eb9294e094bbe456281d574a5cda3e..1a9fcc963e2415570afaa90b0795e802c87c4d9c 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -2,7 +2,6 @@ use crate::{ display_map::ToDisplayPoint, link_go_to_definition::hide_link_definition, movement::surrounding_word, persistence::DB, scroll::ScrollAnchor, Anchor, Autoscroll, Editor, Event, ExcerptId, ExcerptRange, MultiBuffer, MultiBufferSnapshot, NavigationData, ToPoint as _, - FILE_ROW_COLUMN_DELIMITER, }; use anyhow::{Context, Result}; use collections::HashSet; @@ -28,7 +27,7 @@ use std::{ path::{Path, PathBuf}, }; use text::Selection; -use util::{ResultExt, TryFutureExt}; +use util::{paths::FILE_ROW_COLUMN_DELIMITER, ResultExt, TryFutureExt}; use workspace::item::{BreadcrumbText, FollowableItemHandle}; use workspace::{ item::{FollowableItem, Item, ItemEvent, ItemHandle, ProjectItem}, diff --git a/crates/file_finder/src/file_finder.rs b/crates/file_finder/src/file_finder.rs index 6447706358ff55aec82c5a3201c13807f31ee557..fae9bd565cf4da958828e87716ebab83a6a293d9 100644 --- a/crates/file_finder/src/file_finder.rs +++ b/crates/file_finder/src/file_finder.rs @@ -1,4 +1,4 @@ -use editor::{scroll::autoscroll::Autoscroll, Bias, Editor, FILE_ROW_COLUMN_DELIMITER}; +use editor::{scroll::autoscroll::Autoscroll, Bias, Editor}; use fuzzy::PathMatch; use gpui::{ actions, elements::*, AppContext, ModelHandle, MouseState, Task, ViewContext, WeakViewHandle, @@ -14,7 +14,7 @@ use std::{ }, }; use text::Point; -use util::{post_inc, ResultExt}; +use util::{paths::PathLikeWithPosition, post_inc, ResultExt}; use workspace::Workspace; pub type FileFinder = Picker; @@ -25,7 +25,7 @@ pub struct FileFinderDelegate { search_count: usize, latest_search_id: usize, latest_search_did_cancel: bool, - latest_search_query: Option, + latest_search_query: Option>, relative_to: Option>, matches: Vec, selected: Option<(usize, Arc)>, @@ -66,31 +66,9 @@ pub enum Event { struct FileSearchQuery { raw_query: String, file_query_end: Option, - file_row: Option, - file_column: Option, } impl FileSearchQuery { - fn new(raw_query: String) -> Self { - let mut components = raw_query - .as_str() - .splitn(3, FILE_ROW_COLUMN_DELIMITER) - .map(str::trim) - .fuse(); - let file_query = components.next().filter(|str| !str.is_empty()); - let file_row = components.next().and_then(|row| row.parse::().ok()); - let file_column = components.next().and_then(|col| col.parse::().ok()); - - Self { - file_query_end: file_query - .filter(|_| file_row.is_some()) - .map(|query| query.len()), - file_row, - file_column, - raw_query, - } - } - fn path_query(&self) -> &str { match self.file_query_end { Some(file_path_end) => &self.raw_query[..file_path_end], @@ -152,7 +130,7 @@ impl FileFinderDelegate { fn spawn_search( &mut self, - query: FileSearchQuery, + query: PathLikeWithPosition, cx: &mut ViewContext, ) -> Task<()> { let relative_to = self.relative_to.clone(); @@ -183,7 +161,7 @@ impl FileFinderDelegate { cx.spawn(|picker, mut cx| async move { let matches = fuzzy::match_path_sets( candidate_sets.as_slice(), - query.path_query(), + query.path_like.path_query(), relative_to, false, 100, @@ -206,18 +184,18 @@ impl FileFinderDelegate { &mut self, search_id: usize, did_cancel: bool, - query: FileSearchQuery, + query: PathLikeWithPosition, matches: Vec, cx: &mut ViewContext, ) { if search_id >= self.latest_search_id { self.latest_search_id = search_id; if self.latest_search_did_cancel - && Some(query.path_query()) + && Some(query.path_like.path_query()) == self .latest_search_query .as_ref() - .map(|query| query.path_query()) + .map(|query| query.path_like.path_query()) { util::extend_sorted(&mut self.matches, matches.into_iter(), 100, |a, b| b.cmp(a)); } else { @@ -265,7 +243,19 @@ impl PickerDelegate for FileFinderDelegate { cx.notify(); Task::ready(()) } else { - self.spawn_search(FileSearchQuery::new(raw_query), cx) + let raw_query = &raw_query; + let query = PathLikeWithPosition::parse_str(raw_query, |path_like_str| { + Ok::<_, std::convert::Infallible>(FileSearchQuery { + raw_query: raw_query.to_owned(), + file_query_end: if path_like_str == raw_query { + None + } else { + Some(path_like_str.len()) + }, + }) + }) + .expect("infallible"); + self.spawn_search(query, cx) } } @@ -286,12 +276,12 @@ impl PickerDelegate for FileFinderDelegate { let row = self .latest_search_query .as_ref() - .and_then(|query| query.file_row) + .and_then(|query| query.row) .map(|row| row.saturating_sub(1)); let col = self .latest_search_query .as_ref() - .and_then(|query| query.file_column) + .and_then(|query| query.column) .unwrap_or(0) .saturating_sub(1); cx.spawn(|_, mut cx| async move { @@ -477,10 +467,13 @@ mod tests { .latest_search_query .as_ref() .expect("Finder should have a query after the update_matches call"); - assert_eq!(latest_search_query.raw_query, query_inside_file); - assert_eq!(latest_search_query.file_row, Some(file_row)); - assert_eq!(latest_search_query.file_column, Some(file_column as u32)); - assert_eq!(latest_search_query.file_query_end, Some(file_query.len())); + assert_eq!(latest_search_query.path_like.raw_query, query_inside_file); + assert_eq!( + latest_search_query.path_like.file_query_end, + Some(file_query.len()) + ); + assert_eq!(latest_search_query.row, Some(file_row)); + assert_eq!(latest_search_query.column, Some(file_column as u32)); }); let active_pane = cx.read(|cx| workspace.read(cx).active_pane().clone()); @@ -564,10 +557,13 @@ mod tests { .latest_search_query .as_ref() .expect("Finder should have a query after the update_matches call"); - assert_eq!(latest_search_query.raw_query, query_outside_file); - assert_eq!(latest_search_query.file_row, Some(file_row)); - assert_eq!(latest_search_query.file_column, Some(file_column as u32)); - assert_eq!(latest_search_query.file_query_end, Some(file_query.len())); + assert_eq!(latest_search_query.path_like.raw_query, query_outside_file); + assert_eq!( + latest_search_query.path_like.file_query_end, + Some(file_query.len()) + ); + assert_eq!(latest_search_query.row, Some(file_row)); + assert_eq!(latest_search_query.column, Some(file_column as u32)); }); let active_pane = cx.read(|cx| workspace.read(cx).active_pane().clone()); @@ -634,7 +630,7 @@ mod tests { ) }); - let query = FileSearchQuery::new("hi".to_string()); + let query = test_path_like("hi"); finder .update(cx, |f, cx| f.delegate_mut().spawn_search(query.clone(), cx)) .await; @@ -719,8 +715,7 @@ mod tests { }); finder .update(cx, |f, cx| { - f.delegate_mut() - .spawn_search(FileSearchQuery::new("hi".to_string()), cx) + f.delegate_mut().spawn_search(test_path_like("hi"), cx) }) .await; finder.read_with(cx, |f, _| assert_eq!(f.delegate().matches.len(), 7)); @@ -758,8 +753,7 @@ mod tests { // is included in the matching, because the worktree is a single file. finder .update(cx, |f, cx| { - f.delegate_mut() - .spawn_search(FileSearchQuery::new("thf".to_string()), cx) + f.delegate_mut().spawn_search(test_path_like("thf"), cx) }) .await; cx.read(|cx| { @@ -779,8 +773,7 @@ mod tests { // not match anything. finder .update(cx, |f, cx| { - f.delegate_mut() - .spawn_search(FileSearchQuery::new("thf/".to_string()), cx) + f.delegate_mut().spawn_search(test_path_like("thf/"), cx) }) .await; finder.read_with(cx, |f, _| assert_eq!(f.delegate().matches.len(), 0)); @@ -826,8 +819,7 @@ mod tests { // Run a search that matches two files with the same relative path. finder .update(cx, |f, cx| { - f.delegate_mut() - .spawn_search(FileSearchQuery::new("a.t".to_string()), cx) + f.delegate_mut().spawn_search(test_path_like("a.t"), cx) }) .await; @@ -884,8 +876,7 @@ mod tests { finder .update(cx, |f, cx| { - f.delegate_mut() - .spawn_search(FileSearchQuery::new("a.txt".to_string()), cx) + f.delegate_mut().spawn_search(test_path_like("a.txt"), cx) }) .await; @@ -928,8 +919,7 @@ mod tests { }); finder .update(cx, |f, cx| { - f.delegate_mut() - .spawn_search(FileSearchQuery::new("dir".to_string()), cx) + f.delegate_mut().spawn_search(test_path_like("dir"), cx) }) .await; cx.read(|cx| { @@ -937,4 +927,18 @@ mod tests { assert_eq!(finder.delegate().matches.len(), 0); }); } + + fn test_path_like(test_str: &str) -> PathLikeWithPosition { + PathLikeWithPosition::parse_str(test_str, |path_like_str| { + Ok::<_, std::convert::Infallible>(FileSearchQuery { + raw_query: test_str.to_owned(), + file_query_end: if path_like_str == test_str { + None + } else { + Some(path_like_str.len()) + }, + }) + }) + .unwrap() + } } diff --git a/crates/go_to_line/Cargo.toml b/crates/go_to_line/Cargo.toml index f279aca569fd27fbda04bb87086a622e4e11389f..8f99aa366cbad598d00d5cbe24a6c84ede2deaa5 100644 --- a/crates/go_to_line/Cargo.toml +++ b/crates/go_to_line/Cargo.toml @@ -16,3 +16,4 @@ settings = { path = "../settings" } text = { path = "../text" } workspace = { path = "../workspace" } postage.workspace = true +util = { path = "../util" } diff --git a/crates/go_to_line/src/go_to_line.rs b/crates/go_to_line/src/go_to_line.rs index 072ba2f1992eccf6039d4544ccf30cfdc2e9091b..967f17b79457596cf58967bd2258bebf97282d59 100644 --- a/crates/go_to_line/src/go_to_line.rs +++ b/crates/go_to_line/src/go_to_line.rs @@ -1,8 +1,6 @@ use std::sync::Arc; -use editor::{ - display_map::ToDisplayPoint, scroll::autoscroll::Autoscroll, Editor, FILE_ROW_COLUMN_DELIMITER, -}; +use editor::{display_map::ToDisplayPoint, scroll::autoscroll::Autoscroll, Editor}; use gpui::{ actions, elements::*, geometry::vector::Vector2F, AnyViewHandle, AppContext, Axis, Entity, View, ViewContext, ViewHandle, @@ -10,6 +8,7 @@ use gpui::{ use menu::{Cancel, Confirm}; use settings::Settings; use text::{Bias, Point}; +use util::paths::FILE_ROW_COLUMN_DELIMITER; use workspace::{Modal, Workspace}; actions!(go_to_line, [Toggle]); diff --git a/crates/util/src/paths.rs b/crates/util/src/paths.rs index a324b21a31e60f623b8c688da22053d64a53f9a1..280d3cad021507092021a8d8fe217f09ccd1058e 100644 --- a/crates/util/src/paths.rs +++ b/crates/util/src/paths.rs @@ -70,3 +70,40 @@ pub fn compact(path: &Path) -> PathBuf { path.to_path_buf() } } + +pub const FILE_ROW_COLUMN_DELIMITER: char = ':'; + +#[derive(Debug, Clone)] +pub struct PathLikeWithPosition

{ + pub path_like: P, + pub row: Option, + pub column: Option, +} + +impl

PathLikeWithPosition

{ + pub fn parse_str(s: &str, parse_path_like_str: F) -> Result + where + F: Fn(&str) -> Result, + { + let mut components = s.splitn(3, FILE_ROW_COLUMN_DELIMITER).map(str::trim).fuse(); + let path_like_str = components.next().filter(|str| !str.is_empty()); + let row = components.next().and_then(|row| row.parse::().ok()); + let column = components + .next() + .filter(|_| row.is_some()) + .and_then(|col| col.parse::().ok()); + + Ok(match path_like_str { + Some(path_like_str) => Self { + path_like: parse_path_like_str(path_like_str)?, + row, + column, + }, + None => Self { + path_like: parse_path_like_str(s)?, + row: None, + column: None, + }, + }) + } +} From 628558aa3901caf1c44bd62fb85ec889c4507028 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Fri, 12 May 2023 18:13:28 +0300 Subject: [PATCH 09/13] Attempt to open rows and columns from CLI input --- crates/cli/src/cli.rs | 7 ++- crates/cli/src/main.rs | 13 ++-- crates/file_finder/src/file_finder.rs | 2 - crates/util/src/paths.rs | 15 ++++- crates/zed/src/main.rs | 86 ++++++++++++++++++++------- 5 files changed, 91 insertions(+), 32 deletions(-) diff --git a/crates/cli/src/cli.rs b/crates/cli/src/cli.rs index de7b14e14210567075e508bd8d5794a56d5af3de..8281bcb651aa1693cf362f7fb2635e1e09dfa399 100644 --- a/crates/cli/src/cli.rs +++ b/crates/cli/src/cli.rs @@ -1,6 +1,7 @@ pub use ipc_channel::ipc; use serde::{Deserialize, Serialize}; use std::path::PathBuf; +use util::paths::PathLikeWithPosition; #[derive(Serialize, Deserialize)] pub struct IpcHandshake { @@ -10,7 +11,11 @@ pub struct IpcHandshake { #[derive(Debug, Serialize, Deserialize)] pub enum CliRequest { - Open { paths: Vec, wait: bool }, + Open { + // TODO kb old cli won't be able to communicate to new Zed with this change + paths: Vec>, + wait: bool, + }, } #[derive(Debug, Serialize, Deserialize)] diff --git a/crates/cli/src/main.rs b/crates/cli/src/main.rs index 80ec2bbf99e644aaa87a08acc6fa983af3453603..ff7a65c2fcd4a6ee5916e8657c464476607f6692 100644 --- a/crates/cli/src/main.rs +++ b/crates/cli/src/main.rs @@ -21,7 +21,7 @@ use util::paths::PathLikeWithPosition; #[derive(Parser)] #[clap(name = "zed", global_setting(clap::AppSettings::NoAutoVersion))] struct Args { - /// Wait for all of the given paths to be closed before exiting. + /// Wait for all of the given paths to be opened/closed before exiting. #[clap(short, long)] wait: bool, /// A sequence of space-separated paths that you want to open. @@ -78,12 +78,13 @@ fn main() -> Result<()> { paths: args .paths_with_position .into_iter() - // TODO kb continue sendint path with the position further - .map(|path_with_position| path_with_position.path_like) - .map(|path| { - fs::canonicalize(&path).with_context(|| format!("path {path:?} canonicalization")) + .map(|path_with_position| { + path_with_position.convert_path(|path| { + fs::canonicalize(&path) + .with_context(|| format!("path {path:?} canonicalization")) + }) }) - .collect::>>()?, + .collect::>()?, wait: args.wait, })?; diff --git a/crates/file_finder/src/file_finder.rs b/crates/file_finder/src/file_finder.rs index fae9bd565cf4da958828e87716ebab83a6a293d9..063067891a902538e5b861642d6497054cf43ca0 100644 --- a/crates/file_finder/src/file_finder.rs +++ b/crates/file_finder/src/file_finder.rs @@ -295,8 +295,6 @@ impl PickerDelegate for FileFinderDelegate { let point = snapshot .buffer_snapshot .clip_point(Point::new(row, col), Bias::Left); - let point = - snapshot.buffer_snapshot.clip_point(point, Bias::Left); editor.change_selections(Some(Autoscroll::center()), cx, |s| { s.select_ranges([point..point]) }); diff --git a/crates/util/src/paths.rs b/crates/util/src/paths.rs index 280d3cad021507092021a8d8fe217f09ccd1058e..8a0c91aba6cce5ddff2e81359dbac478c4631307 100644 --- a/crates/util/src/paths.rs +++ b/crates/util/src/paths.rs @@ -1,5 +1,7 @@ use std::path::{Path, PathBuf}; +use serde::{Deserialize, Serialize}; + lazy_static::lazy_static! { pub static ref HOME: PathBuf = dirs::home_dir().expect("failed to determine home directory"); pub static ref CONFIG_DIR: PathBuf = HOME.join(".config").join("zed"); @@ -73,7 +75,7 @@ pub fn compact(path: &Path) -> PathBuf { pub const FILE_ROW_COLUMN_DELIMITER: char = ':'; -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Serialize, Deserialize)] pub struct PathLikeWithPosition

{ pub path_like: P, pub row: Option, @@ -106,4 +108,15 @@ impl

PathLikeWithPosition

{ }, }) } + + pub fn convert_path( + self, + mapping: impl FnOnce(P) -> Result, + ) -> Result, E> { + Ok(PathLikeWithPosition { + path_like: mapping(self.path_like)?, + row: self.row, + column: self.column, + }) + } } diff --git a/crates/zed/src/main.rs b/crates/zed/src/main.rs index 60a2fc66bee1ca9746854246ec165c19f59a2ee0..369a6575279c6c5b461f38e3e2a1e7e03124295c 100644 --- a/crates/zed/src/main.rs +++ b/crates/zed/src/main.rs @@ -10,7 +10,7 @@ use cli::{ }; use client::{self, UserStore, ZED_APP_VERSION, ZED_SECRET_CLIENT_TOKEN}; use db::kvp::KEY_VALUE_STORE; -use editor::Editor; +use editor::{scroll::autoscroll::Autoscroll, Editor}; use futures::{ channel::{mpsc, oneshot}, FutureExt, SinkExt, StreamExt, @@ -30,6 +30,7 @@ use settings::{ use simplelog::ConfigBuilder; use smol::process::Command; use std::{ + collections::HashMap, env, ffi::OsStr, fs::OpenOptions, @@ -44,7 +45,9 @@ use std::{ thread, time::Duration, }; +use sum_tree::Bias; use terminal_view::{get_working_directory, TerminalView}; +use text::Point; use util::http::{self, HttpClient}; use welcome::{show_welcome_experience, FIRST_OPEN}; @@ -678,13 +681,29 @@ async fn handle_cli_connection( if let Some(request) = requests.next().await { match request { CliRequest::Open { paths, wait } => { + let mut caret_positions = HashMap::new(); + let paths = if paths.is_empty() { workspace::last_opened_workspace_paths() .await .map(|location| location.paths().to_vec()) - .unwrap_or(paths) + .unwrap_or_default() } else { paths + .into_iter() + .map(|path_with_position| { + let path = path_with_position.path_like; + if let Some(row) = path_with_position.row { + if path.is_file() { + let row = row.saturating_sub(1); + let col = + path_with_position.column.unwrap_or(0).saturating_sub(1); + caret_positions.insert(path.clone(), Point::new(row, col)); + } + } + path + }) + .collect() }; let mut errored = false; @@ -694,11 +713,37 @@ async fn handle_cli_connection( { Ok((workspace, items)) => { let mut item_release_futures = Vec::new(); - cx.update(|cx| { - for (item, path) in items.into_iter().zip(&paths) { - match item { - Some(Ok(item)) => { - let released = oneshot::channel(); + + for (item, path) in items.into_iter().zip(&paths) { + match item { + Some(Ok(item)) => { + if let Some(point) = caret_positions.remove(path) { + // TODO kb does not work + log::info!("@@@@@@@@ {path:?}@{point:?}"); + if let Some(active_editor) = item.downcast::() { + log::info!("@@@@@@@@ editor"); + active_editor + .downgrade() + .update(&mut cx, |editor, cx| { + log::info!("@@@@@@@@ update"); + let snapshot = + editor.snapshot(cx).display_snapshot; + let point = snapshot + .buffer_snapshot + .clip_point(point, Bias::Left); + editor.change_selections( + Some(Autoscroll::center()), + cx, + |s| s.select_ranges([point..point]), + ); + log::info!("@@@@@@@@ finished"); + }) + .log_err(); + } + } + + let released = oneshot::channel(); + cx.update(|cx| { item.on_release( cx, Box::new(move |_| { @@ -706,23 +751,20 @@ async fn handle_cli_connection( }), ) .detach(); - item_release_futures.push(released.1); - } - Some(Err(err)) => { - responses - .send(CliResponse::Stderr { - message: format!( - "error opening {:?}: {}", - path, err - ), - }) - .log_err(); - errored = true; - } - None => {} + }); + item_release_futures.push(released.1); + } + Some(Err(err)) => { + responses + .send(CliResponse::Stderr { + message: format!("error opening {:?}: {}", path, err), + }) + .log_err(); + errored = true; } + None => {} } - }); + } if wait { let background = cx.background(); From 106064c73402c035b92028aecb4b0abf17e1e27c Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Sat, 13 May 2023 11:39:35 +0300 Subject: [PATCH 10/13] Do not break Zed & Zed CLI compatibility --- crates/cli/src/cli.rs | 13 ++++++------- crates/cli/src/main.rs | 5 +++-- crates/util/src/paths.rs | 16 ++++++++++++++++ crates/zed/src/main.rs | 20 ++++++++++++++++---- 4 files changed, 41 insertions(+), 13 deletions(-) diff --git a/crates/cli/src/cli.rs b/crates/cli/src/cli.rs index 8281bcb651aa1693cf362f7fb2635e1e09dfa399..3a0abbaec7240c1c13a636777493ed73df7914d0 100644 --- a/crates/cli/src/cli.rs +++ b/crates/cli/src/cli.rs @@ -1,7 +1,5 @@ pub use ipc_channel::ipc; use serde::{Deserialize, Serialize}; -use std::path::PathBuf; -use util::paths::PathLikeWithPosition; #[derive(Serialize, Deserialize)] pub struct IpcHandshake { @@ -11,11 +9,12 @@ pub struct IpcHandshake { #[derive(Debug, Serialize, Deserialize)] pub enum CliRequest { - Open { - // TODO kb old cli won't be able to communicate to new Zed with this change - paths: Vec>, - wait: bool, - }, + // The filed is named `path` for compatibility, but now CLI can request + // opening a path at a certain row and/or column: `some/path:123` and `some/path:123:456`. + // + // Since Zed CLI has to be installed separately, there can be situations when old CLI is + // querying new Zed editors, support both formats by using `String` here and parsing it on Zed side later. + Open { paths: Vec, wait: bool }, } #[derive(Debug, Serialize, Deserialize)] diff --git a/crates/cli/src/main.rs b/crates/cli/src/main.rs index ff7a65c2fcd4a6ee5916e8657c464476607f6692..d4b75f9533dbb604827834c90e8652e2aa101b98 100644 --- a/crates/cli/src/main.rs +++ b/crates/cli/src/main.rs @@ -79,10 +79,11 @@ fn main() -> Result<()> { .paths_with_position .into_iter() .map(|path_with_position| { - path_with_position.convert_path(|path| { + let path_with_position = path_with_position.convert_path(|path| { fs::canonicalize(&path) .with_context(|| format!("path {path:?} canonicalization")) - }) + })?; + Ok(path_with_position.to_string(|path| path.display().to_string())) }) .collect::>()?, wait: args.wait, diff --git a/crates/util/src/paths.rs b/crates/util/src/paths.rs index 8a0c91aba6cce5ddff2e81359dbac478c4631307..96311fabf87d98f818305a69138c3a6933221208 100644 --- a/crates/util/src/paths.rs +++ b/crates/util/src/paths.rs @@ -119,4 +119,20 @@ impl

PathLikeWithPosition

{ column: self.column, }) } + + pub fn to_string(&self, path_like_to_string: F) -> String + where + F: Fn(&P) -> String, + { + let path_like_string = path_like_to_string(&self.path_like); + if let Some(row) = self.row { + if let Some(column) = self.column { + format!("{path_like_string}:{row}:{column}") + } else { + format!("{path_like_string}:{row}") + } + } else { + path_like_string + } + } } diff --git a/crates/zed/src/main.rs b/crates/zed/src/main.rs index 369a6575279c6c5b461f38e3e2a1e7e03124295c..03fdbf70671e1db87403fde122b832a17f9d0b14 100644 --- a/crates/zed/src/main.rs +++ b/crates/zed/src/main.rs @@ -37,7 +37,7 @@ use std::{ io::Write as _, os::unix::prelude::OsStrExt, panic, - path::PathBuf, + path::{Path, PathBuf}, sync::{ atomic::{AtomicBool, Ordering}, Arc, Weak, @@ -48,7 +48,10 @@ use std::{ use sum_tree::Bias; use terminal_view::{get_working_directory, TerminalView}; use text::Point; -use util::http::{self, HttpClient}; +use util::{ + http::{self, HttpClient}, + paths::PathLikeWithPosition, +}; use welcome::{show_welcome_experience, FIRST_OPEN}; use fs::RealFs; @@ -691,7 +694,16 @@ async fn handle_cli_connection( } else { paths .into_iter() - .map(|path_with_position| { + .filter_map(|path_with_position_string| { + let path_with_position = PathLikeWithPosition::parse_str( + &path_with_position_string, + |path_str| { + Ok::<_, std::convert::Infallible>( + Path::new(path_str).to_path_buf(), + ) + }, + ) + .expect("Infallible"); let path = path_with_position.path_like; if let Some(row) = path_with_position.row { if path.is_file() { @@ -701,7 +713,7 @@ async fn handle_cli_connection( caret_positions.insert(path.clone(), Point::new(row, col)); } } - path + Some(path) }) .collect() }; From 0c6f1038996a42d217c2a5e5b911eb06a669744a Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 16 May 2023 00:18:31 +0300 Subject: [PATCH 11/13] Return proper items on workspace restoration. co-authored-by: Mikayla --- crates/workspace/src/dock.rs | 7 +- crates/workspace/src/persistence/model.rs | 29 +- crates/workspace/src/workspace.rs | 330 ++++++++++++++-------- crates/zed/src/main.rs | 4 + 4 files changed, 239 insertions(+), 131 deletions(-) diff --git a/crates/workspace/src/dock.rs b/crates/workspace/src/dock.rs index 7efcb7f9d31dae6c2dc665bfcd6243843e3f443e..a1fce13df452623c2bf4cf1a66887bf74e21d403 100644 --- a/crates/workspace/src/dock.rs +++ b/crates/workspace/src/dock.rs @@ -462,7 +462,6 @@ mod tests { let (_, _workspace) = cx.add_window(|cx| { Workspace::new( - Some(serialized_workspace), 0, project.clone(), Arc::new(AppState { @@ -480,6 +479,11 @@ mod tests { ) }); + cx.update(|cx| { + Workspace::load_workspace(_workspace.downgrade(), serialized_workspace, Vec::new(), cx) + }) + .await; + cx.foreground().run_until_parked(); //Should terminate } @@ -605,7 +609,6 @@ mod tests { let project = Project::test(fs, [], cx).await; let (window_id, workspace) = cx.add_window(|cx| { Workspace::new( - None, 0, project.clone(), Arc::new(AppState { diff --git a/crates/workspace/src/persistence/model.rs b/crates/workspace/src/persistence/model.rs index a92c369e7a2e47c19fbfbc831a90b6b60edcfcb1..46a8ab49b2406522cdf64abab365a9053755ce56 100644 --- a/crates/workspace/src/persistence/model.rs +++ b/crates/workspace/src/persistence/model.rs @@ -1,5 +1,6 @@ use crate::{ - dock::DockPosition, ItemDeserializers, Member, Pane, PaneAxis, Workspace, WorkspaceId, + dock::DockPosition, item::ItemHandle, ItemDeserializers, Member, Pane, PaneAxis, Workspace, + WorkspaceId, }; use anyhow::{anyhow, Context, Result}; use async_recursion::async_recursion; @@ -97,17 +98,23 @@ impl SerializedPaneGroup { workspace_id: WorkspaceId, workspace: &WeakViewHandle, cx: &mut AsyncAppContext, - ) -> Option<(Member, Option>)> { + ) -> Option<( + Member, + Option>, + Vec>>, + )> { match self { SerializedPaneGroup::Group { axis, children } => { let mut current_active_pane = None; let mut members = Vec::new(); + let mut items = Vec::new(); for child in children { - if let Some((new_member, active_pane)) = child + if let Some((new_member, active_pane, new_items)) = child .deserialize(project, workspace_id, workspace, cx) .await { members.push(new_member); + items.extend(new_items); current_active_pane = current_active_pane.or(active_pane); } } @@ -117,7 +124,7 @@ impl SerializedPaneGroup { } if members.len() == 1 { - return Some((members.remove(0), current_active_pane)); + return Some((members.remove(0), current_active_pane, items)); } Some(( @@ -126,6 +133,7 @@ impl SerializedPaneGroup { members, }), current_active_pane, + items, )) } SerializedPaneGroup::Pane(serialized_pane) => { @@ -133,7 +141,7 @@ impl SerializedPaneGroup { .update(cx, |workspace, cx| workspace.add_pane(cx).downgrade()) .log_err()?; let active = serialized_pane.active; - serialized_pane + let new_items = serialized_pane .deserialize_to(project, &pane, workspace_id, workspace, cx) .await .log_err()?; @@ -143,7 +151,7 @@ impl SerializedPaneGroup { .log_err()? { let pane = pane.upgrade(cx)?; - Some((Member::Pane(pane.clone()), active.then(|| pane))) + Some((Member::Pane(pane.clone()), active.then(|| pane), new_items)) } else { let pane = pane.upgrade(cx)?; workspace @@ -174,7 +182,8 @@ impl SerializedPane { workspace_id: WorkspaceId, workspace: &WeakViewHandle, cx: &mut AsyncAppContext, - ) -> Result<()> { + ) -> Result>>> { + let mut items = Vec::new(); let mut active_item_index = None; for (index, item) in self.children.iter().enumerate() { let project = project.clone(); @@ -192,6 +201,10 @@ impl SerializedPane { .await .log_err(); + items.push(item_handle.clone()); + + log::info!("ACTUALLY SHOWN ITEMS: {:?}", &item_handle); + if let Some(item_handle) = item_handle { workspace.update(cx, |workspace, cx| { let pane_handle = pane_handle @@ -213,7 +226,7 @@ impl SerializedPane { })?; } - anyhow::Ok(()) + anyhow::Ok(items) } } diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 6350b43415ea670796d62b4d3d79348aeab935ba..fe8ea656974d3e2ae09226d9737c4d65723fb591 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -82,7 +82,7 @@ use status_bar::StatusBar; pub use status_bar::StatusItemView; use theme::{Theme, ThemeRegistry}; pub use toolbar::{ToolbarItemLocation, ToolbarItemView}; -use util::{paths, ResultExt}; +use util::{async_iife, paths, ResultExt}; lazy_static! { static ref ZED_WINDOW_SIZE: Option = env::var("ZED_WINDOW_SIZE") @@ -493,7 +493,6 @@ struct FollowerState { impl Workspace { pub fn new( - serialized_workspace: Option, workspace_id: WorkspaceId, project: ModelHandle, app_state: Arc, @@ -659,16 +658,6 @@ impl Workspace { this.project_remote_id_changed(project.read(cx).remote_id(), cx); cx.defer(|this, cx| this.update_window_title(cx)); - if let Some(serialized_workspace) = serialized_workspace { - cx.defer(move |_, cx| { - Self::load_from_serialized_workspace(weak_handle, serialized_workspace, cx) - }); - } else if project.read(cx).is_local() { - if cx.global::().default_dock_anchor != DockAnchor::Expanded { - Dock::show(&mut this, false, cx); - } - } - this } @@ -690,8 +679,7 @@ impl Workspace { ); cx.spawn(|mut cx| async move { - let mut serialized_workspace = - persistence::DB.workspace_for_roots(&abs_paths.as_slice()); + let serialized_workspace = persistence::DB.workspace_for_roots(&abs_paths.as_slice()); let paths_to_open = serialized_workspace .as_ref() @@ -700,8 +688,9 @@ impl Workspace { // Get project paths for all of the abs_paths let mut worktree_roots: HashSet> = Default::default(); - let mut project_paths = Vec::new(); - for path in paths_to_open.iter() { + let mut project_paths: Vec<(PathBuf, Option)> = + Vec::with_capacity(paths_to_open.len()); + for path in paths_to_open.iter().cloned() { if let Some((worktree, project_entry)) = cx .update(|cx| { Workspace::project_path_for_path(project_handle.clone(), &path, true, cx) @@ -710,9 +699,9 @@ impl Workspace { .log_err() { worktree_roots.insert(worktree.read_with(&mut cx, |tree, _| tree.abs_path())); - project_paths.push(Some(project_entry)); + project_paths.push((path, Some(project_entry))); } else { - project_paths.push(None); + project_paths.push((path, None)); } } @@ -732,27 +721,17 @@ impl Workspace { )) }); - let build_workspace = - |cx: &mut ViewContext, - serialized_workspace: Option| { - let mut workspace = Workspace::new( - serialized_workspace, - workspace_id, - project_handle.clone(), - app_state.clone(), - cx, - ); - (app_state.initialize_workspace)(&mut workspace, &app_state, cx); - workspace - }; + let build_workspace = |cx: &mut ViewContext| { + let mut workspace = + Workspace::new(workspace_id, project_handle.clone(), app_state.clone(), cx); + (app_state.initialize_workspace)(&mut workspace, &app_state, cx); + + workspace + }; let workspace = requesting_window_id .and_then(|window_id| { - cx.update(|cx| { - cx.replace_root_view(window_id, |cx| { - build_workspace(cx, serialized_workspace.take()) - }) - }) + cx.update(|cx| cx.replace_root_view(window_id, |cx| build_workspace(cx))) }) .unwrap_or_else(|| { let (bounds, display) = if let Some(bounds) = window_bounds_override { @@ -790,44 +769,21 @@ impl Workspace { // Use the serialized workspace to construct the new window cx.add_window( (app_state.build_window_options)(bounds, display, cx.platform().as_ref()), - |cx| build_workspace(cx, serialized_workspace), + |cx| build_workspace(cx), ) .1 }); let workspace = workspace.downgrade(); notify_if_database_failed(&workspace, &mut cx); - - // Call open path for each of the project paths - // (this will bring them to the front if they were in the serialized workspace) - debug_assert!(paths_to_open.len() == project_paths.len()); - let tasks = paths_to_open - .iter() - .cloned() - .zip(project_paths.into_iter()) - .map(|(abs_path, project_path)| { - let workspace = workspace.clone(); - cx.spawn(|mut cx| { - let fs = app_state.fs.clone(); - async move { - let project_path = project_path?; - if fs.is_file(&abs_path).await { - Some( - workspace - .update(&mut cx, |workspace, cx| { - workspace.open_path(project_path, None, true, cx) - }) - .log_err()? - .await, - ) - } else { - None - } - } - }) - }); - - let opened_items = futures::future::join_all(tasks.into_iter()).await; + let opened_items = open_items( + serialized_workspace, + &workspace, + project_paths, + app_state, + cx, + ) + .await; (workspace, opened_items) }) @@ -2536,13 +2492,15 @@ impl Workspace { } } - fn load_from_serialized_workspace( + pub(crate) fn load_workspace( workspace: WeakViewHandle, serialized_workspace: SerializedWorkspace, + paths_to_open: Vec>, cx: &mut AppContext, - ) { + ) -> Task, anyhow::Error>>>> { cx.spawn(|mut cx| async move { - let (project, dock_pane_handle, old_center_pane) = + let result = async_iife! {{ + let (project, dock_pane_handle, old_center_pane) = workspace.read_with(&cx, |workspace, _| { ( workspace.project().clone(), @@ -2551,74 +2509,109 @@ impl Workspace { ) })?; - serialized_workspace - .dock_pane - .deserialize_to( + let dock_items = serialized_workspace + .dock_pane + .deserialize_to( &project, &dock_pane_handle, serialized_workspace.id, &workspace, &mut cx, - ) - .await?; + ) + .await?; - // Traverse the splits tree and add to things - let center_group = serialized_workspace + // Traverse the splits tree and add to things + let something = serialized_workspace .center_group .deserialize(&project, serialized_workspace.id, &workspace, &mut cx) .await; - // Remove old panes from workspace panes list - workspace.update(&mut cx, |workspace, cx| { - if let Some((center_group, active_pane)) = center_group { - workspace.remove_panes(workspace.center.root.clone(), cx); + let mut center_items = None; + let mut center_group = None; + if let Some((group, active_pane, items)) = something { + center_items = Some(items); + center_group = Some((group, active_pane)) + } - // Swap workspace center group - workspace.center = PaneGroup::with_root(center_group); + let resulting_list = cx.read(|cx| { + let mut opened_items = center_items + .unwrap_or_default() + .into_iter() + .chain(dock_items.into_iter()) + .filter_map(|item| { + let item = item?; + let project_path = item.project_path(cx)?; + Some((project_path, item)) + }) + .collect::>(); - // Change the focus to the workspace first so that we retrigger focus in on the pane. - cx.focus_self(); + paths_to_open + .into_iter() + .map(|path_to_open| { + path_to_open.map(|path_to_open| { + Ok(opened_items.remove(&path_to_open)) + }) + .transpose() + .map(|item| item.flatten()) + .transpose() + }) + .collect::>() + }); - if let Some(active_pane) = active_pane { - cx.focus(&active_pane); - } else { - cx.focus(workspace.panes.last().unwrap()); - } - } else { - let old_center_handle = old_center_pane.and_then(|weak| weak.upgrade(cx)); - if let Some(old_center_handle) = old_center_handle { - cx.focus(&old_center_handle) + // Remove old panes from workspace panes list + workspace.update(&mut cx, |workspace, cx| { + if let Some((center_group, active_pane)) = center_group { + workspace.remove_panes(workspace.center.root.clone(), cx); + + // Swap workspace center group + workspace.center = PaneGroup::with_root(center_group); + + // Change the focus to the workspace first so that we retrigger focus in on the pane. + cx.focus_self(); + + if let Some(active_pane) = active_pane { + cx.focus(&active_pane); + } else { + cx.focus(workspace.panes.last().unwrap()); + } } else { - cx.focus_self() + let old_center_handle = old_center_pane.and_then(|weak| weak.upgrade(cx)); + if let Some(old_center_handle) = old_center_handle { + cx.focus(&old_center_handle) + } else { + cx.focus_self() + } } - } - if workspace.left_sidebar().read(cx).is_open() + if workspace.left_sidebar().read(cx).is_open() != serialized_workspace.left_sidebar_open - { - workspace.toggle_sidebar(SidebarSide::Left, cx); - } + { + workspace.toggle_sidebar(SidebarSide::Left, cx); + } - // Note that without after_window, the focus_self() and - // the focus the dock generates start generating alternating - // focus due to the deferred execution each triggering each other - cx.after_window_update(move |workspace, cx| { - Dock::set_dock_position( - workspace, - serialized_workspace.dock_position, - false, - cx, - ); - }); + // Note that without after_window, the focus_self() and + // the focus the dock generates start generating alternating + // focus due to the deferred execution each triggering each other + cx.after_window_update(move |workspace, cx| { + Dock::set_dock_position( + workspace, + serialized_workspace.dock_position, + false, + cx, + ); + }); - cx.notify(); - })?; + cx.notify(); + })?; - // Serialize ourself to make sure our timestamps and any pane / item changes are replicated - workspace.read_with(&cx, |workspace, cx| workspace.serialize_workspace(cx))?; - anyhow::Ok(()) + // Serialize ourself to make sure our timestamps and any pane / item changes are replicated + workspace.read_with(&cx, |workspace, cx| workspace.serialize_workspace(cx))?; + + Ok::<_, anyhow::Error>(resulting_list) + }}; + + result.await.unwrap_or_default() }) - .detach_and_log_err(cx); } #[cfg(any(test, feature = "test-support"))] @@ -2634,8 +2627,104 @@ impl Workspace { dock_default_item_factory: |_, _| None, background_actions: || &[], }); - Self::new(None, 0, project, app_state, cx) + Self::new(0, project, app_state, cx) + } +} + +async fn open_items( + serialized_workspace: Option, + workspace: &WeakViewHandle, + mut project_paths_to_open: Vec<(PathBuf, Option)>, + app_state: Arc, + mut cx: AsyncAppContext, +) -> Vec>>> { + let mut opened_items = Vec::with_capacity(project_paths_to_open.len()); + + if let Some(serialized_workspace) = serialized_workspace { + // TODO kb + // If the user is opening a serialized workspace, force open the requested paths + // Requested items: (CLI args or whatever) + // Restored items: What came from the database + // Remaining items = Requested - restored + // For each remaining item, call workspace.open_path() (as below) + + let workspace = workspace.clone(); + let restored_items = cx + .update(|cx| { + Workspace::load_workspace( + workspace, + serialized_workspace, + project_paths_to_open + .iter() + .map(|(_, project_path)| project_path) + .cloned() + .collect(), + cx, + ) + }) + .await; + + let restored_project_paths = cx.read(|cx| { + restored_items + .iter() + .filter_map(|item| item.as_ref()?.as_ref().ok()?.project_path(cx)) + .collect::>() + }); + + opened_items = restored_items; + project_paths_to_open + .iter_mut() + .for_each(|(_, project_path)| { + if let Some(project_path_to_open) = project_path { + if restored_project_paths.contains(project_path_to_open) { + *project_path = None; + } + } + }); + } else { + for _ in 0..project_paths_to_open.len() { + opened_items.push(None); + } } + assert!(opened_items.len() == project_paths_to_open.len()); + + let tasks = + project_paths_to_open + .into_iter() + .enumerate() + .map(|(i, (abs_path, project_path))| { + let workspace = workspace.clone(); + cx.spawn(|mut cx| { + let fs = app_state.fs.clone(); + async move { + let file_project_path = project_path?; + if fs.is_file(&abs_path).await { + Some(( + i, + workspace + .update(&mut cx, |workspace, cx| { + workspace.open_path(file_project_path, None, true, cx) + }) + .log_err()? + .await, + )) + } else { + None + } + } + }) + }); + + for maybe_opened_path in futures::future::join_all(tasks.into_iter()) + .await + .into_iter() + { + if let Some((i, path_open_result)) = maybe_opened_path { + opened_items[i] = Some(path_open_result); + } + } + + opened_items } fn notify_if_database_failed(workspace: &WeakViewHandle, cx: &mut AsyncAppContext) { @@ -3008,8 +3097,7 @@ pub fn join_remote_project( let (_, workspace) = cx.add_window( (app_state.build_window_options)(None, None, cx.platform().as_ref()), |cx| { - let mut workspace = - Workspace::new(Default::default(), 0, project, app_state.clone(), cx); + let mut workspace = Workspace::new(0, project, app_state.clone(), cx); (app_state.initialize_workspace)(&mut workspace, &app_state, cx); workspace }, diff --git a/crates/zed/src/main.rs b/crates/zed/src/main.rs index 03fdbf70671e1db87403fde122b832a17f9d0b14..8a659fd0cc4bab35b71547ceac28d223b2d9f603 100644 --- a/crates/zed/src/main.rs +++ b/crates/zed/src/main.rs @@ -729,6 +729,10 @@ async fn handle_cli_connection( for (item, path) in items.into_iter().zip(&paths) { match item { Some(Ok(item)) => { + log::info!("UPDATED ITEMS: {:?}", item); + log::info!( + "caret_positions: {caret_positions:?}, path: {path:?}", + ); if let Some(point) = caret_positions.remove(path) { // TODO kb does not work log::info!("@@@@@@@@ {path:?}@{point:?}"); From be7a58b5082e78794c9f08197a37c4356d29184c Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 16 May 2023 17:37:23 +0300 Subject: [PATCH 12/13] Finalize the CLI opening part --- crates/workspace/src/persistence/model.rs | 2 - crates/workspace/src/workspace.rs | 46 +++++++++-------------- crates/zed/src/main.rs | 9 ----- 3 files changed, 17 insertions(+), 40 deletions(-) diff --git a/crates/workspace/src/persistence/model.rs b/crates/workspace/src/persistence/model.rs index 46a8ab49b2406522cdf64abab365a9053755ce56..dd81109d8c220c2876010be3c8b6917ef94b69ad 100644 --- a/crates/workspace/src/persistence/model.rs +++ b/crates/workspace/src/persistence/model.rs @@ -203,8 +203,6 @@ impl SerializedPane { items.push(item_handle.clone()); - log::info!("ACTUALLY SHOWN ITEMS: {:?}", &item_handle); - if let Some(item_handle) = item_handle { workspace.update(cx, |workspace, cx| { let pane_handle = pane_handle diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index fe8ea656974d3e2ae09226d9737c4d65723fb591..00093639e364ec1be33697095d9bb33335ee28d3 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -681,10 +681,7 @@ impl Workspace { cx.spawn(|mut cx| async move { let serialized_workspace = persistence::DB.workspace_for_roots(&abs_paths.as_slice()); - let paths_to_open = serialized_workspace - .as_ref() - .map(|workspace| workspace.location.paths()) - .unwrap_or(Arc::new(abs_paths)); + let paths_to_open = Arc::new(abs_paths); // Get project paths for all of the abs_paths let mut worktree_roots: HashSet> = Default::default(); @@ -1074,6 +1071,8 @@ impl Workspace { visible: bool, cx: &mut ViewContext, ) -> Task, anyhow::Error>>>> { + log::info!("open paths {:?}", abs_paths); + let fs = self.app_state.fs.clone(); // Sort the paths to ensure we add worktrees for parents before their children. @@ -2512,25 +2511,23 @@ impl Workspace { let dock_items = serialized_workspace .dock_pane .deserialize_to( - &project, - &dock_pane_handle, - serialized_workspace.id, - &workspace, - &mut cx, + &project, + &dock_pane_handle, + serialized_workspace.id, + &workspace, + &mut cx, ) .await?; - // Traverse the splits tree and add to things - let something = serialized_workspace - .center_group - .deserialize(&project, serialized_workspace.id, &workspace, &mut cx) - .await; - let mut center_items = None; let mut center_group = None; - if let Some((group, active_pane, items)) = something { - center_items = Some(items); - center_group = Some((group, active_pane)) + // Traverse the splits tree and add to things + if let Some((group, active_pane, items)) = serialized_workspace + .center_group + .deserialize(&project, serialized_workspace.id, &workspace, &mut cx) + .await { + center_items = Some(items); + center_group = Some((group, active_pane)) } let resulting_list = cx.read(|cx| { @@ -2584,7 +2581,7 @@ impl Workspace { } if workspace.left_sidebar().read(cx).is_open() - != serialized_workspace.left_sidebar_open + != serialized_workspace.left_sidebar_open { workspace.toggle_sidebar(SidebarSide::Left, cx); } @@ -2641,13 +2638,6 @@ async fn open_items( let mut opened_items = Vec::with_capacity(project_paths_to_open.len()); if let Some(serialized_workspace) = serialized_workspace { - // TODO kb - // If the user is opening a serialized workspace, force open the requested paths - // Requested items: (CLI args or whatever) - // Restored items: What came from the database - // Remaining items = Requested - restored - // For each remaining item, call workspace.open_path() (as below) - let workspace = workspace.clone(); let restored_items = cx .update(|cx| { @@ -2656,7 +2646,7 @@ async fn open_items( serialized_workspace, project_paths_to_open .iter() - .map(|(_, project_path)| project_path) + .map(|(_, project_path)| dbg!(project_path)) .cloned() .collect(), cx, @@ -2966,8 +2956,6 @@ pub fn open_paths( Vec, anyhow::Error>>>, )>, > { - log::info!("open paths {:?}", abs_paths); - let app_state = app_state.clone(); let abs_paths = abs_paths.to_vec(); cx.spawn(|mut cx| async move { diff --git a/crates/zed/src/main.rs b/crates/zed/src/main.rs index 8a659fd0cc4bab35b71547ceac28d223b2d9f603..1947095bf5598990304687d891e170d697bc951a 100644 --- a/crates/zed/src/main.rs +++ b/crates/zed/src/main.rs @@ -729,19 +729,11 @@ async fn handle_cli_connection( for (item, path) in items.into_iter().zip(&paths) { match item { Some(Ok(item)) => { - log::info!("UPDATED ITEMS: {:?}", item); - log::info!( - "caret_positions: {caret_positions:?}, path: {path:?}", - ); if let Some(point) = caret_positions.remove(path) { - // TODO kb does not work - log::info!("@@@@@@@@ {path:?}@{point:?}"); if let Some(active_editor) = item.downcast::() { - log::info!("@@@@@@@@ editor"); active_editor .downgrade() .update(&mut cx, |editor, cx| { - log::info!("@@@@@@@@ update"); let snapshot = editor.snapshot(cx).display_snapshot; let point = snapshot @@ -752,7 +744,6 @@ async fn handle_cli_connection( cx, |s| s.select_ranges([point..point]), ); - log::info!("@@@@@@@@ finished"); }) .log_err(); } From 5d4fc9975038a5be6bae3ed9be537ab54c2a6110 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 16 May 2023 20:48:19 +0300 Subject: [PATCH 13/13] Unit test file:row:column parsing --- crates/cli/src/main.rs | 2 +- crates/util/src/paths.rs | 197 +++++++++++++++++++++++++++++++++------ 2 files changed, 170 insertions(+), 29 deletions(-) diff --git a/crates/cli/src/main.rs b/crates/cli/src/main.rs index d4b75f9533dbb604827834c90e8652e2aa101b98..feebbff61b93043b88ed2eaa0603018eb191fd69 100644 --- a/crates/cli/src/main.rs +++ b/crates/cli/src/main.rs @@ -79,7 +79,7 @@ fn main() -> Result<()> { .paths_with_position .into_iter() .map(|path_with_position| { - let path_with_position = path_with_position.convert_path(|path| { + let path_with_position = path_with_position.map_path_like(|path| { fs::canonicalize(&path) .with_context(|| format!("path {path:?} canonicalization")) })?; diff --git a/crates/util/src/paths.rs b/crates/util/src/paths.rs index 96311fabf87d98f818305a69138c3a6933221208..f998fc319fcb7bcdfc6be70c66b482ec247a93b0 100644 --- a/crates/util/src/paths.rs +++ b/crates/util/src/paths.rs @@ -73,43 +73,80 @@ pub fn compact(path: &Path) -> PathBuf { } } +/// A delimiter to use in `path_query:row_number:column_number` strings parsing. pub const FILE_ROW_COLUMN_DELIMITER: char = ':'; -#[derive(Debug, Clone, Serialize, Deserialize)] +/// A representation of a path-like string with optional row and column numbers. +/// Matching values example: `te`, `test.rs:22`, `te:22:5`, etc. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct PathLikeWithPosition

{ pub path_like: P, pub row: Option, + // Absent if row is absent. pub column: Option, } impl

PathLikeWithPosition

{ - pub fn parse_str(s: &str, parse_path_like_str: F) -> Result - where - F: Fn(&str) -> Result, - { - let mut components = s.splitn(3, FILE_ROW_COLUMN_DELIMITER).map(str::trim).fuse(); - let path_like_str = components.next().filter(|str| !str.is_empty()); - let row = components.next().and_then(|row| row.parse::().ok()); - let column = components - .next() - .filter(|_| row.is_some()) - .and_then(|col| col.parse::().ok()); - - Ok(match path_like_str { - Some(path_like_str) => Self { - path_like: parse_path_like_str(path_like_str)?, - row, - column, - }, - None => Self { - path_like: parse_path_like_str(s)?, + /// Parses a string that possibly has `:row:column` suffix. + /// Ignores trailing `:`s, so `test.rs:22:` is parsed as `test.rs:22`. + /// If any of the row/column component parsing fails, the whole string is then parsed as a path like. + pub fn parse_str( + s: &str, + parse_path_like_str: impl Fn(&str) -> Result, + ) -> Result { + let fallback = |fallback_str| { + Ok(Self { + path_like: parse_path_like_str(fallback_str)?, row: None, column: None, - }, - }) + }) + }; + + match s.trim().split_once(FILE_ROW_COLUMN_DELIMITER) { + Some((path_like_str, maybe_row_and_col_str)) => { + let path_like_str = path_like_str.trim(); + let maybe_row_and_col_str = maybe_row_and_col_str.trim(); + if path_like_str.is_empty() { + fallback(s) + } else if maybe_row_and_col_str.is_empty() { + fallback(path_like_str) + } else { + let (row_parse_result, maybe_col_str) = + match maybe_row_and_col_str.split_once(FILE_ROW_COLUMN_DELIMITER) { + Some((maybe_row_str, maybe_col_str)) => { + (maybe_row_str.parse::(), maybe_col_str.trim()) + } + None => (maybe_row_and_col_str.parse::(), ""), + }; + + match row_parse_result { + Ok(row) => { + if maybe_col_str.is_empty() { + Ok(Self { + path_like: parse_path_like_str(path_like_str)?, + row: Some(row), + column: None, + }) + } else { + match maybe_col_str.parse::() { + Ok(col) => Ok(Self { + path_like: parse_path_like_str(path_like_str)?, + row: Some(row), + column: Some(col), + }), + Err(_) => fallback(s), + } + } + } + Err(_) => fallback(s), + } + } + } + None => fallback(s), + } } - pub fn convert_path( + pub fn map_path_like( self, mapping: impl FnOnce(P) -> Result, ) -> Result, E> { @@ -120,10 +157,7 @@ impl

PathLikeWithPosition

{ }) } - pub fn to_string(&self, path_like_to_string: F) -> String - where - F: Fn(&P) -> String, - { + pub fn to_string(&self, path_like_to_string: impl Fn(&P) -> String) -> String { let path_like_string = path_like_to_string(&self.path_like); if let Some(row) = self.row { if let Some(column) = self.column { @@ -136,3 +170,110 @@ impl

PathLikeWithPosition

{ } } } + +#[cfg(test)] +mod tests { + use super::*; + + type TestPath = PathLikeWithPosition; + + fn parse_str(s: &str) -> TestPath { + TestPath::parse_str(s, |s| Ok::<_, std::convert::Infallible>(s.to_string())) + .expect("infallible") + } + + #[test] + fn path_with_position_parsing_positive() { + let input_and_expected = [ + ( + "test_file.rs", + PathLikeWithPosition { + path_like: "test_file.rs".to_string(), + row: None, + column: None, + }, + ), + ( + "test_file.rs:1", + PathLikeWithPosition { + path_like: "test_file.rs".to_string(), + row: Some(1), + column: None, + }, + ), + ( + "test_file.rs:1:2", + PathLikeWithPosition { + path_like: "test_file.rs".to_string(), + row: Some(1), + column: Some(2), + }, + ), + ]; + + for (input, expected) in input_and_expected { + let actual = parse_str(input); + assert_eq!( + actual, expected, + "For positive case input str '{input}', got a parse mismatch" + ); + } + } + + #[test] + fn path_with_position_parsing_negative() { + for input in [ + "test_file.rs:a", + "test_file.rs:a:b", + "test_file.rs::", + "test_file.rs::1", + "test_file.rs:1::", + "test_file.rs::1:2", + "test_file.rs:1::2", + "test_file.rs:1:2:", + "test_file.rs:1:2:3", + ] { + let actual = parse_str(input); + assert_eq!( + actual, + PathLikeWithPosition { + path_like: input.to_string(), + row: None, + column: None, + }, + "For negative case input str '{input}', got a parse mismatch" + ); + } + } + + // Trim off trailing `:`s for otherwise valid input. + #[test] + fn path_with_position_parsing_special() { + let input_and_expected = [ + ( + "test_file.rs:", + PathLikeWithPosition { + path_like: "test_file.rs".to_string(), + row: None, + column: None, + }, + ), + ( + "test_file.rs:1:", + PathLikeWithPosition { + path_like: "test_file.rs".to_string(), + row: Some(1), + column: None, + }, + ), + ]; + + for (input, expected) in input_and_expected { + let actual = parse_str(input); + assert_eq!( + actual, expected, + "For special case input str '{input}', got a parse mismatch" + ); + } + } +}