Finely scope repl events for runs and output clearing (#13872)

Kyle Kelley and Kirill created

Sets up the `cmd-enter` keybinding for the jupyter repl to only apply
when enabled.

Release Notes:

- N/A

---------

Co-authored-by: Kirill <kirill@zed.dev>

Change summary

assets/keymaps/default-macos.json    |  13 ++
crates/editor/src/editor.rs          |   5 +
crates/editor/src/editor_settings.rs |  14 ++
crates/repl/Cargo.toml               |   1 
crates/repl/src/jupyter_settings.rs  |  25 ++--
crates/repl/src/kernels.rs           |   6 +
crates/repl/src/runtime_panel.rs     | 144 +++++++++++++++++------------
crates/repl/src/session.rs           |  77 +++++++++------
8 files changed, 184 insertions(+), 101 deletions(-)

Detailed changes

assets/keymaps/default-macos.json 🔗

@@ -171,7 +171,6 @@
       "enter": "editor::Newline",
       "shift-enter": "editor::Newline",
       "cmd-shift-enter": "editor::NewlineAbove",
-      "cmd-enter": "editor::NewlineBelow",
       "alt-z": "editor::ToggleSoftWrap",
       "cmd-f": "buffer_search::Deploy",
       "cmd-alt-f": [
@@ -197,6 +196,12 @@
       "cmd-alt-e": "editor::SelectEnclosingSymbol"
     }
   },
+  {
+    "context": "Editor && mode == full && !jupyter",
+    "bindings": {
+      "cmd-enter": "editor::NewlineBelow"
+    }
+  },
   {
     "context": "Editor && mode == full && inline_completion",
     "bindings": {
@@ -637,6 +642,12 @@
       "space": "project_panel::Open"
     }
   },
+  {
+    "context": "Editor && jupyter",
+    "bindings": {
+      "cmd-enter": "repl::Run"
+    }
+  },
   {
     "context": "CollabPanel && not_editing",
     "bindings": {

crates/editor/src/editor.rs 🔗

@@ -1927,6 +1927,11 @@ impl Editor {
             EditorMode::AutoHeight { .. } => "auto_height",
             EditorMode::Full => "full",
         };
+
+        if EditorSettings::get_global(cx).jupyter.enabled {
+            key_context.add("jupyter");
+        }
+
         key_context.set("mode", mode);
         if self.pending_rename.is_some() {
             key_context.add("renaming");

crates/editor/src/editor_settings.rs 🔗

@@ -25,6 +25,8 @@ pub struct EditorSettings {
     pub expand_excerpt_lines: u32,
     #[serde(default)]
     pub double_click_in_multibuffer: DoubleClickInMultibuffer,
+    #[serde(default)]
+    pub jupyter: Jupyter,
 }
 
 #[derive(Copy, Clone, Debug, Serialize, Deserialize, PartialEq, Eq, JsonSchema)]
@@ -64,6 +66,15 @@ pub enum DoubleClickInMultibuffer {
     Open,
 }
 
+#[derive(Default, Copy, Clone, Debug, Serialize, Deserialize, PartialEq, Eq, JsonSchema)]
+#[serde(rename_all = "snake_case")]
+pub struct Jupyter {
+    /// Whether the Jupyter feature is enabled.
+    ///
+    /// Default: `false`
+    pub enabled: bool,
+}
+
 #[derive(Copy, Clone, Debug, Serialize, Deserialize, JsonSchema, PartialEq, Eq)]
 pub struct Toolbar {
     pub breadcrumbs: bool,
@@ -217,6 +228,9 @@ pub struct EditorSettingsContent {
     ///
     /// Default: select
     pub double_click_in_multibuffer: Option<DoubleClickInMultibuffer>,
+
+    /// Jupyter REPL settings.
+    pub jupyter: Option<Jupyter>,
 }
 
 // Toolbar related settings

crates/repl/Cargo.toml 🔗

@@ -35,6 +35,7 @@ theme.workspace = true
 terminal_view.workspace = true
 ui.workspace = true
 uuid.workspace = true
+util.workspace = true
 workspace.workspace = true
 
 [dev-dependencies]

crates/repl/src/jupyter_settings.rs 🔗

@@ -1,5 +1,7 @@
 use std::collections::HashMap;
 
+use editor::EditorSettings;
+use gpui::AppContext;
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
 use settings::{Settings, SettingsSources};
@@ -16,18 +18,22 @@ pub enum JupyterDockPosition {
 
 #[derive(Debug, Default)]
 pub struct JupyterSettings {
-    pub enabled: bool,
     pub dock: JupyterDockPosition,
     pub default_width: Pixels,
     pub kernel_selections: HashMap<String, String>,
 }
 
+impl JupyterSettings {
+    pub fn enabled(cx: &AppContext) -> bool {
+        // In order to avoid a circular dependency between `editor` and `repl` crates,
+        // we put the `enable` flag on its settings.
+        // This allows the editor to set up context for key bindings/actions.
+        EditorSettings::get_global(cx).jupyter.enabled
+    }
+}
+
 #[derive(Clone, Serialize, Deserialize, JsonSchema, Debug)]
 pub struct JupyterSettingsContent {
-    /// Whether the Jupyter feature is enabled.
-    ///
-    /// Default: `false`
-    enabled: Option<bool>,
     /// Where to dock the Jupyter panel.
     ///
     /// Default: `right`
@@ -51,7 +57,6 @@ impl JupyterSettingsContent {
 impl Default for JupyterSettingsContent {
     fn default() -> Self {
         JupyterSettingsContent {
-            enabled: Some(false),
             dock: Some(JupyterDockPosition::Right),
             default_width: Some(640.0),
             kernel_selections: Some(HashMap::new()),
@@ -74,9 +79,6 @@ impl Settings for JupyterSettings {
         let mut settings = JupyterSettings::default();
 
         for value in sources.defaults_and_customizations() {
-            if let Some(enabled) = value.enabled {
-                settings.enabled = enabled;
-            }
             if let Some(dock) = value.dock {
                 settings.dock = dock;
             }
@@ -108,9 +110,10 @@ mod tests {
         let store = settings::SettingsStore::test(cx);
         cx.set_global(store);
 
+        EditorSettings::register(cx);
         JupyterSettings::register(cx);
 
-        assert_eq!(JupyterSettings::get_global(cx).enabled, false);
+        assert_eq!(JupyterSettings::enabled(cx), false);
         assert_eq!(
             JupyterSettings::get_global(cx).dock,
             JupyterDockPosition::Right
@@ -136,7 +139,7 @@ mod tests {
                 .unwrap();
         });
 
-        assert_eq!(JupyterSettings::get_global(cx).enabled, true);
+        assert_eq!(JupyterSettings::enabled(cx), true);
         assert_eq!(
             JupyterSettings::get_global(cx).dock,
             JupyterDockPosition::Left

crates/repl/src/kernels.rs 🔗

@@ -160,7 +160,11 @@ impl RunningKernel {
                 kernel_name: Some(format!("zed-{}", kernel_specification.name)),
             };
 
-            let connection_path = dirs::runtime_dir().join(format!("kernel-zed-{entity_id}.json"));
+            let runtime_dir = dirs::runtime_dir();
+            fs.create_dir(&runtime_dir)
+                .await
+                .with_context(|| format!("Failed to create jupyter runtime dir {runtime_dir:?}"))?;
+            let connection_path = runtime_dir.join(format!("kernel-zed-{entity_id}.json"));
             let content = serde_json::to_string(&connection_info)?;
             // write out file to disk for kernel
             fs.atomic_write(connection_path.clone(), content).await?;

crates/repl/src/runtime_panel.rs 🔗

@@ -6,31 +6,31 @@ use crate::{
 use anyhow::{Context as _, Result};
 use collections::HashMap;
 use editor::{Anchor, Editor, RangeToAnchorExt};
+use futures::StreamExt as _;
 use gpui::{
-    actions, prelude::*, AppContext, AsyncWindowContext, Entity, EntityId, EventEmitter,
-    FocusHandle, FocusOutEvent, FocusableView, Subscription, Task, View, WeakView,
+    actions, prelude::*, AppContext, AsyncWindowContext, EntityId, EventEmitter, FocusHandle,
+    FocusOutEvent, FocusableView, Subscription, Task, View, WeakView,
 };
 use language::Point;
 use project::Fs;
 use settings::{Settings as _, SettingsStore};
 use std::{ops::Range, sync::Arc};
 use ui::{prelude::*, ButtonLike, ElevationIndex, KeyBinding};
+use util::ResultExt as _;
 use workspace::{
     dock::{Panel, PanelEvent},
     Workspace,
 };
 
-actions!(repl, [Run, ToggleFocus, ClearOutputs]);
+actions!(repl, [Run, ClearOutputs]);
+actions!(repl_panel, [ToggleFocus]);
 
 pub fn init(cx: &mut AppContext) {
     cx.observe_new_views(
         |workspace: &mut Workspace, _cx: &mut ViewContext<Workspace>| {
-            workspace
-                .register_action(|workspace, _: &ToggleFocus, cx| {
-                    workspace.toggle_panel_focus::<RuntimePanel>(cx);
-                })
-                .register_action(run)
-                .register_action(clear_outputs);
+            workspace.register_action(|workspace, _: &ToggleFocus, cx| {
+                workspace.toggle_panel_focus::<RuntimePanel>(cx);
+            });
         },
     )
     .detach();
@@ -44,6 +44,12 @@ pub struct RuntimePanel {
     sessions: HashMap<EntityId, View<Session>>,
     kernel_specifications: Vec<KernelSpecification>,
     _subscriptions: Vec<Subscription>,
+    _editor_events_task: Task<()>,
+}
+
+pub enum ReplEvent {
+    Run(WeakView<Editor>),
+    ClearOutputs(WeakView<Editor>),
 }
 
 impl RuntimePanel {
@@ -58,26 +64,82 @@ impl RuntimePanel {
 
                     let fs = workspace.app_state().fs.clone();
 
+                    // Make a channel that we receive editor events on (for repl::Run, repl::ClearOutputs)
+                    // This allows us to inject actions on the editor from the repl panel without requiring the editor to
+                    // depend on the `repl` crate.
+                    let (repl_editor_event_tx, mut repl_editor_event_rx) =
+                        futures::channel::mpsc::unbounded::<ReplEvent>();
+
                     let subscriptions = vec![
                         cx.on_focus_in(&focus_handle, Self::focus_in),
                         cx.on_focus_out(&focus_handle, Self::focus_out),
                         cx.observe_global::<SettingsStore>(move |this, cx| {
-                            let settings = JupyterSettings::get_global(cx);
-                            this.set_enabled(settings.enabled, cx);
+                            this.set_enabled(JupyterSettings::enabled(cx), cx);
                         }),
+                        cx.observe_new_views(
+                            move |editor: &mut Editor, cx: &mut ViewContext<Editor>| {
+                                let editor_view = cx.view().downgrade();
+                                let run_event_tx = repl_editor_event_tx.clone();
+                                let clear_event_tx = repl_editor_event_tx.clone();
+                                editor
+                                    .register_action(move |_: &Run, cx: &mut WindowContext| {
+                                        if !JupyterSettings::enabled(cx) {
+                                            return;
+                                        }
+                                        run_event_tx
+                                            .unbounded_send(ReplEvent::Run(editor_view.clone()))
+                                            .ok();
+                                    })
+                                    .detach();
+
+                                let editor_view = cx.view().downgrade();
+                                editor
+                                    .register_action(
+                                        move |_: &ClearOutputs, cx: &mut WindowContext| {
+                                            if !JupyterSettings::enabled(cx) {
+                                                return;
+                                            }
+                                            clear_event_tx
+                                                .unbounded_send(ReplEvent::ClearOutputs(
+                                                    editor_view.clone(),
+                                                ))
+                                                .ok();
+                                        },
+                                    )
+                                    .detach();
+                            },
+                        ),
                     ];
 
-                    let enabled = JupyterSettings::get_global(cx).enabled;
-
-                    Self {
-                        fs,
+                    // Listen for events from the editor on the `repl_editor_event_rx` channel
+                    let _editor_events_task = cx.spawn(
+                        move |this: WeakView<RuntimePanel>, mut cx: AsyncWindowContext| async move {
+                            while let Some(event) = repl_editor_event_rx.next().await {
+                                this.update(&mut cx, |runtime_panel, cx| match event {
+                                    ReplEvent::Run(editor) => {
+                                        runtime_panel.run(editor, cx).log_err();
+                                    }
+                                    ReplEvent::ClearOutputs(editor) => {
+                                        runtime_panel.clear_outputs(editor, cx);
+                                    }
+                                })
+                                .ok();
+                            }
+                        },
+                    );
+
+                    let runtime_panel = Self {
+                        fs: fs.clone(),
                         width: None,
                         focus_handle,
                         kernel_specifications: Vec::new(),
                         sessions: Default::default(),
                         _subscriptions: subscriptions,
-                        enabled,
-                    }
+                        enabled: JupyterSettings::enabled(cx),
+                        _editor_events_task,
+                    };
+
+                    runtime_panel
                 })
             })?;
 
@@ -152,9 +214,11 @@ impl RuntimePanel {
 
     pub fn snippet(
         &self,
-        editor: View<Editor>,
+        editor: WeakView<Editor>,
         cx: &mut ViewContext<Self>,
     ) -> Option<(String, Arc<str>, Range<Anchor>)> {
+        let editor = editor.upgrade()?;
+
         let buffer = editor.read(cx).buffer().read(cx).snapshot(cx);
         let anchor_range = self.selection(editor, cx);
 
@@ -214,8 +278,7 @@ impl RuntimePanel {
 
     pub fn run(
         &mut self,
-        editor: View<Editor>,
-        fs: Arc<dyn Fs>,
+        editor: WeakView<Editor>,
         cx: &mut ViewContext<Self>,
     ) -> anyhow::Result<()> {
         if !self.enabled {
@@ -234,7 +297,8 @@ impl RuntimePanel {
             .with_context(|| format!("No kernel found for language: {language_name}"))?;
 
         let session = self.sessions.entry(entity_id).or_insert_with(|| {
-            let view = cx.new_view(|cx| Session::new(editor, fs.clone(), kernel_specification, cx));
+            let view =
+                cx.new_view(|cx| Session::new(editor, self.fs.clone(), kernel_specification, cx));
             cx.notify();
 
             let subscription = cx.subscribe(
@@ -261,7 +325,7 @@ impl RuntimePanel {
         anyhow::Ok(())
     }
 
-    pub fn clear_outputs(&mut self, editor: View<Editor>, cx: &mut ViewContext<Self>) {
+    pub fn clear_outputs(&mut self, editor: WeakView<Editor>, cx: &mut ViewContext<Self>) {
         let entity_id = editor.entity_id();
         if let Some(session) = self.sessions.get_mut(&entity_id) {
             session.update(cx, |session, cx| {
@@ -272,42 +336,6 @@ impl RuntimePanel {
     }
 }
 
-pub fn run(workspace: &mut Workspace, _: &Run, cx: &mut ViewContext<Workspace>) {
-    let settings = JupyterSettings::get_global(cx);
-    if !settings.enabled {
-        return;
-    }
-
-    let editor = workspace
-        .active_item(cx)
-        .and_then(|item| item.act_as::<Editor>(cx));
-
-    if let (Some(editor), Some(runtime_panel)) = (editor, workspace.panel::<RuntimePanel>(cx)) {
-        runtime_panel.update(cx, |runtime_panel, cx| {
-            runtime_panel
-                .run(editor, workspace.app_state().fs.clone(), cx)
-                .ok();
-        });
-    }
-}
-
-pub fn clear_outputs(workspace: &mut Workspace, _: &ClearOutputs, cx: &mut ViewContext<Workspace>) {
-    let settings = JupyterSettings::get_global(cx);
-    if !settings.enabled {
-        return;
-    }
-
-    let editor = workspace
-        .active_item(cx)
-        .and_then(|item| item.act_as::<Editor>(cx));
-
-    if let (Some(editor), Some(runtime_panel)) = (editor, workspace.panel::<RuntimePanel>(cx)) {
-        runtime_panel.update(cx, |runtime_panel, cx| {
-            runtime_panel.clear_outputs(editor, cx);
-        });
-    }
-}
-
 impl Panel for RuntimePanel {
     fn persistent_name() -> &'static str {
         "RuntimePanel"

crates/repl/src/session.rs 🔗

@@ -10,7 +10,7 @@ use editor::{
     Anchor, AnchorRangeExt as _, Editor,
 };
 use futures::{FutureExt as _, StreamExt as _};
-use gpui::{div, prelude::*, Entity, EventEmitter, Render, Task, View, ViewContext};
+use gpui::{div, prelude::*, EventEmitter, Render, Task, View, ViewContext, WeakView};
 use project::Fs;
 use runtimelib::{
     ExecuteRequest, InterruptRequest, JupyterMessage, JupyterMessageContent, KernelInfoRequest,
@@ -22,16 +22,15 @@ use theme::{ActiveTheme, ThemeSettings};
 use ui::{h_flex, prelude::*, v_flex, ButtonLike, ButtonStyle, Label};
 
 pub struct Session {
-    editor: View<Editor>,
+    editor: WeakView<Editor>,
     kernel: Kernel,
     blocks: HashMap<String, EditorBlock>,
     messaging_task: Task<()>,
     kernel_specification: KernelSpecification,
 }
 
-#[derive(Debug)]
 struct EditorBlock {
-    editor: View<Editor>,
+    editor: WeakView<Editor>,
     code_range: Range<Anchor>,
     block_id: BlockId,
     execution_view: View<ExecutionView>,
@@ -39,11 +38,11 @@ struct EditorBlock {
 
 impl EditorBlock {
     fn new(
-        editor: View<Editor>,
+        editor: WeakView<Editor>,
         code_range: Range<Anchor>,
         status: ExecutionStatus,
         cx: &mut ViewContext<Session>,
-    ) -> Self {
+    ) -> anyhow::Result<Self> {
         let execution_view = cx.new_view(|cx| ExecutionView::new(status, cx));
 
         let block_id = editor.update(cx, |editor, cx| {
@@ -56,14 +55,14 @@ impl EditorBlock {
             };
 
             editor.insert_blocks([block], None, cx)[0]
-        });
+        })?;
 
-        Self {
+        anyhow::Ok(Self {
             editor,
             code_range,
             block_id,
             execution_view,
-        }
+        })
     }
 
     fn handle_message(&mut self, message: &JupyterMessage, cx: &mut ViewContext<Session>) {
@@ -71,17 +70,19 @@ impl EditorBlock {
             execution_view.push_message(&message.content, cx);
         });
 
-        self.editor.update(cx, |editor, cx| {
-            let mut replacements = HashMap::default();
-            replacements.insert(
-                self.block_id,
-                (
-                    Some(self.execution_view.num_lines(cx).saturating_add(1)),
-                    Self::create_output_area_render(self.execution_view.clone()),
-                ),
-            );
-            editor.replace_blocks(replacements, None, cx);
-        })
+        self.editor
+            .update(cx, |editor, cx| {
+                let mut replacements = HashMap::default();
+                replacements.insert(
+                    self.block_id,
+                    (
+                        Some(self.execution_view.num_lines(cx).saturating_add(1)),
+                        Self::create_output_area_render(self.execution_view.clone()),
+                    ),
+                );
+                editor.replace_blocks(replacements, None, cx);
+            })
+            .ok();
     }
 
     fn create_output_area_render(execution_view: View<ExecutionView>) -> RenderBlock {
@@ -118,7 +119,7 @@ impl EditorBlock {
 
 impl Session {
     pub fn new(
-        editor: View<Editor>,
+        editor: WeakView<Editor>,
         fs: Arc<dyn Fs>,
         kernel_specification: KernelSpecification,
         cx: &mut ViewContext<Self>,
@@ -200,14 +201,22 @@ impl Session {
         let blocks_to_remove: HashSet<BlockId> =
             self.blocks.values().map(|block| block.block_id).collect();
 
-        self.editor.update(cx, |editor, cx| {
-            editor.remove_blocks(blocks_to_remove, None, cx);
-        });
+        self.editor
+            .update(cx, |editor, cx| {
+                editor.remove_blocks(blocks_to_remove, None, cx);
+            })
+            .ok();
 
         self.blocks.clear();
     }
 
     pub fn execute(&mut self, code: &str, anchor_range: Range<Anchor>, cx: &mut ViewContext<Self>) {
+        let editor = if let Some(editor) = self.editor.upgrade() {
+            editor
+        } else {
+            return;
+        };
+
         let execute_request = ExecuteRequest {
             code: code.to_string(),
             ..ExecuteRequest::default()
@@ -217,7 +226,7 @@ impl Session {
 
         let mut blocks_to_remove: HashSet<BlockId> = HashSet::default();
 
-        let buffer = self.editor.read(cx).buffer().read(cx).snapshot(cx);
+        let buffer = editor.read(cx).buffer().read(cx).snapshot(cx);
 
         self.blocks.retain(|_key, block| {
             if anchor_range.overlaps(&block.code_range, &buffer) {
@@ -228,9 +237,11 @@ impl Session {
             }
         });
 
-        self.editor.update(cx, |editor, cx| {
-            editor.remove_blocks(blocks_to_remove, None, cx);
-        });
+        self.editor
+            .update(cx, |editor, cx| {
+                editor.remove_blocks(blocks_to_remove, None, cx);
+            })
+            .ok();
 
         let status = match &self.kernel {
             Kernel::RunningKernel(_) => ExecutionStatus::Queued,
@@ -240,7 +251,13 @@ impl Session {
             Kernel::Shutdown => ExecutionStatus::Shutdown,
         };
 
-        let editor_block = EditorBlock::new(self.editor.clone(), anchor_range, status, cx);
+        let editor_block = if let Ok(editor_block) =
+            EditorBlock::new(self.editor.clone(), anchor_range, status, cx)
+        {
+            editor_block
+        } else {
+            return;
+        };
 
         self.blocks
             .insert(message.header.msg_id.clone(), editor_block);
@@ -341,7 +358,7 @@ impl Session {
 }
 
 pub enum SessionEvent {
-    Shutdown(View<Editor>),
+    Shutdown(WeakView<Editor>),
 }
 
 impl EventEmitter<SessionEvent> for Session {}