ui: embed editor cleanups

ui/src/components/mail/compose.rs

@@ -126,14 +126,6 @@ impl ViewMode {
             false
         }
     }
-
-    fn is_embed(&self) -> bool {
-        if let ViewMode::Embed = self {
-            true
-        } else {
-            false
-        }
-    }
 }
 
 impl fmt::Display for Composer {
@@ -543,23 +535,24 @@ impl Component for Composer {
         /* Regardless of view mode, do the following */
         self.form.draw(grid, header_area, context);
         if let Some(ref mut embed_pty) = self.embed {
-            let body_area = (upper_left!(header_area), bottom_right!(body_area));
+            let embed_area = (upper_left!(header_area), bottom_right!(body_area));
-            clear_area(grid, body_area);
             match embed_pty {
                 EmbedStatus::Running(_, _) => {
                     let mut guard = embed_pty.lock().unwrap();
+                    clear_area(grid, embed_area);
                     copy_area(
                         grid,
                         &guard.grid,
-                        body_area,
+                        embed_area,
                         ((0, 0), pos_dec(guard.terminal_size, (1, 1))),
                     );
-                    guard.set_terminal_size((width!(body_area), height!(body_area)));
+                    guard.set_terminal_size((width!(embed_area), height!(embed_area)));
                     context.dirty_areas.push_back(area);
                     self.dirty = false;
                     return;
                 }
                 EmbedStatus::Stopped(_, _) => {
+                    clear_area(grid, body_area);
                     write_string_to_grid(
                         "process has stopped, press 'e' to re-activate",
                         grid,
@@ -569,9 +562,7 @@ impl Component for Composer {
                         body_area,
                         None,
                     );
-                    context.dirty_areas.push_back(body_area);
+                    context.dirty_areas.push_back(area);
-                    self.dirty = false;
-                    return;
                 }
             }
         } else {
@@ -706,7 +697,10 @@ impl Component for Composer {
             }
             _ => {}
         }
-        if self.cursor == Cursor::Headers && self.form.process_event(event, context) {
+        if self.cursor == Cursor::Headers
+            && self.mode.is_edit()
+            && self.form.process_event(event, context)
+        {
             if let UIEvent::InsertInput(_) = event {
                 self.has_changes = true;
             }
@@ -762,6 +756,12 @@ impl Component for Composer {
             }
             UIEvent::EmbedInput((Key::Ctrl('z'), _)) => {
                 self.embed.as_ref().unwrap().lock().unwrap().stop();
+                match self.embed.take() {
+                    Some(EmbedStatus::Running(e, f)) | Some(EmbedStatus::Stopped(e, f)) => {
+                        self.embed = Some(EmbedStatus::Stopped(e, f));
+                    }
+                    _ => {}
+                }
                 context
                     .replies
                     .push_back(UIEvent::ChangeMode(UIMode::Normal));
@@ -824,6 +824,10 @@ impl Component for Composer {
                                     }
                                     _ => {}
                                 }
+                                self.mode = ViewMode::Edit;
+                                context
+                                    .replies
+                                    .push_back(UIEvent::ChangeMode(UIMode::Normal));
                                 self.dirty = true;
                                 return true;
                             }
@@ -852,15 +856,22 @@ impl Component for Composer {
                 self.set_dirty();
                 return true;
             }
-            UIEvent::Input(Key::Char('e')) if self.mode.is_embed() => {
+            UIEvent::Input(Key::Char('e')) if self.embed.is_some() => {
                 self.embed.as_ref().unwrap().lock().unwrap().wake_up();
+                match self.embed.take() {
+                    Some(EmbedStatus::Running(e, f)) | Some(EmbedStatus::Stopped(e, f)) => {
+                        self.embed = Some(EmbedStatus::Running(e, f));
+                    }
+                    _ => {}
+                }
+                self.mode = ViewMode::Embed;
                 context
                     .replies
                     .push_back(UIEvent::ChangeMode(UIMode::Embed));
                 self.set_dirty();
                 return true;
             }
-            UIEvent::Input(Key::Char('e')) => {
+            UIEvent::Input(Key::Char('e')) if self.mode.is_edit() => {
                 /* Edit draft in $EDITOR */
                 let settings = &context.settings;
                 let editor = if let Some(editor_cmd) = settings.composing.editor_cmd.as_ref() {
@@ -901,6 +912,9 @@ impl Component for Composer {
                     context
                         .replies
                         .push_back(UIEvent::ChangeMode(UIMode::Embed));
+                    context.replies.push_back(UIEvent::Fork(ForkType::Embed(
+                        self.embed.as_ref().unwrap().lock().unwrap().child_pid,
+                    )));
                     self.mode = ViewMode::Embed;
                     return true;
                 }

ui/src/state.rs

@@ -196,6 +196,13 @@ impl Drop for State {
     fn drop(&mut self) {
         // When done, restore the defaults to avoid messing with the terminal.
         self.switch_to_main_screen();
+        if let Some(ForkType::Embed(child_pid)) = self.child.take() {
+            use nix::sys::wait::{waitpid, WaitPidFlag};
+            /* Try wait, we don't want to block */
+            if let Err(e) = waitpid(child_pid, Some(WaitPidFlag::WNOHANG)) {
+                eprintln!("Failed to wait on subprocess {}: {}", child_pid, e);
+            }
+        }
     }
 }
 

ui/src/terminal/embed.rs

@@ -6,7 +6,7 @@ use melib::ERROR;
 use nix::fcntl::{open, OFlag};
 use nix::libc::{STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO};
 use nix::pty::{grantpt, posix_openpt, ptsname, unlockpt, Winsize};
-use nix::sys::{stat, wait::waitpid};
+use nix::sys::stat;
 use nix::unistd::{dup2, fork, ForkResult};
 use nix::{ioctl_none_bad, ioctl_write_ptr_bad};
 use std::ffi::CString;
@@ -66,45 +66,49 @@ pub fn create_pty(
             /* Open slave end for pseudoterminal */
             let slave_fd = open(Path::new(&slave_name), OFlag::O_RDWR, stat::Mode::empty())?;
 
-            let child_pid = match fork() {
+            // assign stdin, stdout, stderr to the tty
-                Ok(ForkResult::Child) => {
+            dup2(slave_fd, STDIN_FILENO).unwrap();
-                    // assign stdin, stdout, stderr to the tty
+            dup2(slave_fd, STDOUT_FILENO).unwrap();
-                    dup2(slave_fd, STDIN_FILENO).unwrap();
+            dup2(slave_fd, STDERR_FILENO).unwrap();
-                    dup2(slave_fd, STDOUT_FILENO).unwrap();
+            /* Become session leader */
-                    dup2(slave_fd, STDERR_FILENO).unwrap();
+            nix::unistd::setsid().unwrap();
-                    /* Become session leader */
+            match unsafe { set_controlling_terminal(slave_fd) } {
-                    nix::unistd::setsid().unwrap();
+                Ok(c) if c < 0 => {
-                    match unsafe { set_controlling_terminal(slave_fd) } {
+                    log(
-                        Ok(c) if c < 0 => {
+                        format!(
-                            log(format!("Could not execute `{}`: ioctl(fd, TIOCSCTTY, NULL) returned {}", command, c,), ERROR);
+                            "Could not execute `{}`: ioctl(fd, TIOCSCTTY, NULL) returned {}",
-                            std::process::exit(c);
+                            command, c,
-                        }
+                        ),
-                        Ok(_) => {}
+                        ERROR,
-                        Err(err) => {
+                    );
-                            log(format!("Could not execute `{}`: ioctl(fd, TIOCSCTTY, NULL) returned {}", command, err,), ERROR);
+                    std::process::exit(c);
-                            std::process::exit(-1);
+                }
-                        }
+                Ok(_) => {}
-                    }
+                Err(err) => {
-                    let parts = split_command!(command);
+                    log(
-                    let (cmd, _) = (parts[0], &parts[1..]);
+                        format!(
-                    if let Err(e) = nix::unistd::execv(
+                            "Could not execute `{}`: ioctl(fd, TIOCSCTTY, NULL) returned {}",
-                        &CString::new(cmd).unwrap(),
+                            command, err,
-                        &parts
+                        ),
-                            .iter()
+                        ERROR,
-                            .map(|&a| CString::new(a).unwrap())
+                    );
-                            .collect::<Vec<CString>>(),
-                    ) {
-                        log(format!("Could not execute `{}`: {}", command, e,), ERROR);
-                        std::process::exit(-1);
-                    }
-                    /* This path shouldn't be executed. */
                     std::process::exit(-1);
                 }
-                Ok(ForkResult::Parent { child }) => child,
+            }
-                Err(e) => panic!(e),
+            let parts = split_command!(command);
-            };
+            let (cmd, _) = (parts[0], &parts[1..]);
-            waitpid(child_pid, None).unwrap();
+            if let Err(e) = nix::unistd::execv(
-            std::process::exit(0);
+                &CString::new(cmd).unwrap(),
+                &parts
+                    .iter()
+                    .map(|&a| CString::new(a).unwrap())
+                    .collect::<Vec<CString>>(),
+            ) {
+                log(format!("Could not execute `{}`: {}", command, e,), ERROR);
+                std::process::exit(-1);
+            }
+            /* This path shouldn't be executed. */
+            std::process::exit(-1);
         }
         Ok(ForkResult::Parent { child }) => child,
         Err(e) => panic!(e),

ui/src/terminal/embed/grid.rs

@@ -633,7 +633,7 @@ impl EmbedGrid {
                         .write_all((terminal_size.0).to_string().as_bytes())
                         .unwrap();
                     stdin.write_all(&[b't']).unwrap();
-                    stdin.flush();
+                    stdin.flush().unwrap();
                 } else {
                     debug!("ignoring unknown code {}", EscCode::from((&(*state), byte)));
                 }
@@ -653,7 +653,7 @@ impl EmbedGrid {
                     .write_all((cursor.0 + 1).to_string().as_bytes())
                     .unwrap();
                 stdin.write_all(&[b'R']).unwrap();
-                stdin.flush();
+                stdin.flush().unwrap();
                 *state = State::Normal;
             }
             (b'A', State::Csi1(buf)) => {

ui/src/types.rs

@@ -28,6 +28,7 @@ use super::terminal::*;
 
 use melib::backends::FolderHash;
 use melib::{EnvelopeHash, RefreshEvent};
+use nix::unistd::Pid;
 use std;
 use std::fmt;
 use std::thread;
@@ -69,7 +70,7 @@ pub enum ForkType {
     /// Already finished fork, we only want to restore input/output
     Finished,
     /// Embed pty
-    Embed,
+    Embed(Pid),
     Generic(std::process::Child),
     NewDraft(File, std::process::Child),
 }