diff --git a/README.md b/README.md index 5689034..066b17b 100644 --- a/README.md +++ b/README.md @@ -79,7 +79,7 @@ A few behaviours that are easy to miss: - **Voice messages play**, with their waveform, at 1× to 2× — Telegram's and Discord's, through `mediad --play`, one at a time. - **Discord's server list** waits until chats have loaded before it shows cached servers, so a cold start does not flash an empty or stale tree. - **Signal's Note to Self.** A cached self-conversation is relabelled when the account reconnects. Its messages stay. -- **Search.** `tinyclient --text --search` searches message text in the local cache, newest hits first. The switcher still finds conversations by title, and its "Search messages" row opens a pane of those hits, where clicking one opens its conversation on that message. The networks themselves are not asked. +- **Search.** `tinyclient --text --search` searches message text in the local cache, newest hits first. In the window, the Edit menu's Find opens the search pane: this conversation (Command-F), everywhere (Shift-Command-F; Ctrl-F, Ctrl-Shift-F elsewhere), and the switcher's "Search messages" row lands there too. A hit opens its conversation on that message. The networks themselves are not asked yet. - **Terminal.** `tinyclient --text`, or any build without the `gui` feature, prints account status and the inbox as lines of text. An interactive terminal UI is not built. The socket protocol is enough to write one. - **Platforms.** CI builds and tests macOS, Linux, and Windows on every push (`.github/workflows/ci.yml`). Most development happens on macOS, so reports and fixes from Linux and Windows are especially welcome. The main window closes cleanly on all three. Linux web sign-in, including Discord's session capture, works with WebKitGTK. diff --git a/crates/chatd/src/blobs.rs b/crates/chatd/src/blobs.rs index 6d6a2d1..ef3d8bb 100644 --- a/crates/chatd/src/blobs.rs +++ b/crates/chatd/src/blobs.rs @@ -141,6 +141,9 @@ struct Job { file: FileKey, /// The account's sign-out count when this was asked for. epoch: u64, + /// The picker session's generation when this was asked for. 0 for anything that is not a + /// picker's preview; a close ends exactly the generations it closed. + picker_generation: u64, adapter: Option>, /// Asked for as the picture of a row on screen: it waits while that row is scrolled away. row: bool, @@ -179,8 +182,18 @@ struct Queue { epochs: HashMap, /// Rows' pictures asked for and since scrolled away, newest last. parked: Vec, - /// Accounts whose picker is open, with when it last asked for a preview. - picked: HashMap, + /// Accounts whose picker is open, with when it last asked for a preview and the picker + /// session's generation: a close ends exactly the generations it closed. + picked: HashMap, + /// The highest picker generation each account's sweep has closed. + picker_closed: HashMap, +} + +/// A picker session: the ask that opened it, and the generation its close will end. +#[derive(Clone, Copy)] +struct Picker { + asked: Instant, + generation: u64, } struct Cache { @@ -297,7 +310,7 @@ impl Blobs { // yet; until it can (a `Request::PickerClosed { account }`, or `Watching.picker`), a picker // counts as closed after `PICK_IDLE` with no `Fetch` of a preview. pub fn picker_closed(&self, account: &AccountKey) { - self.cache.close_picker(account); + self.cache.close_picker(account, false); } /// Brings the cache back under its cap. Runs after a fetch lands and at start. @@ -402,9 +415,16 @@ impl Cache { for file in &files { let key = (account.clone(), file.id.clone()); queue.asked.insert(key.clone(), at); - // The picker asking for a preview is the picker being open. + // The picker asking for a preview is the picker being open. Its session's + // generation stands above whatever a sweep has closed, so its jobs land while + // a job of the closed session is dropped. + let mut picker_generation = 0; if is_pick(&file.id) { - queue.picked.insert(account.clone(), at); + picker_generation = next_generation( + queue.picker_closed.get(account).copied().unwrap_or(0), + queue.picked.get(account).map(|picker| picker.generation), + ); + queue.picked.insert(account.clone(), Picker { asked: at, generation: picker_generation }); picking = true; } // A failure stands until `Fetch` says `retry` (the server clears it before @@ -420,6 +440,7 @@ impl Cache { row: screen.rows.contains(&key), file: key, epoch, + picker_generation, adapter: adapter.clone(), }); if queue.waiting.len() > WAITING { @@ -484,16 +505,20 @@ impl Cache { let mut queue = crate::hub::lock(&self.queue); loop { let at = Instant::now(); - let closed: Vec = queue.picked.iter().filter(|(_, asked)| at.duration_since(**asked) >= self.pick_idle).map(|(account, _)| account.clone()).collect(); + let closed = stale(&queue.picked, at, self.pick_idle); if !closed.is_empty() { drop(queue); for account in &closed { - self.close_picker(account); + self.close_picker(account, true); } queue = crate::hub::lock(&self.queue); continue; } - let next = queue.picked.values().map(|asked| self.pick_idle.saturating_sub(at.duration_since(*asked))).min(); + let next = queue + .picked + .values() + .map(|picker| self.pick_idle.saturating_sub(at.duration_since(picker.asked))) + .min(); queue = match next { Some(wait) => self.picks.wait_timeout(queue, wait).map_or_else(|err| err.into_inner().0, |(queue, _)| queue), None => self.picks.wait(queue).unwrap_or_else(std::sync::PoisonError::into_inner), @@ -501,10 +526,22 @@ impl Cache { } } - /// A picker closed: its previews go, on disk and in the store, and none waits to be fetched. - fn close_picker(&self, account: &AccountKey) { + /// A picker closed: its previews go, on disk and in the store, and none waits to be + /// fetched. The sweeper asks `only_if_idle`: an ask that re-registered the picker while + /// the sweep was waking opened the next session, which keeps its registration, its jobs, + /// and its files. + fn close_picker(&self, account: &AccountKey, only_if_idle: bool) { { let mut queue = crate::hub::lock(&self.queue); + if only_if_idle && !queue.picked.get(account).is_none_or(|picker| picker.asked.elapsed() >= self.pick_idle) { + return; + } + // The close ends this session's generation: a job of it is dropped unless a new + // session reopens the picker, narrowing the window against the directory removal + // below. + if let Some(generation) = queue.picked.get(account).map(|picker| picker.generation) { + queue.picker_closed.insert(account.clone(), generation); + } queue.picked.remove(account); let gone = |file: &FileKey| file.0 == *account && is_pick(&file.1); queue.waiting.retain(|job| !gone(&job.file)); @@ -522,6 +559,13 @@ impl Cache { let _ = std::fs::remove_dir(dir.parent().unwrap_or(&dir)); } + /// True when this picker generation has been closed and no newer session is open. + fn closed_since(&self, account: &AccountKey, generation: u64) -> bool { + let queue = crate::hub::lock(&self.queue); + let closed = queue.picker_closed.get(account).copied().unwrap_or(0); + generation <= closed && !queue.picked.contains_key(account) + } + /// Fetches each account's own picture (the rail's) when the account signs in or starts, /// and on each connect if it is not on disk. Looks at the links every [`OWN_EVERY`] (one /// lock); the store only while an account that just started or connected has not had its @@ -736,11 +780,6 @@ impl Cache { } } - /// The account's picker is open. - fn picking(&self, account: &AccountKey) -> bool { - crate::hub::lock(&self.queue).picked.contains_key(account) - } - fn signed_out_since(&self, job: &Job) -> bool { let queue = crate::hub::lock(&self.queue); queue.epochs.get(&job.file.0).copied().unwrap_or(0) != job.epoch @@ -774,6 +813,11 @@ impl Cache { if self.signed_out_since(job) { return None; } + // A job of a picker session the sweep closed (with no newer session open): its + // files went with the close, and the bytes are not fetched only to be thrown away. + if is_pick(id) && self.closed_since(account, job.picker_generation) { + return None; + } if file.path.is_some() { return Some(Ok(file)); } @@ -791,9 +835,17 @@ impl Cache { }; let (bytes, from) = match fetch::download(&self.hosts, agent, &want, buffer) { Ok(fetched) => fetched, - Err(why) => return failed(&file, why), + Err(why) => { + if is_pick(id) && self.closed_since(account, job.picker_generation) { + return None; + } + if is_pick(id) && !last && !dir.exists() { + continue; + } + return failed(&file, why); + } }; - if self.signed_out_since(job) || (is_pick(id) && !self.picking(account)) { + if self.signed_out_since(job) || (is_pick(id) && self.closed_since(account, job.picker_generation)) { let _ = std::fs::remove_file(&part); return None; } @@ -808,9 +860,14 @@ impl Cache { if let Err(err) = std::fs::rename(&part, &path) { let _ = std::fs::remove_file(&part); // The picker closed and took its directory while this arrived: nobody wants it. - if is_pick(id) && !self.picking(account) { + if is_pick(id) && self.closed_since(account, job.picker_generation) { return None; } + // A picker reopened as the sweep took its directory: the second pass makes + // the directory again and fetches once more. + if is_pick(id) && !last && err.kind() == std::io::ErrorKind::NotFound { + continue; + } return failed(&file, format!("The file could not be kept: {err}.")); } let landed = self.hub.store().media_landed(account, id, &path.to_string_lossy(), bytes, now()); @@ -846,3 +903,53 @@ impl Cache { } } } + +/// The generation the picker session after this ask stands at: one above whatever a sweep has +/// closed, and above whatever the last ask opened. A job of the closed generation is below +/// every session that follows, and is dropped before its bytes are kept. +fn next_generation(closed: u64, open: Option) -> u64 { + closed.max(open.unwrap_or(0)) + 1 +} + +/// The pickers whose idle ran out: the accounts whose close ends their generation. +fn stale(picked: &HashMap, at: Instant, idle: Duration) -> Vec { + picked + .iter() + .filter(|(_, picker)| at.duration_since(picker.asked) >= idle) + .map(|(account, _)| account.clone()) + .collect() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn a_close_ends_one_picker_generation_and_an_ask_opens_the_next() { + // The first session stands above no close at all. + assert_eq!(next_generation(0, None), 1); + // An ask while the sweep is closing generation 1 opens generation 2, whose jobs land + // where the close's removals cannot reach. A job of the closed generation stands at 1, + // meets `picker_closed` = 1, and is dropped before its bytes are kept. + assert_eq!(next_generation(1, Some(1)), 2); + assert!(1 < next_generation(1, Some(1))); + } + + #[test] + fn the_sweep_closes_only_what_stood_idle_and_a_reopened_picker_is_never_stale() { + let account = AccountKey { network: Network::Discord, account: "1".into() }; + let at = Instant::now(); + let idle = Duration::from_secs(2); + let mut picked = HashMap::new(); + // Open long enough: the sweep closes it. + picked.insert(account.clone(), Picker { asked: at - Duration::from_secs(3), generation: 1 }); + assert_eq!(stale(&picked, at, idle), std::slice::from_ref(&account)); + // Re-asked a moment ago — the next session's generation: never stale, whatever the + // generation's number. + picked.insert(account.clone(), Picker { asked: at, generation: next_generation(1, Some(1)) }); + assert!(stale(&picked, at, idle).is_empty()); + // With no picker open, nothing is closed. + picked.clear(); + assert!(stale(&picked, at, idle).is_empty()); + } +} diff --git a/crates/chatd/src/server.rs b/crates/chatd/src/server.rs index e0df31b..63325a5 100644 --- a/crates/chatd/src/server.rs +++ b/crates/chatd/src/server.rs @@ -1411,11 +1411,11 @@ impl Chat { Err(err) => internal(err), } } - Request::SearchMessages { conversation, network, query, limit } => { + Request::SearchMessages { conversation, network, space, query, limit } => { let query = query.trim(); let found = match query.is_empty() { true => Ok(Vec::new()), - false => self.hub.store().search_messages(conversation.as_ref(), network, query, limit.min(PAGE_LIMIT)), + false => self.hub.store().search_messages(conversation.as_ref(), network, space.as_ref(), query, limit.min(PAGE_LIMIT)), }; match found { Ok(hits) => Response::MessageHits { hits }, diff --git a/crates/chatd/src/store.rs b/crates/chatd/src/store.rs index f9f1451..c693d17 100644 --- a/crates/chatd/src/store.rs +++ b/crates/chatd/src/store.rs @@ -1501,6 +1501,7 @@ impl Store { &self, conversation: Option<&Key>, network: Option, + space: Option<&Key>, query: &str, limit: u32, ) -> rusqlite::Result> { @@ -1511,11 +1512,12 @@ impl Store { FROM message m JOIN conversation c ON c.network = m.network AND c.account = m.account AND c.id = m.conversation - WHERE m.pending = 0 AND m.text LIKE ?5 ESCAPE '\\' + WHERE m.pending = 0 AND m.text LIKE ?8 ESCAPE '\\' AND (?1 IS NULL OR m.network = ?1) AND (?2 IS NULL OR (m.network = ?2 AND m.account = ?3 AND m.conversation = ?4)) + AND (?5 IS NULL OR (c.network = ?5 AND c.account = ?6 AND c.space = ?7)) AND NOT c.hidden AND c.special != 'request' - ORDER BY m.sent DESC, length(m.id) DESC, m.id DESC LIMIT ?6", + ORDER BY m.sent DESC, length(m.id) DESC, m.id DESC LIMIT ?9", )? .query_map( params![ @@ -1523,6 +1525,9 @@ impl Store { conversation.map(|key| word(&key.network)), conversation.map(|key| key.account.as_str()), conversation.map(|key| key.id.as_str()), + space.map(|key| word(&key.network)), + space.map(|key| key.account.as_str()), + space.map(|key| key.id.as_str()), pattern, limit, ], @@ -2575,37 +2580,60 @@ mod tests { .unwrap(); store.ingest(&request, &said("5", "world of tickets", 50), true).unwrap(); - let hits = store.search_messages(None, None, "world", 10).unwrap(); + let hits = store.search_messages(None, None, None, "world", 10).unwrap(); assert_eq!(hits.iter().map(|hit| hit.message_id.as_str()).collect::>(), ["2", "1"]); assert_eq!((hits[0].sender.as_str(), hits[0].snippet.as_str()), ("mara", "WORLD peace")); assert!(!hits[0].outgoing && hits[0].title == "#general"); assert_eq!(hits[0].conversation, key); // Named conversation and network narrow; a hidden conversation answers nothing at all. - assert_eq!(store.search_messages(Some(&key), None, "world", 10).unwrap().len(), 2); - assert!(store.search_messages(Some(&hidden), None, "world", 10).unwrap().is_empty()); - assert_eq!(store.search_messages(None, Some(Network::Slack), "world", 10).unwrap().len(), 0); - assert_eq!(store.search_messages(None, Some(Network::Discord), "world", 10).unwrap().len(), 2); + assert_eq!(store.search_messages(Some(&key), None, None, "world", 10).unwrap().len(), 2); + assert!(store.search_messages(Some(&hidden), None, None, "world", 10).unwrap().is_empty()); + assert_eq!(store.search_messages(None, Some(Network::Slack), None, "world", 10).unwrap().len(), 0); + assert_eq!(store.search_messages(None, Some(Network::Discord), None, "world", 10).unwrap().len(), 2); // Case goes for nothing, a longer cut carries the marker, and an empty query finds none. - let cut = store.search_messages(None, None, "peace", 1).unwrap(); + let cut = store.search_messages(None, None, None, "peace", 1).unwrap(); assert_eq!(cut.len(), 1); - assert!(store.search_messages(None, None, " ", 10).unwrap().is_empty()); + assert!(store.search_messages(None, None, None, " ", 10).unwrap().is_empty()); // What the user typed means itself: a percent is not a wildcard. store.ingest(&key, &said("5", "we matched 50% there", 50), false).unwrap(); - assert_eq!(store.search_messages(None, None, "50%", 10).unwrap().len(), 1); - assert_eq!(store.search_messages(None, None, "0%_t", 10).unwrap().len(), 0); + assert_eq!(store.search_messages(None, None, None, "50%", 10).unwrap().len(), 1); + assert_eq!(store.search_messages(None, None, None, "0%_t", 10).unwrap().len(), 0); // A match far into a long message is still the hit's window, marked on both ends. let long = format!("{} the needle turns up here {}", "filler ".repeat(60), "and the text goes on".repeat(20)); store.ingest(&key, &said("6", &long, 60), false).unwrap(); - let far = store.search_messages(None, None, "needle", 1).unwrap(); + let far = store.search_messages(None, None, None, "needle", 1).unwrap(); assert_eq!(far.len(), 1); assert!(far[0].snippet.starts_with("...") && far[0].snippet.ends_with("..."), "{}", far[0].snippet); assert!(far[0].snippet.contains("needle turns up"), "{}", far[0].snippet); } + #[test] + fn a_message_search_narrows_to_one_space() { + let (mut store, key) = store(); + let account = key.account(); + for (id, title, space) in [("guilded", "#in the space", Some("guild")), ("loose", "#no space", None)] { + let mut row = ConversationRow::new(id, ConversationKind::Channel, title); + row.space = space.map(|space| space.to_owned()); + store.upsert_conversation(&account, &row).unwrap(); + } + store.ingest(&Key::new(&account, "guilded"), &said("1", "world of the guild", 10), false).unwrap(); + store.ingest(&Key::new(&account, "loose"), &said("2", "world outside", 20), false).unwrap(); + + let space = Key::new(&account, "guild"); + let hits = store.search_messages(None, None, Some(&space), "world", 10).unwrap(); + assert_eq!(hits.iter().map(|hit| hit.message_id.as_str()).collect::>(), ["1"]); + assert_eq!(hits[0].title, "#in the space"); + // The same space id on another network or account holds other rows: nothing answers. + let elsewhere = Key::new(&AccountKey { network: Network::Slack, account: "me".into() }, "guild"); + assert!(store.search_messages(None, None, Some(&elsewhere), "world", 10).unwrap().is_empty()); + // With no space named, the cache answers as a whole. + assert_eq!(store.search_messages(None, None, None, "world", 10).unwrap().len(), 2); + } + #[test] fn pages_walk_a_conversation_both_ways_and_say_when_there_is_more() { let (mut store, key) = store(); diff --git a/crates/chatd/tests/blobs.rs b/crates/chatd/tests/blobs.rs index 45f0f56..426d18f 100644 --- a/crates/chatd/tests/blobs.rs +++ b/crates/chatd/tests/blobs.rs @@ -633,8 +633,9 @@ fn picker_previews_live_in_their_own_directory_and_go_when_the_picker_closes() { (file(&still, MediaKind::Image), files.url("/bytes/4000")), (file("emoji-1", MediaKind::Emoji), files.url("/bytes/500")), ]; - // A picker counts as closed after 2 s without asking. - let chatd = Daemon::start(®istered, &[("TINYCLIENT_PICK_IDLE", "2")]); + // A picker counts as closed after 5 s without asking. Roomy, so a slow or cold machine + // cannot overtake the last section, where a preview is asked for again. + let chatd = Daemon::start(®istered, &[("TINYCLIENT_PICK_IDLE", "5")]); let pick_dir = chatd.dir.join("media/discord/pick/42"); let landed = [chatd.fetched(&sticker), chatd.fetched(&still)]; for file in &landed { diff --git a/crates/chatd/tests/discord.rs b/crates/chatd/tests/discord.rs index 7e038c0..21387a0 100644 --- a/crates/chatd/tests/discord.rs +++ b/crates/chatd/tests/discord.rs @@ -530,6 +530,19 @@ fn mediad() -> PathBuf { chatd.with_file_name(format!("mediad{}", std::env::consts::EXE_SUFFIX)) } +/// A child that is over at once, for the `World` field until the real chatd is spawned. +#[cfg(windows)] +fn placeholder() -> Command { + let mut done = Command::new("cmd"); + done.args(["/C", "exit 0"]); + done +} + +#[cfg(not(windows))] +fn placeholder() -> Command { + Command::new("true") +} + /// The fake Discord and a real chatd pointed at it. struct World { child: Child, @@ -621,9 +634,10 @@ impl World { prepare(&shared); let env = env.iter().map(|(name, value)| ((*name).to_owned(), value.replace("{{API}}", &api))).collect(); - // A placeholder child until the real one is spawned from the finished world. + // A placeholder child until the real one is spawned from the finished world. `true` is + // Unix's; Windows takes its own no-op. let mut world = World { - child: Command::new("true").spawn().unwrap(), + child: placeholder().spawn().unwrap(), dir, discord: shared, api, diff --git a/crates/protocol/src/lib.rs b/crates/protocol/src/lib.rs index 8d598df..d657a6d 100644 --- a/crates/protocol/src/lib.rs +++ b/crates/protocol/src/lib.rs @@ -981,14 +981,17 @@ pub enum Request { limit: u32, }, /// Cached messages whose text contains `query`, newest first. `conversation` narrows to one - /// conversation, `network` to one network. Hidden conversations and message requests stay - /// out of it, as they stay out of everything that counts. From the cache alone: what a - /// network itself finds lands as it answers, and is a later step. + /// conversation, `network` to one network, `space` to one Space's conversations. Hidden + /// conversations and message requests stay out of it, as they stay out of everything that + /// counts. From the cache alone: what a network itself finds lands as it answers, and is a + /// later step. SearchMessages { #[serde(default, skip_serializing_if = "Option::is_none")] conversation: Option, #[serde(default, skip_serializing_if = "Option::is_none")] network: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + space: Option, query: String, limit: u32, }, @@ -1743,19 +1746,20 @@ mod tests { #[test] fn a_message_search_and_its_hits_read_off_the_wire() { let ask: Request = serde_json::from_str(r#"{"op":"search_messages","query":"lichen","limit":20}"#).unwrap(); - let Request::SearchMessages { conversation, network, query, limit } = &ask else { + let Request::SearchMessages { conversation, network, space, query, limit } = &ask else { panic!("not a search") }; - assert!(conversation.is_none() && network.is_none() && query == "lichen" && *limit == 20); + assert!(conversation.is_none() && network.is_none() && space.is_none() && query == "lichen" && *limit == 20); let with_filters = Request::SearchMessages { conversation: Some(Key { network: Network::Slack, account: "T1".into(), id: "C1".into() }), network: Some(Network::Slack), + space: Some(Key { network: Network::Slack, account: "T1".into(), id: "S1".into() }), query: "lichen".into(), limit: 20, }; assert_eq!( serde_json::to_string(&with_filters).unwrap(), - r#"{"op":"search_messages","conversation":{"network":"slack","account":"T1","id":"C1"},"network":"slack","query":"lichen","limit":20}"# + r#"{"op":"search_messages","conversation":{"network":"slack","account":"T1","id":"C1"},"network":"slack","space":{"network":"slack","account":"T1","id":"S1"},"query":"lichen","limit":20}"# ); let hit = MessageHit { conversation: Key { network: Network::Discord, account: "1".into(), id: "C2".into() }, diff --git a/crates/tinyclient/src/gui/app/menubar.rs b/crates/tinyclient/src/gui/app/menubar.rs index ad9ee4e..17aed17 100644 --- a/crates/tinyclient/src/gui/app/menubar.rs +++ b/crates/tinyclient/src/gui/app/menubar.rs @@ -6,9 +6,9 @@ // disabled until those views register them with `.on_action`: Attach File (`AttachFile`, // composer), Show Details (`ShowDetails`, pane), Mute… (`Mute`, profile), Start Call // (`StartCall`, call) and Open in (`OpenInNetwork`, handoff). Items with nothing built -// behind them are left out rather than shown dead: Find (⌘F, ⇧⌘F), Mark as -// Unread, Mark Server as Read, Copy Link, Release Notes and Report a Problem…. They come back -// with what they do. Check for Updates… is here; it becomes Restart to Update once a build is staged. +// behind them are left out rather than shown dead: Mark as Unread, Mark Server as Read, +// Copy Link, Release Notes and Report a Problem…. They come back with what they do, as Find +// (⌘F, ⇧⌘F) did. Check for Updates… is here; it becomes Restart to Update once a build is staged. use super::super::settings; use super::super::sidebar::Mode; @@ -82,6 +82,11 @@ pub fn menus(places: &[(String, usize)], open_network: Option, update_i MenuItem::os_action("Paste", input::Paste, OsAction::Paste), MenuItem::action("Paste and Match Style", input::Paste), MenuItem::os_action("Select All", input::SelectAll, OsAction::SelectAll), + MenuItem::separator(), + MenuItem::Submenu(Menu::new("Find").items([ + MenuItem::action("Search in Conversation", SearchConversation), + MenuItem::action("Search Everywhere", SearchEverywhere), + ])), // macOS adds Emoji & Symbols, Dictation and AutoFill itself. ]), Menu::new("View").items(view), @@ -126,6 +131,8 @@ pub fn bind_keys(cx: &mut App) { KeyBinding::new("secondary-n", NewMessage, None), KeyBinding::new("secondary-u", AttachFile, None), KeyBinding::new("secondary-w", CloseWindow, None), + KeyBinding::new("secondary-f", SearchConversation, None), + KeyBinding::new("secondary-shift-f", SearchEverywhere, None), KeyBinding::new("secondary-=", BiggerText, None), KeyBinding::new("secondary-+", BiggerText, None), KeyBinding::new("secondary--", SmallerText, None), @@ -246,21 +253,27 @@ mod tests { let names: Vec<&str> = bar.iter().map(|menu| menu.name.as_ref()).collect(); assert_eq!(names, ["tinyclient", "File", "Edit", "View", "Conversation", "Window", "Help"]); let items = |menu: &Menu| -> Vec { - menu.items - .iter() - .filter_map(|item| match item { - MenuItem::Action { name, .. } => Some(name.to_string()), - _ => None, - }) - .collect() + fn walk(item: &MenuItem, names: &mut Vec) { + match item { + MenuItem::Action { name, .. } => names.push(name.to_string()), + MenuItem::Submenu(menu) => menu.items.iter().for_each(|item| walk(item, names)), + _ => {} + } + } + let mut names = Vec::new(); + menu.items.iter().for_each(|item| walk(item, &mut names)); + names }; assert!(items(&bar[4]).contains(&"Open in Slack".to_owned())); // The rail's places after Inbox and People, in its order; no second Inbox. assert_eq!(items(&bar[3])[..4], ["Inbox", "People", "Discord", "Moss & Lichen"]); assert!(items(&bar[1]).contains(&"Close Window".to_owned())); + // Find's two searches work: their pane and keys are built. + assert!(items(&bar[2]).contains(&"Search in Conversation".to_owned())); + assert!(items(&bar[2]).contains(&"Search Everywhere".to_owned())); // Nothing is offered that does nothing. let all: Vec = bar.iter().flat_map(items).collect(); - for dead in ["Search in Conversation", "Search Everywhere", "Release Notes", "Report a Problem…", "Mark as Unread", "Copy Link"] { + for dead in ["Release Notes", "Report a Problem…", "Mark as Unread", "Copy Link"] { assert!(!all.contains(&dead.to_owned()), "{dead} is offered"); } assert!(all.contains(&"Check for Updates…".to_owned())); diff --git a/crates/tinyclient/src/gui/conversation/shot.rs b/crates/tinyclient/src/gui/conversation/shot.rs index e44ad8c..0127792 100644 --- a/crates/tinyclient/src/gui/conversation/shot.rs +++ b/crates/tinyclient/src/gui/conversation/shot.rs @@ -353,7 +353,7 @@ pub fn steps(cx: &mut gpui::Context) { "details" => shell.toggle_details(cx), "back" => shell.travel(true, cx), "forward" => shell.travel(false, cx), - "close" => shell.close_right(cx), + "close" => shell.close_right(None, cx), // Escape in the composer with nothing of its own to close. "escape" => shell.composer_escaped(cx), "members-on" => shell.members_switch_on(cx), diff --git a/crates/tinyclient/src/gui/conversation/side.rs b/crates/tinyclient/src/gui/conversation/side.rs index d0af73c..bd91067 100644 --- a/crates/tinyclient/src/gui/conversation/side.rs +++ b/crates/tinyclient/src/gui/conversation/side.rs @@ -9,12 +9,13 @@ use crate::gui::pane::{self, Layer, Showing, Stack, Step}; use super::{Pane, PaneEvent}; use crate::client::ask; use crate::gui::components::{empty_state, glyph}; +use crate::gui::input::{InputEvent, InputKey, TextInput}; use crate::gui::layout::{RightKind, RightPane}; use crate::gui::state::Shell; use crate::gui::theme::{self, Type}; use crate::gui::time; use crate::view::{age, now}; -use gpui::{div, prelude::*, px, AnyElement, Context, Entity, FontWeight, HighlightStyle, MouseButton, SharedString, StyledText, Window}; +use gpui::{div, prelude::*, px, AnyElement, Context, Entity, Focusable, FontWeight, HighlightStyle, MouseButton, SharedString, StyledText, Window}; use protocol::{ConversationKind, Key, Link, Message, MessageHit, Request, Response}; use std::ops::Range; use std::rc::Rc; @@ -28,6 +29,8 @@ pub struct Side { threads: pane::Memory>, /// The message search's page: what was typed, and what the cache holds for it. pub search: Option, + /// The search pane's field and its wiring, alive while the pane is. + search_input: Option, /// The conversation the stack was last fitted to. fitted: Option, /// The kind this view last put in the layout's slot, so another view's change can be told @@ -46,14 +49,64 @@ pub struct Side { room: std::cell::Cell, } -/// One message search's page: what was typed, and what the cache holds for it. +/// One message search's page: the scope asked, what was typed, and what the cache holds for it. #[derive(Default)] pub struct SearchPage { + pub scope: Scope, pub words: String, /// What the cache answered: `None` while the read is outstanding or failed. pub hits: Option>, /// The cache could not be asked. pub failed: bool, + /// The hit the keyboard is on: drawn lit, opened on Enter. + pub picked: Option, + /// How often the cache was asked, so an older answer never lands over a newer one. + asked: u32, +} + +/// How far a search asks: the conversation open now, its Space, or the whole cache. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub enum Scope { + #[default] + Everywhere, + Conversation, + Space, +} + +impl Scope { + fn label(self) -> &'static str { + match self { + Scope::Conversation => "This conversation", + Scope::Space => "This space", + Scope::Everywhere => "Everywhere", + } + } +} + +/// A scope's filters for `SearchMessages`: the one conversation, or the Space's key, or neither. +/// A scope about the conversation falls back to everywhere with nothing open, as the switcher's +/// own search row does. +fn scope_filters(scope: Scope, open: Option<(&Key, Option<&str>)>) -> (Option, Option) { + match (scope, open) { + (Scope::Conversation, Some((key, _))) => (Some(key.clone()), None), + (Scope::Space, Some((key, Some(space)))) => { + (None, Some(Key { network: key.network, account: key.account.clone(), id: space.to_owned() })) + } + _ => (None, None), + } +} + +/// The search pane's field and the subscriptions that carry its keys to the shell. +struct SearchField { + input: Entity, + _subscriptions: Vec, +} + +/// The page a fresh search opens with: the scope, and the words it starts asked for. The +/// switcher's row seeds its words here, so the first ask carries them without relying on the +/// field's `set_text`, which is not an edit and emits nothing. +fn fresh_page(scope: Scope, words: &str) -> SearchPage { + SearchPage { scope, words: words.to_owned(), ..SearchPage::default() } } /// The letters of the query inside a hit's text, for the heavier drawing. @@ -222,7 +275,7 @@ impl Shell { } None => { if self.conversation_state.side.installed.take().is_some() && self.right_kind().is_some() { - self.close_right(cx); + self.close_right(None, cx); } } } @@ -262,6 +315,10 @@ impl Shell { side.switches = pane::close(&side.stack, side.switches); side.stack = Stack::default(); side.installed = None; + // A search pane the ✕ closed goes with its stack, exactly as Esc's closing + // does: no page behind, no field holding the keys. + side.search = None; + side.search_input = None; self.conversation_state.thread.borrow_mut().take(); } Some(RightKind::Info) => { @@ -353,33 +410,197 @@ impl Shell { self.install(cx); } - /// The switcher's "Search messages" row: the pane opens at once and the cache is asked. - pub(in crate::gui) fn open_message_search(&mut self, words: String, cx: &mut Context) { + /// The switcher's "Search messages" row: the pane opens with the words already asked for. + /// The field is filled to match, whose `set_text` is not an edit and emits nothing - the + /// ask carries the words, so nothing depends on it emitting. + pub(in crate::gui) fn open_message_search(&mut self, words: String, window: &mut Window, cx: &mut Context) { + self.new_search_with(Scope::Everywhere, &words, window, cx); + } + + /// Search in the conversation, or everywhere: the pane opens with its field focused, or, + /// standing already, the scope switches and the asking starts again. + pub(in crate::gui) fn open_search(&mut self, scope: Scope, window: &mut Window, cx: &mut Context) { + // When another layer sits on top of Search, starting a search starts over. And a + // conversation scope with nothing open names nothing — it asks everywhere, as the + // switcher's row does, so a chip is lit. + let scope = if scope == Scope::Conversation && self.open_key().is_none() { + Scope::Everywhere + } else { + scope + }; + if !matches!(self.conversation_state.side.stack.top(), Some(Layer::Search)) { + self.conversation_state.side.search = None; + self.conversation_state.side.search_input = None; + self.new_search_with(scope, "", window, cx); + return; + } + if let Some(page) = self.conversation_state.side.search.as_mut() { + page.scope = scope; + page.hits = None; + page.picked = None; + } + self.reask_search(cx); + if let Some(field) = &self.conversation_state.side.search_input { + window.focus(&field.input.focus_handle(cx), cx); + } + cx.notify(); + } + + /// The Edit menu's two finds. + pub(in crate::gui) fn search_here(&mut self, window: &mut Window, cx: &mut Context) { + self.open_search(Scope::Conversation, window, cx); + } + + pub(in crate::gui) fn search_everywhere(&mut self, window: &mut Window, cx: &mut Context) { + self.open_search(Scope::Everywhere, window, cx); + } + + /// A fresh search, seeded with the words it starts asked for. `open_search` and the + /// switcher's row both come through here. + fn new_search_with(&mut self, scope: Scope, words: &str, window: &mut Window, cx: &mut Context) { + let input = cx.new(|cx| TextInput::new("Search messages", cx)); + let changed = cx.subscribe_in(&input, window, |this, _, event: &InputEvent, _, cx| match event { + InputEvent::Changed => this.search_typed(cx), + InputEvent::Submit => this.open_picked_hit(cx), + }); + let keys = cx.subscribe_in(&input, window, |this, _, event: &InputKey, window, cx| match event { + InputKey::Escape => this.close_search(window, cx), + InputKey::Up => this.move_search_pick(false, cx), + InputKey::Down => this.move_search_pick(true, cx), + }); + { + let side = &mut self.conversation_state.side; + side.search_input = Some(SearchField { input, _subscriptions: vec![changed, keys] }); + side.search = Some(fresh_page(scope, words)); + side.stack = pane::open(&side.stack, Layer::Search); + } + self.install(cx); + if let Some(field) = &self.conversation_state.side.search_input { + window.focus(&field.input.focus_handle(cx), cx); + } + if !words.is_empty() { + // The field is filled to match the ask. `set_text` is not an edit and emits + // nothing, which is why the ask above carries the words itself. + if let Some(field) = &self.conversation_state.side.search_input { + field.input.update(cx, |input, cx| input.set_text(words, cx)); + } + } + self.reask_search(cx); + } + + /// Esc in the field: the pane closes and the conversation's keys come back. + fn close_search(&mut self, window: &mut Window, cx: &mut Context) { let side = &mut self.conversation_state.side; - side.search = Some(SearchPage { words: words.clone(), hits: None, failed: false }); - side.stack = pane::open(&side.stack, Layer::Search); + if side.search.take().is_none() { + return; + } + side.stack = Stack { base: None, push: None }; + side.search_input = None; self.install(cx); + self.composer_focus(window, cx); + } + + /// What the field holds now is what the cache is asked for. + fn search_typed(&mut self, cx: &mut Context) { + let words = self + .conversation_state + .side + .search_input + .as_ref() + .map(|field| field.input.read(cx).text().to_owned()) + .unwrap_or_default(); + let Some(page) = self.conversation_state.side.search.as_mut() else { + return; + }; + if words == page.words { + return; + } + page.words = words; + page.hits = None; + page.picked = None; + self.reask_search(cx); + } + + /// Asks the cache for the page's words within its scope. The pane's scopes name the + /// conversation open now, so a switch of conversation re-resolves the next ask. + fn reask_search(&mut self, cx: &mut Context) { + let (words, scope, asked) = { + let Some(page) = self.conversation_state.side.search.as_mut() else { + return; + }; + page.failed = false; + page.asked += 1; + (page.words.clone(), page.scope, page.asked) + }; + if words.trim().is_empty() { + if let Some(page) = self.conversation_state.side.search.as_mut() { + page.hits = None; + } + cx.notify(); + return; + } + let open = self.open_with_space(); + let (conversation, space) = scope_filters(scope, open.as_ref().map(|(key, space)| (key, space.as_deref()))); + let request = Request::SearchMessages { conversation, network: None, space, query: words, limit: protocol::PAGE_LIMIT }; self.off_thread( cx, - { - let asked = words.clone(); - move || ask(&Request::SearchMessages { conversation: None, network: None, query: asked, limit: protocol::PAGE_LIMIT }) - }, + move || ask(&request), move |this, answer, cx| { - let Some(page) = this.conversation_state.side.search.as_mut().filter(|page| page.words == words) else { + let Some(page) = this.conversation_state.side.search.as_mut().filter(|page| page.asked == asked) else { return; }; match answer { Ok(Response::MessageHits { hits }) => page.hits = Some(hits), - // Another answer is not this request's; the read failing is the pane's word. - Ok(_) => {} - Err(_) => page.failed = true, + Ok(_) | Err(_) => page.failed = true, } cx.notify(); }, ); } + /// The conversation open now, with the Space it sits in: what the pane's scopes name. + fn open_with_space(&self) -> Option<(Key, Option)> { + let key = self.open_key()?.clone(); + let mut held = self + .sidebar_state + .held() + .cloned() + .chain(self.trees.values().flat_map(|tree| tree.conversations.iter().cloned())); + let space = held.find(|conversation| conversation.key == key).and_then(|conversation| conversation.space.clone()); + Some((key, space)) + } + + /// The keyboard moves over the hits; Enter opens the one it is on, or the first. + fn move_search_pick(&mut self, down: bool, cx: &mut Context) { + let Some(page) = self.conversation_state.side.search.as_mut() else { + return; + }; + let Some(hits) = page.hits.as_ref() else { + return; + }; + if hits.is_empty() { + return; + } + page.picked = Some(match page.picked { + Some(at) if down => (at + 1).min(hits.len() - 1), + Some(at) => at.saturating_sub(1), + None if down => 0, + None => hits.len() - 1, + }); + cx.notify(); + } + + fn open_picked_hit(&mut self, cx: &mut Context) { + let page = self.conversation_state.side.search.as_ref(); + let picked = page + .and_then(|page| page.hits.as_ref()) + .and_then(|hits| hits.get(page.and_then(|page| page.picked).unwrap_or(0))) + .cloned(); + if let Some(hit) = picked { + self.open_search_hit(&hit, cx); + } + } + /// A search hit was clicked: its conversation opens on the message, or, being open already, /// jumps to it. pub(in crate::gui) fn open_search_hit(&mut self, hit: &MessageHit, cx: &mut Context) { @@ -507,6 +728,8 @@ impl Shell { Step::ClosePane => { let side = &mut self.conversation_state.side; side.stack = Stack { base: None, push: None }; + side.search = None; + side.search_input = None; self.install(cx); // The thread's composer may have had the keys: they go back to the conversation's. if let Some(window) = window { @@ -516,7 +739,7 @@ impl Shell { Step::Latest => { // A pane this view did not put there (search results, a sheet) closes first. if self.conversation_state.side.installed.is_none() && self.right_kind().is_some() { - self.close_right(cx); + self.close_right(window, cx); return true; } if let Some(pane) = self.conversation_state.pane.get().cloned() { @@ -595,7 +818,7 @@ fn draw_side(shell: &Shell, window: &mut Window, cx: &mut Context) -> Any Layer::Members(_) => shell.pane_members(window, cx), Layer::Thread { conversation, .. } => draw_thread(shell, conversation, cx), Layer::Pinned(conversation) => draw_pins(shell, conversation, cx), - Layer::Search => draw_search(shell, cx), + Layer::Search => draw_search(shell, window, cx), }; let crumb = stack.push.as_ref().and(stack.base.as_ref()).map(|base| crumb(base.title(), cx)); div() @@ -642,10 +865,11 @@ fn crumb(base: &'static str, cx: &mut Context) -> AnyElement { /// The message search's pane: what was asked, and the cache's hits, newest first. Every hit is /// the cache's word for it, so a row says "cached"; the networks are a later ask. -fn draw_search(shell: &Shell, cx: &mut Context) -> AnyElement { - let Some(page) = &shell.conversation_state.side.search else { +fn draw_search(shell: &Shell, _window: &mut Window, cx: &mut Context) -> AnyElement { + let Some(page) = shell.conversation_state.side.search.as_ref() else { return div().into_any_element(); }; + let field = shell.conversation_state.side.search_input.as_ref().map(|field| field.input.clone()); let head = div() .h(px(44.)) .flex_none() @@ -656,9 +880,45 @@ fn draw_search(shell: &Shell, cx: &mut Context) -> AnyElement { .border_b_1() .border_color(theme::hairline()) .child(glyph("icons/search.svg", 14., theme::text_soft())) - .child(div().flex_1().min_w_0().truncate().type_ui().text_color(theme::text()).child(page.words.clone())) - .child(div().type_small().text_color(theme::text_soft()).child("cached")); - let body = if page.failed { + .child(match field { + Some(field) => div().flex_1().min_w_0().child(field).into_any_element(), + None => div().flex_1().min_w_0().truncate().type_ui().text_color(theme::text()).child(page.words.clone()).into_any_element(), + }); + // The scope's chips: the conversation open now, its Space, or the whole cache. One is the + // scope asked; a chip about no conversation open is not drawn, for it cannot be asked. + let open = shell.open_with_space(); + let in_space = open.as_ref().and_then(|(_, space)| space.as_deref()).is_some(); + let mut chips = div().flex_none().px(px(8.)).pt(px(8.)).pb(px(2.)).flex().flex_wrap().gap(px(6.)); + for candidate in [Scope::Conversation, Scope::Space, Scope::Everywhere] { + let available = match candidate { + Scope::Conversation => open.is_some(), + Scope::Space => in_space, + Scope::Everywhere => true, + }; + if !available { + continue; + } + let lit = candidate == page.scope; + chips = chips.child( + div() + .id(("search-scope", candidate as usize)) + .px(px(10.)) + .h(px(24.)) + .rounded(px(theme::R_CONTROL)) + .flex() + .items_center() + .type_small() + .cursor_pointer() + .text_color(if lit { theme::text() } else { theme::text_soft() }) + .when(lit, |chip| chip.bg(theme::selected())) + .hover(|style| style.bg(theme::hover())) + .on_click(cx.listener(move |this, _, window, cx| this.open_search(candidate, window, cx))) + .child(candidate.label()), + ); + } + let body = if page.words.trim().is_empty() { + empty_state("icons/search.svg", "Search the cache.", "What was said in what tinyclient has read.").into_any_element() + } else if page.failed { empty_state("icons/search.svg", "tinyclient isn't answering.", "The cache could not be asked. Try again in a moment.") .into_any_element() } else if page.hits.is_none() { @@ -670,6 +930,7 @@ fn draw_search(shell: &Shell, cx: &mut Context) -> AnyElement { let mut rows = div().id("search-hits").flex_1().min_h_0().overflow_y_scroll().flex().flex_col().p(px(4.)); for (ix, hit) in hits.iter().enumerate() { let clicked = hit.clone(); + let picked = page.picked == Some(ix); rows = rows.child( div() .id(("hit", ix)) @@ -681,8 +942,14 @@ fn draw_search(shell: &Shell, cx: &mut Context) -> AnyElement { .flex_col() .gap(px(2.)) .cursor_pointer() + .when(picked, |row| row.bg(theme::selected())) .hover(|style| style.bg(theme::hover())) - .on_click(cx.listener(move |this, _, _, cx| this.open_search_hit(&clicked, cx))) + .on_click(cx.listener(move |this, _, _, cx| { + if let Some(page) = this.conversation_state.side.search.as_mut() { + page.picked = Some(ix); + } + this.open_search_hit(&clicked, cx); + })) .child( div() .flex() @@ -719,6 +986,7 @@ fn draw_search(shell: &Shell, cx: &mut Context) -> AnyElement { .flex_col() .bg(theme::canvas()) .child(head) + .child(chips) .child(body) .into_any_element() } @@ -800,4 +1068,33 @@ mod tests { assert!(!thread_belongs(&key("C_a"), Some(&key("C_b")))); assert!(!thread_belongs(&key("C_a"), None)); } + + #[test] + fn a_scope_names_what_is_open_and_falls_back_to_everywhere() { + let channel = key("C_a"); + let space = Key::new(&AccountKey { network: Network::Slack, account: "T1".into() }, "guild"); + + // Everywhere asks for the whole cache, whatever is open. + assert_eq!(scope_filters(Scope::Everywhere, None), (None, None)); + assert_eq!(scope_filters(Scope::Everywhere, Some((&channel, Some("guild")))), (None, None)); + + // The conversation and its Space name what is open now. + assert_eq!(scope_filters(Scope::Conversation, Some((&channel, Some("guild")))), (Some(channel.clone()), None)); + assert_eq!(scope_filters(Scope::Space, Some((&channel, Some("guild")))), (None, Some(space))); + + // A direct conversation has no Space, and with nothing open both name nothing: the + // cache answers as a whole, as the switcher's row does. + assert_eq!(scope_filters(Scope::Space, Some((&channel, None))), (None, None)); + assert_eq!(scope_filters(Scope::Conversation, None), (None, None)); + assert_eq!(scope_filters(Scope::Space, None), (None, None)); + } + + #[test] + fn the_switcher_s_words_start_asked_for() { + // The switcher's row seeds its words into the page, so the first ask carries them + // without relying on the field's `set_text`, which is not an edit and emits nothing. + let page = fresh_page(Scope::Everywhere, "rules"); + assert_eq!((page.scope, page.words.as_str()), (Scope::Everywhere, "rules")); + assert!(page.hits.is_none() && page.picked.is_none()); + } } diff --git a/crates/tinyclient/src/gui/layout.rs b/crates/tinyclient/src/gui/layout.rs index f58d939..6e79757 100644 --- a/crates/tinyclient/src/gui/layout.rs +++ b/crates/tinyclient/src/gui/layout.rs @@ -58,7 +58,7 @@ pub struct Slots { /// `typing…` in place of a direct row's preview, while someone is. pub typing: Option Option>, /// The switcher's last row: search messages for these words. - pub search: Option)>, + pub search: Option)>, } #[derive(Clone, Copy, PartialEq, Eq, Debug)] @@ -67,8 +67,7 @@ pub enum RightKind { Members, /// One person: who they are and what was shared with them. Info, - // For the search results, which are not built yet. - #[allow(dead_code)] + /// The search pane: the field, its scopes, the cache's hits. Search, } @@ -195,8 +194,14 @@ impl Shell { cx.notify(); } - pub(super) fn close_right(&mut self, cx: &mut Context) { - self.layout_state.right = None; + pub(super) fn close_right(&mut self, window: Option<&mut Window>, cx: &mut Context) { + let was_open = self.layout_state.right.take().is_some(); + // The keys go back to the composer when a pane was open, as Esc's closing of it does. + if was_open { + if let Some(window) = window { + self.composer_focus(window, cx); + } + } cx.notify(); } @@ -420,7 +425,7 @@ impl Shell { .child(glyph("icons/chevron-left.svg", 16., theme::text_soft())) .flex_row_reverse() .on_mouse_down(MouseButton::Left, |_, _, cx| cx.stop_propagation()) - .on_click(cx.listener(|this, _, _, cx| this.close_right(cx))), + .on_click(cx.listener(|this, _, window, cx| this.close_right(Some(window), cx))), ) }) .child( @@ -439,7 +444,7 @@ impl Shell { icon_button("right-close", "icons/x.svg", false) .tooltip(tip("Close", Some("⌘."))) .on_mouse_down(MouseButton::Left, |_, _, cx| cx.stop_propagation()) - .on_click(cx.listener(|this, _, _, cx| this.close_right(cx))), + .on_click(cx.listener(|this, _, window, cx| this.close_right(Some(window), cx))), ); let resizable = !replaces; Some( diff --git a/crates/tinyclient/src/gui/mod.rs b/crates/tinyclient/src/gui/mod.rs index 98d0609..55b0b1f 100644 --- a/crates/tinyclient/src/gui/mod.rs +++ b/crates/tinyclient/src/gui/mod.rs @@ -110,7 +110,9 @@ impl Shell { .on_action(cx.listener(|this, _: &FocusNext, window, cx| this.focus_region(true, window, cx))) .on_action(cx.listener(|this, _: &FocusPrevious, window, cx| this.focus_region(false, window, cx))) .on_key_down(cx.listener(Self::typed)) - .on_action(cx.listener(|this, _: &CloseRight, _, cx| this.close_right(cx))) + .on_action(cx.listener(|this, _: &CloseRight, window, cx| this.close_right(Some(window), cx))) + .on_action(cx.listener(|this, _: &SearchConversation, window, cx| this.search_here(window, cx))) + .on_action(cx.listener(|this, _: &SearchEverywhere, window, cx| this.search_everywhere(window, cx))) .on_action(cx.listener(|this, _: &CloseTop, window, cx| { // With no layer to close, Escape belongs to the conversation. if !this.close_top(window, cx) { diff --git a/crates/tinyclient/src/gui/pane.rs b/crates/tinyclient/src/gui/pane.rs index 667e9f2..b3c7063 100644 --- a/crates/tinyclient/src/gui/pane.rs +++ b/crates/tinyclient/src/gui/pane.rs @@ -37,8 +37,8 @@ pub enum Layer { Pinned(Key), /// A Space's own details, opened from its name in the sidebar. Space(Key), - /// Search results, which never follow. - #[allow(dead_code)] + /// The search pane: the field, its scopes, the cache's hits. It never follows a + /// conversation switch, for it is how the user moves. Search, } diff --git a/crates/tinyclient/src/gui/people.rs b/crates/tinyclient/src/gui/people.rs index 7a0b13a..6c5d966 100644 --- a/crates/tinyclient/src/gui/people.rs +++ b/crates/tinyclient/src/gui/people.rs @@ -284,7 +284,7 @@ impl Shell { Some(peer) if self.right_kind() != Some(RightKind::Info) || shown.as_ref() != Some(&peer) => { self.show_info(peer, cx); } - _ => self.close_right(cx), + _ => self.close_right(None, cx), } } @@ -610,7 +610,7 @@ impl Shell { /// Shows or hides the member list of the open conversation in the right pane. pub(super) fn toggle_members(&mut self, cx: &mut Context) { if self.right_kind() == Some(RightKind::Members) { - self.close_right(cx); + self.close_right(None, cx); return; } let count = self.people_state.members.get().and_then(|members| members.read(cx).total()); diff --git a/crates/tinyclient/src/gui/settings/rows.rs b/crates/tinyclient/src/gui/settings/rows.rs index 91b217c..e260285 100644 --- a/crates/tinyclient/src/gui/settings/rows.rs +++ b/crates/tinyclient/src/gui/settings/rows.rs @@ -206,6 +206,8 @@ pub const SHORTCUTS: &[Shortcut] = &[ key("Edit", "Copy", "⌘C"), key("Edit", "Paste", "⌘V"), key("Edit", "Select All", "⌘A"), + key("Edit", "Search in Conversation", "⌘F"), + key("Edit", "Search Everywhere", "⇧⌘F"), key("View", "Inbox", "⌘1"), key("View", "People", "⌘2"), key("View", "Quick Switcher", "⌘K"), @@ -318,13 +320,18 @@ mod tests { fn every_shortcut_names_a_menu_item_that_exists_and_every_keyed_item_is_listed() { use crate::gui::app::menubar::menus; use gpui::MenuItem; + // A submenu's items belong to their parent menu, as they are drawn. + fn names(item: &MenuItem) -> impl Iterator { + match item { + MenuItem::Action { name, .. } => Box::new(std::iter::once(name.as_ref())) as Box>, + MenuItem::Submenu(menu) => Box::new(menu.items.iter().flat_map(names)) as Box>, + _ => Box::new(std::iter::empty()) as Box>, + } + } let bar = menus(&[], None, "Check for Updates…"); for shortcut in SHORTCUTS.iter().filter(|s| s.menu != "Calls") { let menu = bar.iter().find(|menu| menu.name.as_ref() == shortcut.menu).unwrap_or_else(|| panic!("no {} menu", shortcut.menu)); - let found = menu.items.iter().any(|item| match item { - MenuItem::Action { name, .. } => name.as_ref() == shortcut.command, - _ => false, - }); + let found = menu.items.iter().any(|item| names(item).any(|name| name == shortcut.command)); assert!(found, "{} is not in {}", shortcut.command, shortcut.menu); } // The menu items bound in `menubar::bind_keys` and `gui::bind_keys`. diff --git a/crates/tinyclient/src/gui/sidebar/switcher.rs b/crates/tinyclient/src/gui/sidebar/switcher.rs index 5248e96..fd21f02 100644 --- a/crates/tinyclient/src/gui/sidebar/switcher.rs +++ b/crates/tinyclient/src/gui/sidebar/switcher.rs @@ -495,7 +495,7 @@ impl Shell { Target::Person(person) => self.open_person(&person, cx), Target::Search(words) => { if let Some(search) = self.slots.search { - search(self, words, cx); + search(self, words, window, cx); } } Target::Asking(_) => {} diff --git a/crates/tinyclient/src/main.rs b/crates/tinyclient/src/main.rs index d6abdf2..9fa3dcc 100644 --- a/crates/tinyclient/src/main.rs +++ b/crates/tinyclient/src/main.rs @@ -65,7 +65,7 @@ fn print_status() -> io::Result<()> { /// The shell without a window: what a search of the cache finds, as lines. fn print_search(query: &str) -> io::Result<()> { - let request = Request::SearchMessages { conversation: None, network: None, query: query.to_owned(), limit: PAGE_LIMIT }; + let request = Request::SearchMessages { conversation: None, network: None, space: None, query: query.to_owned(), limit: PAGE_LIMIT }; let hits = match ask(&request)? { Response::MessageHits { hits } => hits, other => return Err(unexpected(other)), diff --git a/docs/architecture.md b/docs/architecture.md index fd281ef..601d6e1 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -37,7 +37,7 @@ How the processes fit together and who may edit what. The contract itself is the | `Threads {space, offset, limit}`, `Topics {forum, offset, limit}` | `Conversations` | store; `Command::OpenThreads` at offset 0 | | `InVoice {network}`, `Around {network}` | `Conversations`; `Around {requests, in_call, online, recent}` | hub (occupants, presence, activity) and store | | `Find {network, query, limit}` | `Conversations` (title contains, newest first, cache only) | store | -| `SearchMessages {conversation, network, query, limit}` | `MessageHits {hits (where, snippet, sender, title)}` (text contains, newest first, cache only) | store | +| `SearchMessages {conversation, network, space, query, limit}` | `MessageHits {hits (where, snippet, sender, title)}` (text contains, newest first, cache only) | store | | `Spaces {network}`, `Tree {space, offset, limit, peek}` | `Spaces`; `Tree {groups (kind, count), conversations, total}` | store; `Command::OpenSpace` | | `Conversation`, `Collapse`, `Star`, `SetDraft` | `Conversation`; `Ok` | store (local only) | | `People {network, relation, query, offset, limit}` | `People {people, total}` | store; `Command::SearchPeople`, debounced |