Finished terminal refactoring

Mikayla Maki created

Change summary

crates/terminal/src/model.rs    | 44 ++++++++++++++--------
crates/terminal/src/terminal.rs | 70 +++++++++++++++++++++++-----------
2 files changed, 75 insertions(+), 39 deletions(-)

Detailed changes

crates/terminal/src/model.rs 🔗

@@ -60,10 +60,9 @@ pub struct TerminalError {
     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
+impl TerminalError {
+    pub fn fmt_directory(&self) -> String {
+        self.directory
             .clone()
             .map(|path| {
                 match path
@@ -79,13 +78,22 @@ impl Display for TerminalError {
                 let default_dir =
                     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(),
+                    Some(dir) => format!("<none specified, using home directory> {}", dir),
+                    None => "<none specified, could not find home directory>".to_string(),
                 }
-            });
+            })
+    }
 
-        let shell = self
-            .shell
+    pub fn shell_to_string(&self) -> Option<String> {
+        self.shell.as_ref().map(|shell| match shell {
+            Shell::System => "<system shell>".to_string(),
+            Shell::Program(p) => p.to_string(),
+            Shell::WithArguments { program, args } => format!("{} {}", program, args.join(" ")),
+        })
+    }
+
+    pub fn fmt_shell(&self) -> String {
+        self.shell
             .clone()
             .map(|shell| match shell {
                 Shell::System => {
@@ -94,7 +102,7 @@ impl Display for TerminalError {
 
                     match pw {
                         Some(pw) => format!("<system defined shell> {}", pw.shell),
-                        None => "<could not access system defined shell>".to_string(),
+                        None => "<could not access the password file>".to_string(),
                     }
                 }
                 Shell::Program(s) => s,
@@ -107,11 +115,17 @@ impl Display for TerminalError {
                     Some(pw) => {
                         format!("<none specified, using system defined shell> {}", pw.shell)
                     }
-                    None => {
-                        "<none specified, could not access system defined shell> {}".to_string()
-                    }
+                    None => "<none specified, could not access the password file> {}".to_string(),
                 }
-            });
+            })
+    }
+}
+
+impl Display for TerminalError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        let dir_string: String = self.fmt_directory();
+
+        let shell = self.fmt_shell();
 
         write!(
             f,
@@ -210,7 +224,6 @@ impl TerminalBuilder {
             pty_tx: Notifier(pty_tx),
             term,
             title: shell_txt.to_string(),
-            associated_directory: working_directory,
         };
 
         Ok(TerminalBuilder {
@@ -245,7 +258,6 @@ pub struct Terminal {
     pty_tx: Notifier,
     term: Arc<FairMutex<Term<ZedListener>>>,
     pub title: String,
-    pub associated_directory: Option<PathBuf>,
 }
 
 impl Terminal {

crates/terminal/src/terminal.rs 🔗

@@ -58,6 +58,7 @@ impl TerminalContent {
 pub struct TerminalView {
     modal: bool,
     content: TerminalContent,
+    associated_directory: Option<PathBuf>,
 }
 
 pub struct ErrorView {
@@ -98,7 +99,8 @@ impl TerminalView {
         let shell = settings.terminal_overrides.shell.clone();
         let envs = settings.terminal_overrides.env.clone(); //Should be short and cheap.
 
-        let content = match TerminalBuilder::new(working_directory, shell, envs, size_info) {
+        let content = match TerminalBuilder::new(working_directory.clone(), shell, envs, size_info)
+        {
             Ok(terminal) => {
                 let terminal = cx.add_model(|cx| terminal.subscribe(cx));
                 let view = cx.add_view(|cx| ConnectedView::from_terminal(terminal, modal, cx));
@@ -115,7 +117,11 @@ impl TerminalView {
         };
         cx.focus(content.handle());
 
-        TerminalView { modal, content }
+        TerminalView {
+            modal,
+            content,
+            associated_directory: working_directory,
+        }
     }
 
     fn from_terminal(
@@ -127,6 +133,7 @@ impl TerminalView {
         TerminalView {
             modal,
             content: TerminalContent::Connected(connected_view),
+            associated_directory: None,
         }
     }
 }
@@ -176,16 +183,39 @@ impl View for ErrorView {
         let settings = cx.global::<Settings>();
         let style = TerminalEl::make_text_style(cx.font_cache(), settings);
 
-        Label::new(
-            format!(
-                "Failed to open the terminal. Info: \n{}",
-                self.error.to_string()
-            ),
-            style,
-        )
-        .aligned()
-        .contained()
-        .boxed()
+        //TODO:
+        //We want markdown style highlighting so we can format the program and working directory with ``
+        //We want a max-width of 75% with word-wrap
+        //We want to be able to select the text
+        //Want to be able to scroll if the error message is massive somehow (resiliency)
+
+        let program_text = {
+            match self.error.shell_to_string() {
+                Some(shell_txt) => format!("Shell Program: `{}`", shell_txt),
+                None => "No program specified".to_string(),
+            }
+        };
+
+        let directory_text = {
+            match self.error.directory.as_ref() {
+                Some(path) => format!("Working directory: `{}`", path.to_string_lossy()),
+                None => "No working directory specified".to_string(),
+            }
+        };
+
+        let error_text = self.error.source.to_string();
+
+        Flex::column()
+            .with_child(
+                Text::new("Failed to open the terminal.".to_string(), style.clone())
+                    .contained()
+                    .boxed(),
+            )
+            .with_child(Text::new(program_text, style.clone()).contained().boxed())
+            .with_child(Text::new(directory_text, style.clone()).contained().boxed())
+            .with_child(Text::new(error_text, style.clone()).contained().boxed())
+            .aligned()
+            .boxed()
     }
 }
 
@@ -217,17 +247,11 @@ impl Item for TerminalView {
         //From what I can tell, there's no  way to tell the current working
         //Directory of the terminal from outside the shell. There might be
         //solutions to this, but they are non-trivial and require more IPC
-        if let TerminalContent::Connected(connected) = &self.content {
-            let associated_directory = connected
-                .read(cx)
-                .handle()
-                .read(cx)
-                .associated_directory
-                .clone();
-            Some(TerminalView::new(associated_directory, false, cx))
-        } else {
-            None
-        }
+        Some(TerminalView::new(
+            self.associated_directory.clone(),
+            false,
+            cx,
+        ))
     }
 
     fn project_path(&self, _cx: &gpui::AppContext) -> Option<ProjectPath> {