History, is history

Conrad Irwin created

Change summary

assets/keymaps/default-linux.json          |   2 
assets/keymaps/default-macos.json          |   2 
crates/agent_ui/src/acp.rs                 |   2 
crates/agent_ui/src/acp/message_editor.rs  | 219 -----------------------
crates/agent_ui/src/acp/message_history.rs |  87 ---------
crates/agent_ui/src/acp/thread_view.rs     |  27 --
crates/agent_ui/src/agent_panel.rs         |  16 -
crates/zed_actions/src/lib.rs              |   4 
8 files changed, 13 insertions(+), 346 deletions(-)

Detailed changes

assets/keymaps/default-linux.json 🔗

@@ -331,8 +331,6 @@
     "use_key_equivalents": true,
     "bindings": {
       "enter": "agent::Chat",
-      "up": "agent::PreviousHistoryMessage",
-      "down": "agent::NextHistoryMessage",
       "shift-ctrl-r": "agent::OpenAgentDiff",
       "ctrl-shift-y": "agent::KeepAll",
       "ctrl-shift-n": "agent::RejectAll"

assets/keymaps/default-macos.json 🔗

@@ -383,8 +383,6 @@
     "use_key_equivalents": true,
     "bindings": {
       "enter": "agent::Chat",
-      "up": "agent::PreviousHistoryMessage",
-      "down": "agent::NextHistoryMessage",
       "shift-ctrl-r": "agent::OpenAgentDiff",
       "cmd-shift-y": "agent::KeepAll",
       "cmd-shift-n": "agent::RejectAll"

crates/agent_ui/src/acp.rs 🔗

@@ -1,11 +1,9 @@
 mod completion_provider;
 mod message_editor;
-mod message_history;
 mod model_selector;
 mod model_selector_popover;
 mod thread_view;
 
-pub use message_history::MessageHistory;
 pub use model_selector::AcpModelSelector;
 pub use model_selector_popover::AcpModelSelectorPopover;
 pub use thread_view::AcpThreadView;

crates/agent_ui/src/acp/message_editor.rs 🔗

@@ -1,5 +1,5 @@
 use crate::acp::completion_provider::ContextPickerCompletionProvider;
-use crate::acp::{MessageHistory, completion_provider::MentionSet};
+use crate::acp::completion_provider::MentionSet;
 use acp_thread::MentionUri;
 use agent_client_protocol as acp;
 use anyhow::Result;
@@ -10,8 +10,7 @@ use editor::{
 };
 use file_icons::FileIcons;
 use gpui::{
-    AppContext, Context, Entity, EventEmitter, FocusHandle, Focusable, Subscription, Task,
-    TextStyle, WeakEntity,
+    AppContext, Context, Entity, EventEmitter, FocusHandle, Focusable, Task, TextStyle, WeakEntity,
 };
 use language::Language;
 use language::{Buffer, BufferSnapshot};
@@ -20,7 +19,7 @@ use project::{CompletionIntent, Project};
 use settings::Settings;
 use std::path::Path;
 use std::rc::Rc;
-use std::{cell::RefCell, sync::Arc};
+use std::sync::Arc;
 use theme::ThemeSettings;
 use ui::{
     ActiveTheme, App, IconName, InteractiveElement, IntoElement, ParentElement, Render,
@@ -28,7 +27,7 @@ use ui::{
 };
 use util::ResultExt;
 use workspace::Workspace;
-use zed_actions::agent::{Chat, NextHistoryMessage, PreviousHistoryMessage};
+use zed_actions::agent::Chat;
 
 pub const MIN_EDITOR_LINES: usize = 4;
 pub const MAX_EDITOR_LINES: usize = 8;
@@ -37,9 +36,6 @@ pub struct MessageEditor {
     editor: Entity<Editor>,
     project: Entity<Project>,
     mention_set: Arc<Mutex<MentionSet>>,
-    history: Rc<RefCell<MessageHistory<Vec<acp::ContentBlock>>>>,
-    message_set_from_history: Option<BufferSnapshot>,
-    _subscription: Subscription,
 }
 
 pub enum MessageEditorEvent {
@@ -52,7 +48,6 @@ impl MessageEditor {
     pub fn new(
         workspace: WeakEntity<Workspace>,
         project: Entity<Project>,
-        history: Rc<RefCell<MessageHistory<Vec<acp::ContentBlock>>>>,
         window: &mut Window,
         cx: &mut Context<Self>,
     ) -> Self {
@@ -95,35 +90,11 @@ impl MessageEditor {
             });
             editor
         });
-        let message_editor_subscription = cx.subscribe(&editor, |this, editor, event, cx| {
-            if let editor::EditorEvent::BufferEdited = &event {
-                let buffer = editor
-                    .read(cx)
-                    .buffer()
-                    .read(cx)
-                    .as_singleton()
-                    .unwrap()
-                    .read(cx)
-                    .snapshot();
-                if let Some(message) = this.message_set_from_history.clone()
-                    && message.version() != buffer.version()
-                {
-                    this.message_set_from_history = None;
-                }
-
-                if this.message_set_from_history.is_none() {
-                    this.history.borrow_mut().reset_position();
-                }
-            }
-        });
 
         Self {
             editor,
             project,
             mention_set,
-            history,
-            message_set_from_history: None,
-            _subscription: message_editor_subscription,
         }
     }
 
@@ -266,69 +237,7 @@ impl MessageEditor {
         });
     }
 
-    fn previous_history_message(
-        &mut self,
-        _: &PreviousHistoryMessage,
-        window: &mut Window,
-        cx: &mut Context<Self>,
-    ) {
-        if self.message_set_from_history.is_none() && !self.editor.read(cx).is_empty(cx) {
-            self.editor.update(cx, |editor, cx| {
-                editor.move_up(&Default::default(), window, cx);
-            });
-            return;
-        }
-
-        self.message_set_from_history = Self::set_draft_message(
-            self.editor.clone(),
-            self.mention_set.clone(),
-            self.project.clone(),
-            self.history
-                .borrow_mut()
-                .prev()
-                .map(|blocks| blocks.as_slice()),
-            window,
-            cx,
-        );
-    }
-
-    fn next_history_message(
-        &mut self,
-        _: &NextHistoryMessage,
-        window: &mut Window,
-        cx: &mut Context<Self>,
-    ) {
-        if self.message_set_from_history.is_none() {
-            self.editor.update(cx, |editor, cx| {
-                editor.move_down(&Default::default(), window, cx);
-            });
-            return;
-        }
-
-        let mut history = self.history.borrow_mut();
-        let next_history = history.next();
-
-        let set_draft_message = Self::set_draft_message(
-            self.editor.clone(),
-            self.mention_set.clone(),
-            self.project.clone(),
-            Some(
-                next_history
-                    .map(|blocks| blocks.as_slice())
-                    .unwrap_or_else(|| &[]),
-            ),
-            window,
-            cx,
-        );
-        // If we reset the text to an empty string because we ran out of history,
-        // we don't want to mark it as coming from the history
-        self.message_set_from_history = if next_history.is_some() {
-            set_draft_message
-        } else {
-            None
-        };
-    }
-
+    #[allow(unused)]
     fn set_draft_message(
         message_editor: Entity<Editor>,
         mention_set: Arc<Mutex<MentionSet>>,
@@ -437,8 +346,6 @@ impl Render for MessageEditor {
     fn render(&mut self, _window: &mut Window, cx: &mut Context<Self>) -> impl IntoElement {
         div()
             .key_context("MessageEditor")
-            .on_action(cx.listener(Self::previous_history_message))
-            .on_action(cx.listener(Self::next_history_message))
             .on_action(cx.listener(Self::chat))
             .flex_1()
             .child({
@@ -474,125 +381,23 @@ impl Render for MessageEditor {
 
 #[cfg(test)]
 mod tests {
-    use std::{cell::RefCell, path::Path, rc::Rc};
+    use std::path::Path;
 
     use agent_client_protocol as acp;
     use fs::FakeFs;
     use gpui::{AppContext, TestAppContext};
     use lsp::{CompletionContext, CompletionTriggerKind};
-    use pretty_assertions::assert_matches;
     use project::{CompletionIntent, Project};
     use serde_json::json;
     use util::path;
     use workspace::Workspace;
 
-    use crate::acp::{
-        MessageHistory, message_editor::MessageEditor, thread_view::tests::init_test,
-    };
-
-    #[gpui::test]
-    async fn test_at_mention_history(cx: &mut TestAppContext) {
-        init_test(cx);
-
-        let history = Rc::new(RefCell::new(MessageHistory::default()));
-        let fs = FakeFs::new(cx.executor());
-        fs.insert_tree("/project", json!({"file": ""})).await;
-        let project = Project::test(fs, [Path::new(path!("/project"))], cx).await;
-
-        let (workspace, cx) =
-            cx.add_window_view(|window, cx| Workspace::test_new(project.clone(), window, cx));
-
-        let message_editor = cx.update(|window, cx| {
-            cx.new(|cx| {
-                MessageEditor::new(
-                    workspace.downgrade(),
-                    project.clone(),
-                    history.clone(),
-                    window,
-                    cx,
-                )
-            })
-        });
-        let editor = message_editor.update(cx, |message_editor, _| message_editor.editor.clone());
-
-        cx.run_until_parked();
-
-        let excerpt_id = editor.update(cx, |editor, cx| {
-            editor
-                .buffer()
-                .read(cx)
-                .excerpt_ids()
-                .into_iter()
-                .next()
-                .unwrap()
-        });
-        let completions = editor.update_in(cx, |editor, window, cx| {
-            editor.set_text("Hello @", window, cx);
-            let buffer = editor.buffer().read(cx).as_singleton().unwrap();
-            let completion_provider = editor.completion_provider().unwrap();
-            completion_provider.completions(
-                excerpt_id,
-                &buffer,
-                text::Anchor::MAX,
-                CompletionContext {
-                    trigger_kind: CompletionTriggerKind::TRIGGER_CHARACTER,
-                    trigger_character: Some("@".into()),
-                },
-                window,
-                cx,
-            )
-        });
-        let [_, completion]: [_; 2] = completions
-            .await
-            .unwrap()
-            .into_iter()
-            .flat_map(|response| response.completions)
-            .collect::<Vec<_>>()
-            .try_into()
-            .unwrap();
-
-        editor.update_in(cx, |editor, window, cx| {
-            let snapshot = editor.buffer().read(cx).snapshot(cx);
-            let start = snapshot
-                .anchor_in_excerpt(excerpt_id, completion.replace_range.start)
-                .unwrap();
-            let end = snapshot
-                .anchor_in_excerpt(excerpt_id, completion.replace_range.end)
-                .unwrap();
-            editor.edit([(start..end, completion.new_text)], cx);
-            (completion.confirm.unwrap())(CompletionIntent::Complete, window, cx);
-        });
-
-        cx.run_until_parked();
-
-        let content = message_editor
-            .update(cx, |message_editor, cx| message_editor.contents(cx))
-            .await
-            .unwrap();
-        assert_eq!(content.len(), 2);
-        assert_matches!(&content[0], &acp::ContentBlock::Text(_));
-        assert_matches!(&content[1], &acp::ContentBlock::Resource(_));
-
-        history.borrow_mut().push(content);
-        message_editor.update_in(cx, |message_editor, window, cx| {
-            message_editor.clear(window, cx);
-            message_editor.previous_history_message(&Default::default(), window, cx);
-        });
-
-        let content = message_editor
-            .update(cx, |message_editor, cx| message_editor.contents(cx))
-            .await
-            .unwrap();
-        assert_eq!(content.len(), 2);
-        assert_matches!(&content[0], &acp::ContentBlock::Text(_));
-        assert_matches!(&content[1], &acp::ContentBlock::Resource(_));
-    }
+    use crate::acp::{message_editor::MessageEditor, thread_view::tests::init_test};
 
     #[gpui::test]
     async fn test_at_mention_removal(cx: &mut TestAppContext) {
         init_test(cx);
 
-        let history = Rc::new(RefCell::new(MessageHistory::default()));
         let fs = FakeFs::new(cx.executor());
         fs.insert_tree("/project", json!({"file": ""})).await;
         let project = Project::test(fs, [Path::new(path!("/project"))], cx).await;
@@ -601,15 +406,7 @@ mod tests {
             cx.add_window_view(|window, cx| Workspace::test_new(project.clone(), window, cx));
 
         let message_editor = cx.update(|window, cx| {
-            cx.new(|cx| {
-                MessageEditor::new(
-                    workspace.downgrade(),
-                    project.clone(),
-                    history.clone(),
-                    window,
-                    cx,
-                )
-            })
+            cx.new(|cx| MessageEditor::new(workspace.downgrade(), project.clone(), window, cx))
         });
         let editor = message_editor.update(cx, |message_editor, _| message_editor.editor.clone());
 

crates/agent_ui/src/acp/message_history.rs 🔗

@@ -1,87 +0,0 @@
-pub struct MessageHistory<T> {
-    items: Vec<T>,
-    current: Option<usize>,
-}
-
-impl<T> Default for MessageHistory<T> {
-    fn default() -> Self {
-        MessageHistory {
-            items: Vec::new(),
-            current: None,
-        }
-    }
-}
-
-impl<T> MessageHistory<T> {
-    pub fn push(&mut self, message: T) {
-        self.current.take();
-        self.items.push(message);
-    }
-
-    pub fn reset_position(&mut self) {
-        self.current.take();
-    }
-
-    pub fn prev(&mut self) -> Option<&T> {
-        if self.items.is_empty() {
-            return None;
-        }
-
-        let new_ix = self
-            .current
-            .get_or_insert(self.items.len())
-            .saturating_sub(1);
-
-        self.current = Some(new_ix);
-        self.items.get(new_ix)
-    }
-
-    pub fn next(&mut self) -> Option<&T> {
-        let current = self.current.as_mut()?;
-        *current += 1;
-
-        self.items.get(*current).or_else(|| {
-            self.current.take();
-            None
-        })
-    }
-}
-#[cfg(test)]
-mod tests {
-    use super::*;
-
-    #[test]
-    fn test_prev_next() {
-        let mut history = MessageHistory::default();
-
-        // Test empty history
-        assert_eq!(history.prev(), None);
-        assert_eq!(history.next(), None);
-
-        // Add some messages
-        history.push("first");
-        history.push("second");
-        history.push("third");
-
-        // Test prev navigation
-        assert_eq!(history.prev(), Some(&"third"));
-        assert_eq!(history.prev(), Some(&"second"));
-        assert_eq!(history.prev(), Some(&"first"));
-        assert_eq!(history.prev(), Some(&"first"));
-
-        assert_eq!(history.next(), Some(&"second"));
-
-        // Test mixed navigation
-        history.push("fourth");
-        assert_eq!(history.prev(), Some(&"fourth"));
-        assert_eq!(history.prev(), Some(&"third"));
-        assert_eq!(history.next(), Some(&"fourth"));
-        assert_eq!(history.next(), None);
-
-        // Test that push resets navigation
-        history.prev();
-        history.prev();
-        history.push("fifth");
-        assert_eq!(history.prev(), Some(&"fifth"));
-    }
-}

crates/agent_ui/src/acp/thread_view.rs 🔗

@@ -25,7 +25,7 @@ use markdown::{HeadingLevelStyles, Markdown, MarkdownElement, MarkdownStyle};
 use project::Project;
 use rope::Point;
 use settings::{Settings as _, SettingsStore};
-use std::{cell::RefCell, collections::BTreeMap, process::ExitStatus, rc::Rc, time::Duration};
+use std::{collections::BTreeMap, process::ExitStatus, rc::Rc, time::Duration};
 use terminal_view::TerminalView;
 use text::Anchor;
 use theme::ThemeSettings;
@@ -39,7 +39,6 @@ use zed_actions::agent::{Chat, ToggleModelSelector};
 
 use crate::acp::AcpModelSelectorPopover;
 use crate::acp::message_editor::{MessageEditor, MessageEditorEvent};
-use crate::acp::message_history::MessageHistory;
 use crate::agent_diff::AgentDiff;
 use crate::ui::{AgentNotification, AgentNotificationEvent};
 use crate::{
@@ -69,7 +68,6 @@ pub struct AcpThreadView {
     plan_expanded: bool,
     editor_expanded: bool,
     terminal_expanded: bool,
-    message_history: Rc<RefCell<MessageHistory<Vec<acp::ContentBlock>>>>,
     _cancel_task: Option<Task<()>>,
     _subscriptions: [Subscription; 2],
 }
@@ -96,19 +94,11 @@ impl AcpThreadView {
         agent: Rc<dyn AgentServer>,
         workspace: WeakEntity<Workspace>,
         project: Entity<Project>,
-        message_history: Rc<RefCell<MessageHistory<Vec<acp::ContentBlock>>>>,
         window: &mut Window,
         cx: &mut Context<Self>,
     ) -> Self {
-        let message_editor = cx.new(|cx| {
-            MessageEditor::new(
-                workspace.clone(),
-                project.clone(),
-                message_history.clone(),
-                window,
-                cx,
-            )
-        });
+        let message_editor =
+            cx.new(|cx| MessageEditor::new(workspace.clone(), project.clone(), window, cx));
 
         let list_state = ListState::new(0, gpui::ListAlignment::Bottom, px(2048.0));
 
@@ -138,7 +128,6 @@ impl AcpThreadView {
             plan_expanded: false,
             editor_expanded: false,
             terminal_expanded: true,
-            message_history,
             _subscriptions: subscriptions,
             _cancel_task: None,
         }
@@ -348,7 +337,6 @@ impl AcpThreadView {
                 this.message_editor.update(cx, |message_editor, cx| {
                     message_editor.clear(window, cx);
                 });
-                this.message_history.borrow_mut().push(contents.clone());
             })?;
             let send = thread.update(cx, |thread, cx| thread.send(contents, cx))?;
             send.await
@@ -3212,14 +3200,7 @@ pub(crate) mod tests {
 
         let thread_view = cx.update(|window, cx| {
             cx.new(|cx| {
-                AcpThreadView::new(
-                    Rc::new(agent),
-                    workspace.downgrade(),
-                    project,
-                    Rc::new(RefCell::new(MessageHistory::default())),
-                    window,
-                    cx,
-                )
+                AcpThreadView::new(Rc::new(agent), workspace.downgrade(), project, window, cx)
             })
         });
         cx.run_until_parked();

crates/agent_ui/src/agent_panel.rs 🔗

@@ -1,4 +1,3 @@
-use std::cell::RefCell;
 use std::ops::{Not, Range};
 use std::path::Path;
 use std::rc::Rc;
@@ -476,8 +475,6 @@ pub struct AgentPanel {
     configuration_subscription: Option<Subscription>,
     local_timezone: UtcOffset,
     active_view: ActiveView,
-    acp_message_history:
-        Rc<RefCell<crate::acp::MessageHistory<Vec<agent_client_protocol::ContentBlock>>>>,
     previous_view: Option<ActiveView>,
     history_store: Entity<HistoryStore>,
     history: Entity<ThreadHistory>,
@@ -765,7 +762,6 @@ impl AgentPanel {
             .unwrap(),
             inline_assist_context_store,
             previous_view: None,
-            acp_message_history: Default::default(),
             history_store: history_store.clone(),
             history: cx.new(|cx| ThreadHistory::new(weak_self, history_store, window, cx)),
             hovered_recent_history_item: None,
@@ -962,7 +958,6 @@ impl AgentPanel {
     ) {
         let workspace = self.workspace.clone();
         let project = self.project.clone();
-        let message_history = self.acp_message_history.clone();
         let fs = self.fs.clone();
 
         const LAST_USED_EXTERNAL_AGENT_KEY: &str = "agent_panel__last_used_external_agent";
@@ -1006,14 +1001,7 @@ impl AgentPanel {
 
             this.update_in(cx, |this, window, cx| {
                 let thread_view = cx.new(|cx| {
-                    crate::acp::AcpThreadView::new(
-                        server,
-                        workspace.clone(),
-                        project,
-                        message_history,
-                        window,
-                        cx,
-                    )
+                    crate::acp::AcpThreadView::new(server, workspace.clone(), project, window, cx)
                 });
 
                 this.set_active_view(ActiveView::ExternalAgentThread { thread_view }, window, cx);
@@ -1567,8 +1555,6 @@ impl AgentPanel {
             self.active_view = new_view;
         }
 
-        self.acp_message_history.borrow_mut().reset_position();
-
         self.focus_handle(cx).focus(window);
     }
 

crates/zed_actions/src/lib.rs 🔗

@@ -285,10 +285,6 @@ pub mod agent {
             ResetOnboarding,
             /// Starts a chat conversation with the agent.
             Chat,
-            /// Displays the previous message in the history.
-            PreviousHistoryMessage,
-            /// Displays the next message in the history.
-            NextHistoryMessage,
             /// Toggles the language model selector dropdown.
             #[action(deprecated_aliases = ["assistant::ToggleModelSelector", "assistant2::ToggleModelSelector"])]
             ToggleModelSelector