Rebasing continues

Mikayla Maki created

Change summary

Cargo.lock                                         |  1 
crates/gpui/src/app.rs                             | 15 ++++++
crates/settings/src/settings.rs                    |  2 
crates/terminal/Cargo.toml                         |  1 
crates/terminal/src/connection.rs                  | 37 ++++-----------
crates/terminal/src/modal.rs                       |  7 ++
crates/terminal/src/terminal.rs                    | 26 +++++++---
crates/terminal/src/tests/terminal_test_context.rs |  2 
8 files changed, 52 insertions(+), 39 deletions(-)

Detailed changes

Cargo.lock 🔗

@@ -5355,6 +5355,7 @@ name = "terminal"
 version = "0.1.0"
 dependencies = [
  "alacritty_terminal",
+ "anyhow",
  "client",
  "dirs 4.0.0",
  "editor",

crates/gpui/src/app.rs 🔗

@@ -1781,6 +1781,21 @@ impl MutableAppContext {
         })
     }
 
+    pub fn try_add_model<T, F>(&mut self, build_model: F) -> Result<ModelHandle<T>>
+    where
+        T: Entity,
+        F: FnOnce(&mut ModelContext<T>) -> Result<T>,
+    {
+        self.update(|this| {
+            let model_id = post_inc(&mut this.next_entity_id);
+            let handle = ModelHandle::new(model_id, &this.cx.ref_counts);
+            let mut cx = ModelContext::new(this, model_id);
+            let model = build_model(&mut cx)?;
+            this.cx.models.insert(model_id, Box::new(model));
+            Ok(handle)
+        })
+    }
+
     pub fn add_window<T, F>(
         &mut self,
         window_options: WindowOptions,

crates/settings/src/settings.rs 🔗

@@ -81,7 +81,7 @@ pub struct TerminalSettings {
     pub working_directory: Option<WorkingDirectory>,
     pub font_size: Option<f32>,
     pub font_family: Option<String>,
-    pub env: Option<Vec<(String, String)>>,
+    pub env: Option<HashMap<String, String>>,
 }
 
 #[derive(Clone, Debug, Deserialize, PartialEq, Eq, JsonSchema)]

crates/terminal/Cargo.toml 🔗

@@ -24,6 +24,7 @@ itertools = "0.10"
 dirs = "4.0.0"
 shellexpand = "2.1.0"
 libc = "0.2"
+anyhow = "1"
 
 
 [dev-dependencies]

crates/terminal/src/connection.rs 🔗

@@ -11,9 +11,10 @@ use alacritty_terminal::{
     tty::{self, setup_env},
     Term,
 };
-use futures::{
-    channel::mpsc::{unbounded, UnboundedSender},
-    StreamExt,
+use anyhow::Result;
+use futures::channel::mpsc::{
+    unbounded::{self, UndboundedSender},
+    UnboundedSender,
 };
 use settings::{Settings, Shell};
 use std::{collections::HashMap, path::PathBuf, sync::Arc};
@@ -59,10 +60,10 @@ impl TerminalConnection {
     pub fn new(
         working_directory: Option<PathBuf>,
         shell: Option<Shell>,
-        env_vars: Option<Vec<(String, String)>>,
+        env: Option<HashMap<String, String>>,
         initial_size: SizeInfo,
         cx: &mut ModelContext<Self>,
-    ) -> TerminalConnection {
+    ) -> Result<TerminalConnection> {
         let pty_config = {
             let shell = shell.and_then(|shell| match shell {
                 Shell::System => None,
@@ -77,12 +78,7 @@ impl TerminalConnection {
             }
         };
 
-        let mut env: HashMap<String, String> = HashMap::new();
-        if let Some(envs) = env_vars {
-            for (var, val) in envs {
-                env.insert(var, val);
-            }
-        }
+        let mut env = env.unwrap_or_else(|| HashMap::new());
 
         //TODO: Properly set the current locale,
         env.insert("LC_ALL".to_string(), "en_US.UTF-8".to_string());
@@ -103,20 +99,7 @@ impl TerminalConnection {
         let term = Arc::new(FairMutex::new(term));
 
         //Setup the pty...
-        let pty = {
-            if let Some(pty) = tty::new(&pty_config, &initial_size, None).ok() {
-                pty
-            } else {
-                let pty_config = PtyConfig {
-                    shell: None,
-                    working_directory: working_directory.clone(),
-                    ..Default::default()
-                };
-
-                tty::new(&pty_config, &initial_size, None)
-                    .expect("Failed with default shell too :(")
-            }
-        };
+        let pty = tty::new(&pty_config, &initial_size, None)?;
 
         let shell = {
             let mut buf = [0; 1024];
@@ -154,13 +137,13 @@ impl TerminalConnection {
         })
         .detach();
 
-        TerminalConnection {
+        Ok(TerminalConnection {
             pty_tx: Notifier(pty_tx),
             term,
             title: shell.to_string(),
             cur_size: initial_size,
             associated_directory: working_directory,
-        }
+        })
     }
 
     ///Takes events from Alacritty and translates them to behavior on this view

crates/terminal/src/modal.rs 🔗

@@ -25,7 +25,12 @@ pub fn deploy_modal(workspace: &mut Workspace, _: &DeployModal, cx: &mut ViewCon
         // No connection was stored, create a new terminal
         if let Some(closed_terminal_handle) = workspace.toggle_modal(cx, |workspace, cx| {
             let wd = get_wd_for_workspace(workspace, cx);
-            let this = cx.add_view(|cx| Terminal::new(wd, true, cx));
+
+            //TODO: Anything other than crash.
+            let this = cx
+                .add_option_view(|cx| Terminal::new(wd, true, cx).ok())
+                .unwrap();
+
             let connection_handle = this.read(cx).connection.clone();
             cx.subscribe(&connection_handle, on_event).detach();
             //Set the global immediately, in case the user opens the command palette

crates/terminal/src/terminal.rs 🔗

@@ -4,7 +4,7 @@ mod modal;
 pub mod terminal_element;
 
 use alacritty_terminal::term::SizeInfo;
-
+use anyhow::Result;
 use connection::{Event, TerminalConnection};
 use dirs::home_dir;
 use gpui::{
@@ -79,7 +79,11 @@ impl Entity for Terminal {
 impl Terminal {
     ///Create a new Terminal view. This spawns a task, a thread, and opens the TTY devices
     ///To get the right working directory from a workspace, use: `get_wd_for_workspace()`
-    fn new(working_directory: Option<PathBuf>, modal: bool, cx: &mut ViewContext<Self>) -> Self {
+    fn new(
+        working_directory: Option<PathBuf>,
+        modal: bool,
+        cx: &mut ViewContext<Self>,
+    ) -> Result<Self> {
         //The details here don't matter, the terminal will be resized on the first layout
         let size_info = SizeInfo::new(
             DEBUG_TERMINAL_WIDTH,
@@ -98,10 +102,11 @@ impl Terminal {
             (shell, envs)
         };
 
-        let connection = cx
-            .add_model(|cx| TerminalConnection::new(working_directory, shell, envs, size_info, cx));
+        let connection = cx.try_add_model(|cx| {
+            TerminalConnection::new(working_directory, shell, envs, size_info, cx)
+        })?;
 
-        Terminal::from_connection(connection, modal, cx)
+        Ok(Terminal::from_connection(connection, modal, cx))
     }
 
     fn from_connection(
@@ -152,7 +157,9 @@ impl Terminal {
     ///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);
-        workspace.add_item(Box::new(cx.add_view(|cx| Terminal::new(wd, false, cx))), cx);
+        if let Some(view) = cx.add_option_view(|cx| Terminal::new(wd, false, cx).ok()) {
+            workspace.add_item(Box::new(view), cx);
+        }
     }
 
     ///Attempt to paste the clipboard into the terminal
@@ -266,13 +273,14 @@ impl Item for Terminal {
 
     fn clone_on_split(&self, cx: &mut ViewContext<Self>) -> Option<Self> {
         //From what I can tell, there's no  way to tell the current working
-        //Directory of the terminal from outside the terminal. There might be
+        //Directory of the terminal from outside the shell. There might be
         //solutions to this, but they are non-trivial and require more IPC
-        Some(Terminal::new(
+        Terminal::new(
             self.connection.read(cx).associated_directory.clone(),
             false,
             cx,
-        ))
+        )
+        .ok()
     }
 
     fn project_path(&self, _cx: &gpui::AppContext) -> Option<ProjectPath> {

crates/terminal/src/tests/terminal_test_context.rs 🔗

@@ -29,7 +29,7 @@ impl<'a> TerminalTestContext<'a> {
         );
 
         let connection =
-            cx.add_model(|cx| TerminalConnection::new(None, None, None, size_info, cx));
+            cx.add_model(|cx| TerminalConnection::new(None, None, None, size_info, cx).unwrap());
 
         TerminalTestContext { cx, connection }
     }