Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 98 additions & 12 deletions codex-rs/tui/src/bottom_pane/chat_composer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -738,14 +738,27 @@ impl ChatComposer {

pub fn set_skill_mentions(&mut self, skills: Option<Vec<SkillMetadata>>) {
self.skills = skills;
self.refresh_mentions_v2_popup_candidates();
self.sync_popups();
}

pub fn set_plugin_mentions(&mut self, plugins: Option<Vec<PluginCapabilitySummary>>) {
self.plugins = plugins;
self.refresh_mentions_v2_popup_candidates();
self.sync_popups();
}

/// Refreshes an open mention catalog when skill or plugin metadata changes.
fn refresh_mentions_v2_popup_candidates(&mut self) {
let ActivePopup::MentionV2(popup) = &mut self.popups.active else {
return;
};
popup.set_candidates(super::mentions_v2::build_search_catalog(
self.skills.as_deref(),
self.plugins.as_deref(),
));
}

pub fn set_plugins_command_enabled(&mut self, enabled: bool) {
self.plugins_command_enabled = enabled;
}
Expand Down Expand Up @@ -3985,25 +3998,31 @@ impl ChatComposer {
.send(AppEvent::StartFileSearch(String::new()));
self.popups.current_file_query = None;
} else {
self.app_event_tx
.send(AppEvent::StartFileSearch(query.clone()));
self.popups.current_file_query = Some(query.clone());
let new_popup = !matches!(self.popups.active, ActivePopup::MentionV2(_));
if new_popup {
// A fresh popup has no cached matches, and the app-owned file-search manager can
// retain an identical query. Reset it before issuing the query so results arrive.
self.app_event_tx
.send(AppEvent::StartFileSearch(String::new()));
}
if new_popup || self.popups.current_file_query.as_deref() != Some(query.as_str()) {
self.app_event_tx
.send(AppEvent::StartFileSearch(query.clone()));
self.popups.current_file_query = Some(query.clone());
}
}

let candidates = super::mentions_v2::build_search_catalog(
self.skills.as_deref(),
self.plugins.as_deref(),
);

match &mut self.popups.active {
ActivePopup::MentionV2(popup) => {
popup.set_query(&query);
popup.set_candidates(candidates);
}
_ => {
let mut popup = MentionV2Popup::new(candidates);
popup.set_query(&query);
self.popups.active = ActivePopup::MentionV2(popup);
let candidates = super::mentions_v2::build_search_catalog(
self.skills.as_deref(),
self.plugins.as_deref(),
);
self.popups.active =
ActivePopup::MentionV2(MentionV2Popup::new(candidates, &query));
}
}

Expand Down Expand Up @@ -7428,6 +7447,73 @@ mod tests {
);
}

#[test]
fn bare_unified_mention_resets_an_inherited_file_search() {
let (mut composer, mut rx) = new_test_composer();
composer.set_mentions_v2_enabled(/*enabled*/ true);

composer.insert_str("@");
assert!(matches!(
rx.try_recv(),
Ok(AppEvent::StartFileSearch(query)) if query.is_empty()
));

composer.insert_str("s");
assert!(matches!(
rx.try_recv(),
Ok(AppEvent::StartFileSearch(query)) if query == "s"
));
}

#[test]
fn reopening_identical_unified_mention_restarts_file_search() {
let (mut composer, mut rx) = new_test_composer();
composer.set_mentions_v2_enabled(/*enabled*/ true);
composer.set_text_content("@foo @foo".to_string(), Vec::new(), Vec::new());
composer.draft.textarea.set_cursor("@f".len());
composer.sync_popups();
assert!(matches!(
rx.try_recv(),
Ok(AppEvent::StartFileSearch(query)) if query.is_empty()
));
assert!(matches!(
rx.try_recv(),
Ok(AppEvent::StartFileSearch(query)) if query == "foo"
));

composer.on_file_search_result("foo".to_string(), Vec::new());
let _ = composer.handle_key_event(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE));
assert!(matches!(composer.popups.active, ActivePopup::None));

composer.draft.textarea.set_cursor("@foo @foo".len());
composer.sync_popups();
assert!(matches!(composer.popups.active, ActivePopup::MentionV2(_)));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Add snapshot coverage for refreshed mention results

When an identical mention popup is reopened, this test only verifies the popup variant and queued search events; it never delivers a fresh nonempty result or renders the resulting rows. The user-visible refresh behavior could therefore regress to stale or empty output while these tests continue passing. Add an insta snapshot that reopens or restores the popup, supplies refreshed matches, and verifies the rendered rows.

AGENTS.md reference: AGENTS.md:L180-L187

Useful? React with 👍 / 👎.

let queries = std::iter::from_fn(|| rx.try_recv().ok())
.filter_map(|event| match event {
AppEvent::StartFileSearch(query) => Some(query),
_ => None,
})
.collect::<Vec<_>>();
assert_eq!(queries, vec![String::new(), "foo".to_string()]);
}

#[test]
fn restored_unified_mention_restarts_file_search() {
let (mut composer, mut rx) = new_test_composer();
composer.set_mentions_v2_enabled(/*enabled*/ true);

composer.set_text_content("@foo".to_string(), Vec::new(), Vec::new());

assert!(matches!(composer.popups.active, ActivePopup::MentionV2(_)));
let queries = std::iter::from_fn(|| rx.try_recv().ok())
.filter_map(|event| match event {
AppEvent::StartFileSearch(query) => Some(query),
_ => None,
})
.collect::<Vec<_>>();
assert_eq!(queries, vec![String::new(), "foo".to_string()]);
}

#[test]
fn mention_popup_type_prefixes_snapshot() {
snapshot_composer_state_with_width(
Expand Down
56 changes: 31 additions & 25 deletions codex-rs/tui/src/bottom_pane/mentions_v2/popup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,84 +16,90 @@ pub(crate) struct Popup {
query: String,
file_search: FileSearch,
candidates: Vec<Candidate>,
rows: Vec<SearchResult>,
search_mode: SearchMode,
state: ScrollState,
}

impl Popup {
pub(crate) fn new(candidates: Vec<Candidate>) -> Self {
Self {
query: String::new(),
file_search: FileSearch::default(),
pub(crate) fn new(candidates: Vec<Candidate>, query: &str) -> Self {
let mut file_search = FileSearch::default();
file_search.set_query(query);
let mut popup = Self {
query: query.to_string(),
file_search,
candidates,
rows: Vec::new(),
search_mode: SearchMode::Results,
state: ScrollState::new(),
}
};
popup.refresh_rows();
popup
}

pub(crate) fn set_candidates(&mut self, candidates: Vec<Candidate>) {
self.candidates = candidates;
self.clamp_selection();
self.refresh_rows();
}

pub(crate) fn set_query(&mut self, query: &str) {
if self.query == query {
return;
}
self.query = query.to_string();
self.file_search.set_query(query);
self.clamp_selection();
self.refresh_rows();
}

pub(crate) fn set_file_matches(&mut self, query: &str, matches: Vec<FileMatch>) {
self.file_search.set_matches(query, matches);
self.clamp_selection();
self.refresh_rows();
}

pub(crate) fn selected(&self) -> Option<Selection> {
let rows = self.rows();
let idx = self.state.selected_idx?;
rows.get(idx).map(|row| row.selection.clone())
self.rows.get(idx).map(|row| row.selection.clone())
}

pub(crate) fn move_up(&mut self) {
let len = self.rows().len();
let len = self.rows.len();
self.state.move_up_wrap(len);
self.state.ensure_visible(len, MAX_POPUP_ROWS.min(len));
}

pub(crate) fn move_down(&mut self) {
let len = self.rows().len();
let len = self.rows.len();
self.state.move_down_wrap(len);
self.state.ensure_visible(len, MAX_POPUP_ROWS.min(len));
}

pub(crate) fn previous_search_mode(&mut self) {
self.search_mode = self.search_mode.previous();
self.clamp_selection();
self.refresh_rows();
}

pub(crate) fn next_search_mode(&mut self) {
self.search_mode = self.search_mode.next();
self.clamp_selection();
self.refresh_rows();
}

pub(crate) fn calculate_required_height(&self, _width: u16) -> u16 {
let visible = self.rows().len().clamp(1, MAX_POPUP_ROWS);
let visible = self.rows.len().clamp(1, MAX_POPUP_ROWS);
(visible as u16).saturating_add(2)
}

fn clamp_selection(&mut self) {
let len = self.rows().len();
self.state.clamp_selection(len);
self.state.ensure_visible(len, MAX_POPUP_ROWS.min(len));
}

fn rows(&self) -> Vec<SearchResult> {
filtered_candidates(
/// Rebuilds cached rows and keeps selection valid after search inputs change.
fn refresh_rows(&mut self) {
self.rows = filtered_candidates(
&self.candidates,
&self.file_search.matches,
&self.query,
self.search_mode,
self.file_search.should_show_matches(),
)
);
let len = self.rows.len();
self.state.clamp_selection(len);
self.state.ensure_visible(len, MAX_POPUP_ROWS.min(len));
}
}

Expand All @@ -102,7 +108,7 @@ impl WidgetRef for Popup {
render_popup(
area,
buf,
&self.rows(),
&self.rows,
&self.state,
self.file_search.empty_message(),
self.search_mode,
Expand Down
Loading