Merge pull request #1356 from zed-industries/differentiate-same-tab-titles

Antonio Scandurra created

Differentiate among tabs with the same name

Change summary

crates/diagnostics/src/diagnostics.rs |  7 +
crates/editor/src/editor.rs           |  3 
crates/editor/src/items.rs            | 92 +++++++++++++++++++++++++++-
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   | 11 ++-
crates/terminal/src/terminal.rs       |  7 +
crates/theme/src/theme.rs             |  1 
crates/workspace/src/pane.rs          | 43 ++++++++++++
crates/workspace/src/workspace.rs     | 90 ++++++++++++++++++++++++++-
styles/src/styleTree/workspace.ts     |  4 +
12 files changed, 248 insertions(+), 28 deletions(-)

Detailed changes

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<usize>,
+        style: &theme::Tab,
+        cx: &AppContext,
+    ) -> ElementBox {
         render_summary(
             &self.summary,
             &style.label.text,

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,
@@ -1073,7 +1074,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)
     }
 

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,12 +12,18 @@ 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};
 
 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(
@@ -292,9 +300,44 @@ 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<Cow<'a, str>> {
+        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<usize>,
+        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)?;
+                let description = path.to_string_lossy();
+                Some(
+                    Label::new(
+                        if description.len() > MAX_TAB_TITLE_LEN {
+                            description[..MAX_TAB_TITLE_LEN].to_string() + "…"
+                        } else {
+                            description.into()
+                        },
+                        style.description.text.clone(),
+                    )
+                    .contained()
+                    .with_style(style.description.container)
+                    .aligned()
+                    .boxed(),
+                )
+            }))
+            .boxed()
     }
 
     fn project_path(&self, cx: &AppContext) -> Option<ProjectPath> {
@@ -534,3 +577,42 @@ impl StatusItemView for CursorPosition {
         cx.notify();
     }
 }
+
+fn path_for_buffer<'a>(
+    buffer: &ModelHandle<MultiBuffer>,
+    mut height: usize,
+    include_filename: bool,
+    cx: &'a AppContext,
+) -> Option<Cow<'a, Path>> {
+    let file = buffer.read(cx).as_singleton()?.read(cx).file()?;
+    // Ensure we always render at least the filename.
+    height += 1;
+
+    let mut prefix = file.path().as_ref();
+    while height > 0 {
+        if let Some(parent) = prefix.parent() {
+            prefix = parent;
+            height -= 1;
+        } else {
+            break;
+        }
+    }
+
+    // Here we could have just always used `full_path`, but that is very
+    // allocation-heavy and so we try to use a `Cow<Path>` 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())
+        } else {
+            Some(full_path.parent().unwrap().to_path_buf().into())
+        }
+    } else {
+        let mut path = file.path().strip_prefix(prefix).unwrap();
+        if !include_filename {
+            path = path.parent().unwrap();
+        }
+        Some(path.into())
+    }
+}

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();
             }
         }
 

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;
 

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 {

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<WeakModelHandle<Project>, WeakViewHandle<ProjectSearchView>>);
 
@@ -220,7 +218,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<usize>,
+        tab_theme: &theme::Tab,
+        cx: &gpui::AppContext,
+    ) -> ElementBox {
         let settings = cx.global::<Settings>();
         let search_theme = &settings.theme.search;
         Flex::row()

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<usize>,
+        tab_theme: &theme::Tab,
+        cx: &gpui::AppContext,
+    ) -> ElementBox {
         let settings = cx.global::<Settings>();
         let search_theme = &settings.theme.search; //TODO properly integrate themes
 

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,

crates/workspace/src/pane.rs 🔗

@@ -863,8 +863,10 @@ impl Pane {
             } else {
                 None
             };
+
             let mut row = Flex::row().scrollable::<Tabs, _>(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({
@@ -873,7 +875,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()
@@ -994,6 +996,43 @@ impl Pane {
             row.boxed()
         })
     }
+
+    fn tab_details(&self, cx: &AppContext) -> Vec<usize> {
+        let mut tab_details = (0..self.items.len()).map(|_| 0).collect::<Vec<_>>();
+
+        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 {

crates/workspace/src/workspace.rs 🔗

@@ -256,7 +256,11 @@ pub trait Item: View {
     fn navigate(&mut self, _: Box<dyn Any>, _: &mut ViewContext<Self>) -> bool {
         false
     }
-    fn tab_content(&self, style: &theme::Tab, cx: &AppContext) -> ElementBox;
+    fn tab_description<'a>(&'a self, _: usize, _: &'a AppContext) -> Option<Cow<'a, str>> {
+        None
+    }
+    fn tab_content(&self, detail: Option<usize>, style: &theme::Tab, cx: &AppContext)
+        -> ElementBox;
     fn project_path(&self, cx: &AppContext) -> Option<ProjectPath>;
     fn project_entry_ids(&self, cx: &AppContext) -> SmallVec<[ProjectEntryId; 3]>;
     fn is_singleton(&self, cx: &AppContext) -> bool;
@@ -409,7 +413,9 @@ impl<T: FollowableItem> FollowableItemHandle for ViewHandle<T> {
 }
 
 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<Cow<'a, str>>;
+    fn tab_content(&self, detail: Option<usize>, style: &theme::Tab, cx: &AppContext)
+        -> ElementBox;
     fn project_path(&self, cx: &AppContext) -> Option<ProjectPath>;
     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<T: Item> ItemHandle for ViewHandle<T> {
-    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<Cow<'a, str>> {
+        self.read(cx).tab_description(detail, cx)
+    }
+
+    fn tab_content(
+        &self,
+        detail: Option<usize>,
+        style: &theme::Tab,
+        cx: &AppContext,
+    ) -> ElementBox {
+        self.read(cx).tab_content(detail, style, cx)
     }
 
     fn project_path(&self, cx: &AppContext) -> Option<ProjectPath> {
@@ -2686,11 +2701,62 @@ fn open_new(app_state: &Arc<AppState>, 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();
@@ -3211,6 +3277,8 @@ mod tests {
         project_entry_ids: Vec<ProjectEntryId>,
         project_path: Option<ProjectPath>,
         nav_history: Option<ItemNavHistory>,
+        tab_descriptions: Option<Vec<&'static str>>,
+        tab_detail: Cell<Option<usize>>,
     }
 
     enum TestItemEvent {
@@ -3230,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(),
             }
         }
     }
@@ -3247,6 +3317,8 @@ mod tests {
                 project_path: None,
                 is_singleton: true,
                 nav_history: None,
+                tab_descriptions: None,
+                tab_detail: Default::default(),
             }
         }
 
@@ -3277,7 +3349,15 @@ mod tests {
     }
 
     impl Item for TestItem {
-        fn tab_content(&self, _: &theme::Tab, _: &AppContext) -> ElementBox {
+        fn tab_description<'a>(&'a self, detail: usize, _: &'a AppContext) -> Option<Cow<'a, str>> {
+            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<usize>, _: &theme::Tab, _: &AppContext) -> ElementBox {
+            self.tab_detail.set(detail);
             Empty::new().boxed()
         }
 

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 = {