mirror of
https://github.com/openai/codex.git
synced 2026-10-20 17:32:37 +00:00
Keep unified mention results fresh (#35365)
## What changed - Restart file search whenever a unified mention popup opens so restored or repeated queries receive results instead of inheriting stale search state. - Cache the popup's filtered rows and refresh them when the query, file matches, search mode, skills, or plugins change. ## Testing - Add regression tests for bare, reopened, and restored unified mention searches. GitOrigin-RevId: 9d38438fade01a0552e03cb05e023ee64af60a4c
This commit is contained in:
@@ -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;
|
||||
}
|
||||
@@ -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));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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(_)));
|
||||
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(
|
||||
|
||||
@@ -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));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user