Compiling with new result based error handling

Mikayla Maki created

Change summary

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 
crates/terminal/src/terminal_element/terminal_layout_context.rs |   3 
crates/terminal/src/tests/terminal_test_context.rs              |  42 
6 files changed, 198 insertions(+), 165 deletions(-)

Detailed changes

crates/terminal/src/connection.rs 🔗

@@ -56,15 +56,16 @@ impl EventListener for ZedListener {
 
 #[derive(Error, Debug)]
 pub struct TerminalError {
-    directory: Option<PathBuf>,
-    shell: Option<Shell>,
-    source: std::io::Error,
+    pub directory: Option<PathBuf>,
+    pub shell: Option<Shell>,
+    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!("<none specified, using home> {}", dir),
                     None => "<none specified, could not find home>".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>) -> Terminal {
+    pub fn connect(mut self, cx: &mut ModelContext<Terminal>) -> 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();

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::<Option<StoredConnection>>(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::<Option<StoredConnection>>(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)));
         }
     }

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<ModelHandle<Terminal>, TerminalError>);
+
 ///A terminal view, maintains the PTY's file handles and communicates with the terminal
 pub struct TerminalView {
-    connection: Result<ModelHandle<Terminal>, 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::<Settings>();
-            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::<Settings>();
+        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<ModelHandle<Terminal>, TerminalError>,
+        connection: TerminalConnection,
         modal: bool,
         cx: &mut ViewContext<Self>,
     ) -> 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>) {
-        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<Workspace>) {
         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>) {
+        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>) {
         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<Self>) {
         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>) {
         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>) {
         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>) {
         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>) {
         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>) {
         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::<Settings>();
+                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))
     }

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<TerminalConnection>,
+    connection: WeakModelHandle<Terminal>,
     view: WeakViewHandle<TerminalView>,
     modal: bool,
 }
@@ -228,7 +228,7 @@ pub struct LayoutState {
 impl TerminalEl {
     pub fn new(
         view: WeakViewHandle<TerminalView>,
-        connection: WeakModelHandle<TerminalConnection>,
+        connection: WeakModelHandle<Terminal>,
         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::<Settings>(), &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)
             }

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

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<TerminalConnection>,
+    pub connection: ModelHandle<Terminal>,
 }
 
 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::<String>())
-                    .collect::<Vec<String>>()
-                    .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::<String>())
+                .collect::<Vec<String>>()
+                .join("\n")
+        })
     }
 }