From 07d269234f4872df5ef02b711396358df56fc63a Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Thu, 14 Jul 2022 11:49:10 +0200 Subject: [PATCH 1/4] Differentiate among tabs with the same name This commit introduces a new, optional `Item::tab_description` method that lets implementers define a description for the tab with a certain `detail`. When two or more tabs match the same description, we will increase the `detail` until tabs don't match anymore or increasing the `detail` doesn't disambiguate tabs any further. As soon as we find a valid `detail` that disambiguates tabs enough, we will pass it to `Item::tab_content`. In `Editor`, this is implemented by showing more and more of the path's suffix as `detail` is increased. --- crates/diagnostics/src/diagnostics.rs | 7 ++- crates/editor/src/editor.rs | 2 +- crates/editor/src/items.rs | 82 +++++++++++++++++++++++++-- crates/editor/src/multi_buffer.rs | 9 +-- crates/language/src/buffer.rs | 4 +- crates/project/src/worktree.rs | 5 +- crates/search/src/project_search.rs | 7 ++- crates/terminal/src/terminal.rs | 7 ++- crates/theme/src/theme.rs | 1 + crates/workspace/src/pane.rs | 43 +++++++++++++- crates/workspace/src/workspace.rs | 25 ++++++-- styles/src/styleTree/workspace.ts | 4 ++ 12 files changed, 171 insertions(+), 25 deletions(-) diff --git a/crates/diagnostics/src/diagnostics.rs b/crates/diagnostics/src/diagnostics.rs index ecc1b2df68..1e89ba0853 100644 --- a/crates/diagnostics/src/diagnostics.rs +++ b/crates/diagnostics/src/diagnostics.rs @@ -501,7 +501,12 @@ impl ProjectDiagnosticsEditor { } impl workspace::Item for ProjectDiagnosticsEditor { - fn tab_content(&self, style: &theme::Tab, cx: &AppContext) -> ElementBox { + fn tab_content( + &self, + _detail: Option, + style: &theme::Tab, + cx: &AppContext, + ) -> ElementBox { render_summary( &self.summary, &style.label.text, diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 6adefcf62a..b0373c0fcb 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -1073,7 +1073,7 @@ impl Editor { &self.buffer } - pub fn title(&self, cx: &AppContext) -> String { + pub fn title<'a>(&self, cx: &'a AppContext) -> Cow<'a, str> { self.buffer().read(cx).title(cx) } diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index 0e3aca1447..f703acdfdb 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -1,4 +1,6 @@ -use crate::{Anchor, Autoscroll, Editor, Event, ExcerptId, NavigationData, ToPoint as _}; +use crate::{ + Anchor, Autoscroll, Editor, Event, ExcerptId, MultiBuffer, NavigationData, ToPoint as _, +}; use anyhow::{anyhow, Result}; use futures::FutureExt; use gpui::{ @@ -10,7 +12,12 @@ use project::{File, Project, ProjectEntryId, ProjectPath}; use rpc::proto::{self, update_view}; use settings::Settings; use smallvec::SmallVec; -use std::{fmt::Write, path::PathBuf, time::Duration}; +use std::{ + borrow::Cow, + fmt::Write, + path::{Path, PathBuf}, + time::Duration, +}; use text::{Point, Selection}; use util::TryFutureExt; use workspace::{FollowableItem, Item, ItemHandle, ItemNavHistory, ProjectItem, StatusItemView}; @@ -292,9 +299,39 @@ impl Item for Editor { } } - fn tab_content(&self, style: &theme::Tab, cx: &AppContext) -> ElementBox { - let title = self.title(cx); - Label::new(title, style.label.clone()).boxed() + fn tab_description<'a>(&'a self, detail: usize, cx: &'a AppContext) -> Option> { + match path_for_buffer(&self.buffer, detail, true, cx)? { + Cow::Borrowed(path) => Some(path.to_string_lossy()), + Cow::Owned(path) => Some(path.to_string_lossy().to_string().into()), + } + } + + fn tab_content( + &self, + detail: Option, + style: &theme::Tab, + cx: &AppContext, + ) -> ElementBox { + Flex::row() + .with_child( + Label::new(self.title(cx).into(), style.label.clone()) + .aligned() + .boxed(), + ) + .with_children(detail.and_then(|detail| { + let path = path_for_buffer(&self.buffer, detail, false, cx)?; + Some( + Label::new( + path.to_string_lossy().into(), + style.description.text.clone(), + ) + .contained() + .with_style(style.description.container) + .aligned() + .boxed(), + ) + })) + .boxed() } fn project_path(&self, cx: &AppContext) -> Option { @@ -534,3 +571,38 @@ impl StatusItemView for CursorPosition { cx.notify(); } } + +fn path_for_buffer<'a>( + buffer: &ModelHandle, + mut depth: usize, + include_filename: bool, + cx: &'a AppContext, +) -> Option> { + let file = buffer.read(cx).as_singleton()?.read(cx).file()?; + + let mut path = file.path().as_ref(); + depth += 1; + while depth > 0 { + if let Some(parent) = path.parent() { + path = parent; + depth -= 1; + } else { + break; + } + } + + if depth > 0 { + let full_path = file.full_path(cx); + if include_filename { + Some(full_path.into()) + } else { + Some(full_path.parent().unwrap().to_path_buf().into()) + } + } else { + let mut path = file.path().strip_prefix(path).unwrap(); + if !include_filename { + path = path.parent().unwrap(); + } + Some(path.into()) + } +} diff --git a/crates/editor/src/multi_buffer.rs b/crates/editor/src/multi_buffer.rs index 5f069d223f..d5b85b0aee 100644 --- a/crates/editor/src/multi_buffer.rs +++ b/crates/editor/src/multi_buffer.rs @@ -14,6 +14,7 @@ use language::{ use settings::Settings; use smallvec::SmallVec; use std::{ + borrow::Cow, cell::{Ref, RefCell}, cmp, fmt, io, iter::{self, FromIterator}, @@ -1194,14 +1195,14 @@ impl MultiBuffer { .collect() } - pub fn title(&self, cx: &AppContext) -> String { - if let Some(title) = self.title.clone() { - return title; + pub fn title<'a>(&'a self, cx: &'a AppContext) -> Cow<'a, str> { + if let Some(title) = self.title.as_ref() { + return title.into(); } if let Some(buffer) = self.as_singleton() { if let Some(file) = buffer.read(cx).file() { - return file.file_name(cx).to_string_lossy().into(); + return file.file_name(cx).to_string_lossy(); } } diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index 0d56ac1979..ee24539287 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -20,7 +20,7 @@ use std::{ any::Any, cmp::{self, Ordering}, collections::{BTreeMap, HashMap}, - ffi::OsString, + ffi::OsStr, future::Future, iter::{self, Iterator, Peekable}, mem, @@ -185,7 +185,7 @@ pub trait File: Send + Sync { /// Returns the last component of this handle's absolute path. If this handle refers to the root /// of its worktree, then this method will return the name of the worktree itself. - fn file_name(&self, cx: &AppContext) -> OsString; + fn file_name<'a>(&'a self, cx: &'a AppContext) -> &'a OsStr; fn is_deleted(&self) -> bool; diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index dbd4b443a1..cc972b9bcd 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -1646,11 +1646,10 @@ impl language::File for File { /// Returns the last component of this handle's absolute path. If this handle refers to the root /// of its worktree, then this method will return the name of the worktree itself. - fn file_name(&self, cx: &AppContext) -> OsString { + fn file_name<'a>(&'a self, cx: &'a AppContext) -> &'a OsStr { self.path .file_name() - .map(|name| name.into()) - .unwrap_or_else(|| OsString::from(&self.worktree.read(cx).root_name)) + .unwrap_or_else(|| OsStr::new(&self.worktree.read(cx).root_name)) } fn is_deleted(&self) -> bool { diff --git a/crates/search/src/project_search.rs b/crates/search/src/project_search.rs index e1acc6a771..5098222ae0 100644 --- a/crates/search/src/project_search.rs +++ b/crates/search/src/project_search.rs @@ -220,7 +220,12 @@ impl Item for ProjectSearchView { .update(cx, |editor, cx| editor.deactivated(cx)); } - fn tab_content(&self, tab_theme: &theme::Tab, cx: &gpui::AppContext) -> ElementBox { + fn tab_content( + &self, + _detail: Option, + tab_theme: &theme::Tab, + cx: &gpui::AppContext, + ) -> ElementBox { let settings = cx.global::(); let search_theme = &settings.theme.search; Flex::row() diff --git a/crates/terminal/src/terminal.rs b/crates/terminal/src/terminal.rs index 12c092d6e6..71587ef135 100644 --- a/crates/terminal/src/terminal.rs +++ b/crates/terminal/src/terminal.rs @@ -324,7 +324,12 @@ impl View for Terminal { } impl Item for Terminal { - fn tab_content(&self, tab_theme: &theme::Tab, cx: &gpui::AppContext) -> ElementBox { + fn tab_content( + &self, + _detail: Option, + tab_theme: &theme::Tab, + cx: &gpui::AppContext, + ) -> ElementBox { let settings = cx.global::(); let search_theme = &settings.theme.search; //TODO properly integrate themes diff --git a/crates/theme/src/theme.rs b/crates/theme/src/theme.rs index 7936a9b6bb..2299bc3477 100644 --- a/crates/theme/src/theme.rs +++ b/crates/theme/src/theme.rs @@ -93,6 +93,7 @@ pub struct Tab { pub container: ContainerStyle, #[serde(flatten)] pub label: LabelStyle, + pub description: ContainedText, pub spacing: f32, pub icon_width: f32, pub icon_close: Color, diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 87b6ea7547..6862cc0028 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -840,8 +840,10 @@ impl Pane { } else { None }; + let mut row = Flex::row().scrollable::(1, autoscroll, cx); - for (ix, item) in self.items.iter().enumerate() { + for (ix, (item, detail)) in self.items.iter().zip(self.tab_details(cx)).enumerate() { + let detail = if detail == 0 { None } else { Some(detail) }; let is_active = ix == self.active_item_index; row.add_child({ @@ -850,7 +852,7 @@ impl Pane { } else { theme.workspace.tab.clone() }; - let title = item.tab_content(&tab_style, cx); + let title = item.tab_content(detail, &tab_style, cx); let mut style = if is_active { theme.workspace.active_tab.clone() @@ -971,6 +973,43 @@ impl Pane { row.boxed() }) } + + fn tab_details(&self, cx: &AppContext) -> Vec { + let mut tab_details = (0..self.items.len()).map(|_| 0).collect::>(); + + let mut tab_descriptions = HashMap::default(); + let mut done = false; + while !done { + done = true; + + // Store item indices by their tab description. + for (ix, (item, detail)) in self.items.iter().zip(&tab_details).enumerate() { + if let Some(description) = item.tab_description(*detail, cx) { + if *detail == 0 + || Some(&description) != item.tab_description(detail - 1, cx).as_ref() + { + tab_descriptions + .entry(description) + .or_insert(Vec::new()) + .push(ix); + } + } + } + + // If two or more items have the same tab description, increase their level + // of detail and try again. + for (_, item_ixs) in tab_descriptions.drain() { + if item_ixs.len() > 1 { + done = false; + for ix in item_ixs { + tab_details[ix] += 1; + } + } + } + } + + tab_details + } } impl Entity for Pane { diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index da62fe7e54..d72704da01 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -256,7 +256,11 @@ pub trait Item: View { fn navigate(&mut self, _: Box, _: &mut ViewContext) -> bool { false } - fn tab_content(&self, style: &theme::Tab, cx: &AppContext) -> ElementBox; + fn tab_description<'a>(&'a self, _: usize, _: &'a AppContext) -> Option> { + None + } + fn tab_content(&self, detail: Option, style: &theme::Tab, cx: &AppContext) + -> ElementBox; fn project_path(&self, cx: &AppContext) -> Option; fn project_entry_ids(&self, cx: &AppContext) -> SmallVec<[ProjectEntryId; 3]>; fn is_singleton(&self, cx: &AppContext) -> bool; @@ -409,7 +413,9 @@ impl FollowableItemHandle for ViewHandle { } pub trait ItemHandle: 'static + fmt::Debug { - fn tab_content(&self, style: &theme::Tab, cx: &AppContext) -> ElementBox; + fn tab_description<'a>(&self, detail: usize, cx: &'a AppContext) -> Option>; + fn tab_content(&self, detail: Option, style: &theme::Tab, cx: &AppContext) + -> ElementBox; fn project_path(&self, cx: &AppContext) -> Option; fn project_entry_ids(&self, cx: &AppContext) -> SmallVec<[ProjectEntryId; 3]>; fn is_singleton(&self, cx: &AppContext) -> bool; @@ -463,8 +469,17 @@ impl dyn ItemHandle { } impl ItemHandle for ViewHandle { - fn tab_content(&self, style: &theme::Tab, cx: &AppContext) -> ElementBox { - self.read(cx).tab_content(style, cx) + fn tab_description<'a>(&self, detail: usize, cx: &'a AppContext) -> Option> { + self.read(cx).tab_description(detail, cx) + } + + fn tab_content( + &self, + detail: Option, + style: &theme::Tab, + cx: &AppContext, + ) -> ElementBox { + self.read(cx).tab_content(detail, style, cx) } fn project_path(&self, cx: &AppContext) -> Option { @@ -3277,7 +3292,7 @@ mod tests { } impl Item for TestItem { - fn tab_content(&self, _: &theme::Tab, _: &AppContext) -> ElementBox { + fn tab_content(&self, _: Option, _: &theme::Tab, _: &AppContext) -> ElementBox { Empty::new().boxed() } diff --git a/styles/src/styleTree/workspace.ts b/styles/src/styleTree/workspace.ts index 36d47bed92..ef5b1fe69c 100644 --- a/styles/src/styleTree/workspace.ts +++ b/styles/src/styleTree/workspace.ts @@ -27,6 +27,10 @@ export default function workspace(theme: Theme) { left: 8, right: 8, }, + description: { + margin: { left: 6, top: 1 }, + ...text(theme, "sans", "muted", { size: "2xs" }) + } }; const activeTab = { From fd5cb02ea9a1eb3c6945a182752e650ba501c2bf Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Thu, 14 Jul 2022 15:12:16 +0200 Subject: [PATCH 2/4] Truncate description in tab title when it is too long --- crates/editor/src/editor.rs | 1 + crates/editor/src/items.rs | 8 +++++++- crates/search/src/project_search.rs | 4 +--- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index b0373c0fcb..72ba6d60af 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -35,6 +35,7 @@ use gpui::{ }; use highlight_matching_bracket::refresh_matching_bracket_highlights; use hover_popover::{hide_hover, HoverState}; +pub use items::MAX_TAB_TITLE_LEN; pub use language::{char_kind, CharKind}; use language::{ BracketPair, Buffer, CodeAction, CodeLabel, Completion, Diagnostic, DiagnosticSeverity, diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index f703acdfdb..8f215076bb 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -23,6 +23,7 @@ use util::TryFutureExt; use workspace::{FollowableItem, Item, ItemHandle, ItemNavHistory, ProjectItem, StatusItemView}; pub const FORMAT_TIMEOUT: Duration = Duration::from_secs(2); +pub const MAX_TAB_TITLE_LEN: usize = 24; impl FollowableItem for Editor { fn from_state_proto( @@ -320,9 +321,14 @@ impl Item for Editor { ) .with_children(detail.and_then(|detail| { let path = path_for_buffer(&self.buffer, detail, false, cx)?; + let description = path.to_string_lossy(); Some( Label::new( - path.to_string_lossy().into(), + if description.len() > MAX_TAB_TITLE_LEN { + description[..MAX_TAB_TITLE_LEN].to_string() + "…" + } else { + description.into() + }, style.description.text.clone(), ) .contained() diff --git a/crates/search/src/project_search.rs b/crates/search/src/project_search.rs index 5098222ae0..622b84633c 100644 --- a/crates/search/src/project_search.rs +++ b/crates/search/src/project_search.rs @@ -4,7 +4,7 @@ use crate::{ ToggleWholeWord, }; use collections::HashMap; -use editor::{Anchor, Autoscroll, Editor, MultiBuffer, SelectAll}; +use editor::{Anchor, Autoscroll, Editor, MultiBuffer, SelectAll, MAX_TAB_TITLE_LEN}; use gpui::{ actions, elements::*, platform::CursorStyle, Action, AppContext, ElementBox, Entity, ModelContext, ModelHandle, MutableAppContext, RenderContext, Subscription, Task, View, @@ -26,8 +26,6 @@ use workspace::{ actions!(project_search, [Deploy, SearchInNew, ToggleFocus]); -const MAX_TAB_TITLE_LEN: usize = 24; - #[derive(Default)] struct ActiveSearches(HashMap, WeakViewHandle>); From 49ef33090cc7c42e055b1b7b52bf53fb40171fd5 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Thu, 14 Jul 2022 16:42:30 +0200 Subject: [PATCH 3/4] Add test for tab disambiguation --- crates/workspace/src/workspace.rs | 67 ++++++++++++++++++++++++++++++- 1 file changed, 66 insertions(+), 1 deletion(-) diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index d72704da01..3aa85c4a95 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -2701,11 +2701,62 @@ fn open_new(app_state: &Arc, cx: &mut MutableAppContext) { #[cfg(test)] mod tests { + use std::cell::Cell; + use super::*; use gpui::{executor::Deterministic, ModelHandle, TestAppContext, ViewContext}; use project::{FakeFs, Project, ProjectEntryId}; use serde_json::json; + #[gpui::test] + async fn test_tab_disambiguation(cx: &mut TestAppContext) { + cx.foreground().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.clone(), cx)); + + // Adding an item with no ambiguity renders the tab without detail. + let item1 = cx.add_view(window_id, |_| { + let mut item = TestItem::new(); + item.tab_descriptions = Some(vec!["c", "b1/c", "a/b1/c"]); + item + }); + workspace.update(cx, |workspace, cx| { + workspace.add_item(Box::new(item1.clone()), cx); + }); + item1.read_with(cx, |item, _| assert_eq!(item.tab_detail.get(), None)); + + // Adding an item that creates ambiguity increases the level of detail on + // both tabs. + let item2 = cx.add_view(window_id, |_| { + let mut item = TestItem::new(); + item.tab_descriptions = Some(vec!["c", "b2/c", "a/b2/c"]); + item + }); + workspace.update(cx, |workspace, cx| { + workspace.add_item(Box::new(item2.clone()), cx); + }); + item1.read_with(cx, |item, _| assert_eq!(item.tab_detail.get(), Some(1))); + item2.read_with(cx, |item, _| assert_eq!(item.tab_detail.get(), Some(1))); + + // Adding an item that creates ambiguity increases the level of detail only + // on the ambiguous tabs. In this case, the ambiguity can't be resolved so + // we stop at the highest detail available. + let item3 = cx.add_view(window_id, |_| { + let mut item = TestItem::new(); + item.tab_descriptions = Some(vec!["c", "b2/c", "a/b2/c"]); + item + }); + workspace.update(cx, |workspace, cx| { + workspace.add_item(Box::new(item3.clone()), cx); + }); + item1.read_with(cx, |item, _| assert_eq!(item.tab_detail.get(), Some(1))); + item2.read_with(cx, |item, _| assert_eq!(item.tab_detail.get(), Some(3))); + item3.read_with(cx, |item, _| assert_eq!(item.tab_detail.get(), Some(3))); + } + #[gpui::test] async fn test_tracking_active_path(cx: &mut TestAppContext) { cx.foreground().forbid_parking(); @@ -3226,6 +3277,8 @@ mod tests { project_entry_ids: Vec, project_path: Option, nav_history: Option, + tab_descriptions: Option>, + tab_detail: Cell>, } enum TestItemEvent { @@ -3245,6 +3298,8 @@ mod tests { project_entry_ids: self.project_entry_ids.clone(), project_path: self.project_path.clone(), nav_history: None, + tab_descriptions: None, + tab_detail: Default::default(), } } } @@ -3262,6 +3317,8 @@ mod tests { project_path: None, is_singleton: true, nav_history: None, + tab_descriptions: None, + tab_detail: Default::default(), } } @@ -3292,7 +3349,15 @@ mod tests { } impl Item for TestItem { - fn tab_content(&self, _: Option, _: &theme::Tab, _: &AppContext) -> ElementBox { + fn tab_description<'a>(&'a self, detail: usize, _: &'a AppContext) -> Option> { + self.tab_descriptions.as_ref().and_then(|descriptions| { + let description = *descriptions.get(detail).or(descriptions.last())?; + Some(description.into()) + }) + } + + fn tab_content(&self, detail: Option, _: &theme::Tab, _: &AppContext) -> ElementBox { + self.tab_detail.set(detail); Empty::new().boxed() } From d4ee372eabe670f1c7d94b3241cec45ea8a5395f Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Thu, 14 Jul 2022 16:46:45 +0200 Subject: [PATCH 4/4] :art: --- crates/editor/src/items.rs | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index 8f215076bb..5bb8d4d0b2 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -580,24 +580,28 @@ impl StatusItemView for CursorPosition { fn path_for_buffer<'a>( buffer: &ModelHandle, - mut depth: usize, + mut height: usize, include_filename: bool, cx: &'a AppContext, ) -> Option> { let file = buffer.read(cx).as_singleton()?.read(cx).file()?; + // Ensure we always render at least the filename. + height += 1; - let mut path = file.path().as_ref(); - depth += 1; - while depth > 0 { - if let Some(parent) = path.parent() { - path = parent; - depth -= 1; + let mut prefix = file.path().as_ref(); + while height > 0 { + if let Some(parent) = prefix.parent() { + prefix = parent; + height -= 1; } else { break; } } - if depth > 0 { + // Here we could have just always used `full_path`, but that is very + // allocation-heavy and so we try to use a `Cow` if we haven't + // traversed all the way up to the worktree's root. + if height > 0 { let full_path = file.full_path(cx); if include_filename { Some(full_path.into()) @@ -605,7 +609,7 @@ fn path_for_buffer<'a>( Some(full_path.parent().unwrap().to_path_buf().into()) } } else { - let mut path = file.path().strip_prefix(path).unwrap(); + let mut path = file.path().strip_prefix(prefix).unwrap(); if !include_filename { path = path.parent().unwrap(); }