From 061dde5a9bd98462e4dba41afc5c4182739f93b4 Mon Sep 17 00:00:00 2001 From: Mikayla Maki Date: Wed, 20 Jul 2022 16:48:40 -0700 Subject: [PATCH] Compiling with new result based error handling --- crates/terminal/src/connection.rs | 20 +- crates/terminal/src/modal.rs | 18 +- crates/terminal/src/terminal.rs | 181 +++++++++++------- crates/terminal/src/terminal_element.rs | 99 +++++----- .../terminal_layout_context.rs | 3 +- .../src/tests/terminal_test_context.rs | 42 ++-- 6 files changed, 198 insertions(+), 165 deletions(-) diff --git a/crates/terminal/src/connection.rs b/crates/terminal/src/connection.rs index 2672a68457270afe96361843f0c64d456c53bd2a..48160cd8776c97707f17b4a72f7fa77be0451bcc 100644 --- a/crates/terminal/src/connection.rs +++ b/crates/terminal/src/connection.rs @@ -56,15 +56,16 @@ impl EventListener for ZedListener { #[derive(Error, Debug)] pub struct TerminalError { - directory: Option, - shell: Option, - source: std::io::Error, + pub directory: Option, + pub shell: Option, + pub source: std::io::Error, } impl Display for TerminalError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { let dir_string: String = self .directory + .clone() .map(|path| { match path .into_os_string() @@ -77,7 +78,7 @@ impl Display for TerminalError { }) .unwrap_or_else(|| { let default_dir = - dirs::home_dir().map(|buf| buf.into_os_string().to_string_lossy()); + dirs::home_dir().map(|buf| buf.into_os_string().to_string_lossy().to_string()); match default_dir { Some(dir) => format!(" {}", dir), None => "".to_string(), @@ -86,6 +87,7 @@ impl Display for TerminalError { let shell = self .shell + .clone() .map(|shell| match shell { Shell::System => { let mut buf = [0; 1024]; @@ -160,7 +162,7 @@ impl DisconnectedPTY { setup_env(&config); //Spawn a task so the Alacritty EventLoop can communicate with us in a view context - let (events_tx, mut events_rx) = unbounded(); + let (events_tx, events_rx) = unbounded(); //Set up the terminal... let term = Term::new(&config, &initial_size, ZedListener(events_tx.clone())); @@ -218,7 +220,7 @@ impl DisconnectedPTY { }) } - pub fn connect(self, cx: &mut ModelContext) -> Terminal { + pub fn connect(mut self, cx: &mut ModelContext) -> Terminal { cx.spawn_weak(|this, mut cx| async move { //Listen for terminal events while let Some(event) = self.events_rx.next().await { @@ -417,12 +419,6 @@ impl Terminal { } } -impl Drop for DisconnectedPTY { - fn drop(&mut self) { - self.terminal.pty_tx.0.send(Msg::Shutdown).ok(); - } -} - impl Drop for Terminal { fn drop(&mut self) { self.pty_tx.0.send(Msg::Shutdown).ok(); diff --git a/crates/terminal/src/modal.rs b/crates/terminal/src/modal.rs index cabfc5b44ca2539a228f43c0647f905d8bc1f778..00f338825b08bca56a1aca74d9310601eb1d0126 100644 --- a/crates/terminal/src/modal.rs +++ b/crates/terminal/src/modal.rs @@ -16,7 +16,13 @@ pub fn deploy_modal(workspace: &mut Workspace, _: &DeployModal, cx: &mut ViewCon if let Some(StoredConnection(stored_connection)) = possible_connection { // Create a view from the stored connection workspace.toggle_modal(cx, |_, cx| { - cx.add_view(|cx| TerminalView::from_connection(stored_connection.clone(), true, cx)) + cx.add_view(|cx| { + TerminalView::from_connection( + crate::TerminalConnection(Ok(stored_connection.clone())), + true, + cx, + ) + }) }); cx.set_global::>(Some(StoredConnection( stored_connection.clone(), @@ -28,7 +34,7 @@ pub fn deploy_modal(workspace: &mut Workspace, _: &DeployModal, cx: &mut ViewCon let this = cx.add_view(|cx| TerminalView::new(wd, true, cx)); - let connection_handle = this.read(cx).connection.clone(); + let connection_handle = this.read(cx).connection.0.as_ref().unwrap().clone(); cx.subscribe(&connection_handle, on_event).detach(); //Set the global immediately, in case the user opens the command palette cx.set_global::>(Some(StoredConnection( @@ -36,7 +42,13 @@ pub fn deploy_modal(workspace: &mut Workspace, _: &DeployModal, cx: &mut ViewCon ))); this }) { - let connection = closed_terminal_handle.read(cx).connection.clone(); + let connection = closed_terminal_handle + .read(cx) + .connection + .0 + .as_ref() + .unwrap() + .clone(); cx.set_global(Some(StoredConnection(connection))); } } diff --git a/crates/terminal/src/terminal.rs b/crates/terminal/src/terminal.rs index 67e18409c7fffaa58a2fc252f07985895802f7d4..28943a1fbb0a216b8ae7bd71db2f53528da7ce89 100644 --- a/crates/terminal/src/terminal.rs +++ b/crates/terminal/src/terminal.rs @@ -15,7 +15,7 @@ use project::{LocalWorktree, Project, ProjectPath}; use settings::{Settings, WorkingDirectory}; use smallvec::SmallVec; use std::path::{Path, PathBuf}; -use terminal_element::TerminalDimensions; +use terminal_element::{terminal_layout_context::TerminalLayoutData, TerminalDimensions}; use workspace::{Item, Workspace}; @@ -62,9 +62,12 @@ pub fn init(cx: &mut MutableAppContext) { cx.add_action(TerminalView::clear); } +//New Type to make terminal connection's easier +struct TerminalConnection(Result, TerminalError>); + ///A terminal view, maintains the PTY's file handles and communicates with the terminal pub struct TerminalView { - connection: Result, TerminalError>, + connection: TerminalConnection, has_new_content: bool, //Currently using iTerm bell, show bell emoji in tab until input is received has_bell: bool, @@ -87,12 +90,9 @@ impl TerminalView { vec2f(DEBUG_TERMINAL_WIDTH, DEBUG_TERMINAL_HEIGHT), ); - let (shell, envs) = { - let settings = cx.global::(); - let shell = settings.terminal_overrides.shell.clone(); - let envs = settings.terminal_overrides.env.clone(); //Should be short and cheap. - (shell, envs) - }; + let settings = cx.global::(); + let shell = settings.terminal_overrides.shell.clone(); + let envs = settings.terminal_overrides.env.clone(); //Should be short and cheap. let connection = DisconnectedPTY::new(working_directory, shell, envs, size_info) .map(|pty| cx.add_model(|cx| pty.connect(cx))) @@ -103,35 +103,36 @@ impl TerminalView { } }); - TerminalView::from_connection(connection, modal, cx) + TerminalView::from_connection(TerminalConnection(connection), modal, cx) } fn from_connection( - connection: Result, TerminalError>, + connection: TerminalConnection, modal: bool, cx: &mut ViewContext, ) -> TerminalView { - cx.observe(&connection, |_, _, cx| cx.notify()).detach(); - cx.subscribe(&connection, |this, _, event, cx| match event { - Event::Wakeup => { - if cx.is_self_focused() { - cx.notify() - } else { - this.has_new_content = true; - cx.emit(Event::TitleChanged); - } - } - Event::Bell => { - this.has_bell = true; - cx.emit(Event::TitleChanged); + match connection.0.as_ref() { + Ok(conn) => { + cx.observe(conn, |_, _, cx| cx.notify()).detach(); + cx.subscribe(conn, |this, _, event, cx| match event { + Event::Wakeup => { + if cx.is_self_focused() { + cx.notify() + } else { + this.has_new_content = true; + cx.emit(Event::TitleChanged); + } + } + Event::Bell => { + this.has_bell = true; + cx.emit(Event::TitleChanged); + } + _ => cx.emit(*event), + }) + .detach(); } - // Event::Input => { - // this.has_bell = false; - // cx.emit(Event::TitleChanged); - // } - _ => cx.emit(*event), - }) - .detach(); + Err(_) => { /* Leave it as is */ } + } TerminalView { connection, @@ -146,13 +147,6 @@ impl TerminalView { cx.emit(Event::TitleChanged); } - fn clear(&mut self, _: &Clear, cx: &mut ViewContext) { - self.connection - .read(cx) - .get_terminal() - .map(|term| term.clear()); - } - ///Create a new Terminal in the current working directory or the user's home directory fn deploy(workspace: &mut Workspace, _: &Deploy, cx: &mut ViewContext) { let wd = get_wd_for_workspace(workspace, cx); @@ -160,63 +154,85 @@ impl TerminalView { workspace.add_item(Box::new(view), cx); } + fn clear(&mut self, _: &Clear, cx: &mut ViewContext) { + self.connection + .0 + .as_ref() + .map(|term_handle| term_handle.read(cx).clear()) + .ok(); + } + ///Attempt to paste the clipboard into the terminal fn copy(&mut self, _: &Copy, cx: &mut ViewContext) { self.connection - .read(cx) - .get_terminal() - .and_then(|term| term.copy()) - .map(|text| cx.write_to_clipboard(ClipboardItem::new(text))); + .0 + .as_ref() + .map(|handle| handle.read(cx)) + .map(|term| term.copy()) + .map(|text| text.map(|text| cx.write_to_clipboard(ClipboardItem::new(text)))) + .ok(); } ///Attempt to paste the clipboard into the terminal fn paste(&mut self, _: &Paste, cx: &mut ViewContext) { cx.read_from_clipboard().map(|item| { self.connection - .read(cx) - .get_terminal() - .map(|term| term.paste(item.text())); + .0 + .as_ref() + .map(|handle| handle.read(cx)) + .map(|term| term.paste(item.text())) + .ok(); }); } ///Synthesize the keyboard event corresponding to 'up' fn up(&mut self, _: &Up, cx: &mut ViewContext) { self.connection - .read(cx) - .get_terminal() - .map(|term| term.try_keystroke(&Keystroke::parse("up").unwrap())); + .0 + .as_ref() + .map(|handle| handle.read(cx)) + .map(|term| term.try_keystroke(&Keystroke::parse("up").unwrap())) + .ok(); } ///Synthesize the keyboard event corresponding to 'down' fn down(&mut self, _: &Down, cx: &mut ViewContext) { self.connection - .read(cx) - .get_terminal() - .map(|term| term.try_keystroke(&Keystroke::parse("down").unwrap())); + .0 + .as_ref() + .map(|handle| handle.read(cx)) + .map(|term| term.try_keystroke(&Keystroke::parse("down").unwrap())) + .ok(); } ///Synthesize the keyboard event corresponding to 'ctrl-c' fn ctrl_c(&mut self, _: &CtrlC, cx: &mut ViewContext) { self.connection - .read(cx) - .get_terminal() - .map(|term| term.try_keystroke(&Keystroke::parse("ctrl-c").unwrap())); + .0 + .as_ref() + .map(|handle| handle.read(cx)) + .map(|term| term.try_keystroke(&Keystroke::parse("ctrl-c").unwrap())) + .ok(); } ///Synthesize the keyboard event corresponding to 'escape' fn escape(&mut self, _: &Escape, cx: &mut ViewContext) { self.connection - .read(cx) - .get_terminal() - .map(|term| term.try_keystroke(&Keystroke::parse("escape").unwrap())); + .0 + .as_ref() + .map(|handle| handle.read(cx)) + .map(|term| term.try_keystroke(&Keystroke::parse("escape").unwrap())) + .ok(); } ///Synthesize the keyboard event corresponding to 'enter' fn enter(&mut self, _: &Enter, cx: &mut ViewContext) { self.connection - .read(cx) - .get_terminal() - .map(|term| term.try_keystroke(&Keystroke::parse("enter").unwrap())); + .0 + .as_ref() + .map(|handle| handle.read(cx)) + .map(|term| term.try_keystroke(&Keystroke::parse("enter").unwrap())) + .ok(); } } @@ -226,9 +242,34 @@ impl View for TerminalView { } fn render(&mut self, cx: &mut gpui::RenderContext<'_, Self>) -> ElementBox { - let element = { - let connection_handle = self.connection.clone().downgrade(); - TerminalEl::new(cx.handle(), connection_handle, self.modal).contained() + let element = match self.connection.0.as_ref() { + Ok(handle) => { + let connection_handle = handle.clone().downgrade(); + TerminalEl::new(cx.handle(), connection_handle, self.modal).contained() + } + Err(e) => { + let settings = cx.global::(); + let style = TerminalLayoutData::make_text_style(cx.font_cache(), settings); + + Flex::column() + .with_child( + Flex::row() + .with_child( + Label::new( + format!( + "Failed to open the terminal. Info: \n{}", + e.to_string() + ), + style, + ) + .boxed(), + ) + .aligned() + .boxed(), + ) + .aligned() + .contained() + } }; if self.modal { @@ -261,9 +302,9 @@ impl Item for TerminalView { tab_theme: &theme::Tab, cx: &gpui::AppContext, ) -> ElementBox { - let title = match self.connection.read(cx) { - TerminalConnection::Connected(conn) => conn.title.clone(), - TerminalConnection::Disconnected { .. } => "Terminal".to_string(), //TODO ask nate about htis + let title = match self.connection.0.as_ref() { + Ok(handle) => handle.read(cx).title.clone(), + Err(_) => "Terminal".to_string(), }; Flex::row() @@ -281,12 +322,10 @@ impl Item for TerminalView { //Directory of the terminal from outside the shell. There might be //solutions to this, but they are non-trivial and require more IPC - let wd = self - .connection - .read(cx) - .get_terminal() - .and_then(|term| term.associated_directory.clone()) - .clone(); + let wd = match self.connection.0.as_ref() { + Ok(term_handle) => term_handle.read(cx).associated_directory.clone(), + Err(e) => e.directory.clone(), + }; Some(TerminalView::new(wd, false, cx)) } diff --git a/crates/terminal/src/terminal_element.rs b/crates/terminal/src/terminal_element.rs index 4a92f30365805ccba7a60c61f4431d7650ebf5c5..3383359f5c8063564d6e08128925ceaaace22838 100644 --- a/crates/terminal/src/terminal_element.rs +++ b/crates/terminal/src/terminal_element.rs @@ -32,7 +32,7 @@ use util::ResultExt; use std::{cmp::min, ops::Range}; use std::{fmt::Debug, ops::Sub}; -use crate::{color_translation::convert_color, connection::TerminalConnection, TerminalView}; +use crate::{color_translation::convert_color, connection::Terminal, TerminalView}; use self::terminal_layout_context::TerminalLayoutData; @@ -44,7 +44,7 @@ const ALACRITTY_SCROLL_MULTIPLIER: f32 = 3.; ///The GPUI element that paints the terminal. ///We need to keep a reference to the view for mouse events, do we need it for any other terminal stuff, or can we move that to connection? pub struct TerminalEl { - connection: WeakModelHandle, + connection: WeakModelHandle, view: WeakViewHandle, modal: bool, } @@ -228,7 +228,7 @@ pub struct LayoutState { impl TerminalEl { pub fn new( view: WeakViewHandle, - connection: WeakModelHandle, + connection: WeakModelHandle, modal: bool, ) -> TerminalEl { TerminalEl { @@ -255,19 +255,17 @@ impl TerminalEl { MouseButton::Left, move |MouseButtonEvent { position, .. }, cx| { if let Some(conn_handle) = mouse_down_connection.upgrade(cx.app) { - conn_handle.update(cx.app, |connection, cx| { - connection.get_terminal().map(|terminal| { - let (point, side) = mouse_to_cell_data( - position, - origin, - cur_size, - terminal.get_display_offset(), - ); - - terminal.mouse_down(point, side); - - cx.notify(); - }); + conn_handle.update(cx.app, |terminal, cx| { + let (point, side) = mouse_to_cell_data( + position, + origin, + cur_size, + terminal.get_display_offset(), + ); + + terminal.mouse_down(point, side); + + cx.notify(); }) } }, @@ -282,19 +280,17 @@ impl TerminalEl { cx| { cx.focus_parent_view(); if let Some(conn_handle) = click_connection.upgrade(cx.app) { - conn_handle.update(cx.app, |connection, cx| { - connection.get_terminal().map(|terminal| { - let (point, side) = mouse_to_cell_data( - position, - origin, - cur_size, - terminal.get_display_offset(), - ); - - terminal.click(point, side, click_count); - - cx.notify(); - }) + conn_handle.update(cx.app, |terminal, cx| { + let (point, side) = mouse_to_cell_data( + position, + origin, + cur_size, + terminal.get_display_offset(), + ); + + terminal.click(point, side, click_count); + + cx.notify(); }); } }, @@ -303,19 +299,17 @@ impl TerminalEl { MouseButton::Left, move |_, MouseMovedEvent { position, .. }, cx| { if let Some(conn_handle) = drag_connection.upgrade(cx.app) { - conn_handle.update(cx.app, |connection, cx| { - connection.get_terminal().map(|terminal| { - let (point, side) = mouse_to_cell_data( - position, - origin, - cur_size, - terminal.get_display_offset(), - ); - - terminal.drag(point, side); - - cx.notify() - }); + conn_handle.update(cx.app, |terminal, cx| { + let (point, side) = mouse_to_cell_data( + position, + origin, + cur_size, + terminal.get_display_offset(), + ); + + terminal.drag(point, side); + + cx.notify() }); } }, @@ -336,13 +330,7 @@ impl Element for TerminalEl { let layout = TerminalLayoutData::new(cx.global::(), &cx.font_cache(), constraint.max); - let terminal = self - .connection - .upgrade(cx) - .unwrap() - .read(cx) - .get_terminal() - .unwrap(); + let terminal = self.connection.upgrade(cx).unwrap().read(cx); let (cursor, cells, rects, highlights) = terminal.render_lock(Some(layout.size.clone()), |content| { @@ -488,12 +476,11 @@ impl Element for TerminalEl { let vertical_scroll = (delta.y() / layout.size.line_height) * ALACRITTY_SCROLL_MULTIPLIER; - self.connection - .upgrade(cx.app) - .and_then(|handle| handle.read(cx.app).get_terminal()) - .map(|terminal| { - terminal.scroll(Scroll::Delta(vertical_scroll.round() as i32)); - }); + self.connection.upgrade(cx.app).map(|terminal| { + terminal + .read(cx.app) + .scroll(Scroll::Delta(vertical_scroll.round() as i32)); + }); }) .is_some(), Event::KeyDown(KeyDownEvent { keystroke, .. }) => { @@ -508,7 +495,7 @@ impl Element for TerminalEl { self.connection .upgrade(cx.app) - .and_then(|model_handle| model_handle.read(cx.app).get_terminal()) + .map(|model_handle| model_handle.read(cx.app)) .map(|term| term.try_keystroke(keystroke)) .unwrap_or(false) } diff --git a/crates/terminal/src/terminal_element/terminal_layout_context.rs b/crates/terminal/src/terminal_element/terminal_layout_context.rs index 9484ef08246ada5902d4a83cca17c8777d7f0b6c..6128d17828487f4deddf5e057103ed909fc653fd 100644 --- a/crates/terminal/src/terminal_element/terminal_layout_context.rs +++ b/crates/terminal/src/terminal_element/terminal_layout_context.rs @@ -14,6 +14,7 @@ impl<'a> TerminalLayoutData<'a> { let terminal_theme = &settings.theme.terminal; let line_height = font_cache.line_height(text_style.font_size); + let cell_width = font_cache.em_advance(text_style.font_id, text_style.font_size); let dimensions = TerminalDimensions::new(line_height, cell_width, constraint); @@ -26,7 +27,7 @@ impl<'a> TerminalLayoutData<'a> { } ///Configures a text style from the current settings. - fn make_text_style(font_cache: &FontCache, settings: &Settings) -> TextStyle { + pub fn make_text_style(font_cache: &FontCache, settings: &Settings) -> TextStyle { // Pull the font family from settings properly overriding let family_id = settings .terminal_overrides diff --git a/crates/terminal/src/tests/terminal_test_context.rs b/crates/terminal/src/tests/terminal_test_context.rs index ec967fb183cb54237d41cbfcc00b022fd15a2333..429d397d1f1a1cabc5186569f89d1eade082cd6e 100644 --- a/crates/terminal/src/tests/terminal_test_context.rs +++ b/crates/terminal/src/tests/terminal_test_context.rs @@ -4,13 +4,14 @@ use gpui::{geometry::vector::vec2f, AppContext, ModelHandle, ReadModelWith, Test use itertools::Itertools; use crate::{ - connection::TerminalConnection, terminal_element::TerminalDimensions, DEBUG_CELL_WIDTH, - DEBUG_LINE_HEIGHT, DEBUG_TERMINAL_HEIGHT, DEBUG_TERMINAL_WIDTH, + connection::{DisconnectedPTY, Terminal}, + terminal_element::TerminalDimensions, + DEBUG_CELL_WIDTH, DEBUG_LINE_HEIGHT, DEBUG_TERMINAL_HEIGHT, DEBUG_TERMINAL_WIDTH, }; pub struct TerminalTestContext<'a> { pub cx: &'a mut TestAppContext, - pub connection: ModelHandle, + pub connection: ModelHandle, } impl<'a> TerminalTestContext<'a> { @@ -23,8 +24,11 @@ impl<'a> TerminalTestContext<'a> { vec2f(DEBUG_TERMINAL_WIDTH, DEBUG_TERMINAL_HEIGHT), ); - let connection = - cx.add_model(|cx| TerminalConnection::new_tty(None, None, None, size_info, cx)); + let connection = cx.add_model(|cx| { + DisconnectedPTY::new(None, None, None, size_info) + .unwrap() + .connect(cx) + }); TerminalTestContext { cx, connection } } @@ -35,11 +39,8 @@ impl<'a> TerminalTestContext<'a> { { let command = command.to_string(); self.connection.update(self.cx, |connection, _| { - connection.get_terminal().unwrap().write_to_pty(command); - connection - .get_terminal() - .unwrap() - .write_to_pty("\r".to_string()); + connection.write_to_pty(command); + connection.write_to_pty("\r".to_string()); }); self.connection @@ -55,18 +56,15 @@ impl<'a> TerminalTestContext<'a> { }) } - fn grid_as_str(connection: &TerminalConnection) -> String { - connection - .get_terminal() - .unwrap() - .render_lock(None, |content| { - let lines = content.display_iter.group_by(|i| i.point.line.0); - lines - .into_iter() - .map(|(_, line)| line.map(|i| i.c).collect::()) - .collect::>() - .join("\n") - }) + fn grid_as_str(connection: &Terminal) -> String { + connection.render_lock(None, |content| { + let lines = content.display_iter.group_by(|i| i.point.line.0); + lines + .into_iter() + .map(|(_, line)| line.map(|i| i.c).collect::()) + .collect::>() + .join("\n") + }) } }