repl: Fix duplicate output inside notebooks (#48616)

MostlyK created

- Render the output only when needed, fixes the duplicate output that
can happen after opening a saved notebook.
- Vim in Jupyter View with j/k navigation across notebook cells

Release Notes:

- N/A

Change summary

assets/keymaps/vim.json                 |   8 +
crates/repl/src/notebook/cell.rs        |  96 ++++----------------
crates/repl/src/notebook/notebook_ui.rs | 123 +++++++++++++++++++++++++++
crates/zed_actions/src/lib.rs           |  14 +++
4 files changed, 164 insertions(+), 77 deletions(-)

Detailed changes

assets/keymaps/vim.json 🔗

@@ -1110,4 +1110,12 @@
       "shift-g": "menu::SelectLast",
     },
   },
+  {
+    "context": "NotebookEditor > Editor && VimControl && vim_mode == normal",
+
+    "bindings": {
+      "j": "notebook::NotebookMoveDown",
+      "k": "notebook::NotebookMoveUp",
+    },
+  },
 ]

crates/repl/src/notebook/cell.rs 🔗

@@ -2,11 +2,12 @@
 use std::sync::Arc;
 use std::time::{Duration, Instant};
 
-use editor::{Editor, EditorMode, MultiBuffer};
+use editor::{Editor, EditorMode, MultiBuffer, SizingBehavior};
 use futures::future::Shared;
 use gpui::{
-    App, Entity, EventEmitter, Focusable, Hsla, InteractiveElement, RetainAllImageCache,
-    StatefulInteractiveElement, Task, TextStyleRefinement, image_cache, prelude::*,
+    App, Entity, EventEmitter, Focusable, Hsla, InteractiveElement, KeyContext,
+    RetainAllImageCache, StatefulInteractiveElement, Task, TextStyleRefinement, image_cache,
+    prelude::*,
 };
 use language::{Buffer, Language, LanguageRegistry};
 use markdown::{Markdown, MarkdownElement, MarkdownStyle};
@@ -357,9 +358,10 @@ impl MarkdownCell {
 
         let editor = cx.new(|cx| {
             let mut editor = Editor::new(
-                EditorMode::AutoHeight {
-                    min_lines: 1,
-                    max_lines: Some(1024),
+                EditorMode::Full {
+                    scale_ui_elements_with_buffer_font_size: false,
+                    show_active_line_background: false,
+                    sizing_behavior: SizingBehavior::SizeByContent,
                 },
                 multi_buffer,
                 None,
@@ -378,6 +380,7 @@ impl MarkdownCell {
 
             editor.set_show_gutter(false, cx);
             editor.set_text_style_refinement(refinement);
+            editor.set_use_modal_editing(true);
             editor
         });
 
@@ -625,9 +628,10 @@ impl CodeCell {
 
         let editor_view = cx.new(|cx| {
             let mut editor = Editor::new(
-                EditorMode::AutoHeight {
-                    min_lines: 1,
-                    max_lines: Some(1024),
+                EditorMode::Full {
+                    scale_ui_elements_with_buffer_font_size: false,
+                    show_active_line_background: false,
+                    sizing_behavior: SizingBehavior::SizeByContent,
                 },
                 multi_buffer,
                 None,
@@ -646,6 +650,7 @@ impl CodeCell {
 
             editor.set_show_gutter(false, cx);
             editor.set_text_style_refinement(refinement);
+            editor.set_use_modal_editing(true);
             editor
         });
 
@@ -700,9 +705,10 @@ impl CodeCell {
 
         let editor_view = cx.new(|cx| {
             let mut editor = Editor::new(
-                EditorMode::AutoHeight {
-                    min_lines: 1,
-                    max_lines: Some(1024),
+                EditorMode::Full {
+                    scale_ui_elements_with_buffer_font_size: false,
+                    show_active_line_background: false,
+                    sizing_behavior: SizingBehavior::SizeByContent,
                 },
                 multi_buffer,
                 None,
@@ -722,6 +728,7 @@ impl CodeCell {
             editor.set_text(source.clone(), window, cx);
             editor.set_show_gutter(false, cx);
             editor.set_text_style_refinement(refinement);
+            editor.set_use_modal_editing(true);
             editor
         });
 
@@ -1117,71 +1124,6 @@ impl Render for CodeCell {
                         ),
                     ),
             )
-            // Output portion
-            .child(
-                h_flex()
-                    .w_full()
-                    .pr_6()
-                    .rounded_xs()
-                    .items_start()
-                    .gap(DynamicSpacing::Base08.rems(cx))
-                    .bg(self.selected_bg_color(window, cx))
-                    .child(self.gutter_output(window, cx))
-                    .child(
-                        div().py_1p5().w_full().child(
-                            div()
-                                .flex()
-                                .size_full()
-                                .flex_1()
-                                .py_3()
-                                .px_5()
-                                .rounded_lg()
-                                .border_1()
-                                .child(
-                                    div()
-                                        .id((ElementId::from(self.id.to_string()), "output-scroll"))
-                                        .w_full()
-                                        .when_some(output_max_width, |div, max_w| {
-                                            div.max_w(max_w).overflow_x_scroll()
-                                        })
-                                        .when_some(output_max_height, |div, max_h| {
-                                            div.max_h(max_h).overflow_y_scroll()
-                                        })
-                                        .children(self.outputs.iter().map(|output| {
-                                            let content = match output {
-                                                Output::Plain { content, .. } => {
-                                                    Some(content.clone().into_any_element())
-                                                }
-                                                Output::Markdown { content, .. } => {
-                                                    Some(content.clone().into_any_element())
-                                                }
-                                                Output::Stream { content, .. } => {
-                                                    Some(content.clone().into_any_element())
-                                                }
-                                                Output::Image { content, .. } => {
-                                                    Some(content.clone().into_any_element())
-                                                }
-                                                Output::Message(message) => Some(
-                                                    div().child(message.clone()).into_any_element(),
-                                                ),
-                                                Output::Table { content, .. } => {
-                                                    Some(content.clone().into_any_element())
-                                                }
-                                                Output::Json { content, .. } => {
-                                                    Some(content.clone().into_any_element())
-                                                }
-                                                Output::ErrorOutput(error_view) => {
-                                                    error_view.render(window, cx)
-                                                }
-                                                Output::ClearOutputWaitMarker => None,
-                                            };
-
-                                            div().children(content)
-                                        })),
-                                ),
-                        ),
-                    ),
-            )
             .when(
                 self.has_outputs() || self.execution_duration.is_some() || self.is_executing,
                 |this| {

crates/repl/src/notebook/notebook_ui.rs 🔗

@@ -5,6 +5,7 @@ use std::{path::PathBuf, sync::Arc};
 use anyhow::{Context as _, Result};
 use client::proto::ViewId;
 use collections::HashMap;
+use editor::DisplayPoint;
 use feature_flags::{FeatureFlagAppExt as _, NotebookFeatureFlag};
 use futures::FutureExt;
 use futures::future::Shared;
@@ -40,6 +41,7 @@ use picker::Picker;
 use runtimelib::{ExecuteRequest, JupyterMessage, JupyterMessageContent};
 use ui::PopoverMenuHandle;
 use zed_actions::editor::{MoveDown, MoveUp};
+use zed_actions::notebook::{NotebookMoveDown, NotebookMoveUp};
 
 actions!(
     notebook,
@@ -1295,6 +1297,127 @@ impl Render for NotebookEditor {
                     }
                 }
             }))
+            .on_action(cx.listener(|this, _: &NotebookMoveDown, window, cx| {
+                let Some(cell_id) = this.cell_order.get(this.selected_cell_index) else {
+                    return;
+                };
+                let Some(cell) = this.cell_map.get(cell_id) else {
+                    return;
+                };
+
+                let editor = match cell {
+                    Cell::Code(cell) => cell.read(cx).editor().clone(),
+                    Cell::Markdown(cell) => cell.read(cx).editor().clone(),
+                    _ => return,
+                };
+
+                let is_at_last_line = editor.update(cx, |editor, cx| {
+                    let display_snapshot = editor.display_snapshot(cx);
+                    let selections = editor.selections.all_display(&display_snapshot);
+                    if let Some(selection) = selections.last() {
+                        let head = selection.head();
+                        let cursor_row = head.row();
+                        let max_row = display_snapshot.max_point().row();
+
+                        cursor_row >= max_row
+                    } else {
+                        false
+                    }
+                });
+
+                if is_at_last_line {
+                    this.select_next(&menu::SelectNext, window, cx);
+                    if let Some(cell_id) = this.cell_order.get(this.selected_cell_index) {
+                        if let Some(cell) = this.cell_map.get(cell_id) {
+                            match cell {
+                                Cell::Code(cell) => {
+                                    let editor = cell.read(cx).editor().clone();
+                                    editor.update(cx, |editor, cx| {
+                                        editor.move_to_beginning(&Default::default(), window, cx);
+                                    });
+                                    editor.focus_handle(cx).focus(window, cx);
+                                }
+                                Cell::Markdown(cell) => {
+                                    cell.update(cx, |cell, cx| {
+                                        cell.set_editing(true);
+                                        cx.notify();
+                                    });
+                                    let editor = cell.read(cx).editor().clone();
+                                    editor.update(cx, |editor, cx| {
+                                        editor.move_to_beginning(&Default::default(), window, cx);
+                                    });
+                                    editor.focus_handle(cx).focus(window, cx);
+                                }
+                                _ => {}
+                            }
+                        }
+                    }
+                } else {
+                    editor.update(cx, |editor, cx| {
+                        editor.move_down(&Default::default(), window, cx);
+                    });
+                }
+            }))
+            .on_action(cx.listener(|this, _: &NotebookMoveUp, window, cx| {
+                let Some(cell_id) = this.cell_order.get(this.selected_cell_index) else {
+                    return;
+                };
+                let Some(cell) = this.cell_map.get(cell_id) else {
+                    return;
+                };
+
+                let editor = match cell {
+                    Cell::Code(cell) => cell.read(cx).editor().clone(),
+                    Cell::Markdown(cell) => cell.read(cx).editor().clone(),
+                    _ => return,
+                };
+
+                let is_at_first_line = editor.update(cx, |editor, cx| {
+                    let display_snapshot = editor.display_snapshot(cx);
+                    let selections = editor.selections.all_display(&display_snapshot);
+                    if let Some(selection) = selections.first() {
+                        let head = selection.head();
+                        let cursor_row = head.row();
+
+                        cursor_row.0 == 0
+                    } else {
+                        false
+                    }
+                });
+
+                if is_at_first_line {
+                    this.select_previous(&menu::SelectPrevious, window, cx);
+                    if let Some(cell_id) = this.cell_order.get(this.selected_cell_index) {
+                        if let Some(cell) = this.cell_map.get(cell_id) {
+                            match cell {
+                                Cell::Code(cell) => {
+                                    let editor = cell.read(cx).editor().clone();
+                                    editor.update(cx, |editor, cx| {
+                                        editor.move_to_end(&Default::default(), window, cx);
+                                    });
+                                    editor.focus_handle(cx).focus(window, cx);
+                                }
+                                Cell::Markdown(cell) => {
+                                    cell.update(cx, |cell, cx| {
+                                        cell.set_editing(true);
+                                        cx.notify();
+                                    });
+                                    let editor = cell.read(cx).editor().clone();
+                                    editor.update(cx, |editor, cx| {
+                                        editor.move_to_end(&Default::default(), window, cx);
+                                    });
+                                    editor.focus_handle(cx).focus(window, cx);
+                                }
+                                _ => {}
+                            }
+                        }
+                    }
+                } else {
+                    editor.update(cx, |editor, cx| {
+                        editor.move_up(&Default::default(), window, cx);
+                    });
+                }
+            }))
             .on_action(
                 cx.listener(|this, action, window, cx| this.restart_kernel(action, window, cx)),
             )

crates/zed_actions/src/lib.rs 🔗

@@ -736,3 +736,17 @@ pub mod preview {
         );
     }
 }
+
+pub mod notebook {
+    use gpui::actions;
+
+    actions!(
+        notebook,
+        [
+            /// Move to down in cells
+            NotebookMoveDown,
+            /// Move to up in cells
+            NotebookMoveUp,
+        ]
+    );
+}