diff --git a/TASKS.md b/TASKS.md index a6bcd5c..ea91233 100644 --- a/TASKS.md +++ b/TASKS.md @@ -262,3 +262,4 @@ Per-component lines live in `tasks/`. Tags there: | T23b | 2 | FAIL | the gallery's narrow passage built a vec of one range, which clippy refuses, so the checks failed | | T23b | 3 | PASS | | | T23c | 1 | FAIL | suggestion arrows landed on disabled rows and left the cursor out of view; a dot ended a file mention; the drop veil never showed, since gpui styles a drag-over only on an element with a hitbox; a parameter's places came from its step alone; the gallery's prompt menu and Reset only logged | +| T23c | 2 | FAIL | a slash inside a path read as a new trigger, closing the file suggestions; a list whose first rows were disabled opened with its cursor out of view | diff --git a/src/chat/tests/compose.rs b/src/chat/tests/compose.rs index 21f15f4..03662e9 100644 --- a/src/chat/tests/compose.rs +++ b/src/chat/tests/compose.rs @@ -30,9 +30,13 @@ impl Render for Compose { .commands(self.commands.clone(), move |value, _, cx| { command.update(cx, |compose, _| compose.picked.push(value.clone())) }) - .context([Choice::new("lift", "lift.rs")], move |value, _, cx| { - mention.update(cx, |compose, _| compose.picked.push(value.clone())) - }) + .context( + [ + Choice::new("lift", "lift.rs"), + Choice::new("src", "src/lift.rs"), + ], + move |value, _, cx| mention.update(cx, |compose, _| compose.picked.push(value.clone())), + ) .on_drop(|_, _, _| {}); let input = if self.busy { input.busy(|_, _| {}) @@ -170,14 +174,19 @@ fn a_file_name_keeps_its_suggestions_past_a_dot(cx: &mut TestAppContext) { } #[gpui::test] -fn arrows_pass_disabled_rows_and_keep_the_cursor_in_view(cx: &mut TestAppContext) { - let (compose, cx) = open(false, numbered(&[1]), cx); - cx.simulate_input("/"); - frames(cx); - for _ in 0..16 { - cx.simulate_keystrokes("down"); - frames(cx); - } +fn a_path_keeps_its_suggestions_past_a_slash(cx: &mut TestAppContext) { + let (compose, cx) = open(false, summarize(), cx); + cx.simulate_input("see @src/"); + settle(cx); + cx.simulate_keystrokes("enter"); + settle(cx); + assert_eq!(text(&compose, cx), "see "); + assert_eq!(picked(&compose, cx), ["src"]); + assert_eq!(compose.read_with(cx, |compose, _| compose.sent), 0); +} + +/// Clicks near the foot of the command list that hangs under the slash. +fn click_list_foot(compose: &Entity, cx: &mut VisualTestContext) { let (anchor, height) = cx.update(|window, cx| { let anchor = compose.read(cx).field.read(cx).bounds_for(0); let height = cx.theme().list_max_height().to_pixels(window.rem_size()); @@ -189,6 +198,32 @@ fn arrows_pass_disabled_rows_and_keep_the_cursor_in_view(cx: &mut TestAppContext ); cx.simulate_mouse_move(foot, None, Modifiers::none()); cx.simulate_click(foot, Modifiers::none()); +} + +#[gpui::test] +fn the_list_opens_on_its_first_enabled_row_in_view(cx: &mut TestAppContext) { + let disabled: Vec = (0..17).collect(); + let (compose, cx) = open(false, numbered(&disabled), cx); + cx.simulate_input("/"); + frames(cx); + click_list_foot(&compose, cx); + assert_eq!( + picked(&compose, cx), + ["c17"], + "the list opens scrolled to its cursor" + ); +} + +#[gpui::test] +fn arrows_pass_disabled_rows_and_keep_the_cursor_in_view(cx: &mut TestAppContext) { + let (compose, cx) = open(false, numbered(&[1]), cx); + cx.simulate_input("/"); + frames(cx); + for _ in 0..16 { + cx.simulate_keystrokes("down"); + frames(cx); + } + click_list_foot(&compose, cx); assert_eq!( picked(&compose, cx), ["c17"], diff --git a/src/forms/combobox.rs b/src/forms/combobox.rs index 492432a..c65ef12 100644 --- a/src/forms/combobox.rs +++ b/src/forms/combobox.rs @@ -256,6 +256,7 @@ impl RenderOnce for Combobox { pick, dismiss: None, scroll: Some(&scroll), + reveal: None, } .render(window, cx), ) diff --git a/src/forms/masked.rs b/src/forms/masked.rs index 1261499..8eecf04 100644 --- a/src/forms/masked.rs +++ b/src/forms/masked.rs @@ -303,6 +303,7 @@ impl RenderOnce for PhoneInput { }) })), scroll: None, + reveal: None, } .render(window, cx), ) diff --git a/src/forms/mention.rs b/src/forms/mention.rs index a719363..89a2e52 100644 --- a/src/forms/mention.rs +++ b/src/forms/mention.rs @@ -7,7 +7,7 @@ use gpui::{ use super::{ Highlight, Input, TextInput, - options::{Choice, Pick, Popup, step}, + options::{Choice, Pick, Popup, Run, step}, text::{Down, Enter, Up}, }; use crate::theme::ActiveTheme; @@ -19,7 +19,7 @@ pub(crate) fn one_word(ch: char) -> bool { ch.is_alphanumeric() || ch == '_' } -/// The trigger being typed, its query all `word` characters: its byte offset and char, ending at `caret`. +/// The trigger that opens the word ending at `caret`, its query all `word` characters: its byte offset and char. pub(crate) fn active_trigger( text: &str, caret: usize, @@ -27,17 +27,14 @@ pub(crate) fn active_trigger( word: fn(char) -> bool, ) -> Option<(usize, char)> { let before = &text[..caret]; - let (ix, ch) = before + let start = before .char_indices() .rev() - .take_while(|(_, ch)| !ch.is_whitespace()) - .find(|(_, ch)| triggers.contains(ch))?; - let opens = before[..ix] - .chars() - .next_back() - .is_none_or(char::is_whitespace); - let query = before[ix + ch.len_utf8()..].chars().all(word); - (opens && query).then_some((ix, ch)) + .find(|(_, ch)| ch.is_whitespace()) + .map_or(0, |(ix, ch)| ix + ch.len_utf8()); + let mut chars = before[start..].chars(); + let trigger = chars.next().filter(|ch| triggers.contains(ch))?; + chars.all(word).then_some((start, trigger)) } /// Replaces the trigger and its query, `at..caret`, with `text`, as one undo step. @@ -107,6 +104,7 @@ struct Picking { highlighted: usize, dismissed: Option, scroll: ScrollHandle, + revealed: Option, } /// The row the cursor rests on: `at` kept in range and off disabled rows. @@ -118,16 +116,15 @@ fn usable(rows: &[Choice], at: usize) -> usize { } } -/// Moves the cursor `by` rows past disabled ones and scrolls it into view. +/// Moves the cursor `by` rows past disabled ones. fn move_cursor(picking: &Entity, rows: &[Choice], by: isize, cx: &mut App) { picking.update(cx, |picking, cx| { picking.highlighted = step(rows, picking.highlighted, by); - picking.scroll.scroll_to_item(picking.highlighted); cx.notify(); }); } -/// Rows offered at a trigger in a field's text, and what a pick does: arrows choose past disabled rows, Enter or a press picks, Escape dismisses until the trigger ends. +/// Rows offered at a trigger in a field's text, and what a pick does: arrows choose past disabled rows, the cursor stays in view, Enter or a press picks, Escape dismisses until the trigger ends. pub(crate) struct Suggestions { pub id: ElementId, pub state: Entity, @@ -152,10 +149,23 @@ impl Suggestions { && !self.rows.is_empty() && self.trigger.is_some_and(|ix| dismissed != Some(ix)); let rows: Rc<[Choice]> = self.rows.into(); + let shown = open && anchor.is_some(); let highlighted = usable(&rows, picking.read(cx).highlighted); - if highlighted != picking.read(cx).highlighted { - picking.update(cx, |picking, _| picking.highlighted = highlighted); + let revealed = picking.read(cx).revealed.filter(|_| shown); + let stored = (picking.read(cx).highlighted, picking.read(cx).revealed); + if (highlighted, revealed) != stored { + picking.update(cx, |picking, _| { + picking.highlighted = highlighted; + picking.revealed = revealed; + }); } + let reveal = (shown && revealed != Some(highlighted)).then(|| { + let done = picking.clone(); + let mark: Run = Rc::new(move |_, cx| { + done.update(cx, |picking, _| picking.revealed = Some(highlighted)) + }); + (highlighted, mark) + }); let scroll = picking.read(cx).scroll.clone(); log::debug!( "suggestions {:?}: open {open}, {} rows, anchor {anchor:?}", @@ -214,6 +224,7 @@ impl Suggestions { pick: self.pick, dismiss: None, scroll: Some(&scroll), + reveal, } .render(window, cx), ) @@ -333,8 +344,15 @@ mod tests { assert_eq!(at("hi @ada lovelace"), None); assert_eq!(at("go #rel"), Some((3, '#'))); assert_eq!(at("see @lift."), None, "a handle is one word"); - let file = active_trigger("see @lift.rs", 12, &marks, |ch| !ch.is_whitespace()); - assert_eq!(file, Some((4, '@')), "a file name holds dots"); + let file = + |text: &str| active_trigger(text, text.len(), &['/', '@'], |ch| !ch.is_whitespace()); + assert_eq!( + file("see @lift.rs"), + Some((4, '@')), + "a file name holds dots" + ); + assert_eq!(file("see @src/"), Some((4, '@')), "a path holds slashes"); + assert_eq!(file("/usr/bin"), Some((0, '/'))); } #[test] diff --git a/src/forms/multi.rs b/src/forms/multi.rs index 1578b76..3856386 100644 --- a/src/forms/multi.rs +++ b/src/forms/multi.rs @@ -222,6 +222,7 @@ impl RenderOnce for MultiSelect { Picker::show(&close, false, at, cx) })), scroll: Some(&scroll), + reveal: None, } .render(window, cx), ) diff --git a/src/forms/options.rs b/src/forms/options.rs index 693a379..571cf5d 100644 --- a/src/forms/options.rs +++ b/src/forms/options.rs @@ -3,7 +3,7 @@ use std::rc::Rc; use gpui::{ Animation, AnimationExt, AnyElement, App, Bounds, Corner, Div, ElementId, InteractiveElement, IntoElement, MouseButton, ParentElement, Pixels, ScrollHandle, SharedString, Stateful, - StatefulInteractiveElement, Styled, Window, anchored, deferred, div, prelude::*, + StatefulInteractiveElement, Styled, Window, anchored, canvas, deferred, div, prelude::*, }; use crate::{ @@ -221,6 +221,8 @@ pub(crate) struct Popup<'a> { pub pick: Pick, pub dismiss: Option, pub scroll: Option<&'a ScrollHandle>, + /// A row to scroll into view once the list has its bounds, and what marks it done. + pub reveal: Option<(usize, Run)>, } impl Popup<'_> { @@ -259,7 +261,24 @@ impl Popup<'_> { .when_some(self.dismiss, |list, dismiss| { list.on_mouse_down_out(move |_, window, cx| dismiss(window, cx)) }) - .children(rows); + .children(rows) + .when_some( + self.scroll.zip(self.reveal), + |list, (handle, (ix, done))| { + let handle = handle.clone(); + list.child( + canvas( + move |_, window, cx| { + handle.scroll_to_item(ix); + window.request_animation_frame(); + done(window, cx); + }, + |_, _, _, _| {}, + ) + .absolute(), + ) + }, + ); float(anchor, self.rows.len(), list, window, cx) } } diff --git a/src/forms/search.rs b/src/forms/search.rs index b605bf6..eef13b6 100644 --- a/src/forms/search.rs +++ b/src/forms/search.rs @@ -113,6 +113,7 @@ impl RenderOnce for SearchInput { pick, dismiss: None, scroll: None, + reveal: None, } .render(window, cx), ) diff --git a/src/forms/select.rs b/src/forms/select.rs index 7b2874a..9631cd8 100644 --- a/src/forms/select.rs +++ b/src/forms/select.rs @@ -224,6 +224,7 @@ pub(crate) fn listing( Picker::show(&close, false, at, cx) })), scroll: Some(&scroll), + reveal: None, } .render(window, cx), )