From fe1f55cbfd84ae7de8bad0e3b854f349f15c5ef9 Mon Sep 17 00:00:00 2001 From: Marshall Bowers Date: Mon, 22 Jul 2024 19:02:11 -0400 Subject: [PATCH] repl: Iterate on design of REPL sessions view (#14987) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR iterates on the design of the REPL sessions view. We now use the same component for both available kernels and running ones to provide some consistency between the two modes: Screenshot 2024-07-22 at 6 49 08 PM Screenshot 2024-07-22 at 6 49 14 PM Release Notes: - N/A --- crates/repl/src/components.rs | 3 + .../repl/src/components/kernel_list_item.rs | 60 +++++++++++ crates/repl/src/kernels.rs | 43 ++++---- crates/repl/src/repl.rs | 27 ++--- crates/repl/src/repl_sessions_ui.rs | 99 ++++++++++--------- crates/repl/src/session.rs | 81 ++++++++------- 6 files changed, 192 insertions(+), 121 deletions(-) create mode 100644 crates/repl/src/components.rs create mode 100644 crates/repl/src/components/kernel_list_item.rs diff --git a/crates/repl/src/components.rs b/crates/repl/src/components.rs new file mode 100644 index 0000000000..5002ae6eb1 --- /dev/null +++ b/crates/repl/src/components.rs @@ -0,0 +1,3 @@ +mod kernel_list_item; + +pub use kernel_list_item::*; diff --git a/crates/repl/src/components/kernel_list_item.rs b/crates/repl/src/components/kernel_list_item.rs new file mode 100644 index 0000000000..9de6ae2d43 --- /dev/null +++ b/crates/repl/src/components/kernel_list_item.rs @@ -0,0 +1,60 @@ +use gpui::AnyElement; +use ui::{prelude::*, Indicator, ListItem}; + +use crate::KernelSpecification; + +#[derive(IntoElement)] +pub struct KernelListItem { + kernel_specification: KernelSpecification, + status_color: Color, + buttons: Vec, + children: Vec, +} + +impl KernelListItem { + pub fn new(kernel_specification: KernelSpecification) -> Self { + Self { + kernel_specification, + status_color: Color::Disabled, + buttons: Vec::new(), + children: Vec::new(), + } + } + + pub fn status_color(mut self, color: Color) -> Self { + self.status_color = color; + self + } + + pub fn button(mut self, button: impl IntoElement) -> Self { + self.buttons.push(button.into_any_element()); + self + } + + pub fn buttons(mut self, buttons: impl IntoIterator) -> Self { + self.buttons + .extend(buttons.into_iter().map(|button| button.into_any_element())); + self + } +} + +impl ParentElement for KernelListItem { + fn extend(&mut self, elements: impl IntoIterator) { + self.children.extend(elements); + } +} + +impl RenderOnce for KernelListItem { + fn render(self, _cx: &mut WindowContext) -> impl IntoElement { + ListItem::new(SharedString::from(self.kernel_specification.name.clone())) + .selectable(false) + .start_slot( + h_flex() + .size_3() + .justify_center() + .child(Indicator::dot().color(self.status_color)), + ) + .children(self.children) + .end_slot(h_flex().gap_2().children(self.buttons)) + } +} diff --git a/crates/repl/src/kernels.rs b/crates/repl/src/kernels.rs index 7e9d6cad75..73b00cb55f 100644 --- a/crates/repl/src/kernels.rs +++ b/crates/repl/src/kernels.rs @@ -18,7 +18,6 @@ use std::{ path::PathBuf, sync::Arc, }; -use ui::{Color, Indicator}; #[derive(Debug, Clone)] pub struct KernelSpecification { @@ -72,15 +71,6 @@ async fn peek_ports(ip: IpAddr) -> Result<[u16; 5]> { Ok(ports) } -#[derive(Debug)] -pub enum Kernel { - RunningKernel(RunningKernel), - StartingKernel(Shared>), - ErroredLaunch(String), - ShuttingDown, - Shutdown, -} - #[derive(Debug, Clone)] pub enum KernelStatus { Idle, @@ -90,6 +80,7 @@ pub enum KernelStatus { ShuttingDown, Shutdown, } + impl KernelStatus { pub fn is_connected(&self) -> bool { match self { @@ -127,20 +118,16 @@ impl From<&Kernel> for KernelStatus { } } -impl Kernel { - pub fn dot(&self) -> Indicator { - match self { - Kernel::RunningKernel(kernel) => match kernel.execution_state { - ExecutionState::Idle => Indicator::dot().color(Color::Success), - ExecutionState::Busy => Indicator::dot().color(Color::Modified), - }, - Kernel::StartingKernel(_) => Indicator::dot().color(Color::Modified), - Kernel::ErroredLaunch(_) => Indicator::dot().color(Color::Error), - Kernel::ShuttingDown => Indicator::dot().color(Color::Modified), - Kernel::Shutdown => Indicator::dot().color(Color::Disabled), - } - } +#[derive(Debug)] +pub enum Kernel { + RunningKernel(RunningKernel), + StartingKernel(Shared>), + ErroredLaunch(String), + ShuttingDown, + Shutdown, +} +impl Kernel { pub fn status(&self) -> KernelStatus { self.into() } @@ -162,6 +149,16 @@ impl Kernel { _ => {} } } + + pub fn is_shutting_down(&self) -> bool { + match self { + Kernel::ShuttingDown => true, + Kernel::RunningKernel(_) + | Kernel::StartingKernel(_) + | Kernel::ErroredLaunch(_) + | Kernel::Shutdown => false, + } + } } pub struct RunningKernel { diff --git a/crates/repl/src/repl.rs b/crates/repl/src/repl.rs index 046397ffa2..41e90e93f9 100644 --- a/crates/repl/src/repl.rs +++ b/crates/repl/src/repl.rs @@ -1,9 +1,4 @@ -use async_dispatcher::{set_dispatcher, Dispatcher, Runnable}; -use gpui::{AppContext, PlatformDispatcher}; -use project::Fs; -use settings::Settings as _; -use std::{sync::Arc, time::Duration}; - +mod components; mod jupyter_settings; mod kernels; mod outputs; @@ -13,14 +8,22 @@ mod repl_store; mod session; mod stdio; -pub use jupyter_settings::JupyterSettings; -pub use kernels::{Kernel, KernelSpecification, KernelStatus}; -pub use repl_editor::*; -pub use repl_sessions_ui::{ClearOutputs, Interrupt, ReplSessionsPage, Run, Sessions, Shutdown}; -pub use runtimelib::ExecutionState; -pub use session::Session; +use std::{sync::Arc, time::Duration}; +use async_dispatcher::{set_dispatcher, Dispatcher, Runnable}; +use gpui::{AppContext, PlatformDispatcher}; +use project::Fs; +pub use runtimelib::ExecutionState; +use settings::Settings as _; + +pub use crate::jupyter_settings::JupyterSettings; +pub use crate::kernels::{Kernel, KernelSpecification, KernelStatus}; +pub use crate::repl_editor::*; +pub use crate::repl_sessions_ui::{ + ClearOutputs, Interrupt, ReplSessionsPage, Run, Sessions, Shutdown, +}; use crate::repl_store::ReplStore; +pub use crate::session::Session; fn zed_dispatcher(cx: &mut AppContext) -> impl Dispatcher { struct ZedDispatcher { diff --git a/crates/repl/src/repl_sessions_ui.rs b/crates/repl/src/repl_sessions_ui.rs index f92212deac..7960eae0ed 100644 --- a/crates/repl/src/repl_sessions_ui.rs +++ b/crates/repl/src/repl_sessions_ui.rs @@ -1,6 +1,7 @@ use editor::Editor; use gpui::{ - actions, prelude::*, AppContext, EventEmitter, FocusHandle, FocusableView, Subscription, View, + actions, prelude::*, AnyElement, AppContext, EventEmitter, FocusHandle, FocusableView, + Subscription, View, }; use ui::{prelude::*, ButtonLike, ElevationIndex, KeyBinding}; use util::ResultExt as _; @@ -8,6 +9,7 @@ use workspace::item::ItemEvent; use workspace::WorkspaceId; use workspace::{item::Item, Workspace}; +use crate::components::KernelListItem; use crate::jupyter_settings::JupyterSettings; use crate::repl_store::ReplStore; @@ -187,15 +189,10 @@ impl Render for ReplSessionsPage { // install kernels. It can be assumed they don't have a running kernel if we have no // specifications. if kernel_specifications.is_empty() { - return v_flex() - .p_4() - .size_full() - .gap_2() - .child(Label::new("No Jupyter Kernels Available").size(LabelSize::Large)) - .child( - Label::new("To start interactively running code in your editor, you need to install and configure Jupyter kernels.") - .size(LabelSize::Default), - ) + let instructions = "To start interactively running code in your editor, you need to install and configure Jupyter kernels."; + + return ReplSessionsContainer::new("No Jupyter Kernels Available") + .child(Label::new(instructions)) .child( h_flex().w_full().p_4().justify_center().gap_2().child( ButtonLike::new("install-kernels") @@ -209,48 +206,62 @@ impl Render for ReplSessionsPage { ) }), ), - ) - .into_any_element(); + ); } // When there are no sessions, show the command to run code in an editor if sessions.is_empty() { - return v_flex() - .p_4() - .size_full() - .gap_2() - .child(Label::new("No Jupyter Kernel Sessions").size(LabelSize::Large)) + let instructions = "To run code in a Jupyter kernel, select some code and use the 'repl::Run' command."; + + return ReplSessionsContainer::new("No Jupyter Kernel Sessions") .child( - v_flex().child( - Label::new("To run code in a Jupyter kernel, select some code and use the 'repl::Run' command.") - .size(LabelSize::Default) - ) - .children( - KeyBinding::for_action(&Run, cx) - .map(|binding| - binding.into_any_element() - ) - ) + v_flex() + .child(Label::new(instructions)) + .children(KeyBinding::for_action(&Run, cx)), ) .child(Label::new("Kernels available").size(LabelSize::Large)) - .children( - kernel_specifications.into_iter().map(|spec| { - h_flex().gap_2().child(Label::new(spec.name.clone())) - .child(Label::new(spec.kernelspec.language.clone()).color(Color::Muted)) - }) - ) - - .into_any_element(); + .children(kernel_specifications.into_iter().map(|spec| { + KernelListItem::new(spec.clone()).child( + h_flex() + .gap_2() + .child(Label::new(spec.name)) + .child(Label::new(spec.kernelspec.language).color(Color::Muted)), + ) + })); } - v_flex() - .p_4() - .child(Label::new("Jupyter Kernel Sessions").size(LabelSize::Large)) - .children( - sessions - .into_iter() - .map(|session| session.clone().into_any_element()), - ) - .into_any_element() + ReplSessionsContainer::new("Jupyter Kernel Sessions").children(sessions) + } +} + +#[derive(IntoElement)] +struct ReplSessionsContainer { + title: SharedString, + children: Vec, +} + +impl ReplSessionsContainer { + pub fn new(title: impl Into) -> Self { + Self { + title: title.into(), + children: Vec::new(), + } + } +} + +impl ParentElement for ReplSessionsContainer { + fn extend(&mut self, elements: impl IntoIterator) { + self.children.extend(elements) + } +} + +impl RenderOnce for ReplSessionsContainer { + fn render(self, _cx: &mut WindowContext) -> impl IntoElement { + v_flex() + .p_4() + .gap_2() + .size_full() + .child(Label::new(self.title).size(LabelSize::Large)) + .children(self.children) } } diff --git a/crates/repl/src/session.rs b/crates/repl/src/session.rs index 34501ecae1..dfe1c78b48 100644 --- a/crates/repl/src/session.rs +++ b/crates/repl/src/session.rs @@ -1,3 +1,4 @@ +use crate::components::KernelListItem; use crate::{ kernels::{Kernel, KernelSpecification, RunningKernel}, outputs::{ExecutionStatus, ExecutionView, LineHeight as _}, @@ -19,12 +20,13 @@ use gpui::{ use language::Point; use project::Fs; use runtimelib::{ - ExecuteRequest, InterruptRequest, JupyterMessage, JupyterMessageContent, ShutdownRequest, + ExecuteRequest, ExecutionState, InterruptRequest, JupyterMessage, JupyterMessageContent, + ShutdownRequest, }; use settings::Settings as _; use std::{env::temp_dir, ops::Range, sync::Arc, time::Duration}; use theme::{ActiveTheme, ThemeSettings}; -use ui::{h_flex, prelude::*, v_flex, ButtonLike, ButtonStyle, IconButtonShape, Label, Tooltip}; +use ui::{prelude::*, IconButtonShape, Tooltip}; pub struct Session { editor: WeakView, @@ -561,52 +563,47 @@ impl EventEmitter for Session {} impl Render for Session { fn render(&mut self, cx: &mut ViewContext) -> impl IntoElement { - let mut buttons = vec![]; - - buttons.push( - ButtonLike::new("shutdown") - .child(Label::new("Shutdown")) - .style(ButtonStyle::Subtle) - .on_click(cx.listener(move |session, _, cx| { - session.shutdown(cx); - })), - ); - - let status_text = match &self.kernel { - Kernel::RunningKernel(kernel) => { - buttons.push( - ButtonLike::new("interrupt") - .child(Label::new("Interrupt")) + let (status_text, interrupt_button) = match &self.kernel { + Kernel::RunningKernel(kernel) => ( + kernel + .kernel_info + .as_ref() + .map(|info| info.language_info.name.clone()), + Some( + Button::new("interrupt", "Interrupt") .style(ButtonStyle::Subtle) .on_click(cx.listener(move |session, _, cx| { session.interrupt(cx); })), - ); - let mut name = self.kernel_specification.name.clone(); - - if let Some(info) = &kernel.kernel_info { - name.push_str(" ("); - name.push_str(&info.language_info.name); - name.push_str(")"); - } - name - } - Kernel::StartingKernel(_) => format!("{} (Starting)", self.kernel_specification.name), - Kernel::ErroredLaunch(err) => { - format!("{} (Error: {})", self.kernel_specification.name, err) - } - Kernel::ShuttingDown => format!("{} (Shutting Down)", self.kernel_specification.name), - Kernel::Shutdown => format!("{} (Shutdown)", self.kernel_specification.name), + ), + ), + Kernel::StartingKernel(_) => (Some("Starting".into()), None), + Kernel::ErroredLaunch(err) => (Some(format!("Error: {err}")), None), + Kernel::ShuttingDown => (Some("Shutting Down".into()), None), + Kernel::Shutdown => (Some("Shutdown".into()), None), }; - return v_flex() - .gap_1() - .child( - h_flex() - .gap_2() - .child(self.kernel.dot()) - .child(Label::new(status_text)), + KernelListItem::new(self.kernel_specification.clone()) + .status_color(match &self.kernel { + Kernel::RunningKernel(kernel) => match kernel.execution_state { + ExecutionState::Idle => Color::Success, + ExecutionState::Busy => Color::Modified, + }, + Kernel::StartingKernel(_) => Color::Modified, + Kernel::ErroredLaunch(_) => Color::Error, + Kernel::ShuttingDown => Color::Modified, + Kernel::Shutdown => Color::Disabled, + }) + .child(Label::new(self.kernel_specification.name.clone())) + .children(status_text.map(|status_text| Label::new(format!("({status_text})")))) + .button( + Button::new("shutdown", "Shutdown") + .style(ButtonStyle::Subtle) + .disabled(self.kernel.is_shutting_down()) + .on_click(cx.listener(move |session, _, cx| { + session.shutdown(cx); + })), ) - .child(h_flex().gap_2().children(buttons)); + .buttons(interrupt_button) } }