From 9c02239afaed12a3e32e61f9be736a168893d07d Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Thu, 16 May 2024 09:14:08 -0600 Subject: [PATCH] chat: Only autocomplete active people (#11892) Release Notes: - chat: Updated name autocompletion to only consider active users --------- Co-authored-by: Marshall Bowers --- Cargo.lock | 1 - crates/client/src/user.rs | 11 ++ crates/collab_ui/Cargo.toml | 1 - crates/collab_ui/src/chat_panel.rs | 23 +-- .../src/chat_panel/message_editor.rs | 186 ++++-------------- 5 files changed, 54 insertions(+), 168 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index fb3e154d28..fc83b03dd9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2412,7 +2412,6 @@ dependencies = [ "call", "channel", "client", - "clock", "collections", "db", "dev_server_projects", diff --git a/crates/client/src/user.rs b/crates/client/src/user.rs index aa28a1cf8e..91fa16f157 100644 --- a/crates/client/src/user.rs +++ b/crates/client/src/user.rs @@ -89,6 +89,7 @@ pub enum ContactRequestStatus { pub struct UserStore { users: HashMap>, + by_github_login: HashMap, participant_indices: HashMap, update_contacts_tx: mpsc::UnboundedSender, current_user: watch::Receiver>>, @@ -144,6 +145,7 @@ impl UserStore { ]; Self { users: Default::default(), + by_github_login: Default::default(), current_user: current_user_rx, contacts: Default::default(), incoming_contact_requests: Default::default(), @@ -231,6 +233,7 @@ impl UserStore { #[cfg(feature = "test-support")] pub fn clear_cache(&mut self) { self.users.clear(); + self.by_github_login.clear(); } async fn handle_update_invite_info( @@ -644,6 +647,12 @@ impl UserStore { }) } + pub fn cached_user_by_github_login(&self, github_login: &str) -> Option> { + self.by_github_login + .get(github_login) + .and_then(|id| self.users.get(id).cloned()) + } + pub fn current_user(&self) -> Option> { self.current_user.borrow().clone() } @@ -670,6 +679,8 @@ impl UserStore { this.update(&mut cx, |this, _| { for user in &users { this.users.insert(user.id, user.clone()); + this.by_github_login + .insert(user.github_login.clone(), user.id); } }) .ok(); diff --git a/crates/collab_ui/Cargo.toml b/crates/collab_ui/Cargo.toml index a549dfb843..01da2ac15b 100644 --- a/crates/collab_ui/Cargo.toml +++ b/crates/collab_ui/Cargo.toml @@ -34,7 +34,6 @@ auto_update.workspace = true call.workspace = true channel.workspace = true client.workspace = true -clock.workspace = true collections.workspace = true db.workspace = true editor.workspace = true diff --git a/crates/collab_ui/src/chat_panel.rs b/crates/collab_ui/src/chat_panel.rs index 919e296f16..2b05b9390d 100644 --- a/crates/collab_ui/src/chat_panel.rs +++ b/crates/collab_ui/src/chat_panel.rs @@ -78,12 +78,14 @@ impl ChatPanel { let fs = workspace.app_state().fs.clone(); let client = workspace.app_state().client.clone(); let channel_store = ChannelStore::global(cx); + let user_store = workspace.app_state().user_store.clone(); let languages = workspace.app_state().languages.clone(); let input_editor = cx.new_view(|cx| { MessageEditor::new( languages.clone(), - channel_store.clone(), + user_store.clone(), + None, cx.new_view(|cx| Editor::auto_height(4, cx)), cx, ) @@ -231,19 +233,12 @@ impl ChatPanel { fn set_active_chat(&mut self, chat: Model, cx: &mut ViewContext) { if self.active_chat.as_ref().map(|e| &e.0) != Some(&chat) { - let channel_id = chat.read(cx).channel_id; - { - self.markdown_data.clear(); - - let chat = chat.read(cx); - let channel_name = chat.channel(cx).map(|channel| channel.name.clone()); - let message_count = chat.message_count(); - self.message_list.reset(message_count); - self.message_editor.update(cx, |editor, cx| { - editor.set_channel(channel_id, channel_name, cx); - editor.clear_reply_to_message_id(); - }); - }; + self.markdown_data.clear(); + self.message_list.reset(chat.read(cx).message_count()); + self.message_editor.update(cx, |editor, cx| { + editor.set_channel_chat(chat.clone(), cx); + editor.clear_reply_to_message_id(); + }); let subscription = cx.subscribe(&chat, Self::channel_did_change); self.active_chat = Some((chat, subscription)); self.acknowledge_last_message(cx); diff --git a/crates/collab_ui/src/chat_panel/message_editor.rs b/crates/collab_ui/src/chat_panel/message_editor.rs index 8126c9c88c..35439a4108 100644 --- a/crates/collab_ui/src/chat_panel/message_editor.rs +++ b/crates/collab_ui/src/chat_panel/message_editor.rs @@ -1,12 +1,12 @@ use anyhow::Result; -use channel::{ChannelMembership, ChannelStore, MessageParams}; -use client::{ChannelId, UserId}; -use collections::{HashMap, HashSet}; +use channel::{ChannelChat, ChannelStore, MessageParams}; +use client::{UserId, UserStore}; +use collections::HashSet; use editor::{AnchorRangeExt, CompletionProvider, Editor, EditorElement, EditorStyle}; use fuzzy::{StringMatch, StringMatchCandidate}; use gpui::{ AsyncWindowContext, FocusableView, FontStyle, FontWeight, HighlightStyle, IntoElement, Model, - Render, SharedString, Task, TextStyle, View, ViewContext, WeakView, WhiteSpace, + Render, Task, TextStyle, View, ViewContext, WeakView, WhiteSpace, }; use language::{ language_settings::SoftWrap, Anchor, Buffer, BufferSnapshot, CodeLabel, LanguageRegistry, @@ -31,11 +31,10 @@ lazy_static! { pub struct MessageEditor { pub editor: View, - channel_store: Model, - channel_members: HashMap, + user_store: Model, + channel_chat: Option>, mentions: Vec, mentions_task: Option>, - channel_id: Option, reply_to_message_id: Option, edit_message_id: Option, } @@ -81,7 +80,8 @@ impl CompletionProvider for MessageEditorCompletionProvider { impl MessageEditor { pub fn new( language_registry: Arc, - channel_store: Model, + user_store: Model, + channel_chat: Option>, editor: View, cx: &mut ViewContext, ) -> Self { @@ -127,9 +127,8 @@ impl MessageEditor { Self { editor, - channel_store, - channel_members: HashMap::default(), - channel_id: None, + user_store, + channel_chat, mentions: Vec::new(), mentions_task: None, reply_to_message_id: None, @@ -161,12 +160,13 @@ impl MessageEditor { self.edit_message_id = None; } - pub fn set_channel( - &mut self, - channel_id: ChannelId, - channel_name: Option, - cx: &mut ViewContext, - ) { + pub fn set_channel_chat(&mut self, chat: Model, cx: &mut ViewContext) { + let channel_id = chat.read(cx).channel_id; + self.channel_chat = Some(chat); + let channel_name = ChannelStore::global(cx) + .read(cx) + .channel_for_id(channel_id) + .map(|channel| channel.name.clone()); self.editor.update(cx, |editor, cx| { if let Some(channel_name) = channel_name { editor.set_placeholder_text(format!("Message #{channel_name}"), cx); @@ -174,31 +174,6 @@ impl MessageEditor { editor.set_placeholder_text("Message Channel", cx); } }); - self.channel_id = Some(channel_id); - self.refresh_users(cx); - } - - pub fn refresh_users(&mut self, cx: &mut ViewContext) { - if let Some(channel_id) = self.channel_id { - let members = self.channel_store.update(cx, |store, cx| { - store.get_channel_member_details(channel_id, cx) - }); - cx.spawn(|this, mut cx| async move { - let members = members.await?; - this.update(&mut cx, |this, cx| this.set_members(members, cx))?; - anyhow::Ok(()) - }) - .detach_and_log_err(cx); - } - } - - pub fn set_members(&mut self, members: Vec, _: &mut ViewContext) { - self.channel_members.clear(); - self.channel_members.extend( - members - .into_iter() - .map(|member| (member.user.github_login.clone(), member.user.id)), - ); } pub fn take_message(&mut self, cx: &mut ViewContext) -> MessageParams { @@ -368,13 +343,19 @@ impl MessageEditor { let start_anchor = buffer.read(cx).anchor_before(start_offset); let mut names = HashSet::default(); - for (github_login, _) in self.channel_members.iter() { - names.insert(github_login.clone()); - } - if let Some(channel_id) = self.channel_id { - for participant in self.channel_store.read(cx).channel_participants(channel_id) { + if let Some(chat) = self.channel_chat.as_ref() { + let chat = chat.read(cx); + for participant in ChannelStore::global(cx) + .read(cx) + .channel_participants(chat.channel_id) + { names.insert(participant.github_login.clone()); } + for message in chat + .messages_in_range(chat.message_count().saturating_sub(100)..chat.message_count()) + { + names.insert(message.sender.github_login.clone()); + } } let candidates = names @@ -481,11 +462,15 @@ impl MessageEditor { text.clear(); text.extend(buffer.text_for_range(range.clone())); if let Some(username) = text.strip_prefix('@') { - if let Some(user_id) = this.channel_members.get(username) { + if let Some(user) = this + .user_store + .read(cx) + .cached_user_by_github_login(username) + { let start = multi_buffer.anchor_after(range.start); let end = multi_buffer.anchor_after(range.end); - mentioned_user_ids.push(*user_id); + mentioned_user_ids.push(user.id); anchor_ranges.push(start..end); } } @@ -550,106 +535,3 @@ impl Render for MessageEditor { )) } } - -#[cfg(test)] -mod tests { - use super::*; - use client::{Client, User, UserStore}; - use clock::FakeSystemClock; - use gpui::TestAppContext; - use http::FakeHttpClient; - use language::{Language, LanguageConfig}; - use project::Project; - use rpc::proto; - use settings::SettingsStore; - use util::test::marked_text_ranges; - - #[gpui::test] - async fn test_message_editor(cx: &mut TestAppContext) { - let language_registry = init_test(cx); - - let (editor, cx) = cx.add_window_view(|cx| { - MessageEditor::new( - language_registry, - ChannelStore::global(cx), - cx.new_view(|cx| Editor::auto_height(4, cx)), - cx, - ) - }); - cx.executor().run_until_parked(); - - editor.update(cx, |editor, cx| { - editor.set_members( - vec![ - ChannelMembership { - user: Arc::new(User { - github_login: "a-b".into(), - id: 101, - avatar_uri: "avatar_a-b".into(), - }), - kind: proto::channel_member::Kind::Member, - role: proto::ChannelRole::Member, - }, - ChannelMembership { - user: Arc::new(User { - github_login: "C_D".into(), - id: 102, - avatar_uri: "avatar_C_D".into(), - }), - kind: proto::channel_member::Kind::Member, - role: proto::ChannelRole::Member, - }, - ], - cx, - ); - - editor.editor.update(cx, |editor, cx| { - editor.set_text("Hello, @a-b! Have you met @C_D?", cx) - }); - }); - - cx.executor().advance_clock(MENTIONS_DEBOUNCE_INTERVAL); - - editor.update(cx, |editor, cx| { - let (text, ranges) = marked_text_ranges("Hello, «@a-b»! Have you met «@C_D»?", false); - assert_eq!( - editor.take_message(cx), - MessageParams { - text, - mentions: vec![(ranges[0].clone(), 101), (ranges[1].clone(), 102)], - reply_to_message_id: None - } - ); - }); - } - - fn init_test(cx: &mut TestAppContext) -> Arc { - cx.update(|cx| { - let settings = SettingsStore::test(cx); - cx.set_global(settings); - - let clock = Arc::new(FakeSystemClock::default()); - let http = FakeHttpClient::with_404_response(); - let client = Client::new(clock, http.clone(), cx); - let user_store = cx.new_model(|cx| UserStore::new(client.clone(), cx)); - theme::init(theme::LoadThemes::JustBase, cx); - Project::init_settings(cx); - language::init(cx); - editor::init(cx); - client::init(&client, cx); - channel::init(&client, user_store, cx); - - MessageEditorSettings::register(cx); - }); - - let language_registry = Arc::new(LanguageRegistry::test(cx.executor())); - language_registry.add(Arc::new(Language::new( - LanguageConfig { - name: "Markdown".into(), - ..Default::default() - }, - Some(tree_sitter_markdown::language()), - ))); - language_registry - } -}