Fix bugs resulting from refactoring the terminal into project and workspace halves

Mikayla Maki created

Change summary

crates/db/src/query.rs                              |  4 
crates/settings/src/settings.rs                     | 28 ++++++
crates/terminal/src/terminal.rs                     |  7 
crates/terminal_view/src/persistence.rs             | 22 ++--
crates/terminal_view/src/terminal_container_view.rs | 62 ++++++--------
crates/terminal_view/src/terminal_element.rs        |  9 -
crates/terminal_view/src/terminal_view.rs           | 45 +++++++---
7 files changed, 101 insertions(+), 76 deletions(-)

Detailed changes

crates/db/src/query.rs 🔗

@@ -199,10 +199,10 @@ macro_rules! query {
             use $crate::anyhow::Context;
 
 
-            self.write(|connection| {
+            self.write(move |connection| {
                 let sql_stmt = $crate::sqlez_macros::sql!($($sql)+);
 
-                connection.select_row_bound::<($($arg_type),+), $return_type>(indoc! { $sql })?(($($arg),+))
+                connection.select_row_bound::<($($arg_type),+), $return_type>(sql_stmt)?(($($arg),+))
                     .context(::std::format!(
                         "Error in {}, select_row_bound failed to execute or parse for: {}",
                         ::std::stringify!($id),

crates/settings/src/settings.rs 🔗

@@ -199,7 +199,7 @@ impl Default for Shell {
     }
 }
 
-#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq, JsonSchema)]
+#[derive(Clone, Copy, Debug, Serialize, Deserialize, PartialEq, Eq, JsonSchema)]
 #[serde(rename_all = "snake_case")]
 pub enum AlternateScroll {
     On,
@@ -473,6 +473,32 @@ impl Settings {
         })
     }
 
+    pub fn terminal_scroll(&self) -> AlternateScroll {
+        *self.terminal_overrides.alternate_scroll.as_ref().unwrap_or(
+            self.terminal_defaults
+                .alternate_scroll
+                .as_ref()
+                .unwrap_or_else(|| &AlternateScroll::On),
+        )
+    }
+
+    pub fn terminal_shell(&self) -> Option<Shell> {
+        self.terminal_overrides
+            .shell
+            .as_ref()
+            .or(self.terminal_defaults.shell.as_ref())
+            .cloned()
+    }
+
+    pub fn terminal_env(&self) -> HashMap<String, String> {
+        self.terminal_overrides.env.clone().unwrap_or_else(|| {
+            self.terminal_defaults
+                .env
+                .clone()
+                .unwrap_or_else(|| HashMap::default())
+        })
+    }
+
     #[cfg(any(test, feature = "test-support"))]
     pub fn test(cx: &gpui::AppContext) -> Settings {
         Settings {

crates/terminal/src/terminal.rs 🔗

@@ -269,9 +269,9 @@ impl TerminalBuilder {
     pub fn new(
         working_directory: Option<PathBuf>,
         shell: Option<Shell>,
-        env: Option<HashMap<String, String>>,
+        mut env: HashMap<String, String>,
         blink_settings: Option<TerminalBlink>,
-        alternate_scroll: &AlternateScroll,
+        alternate_scroll: AlternateScroll,
         window_id: usize,
     ) -> Result<TerminalBuilder> {
         let pty_config = {
@@ -288,10 +288,9 @@ impl TerminalBuilder {
             }
         };
 
-        let mut env = env.unwrap_or_default();
-
         //TODO: Properly set the current locale,
         env.insert("LC_ALL".to_string(), "en_US.UTF-8".to_string());
+        env.insert("ZED_TERM".to_string(), true.to_string());
 
         let alac_scrolling = Scrolling::default();
         // alac_scrolling.set_history((BACK_BUFFER_SIZE * 2) as u32);

crates/terminal_view/src/persistence.rs 🔗

@@ -1,20 +1,18 @@
 use std::path::PathBuf;
 
 use db::{define_connection, query, sqlez_macros::sql};
-use workspace::{WorkspaceDb, WorkspaceId};
-
-type ModelId = usize;
+use workspace::{ItemId, WorkspaceDb, WorkspaceId};
 
 define_connection! {
     pub static ref TERMINAL_DB: TerminalDb<WorkspaceDb> =
         &[sql!(
             CREATE TABLE terminals (
                 workspace_id INTEGER,
-                model_id INTEGER UNIQUE,
+                item_id INTEGER UNIQUE,
                 working_directory BLOB,
-                PRIMARY KEY(workspace_id, model_id),
+                PRIMARY KEY(workspace_id, item_id),
                 FOREIGN KEY(workspace_id) REFERENCES workspaces(workspace_id)
-                    ON DELETE CASCADE
+                ON DELETE CASCADE
             ) STRICT;
         )];
 }
@@ -24,7 +22,7 @@ impl TerminalDb {
        pub async fn update_workspace_id(
             new_id: WorkspaceId,
             old_id: WorkspaceId,
-            item_id: ModelId
+            item_id: ItemId
         ) -> Result<()> {
             UPDATE terminals
             SET workspace_id = ?
@@ -34,8 +32,8 @@ impl TerminalDb {
 
     query! {
         pub async fn save_working_directory(
-            item_id: ModelId,
-            workspace_id: i64,
+            item_id: ItemId,
+            workspace_id: WorkspaceId,
             working_directory: PathBuf
         ) -> Result<()> {
             INSERT OR REPLACE INTO terminals(item_id, workspace_id, working_directory)
@@ -44,10 +42,10 @@ impl TerminalDb {
     }
 
     query! {
-        pub fn get_working_directory(item_id: ModelId, workspace_id: WorkspaceId) -> Result<Option<PathBuf>> {
-            SELECT working_directory
-            FROM terminals
+        pub async fn take_working_directory(item_id: ItemId, workspace_id: WorkspaceId) -> Result<Option<PathBuf>> {
+            DELETE FROM terminals
             WHERE item_id = ? AND workspace_id = ?
+            RETURNING working_directory
         }
     }
 }

crates/terminal_view/src/terminal_container_view.rs 🔗

@@ -22,7 +22,7 @@ use workspace::{
 use workspace::{register_deserializable_item, Pane, WorkspaceId};
 
 use project::{LocalWorktree, Project, ProjectPath};
-use settings::{AlternateScroll, Settings, WorkingDirectory};
+use settings::{Settings, WorkingDirectory};
 use smallvec::SmallVec;
 use std::ops::RangeInclusive;
 use std::path::{Path, PathBuf};
@@ -99,25 +99,13 @@ impl TerminalContainer {
     pub fn new(
         working_directory: Option<PathBuf>,
         modal: bool,
-        _workspace_id: WorkspaceId,
+        workspace_id: WorkspaceId,
         cx: &mut ViewContext<Self>,
     ) -> Self {
         let settings = cx.global::<Settings>();
-        let shell = settings.terminal_overrides.shell.clone();
-        let envs = settings.terminal_overrides.env.clone(); //Should be short and cheap.
-
-        //TODO: move this pattern to settings
-        let scroll = settings
-            .terminal_overrides
-            .alternate_scroll
-            .as_ref()
-            .unwrap_or(
-                settings
-                    .terminal_defaults
-                    .alternate_scroll
-                    .as_ref()
-                    .unwrap_or_else(|| &AlternateScroll::On),
-            );
+        let shell = settings.terminal_shell();
+        let envs = settings.terminal_env();
+        let scroll = settings.terminal_scroll();
 
         let content = match TerminalBuilder::new(
             working_directory.clone(),
@@ -129,7 +117,10 @@ impl TerminalContainer {
         ) {
             Ok(terminal) => {
                 let terminal = cx.add_model(|cx| terminal.subscribe(cx));
-                let view = cx.add_view(|cx| TerminalView::from_terminal(terminal, modal, cx));
+                let item_id = cx.view_id();
+                let view = cx.add_view(|cx| {
+                    TerminalView::from_terminal(terminal, modal, workspace_id, item_id, cx)
+                });
 
                 cx.subscribe(&view, |_this, _content, event, cx| cx.emit(*event))
                     .detach();
@@ -394,25 +385,26 @@ impl Item for TerminalContainer {
         item_id: workspace::ItemId,
         cx: &mut ViewContext<Pane>,
     ) -> Task<anyhow::Result<ViewHandle<Self>>> {
-        let working_directory = TERMINAL_DB.get_working_directory(item_id, workspace_id);
-        Task::ready(Ok(cx.add_view(|cx| {
-            TerminalContainer::new(
-                working_directory.log_err().flatten(),
-                false,
-                workspace_id,
-                cx,
-            )
-        })))
+        cx.spawn(|pane, mut cx| async move {
+            let cwd = TERMINAL_DB
+                .take_working_directory(item_id, workspace_id)
+                .await
+                .log_err()
+                .flatten();
+
+            cx.update(|cx| {
+                Ok(cx.add_view(pane, |cx| {
+                    TerminalContainer::new(cwd, false, workspace_id, cx)
+                }))
+            })
+        })
     }
 
-    fn added_to_workspace(&mut self, _workspace: &mut Workspace, cx: &mut ViewContext<Self>) {
-        if let Some(_connected) = self.connected() {
-            // let id = workspace.database_id();
-            // let terminal_handle = connected.read(cx).terminal().clone();
-            //TODO
-            cx.background()
-                .spawn(TERMINAL_DB.update_workspace_id(0, 0, 0))
-                .detach();
+    fn added_to_workspace(&mut self, workspace: &mut Workspace, cx: &mut ViewContext<Self>) {
+        if let Some(connected) = self.connected() {
+            connected.update(cx, |connected_view, cx| {
+                connected_view.added_to_workspace(workspace.database_id(), cx);
+            })
         }
     }
 }

crates/terminal_view/src/terminal_element.rs 🔗

@@ -18,7 +18,7 @@ use ordered_float::OrderedFloat;
 use settings::Settings;
 use terminal::{
     alacritty_terminal::{
-        ansi::{Color as AnsiColor, CursorShape as AlacCursorShape, NamedColor},
+        ansi::{Color as AnsiColor, Color::Named, CursorShape as AlacCursorShape, NamedColor},
         grid::Dimensions,
         index::Point,
         term::{cell::Flags, TermMode},
@@ -198,10 +198,7 @@ impl TerminalElement {
 
                 //Expand background rect range
                 {
-                    if matches!(
-                        bg,
-                        terminal::alacritty_terminal::ansi::Color::Named(NamedColor::Background)
-                    ) {
+                    if matches!(bg, Named(NamedColor::Background)) {
                         //Continue to next cell, resetting variables if nescessary
                         cur_alac_color = None;
                         if let Some(rect) = cur_rect {
@@ -639,7 +636,7 @@ impl Element for TerminalElement {
 
         //Layout cursor. Rectangle is used for IME, so we should lay it out even
         //if we don't end up showing it.
-        let cursor = if let terminal::alacritty_terminal::ansi::CursorShape::Hidden = cursor.shape {
+        let cursor = if let AlacCursorShape::Hidden = cursor.shape {
             None
         } else {
             let cursor_point = DisplayCursor::from(cursor.point, *display_offset);

crates/terminal_view/src/terminal_view.rs 🔗

@@ -1,4 +1,4 @@
-use std::{ops::RangeInclusive, path::PathBuf, time::Duration};
+use std::{ops::RangeInclusive, time::Duration};
 
 use context_menu::{ContextMenu, ContextMenuItem};
 use gpui::{
@@ -21,7 +21,7 @@ use terminal::{
     Terminal,
 };
 use util::ResultExt;
-use workspace::pane;
+use workspace::{pane, ItemId, WorkspaceId};
 
 use crate::{persistence::TERMINAL_DB, terminal_element::TerminalElement, Event};
 
@@ -75,6 +75,8 @@ pub struct TerminalView {
     blinking_on: bool,
     blinking_paused: bool,
     blink_epoch: usize,
+    workspace_id: WorkspaceId,
+    item_id: ItemId,
 }
 
 impl Entity for TerminalView {
@@ -85,6 +87,8 @@ impl TerminalView {
     pub fn from_terminal(
         terminal: ModelHandle<Terminal>,
         modal: bool,
+        workspace_id: WorkspaceId,
+        item_id: ItemId,
         cx: &mut ViewContext<Self>,
     ) -> Self {
         cx.observe(&terminal, |_, _, cx| cx.notify()).detach();
@@ -102,20 +106,20 @@ impl TerminalView {
             }
             Event::BlinkChanged => this.blinking_on = !this.blinking_on,
             Event::TitleChanged => {
-                // if let Some(foreground_info) = &terminal.read(cx).foreground_process_info {
-                // let cwd = foreground_info.cwd.clone();
-                //TODO
-                // let item_id = self.item_id;
-                // let workspace_id = self.workspace_id;
-                cx.background()
-                    .spawn(async move {
-                        TERMINAL_DB
-                            .save_working_directory(0, 0, PathBuf::new())
-                            .await
-                            .log_err();
-                    })
-                    .detach();
-                // }
+                if let Some(foreground_info) = &this.terminal().read(cx).foreground_process_info {
+                    let cwd = foreground_info.cwd.clone();
+
+                    let item_id = this.item_id;
+                    let workspace_id = this.workspace_id;
+                    cx.background()
+                        .spawn(async move {
+                            TERMINAL_DB
+                                .save_working_directory(item_id, workspace_id, cwd)
+                                .await
+                                .log_err();
+                        })
+                        .detach();
+                }
             }
             _ => cx.emit(*event),
         })
@@ -131,6 +135,8 @@ impl TerminalView {
             blinking_on: false,
             blinking_paused: false,
             blink_epoch: 0,
+            workspace_id,
+            item_id,
         }
     }
 
@@ -282,6 +288,13 @@ impl TerminalView {
         &self.terminal
     }
 
+    pub fn added_to_workspace(&mut self, new_id: WorkspaceId, cx: &mut ViewContext<Self>) {
+        cx.background()
+            .spawn(TERMINAL_DB.update_workspace_id(new_id, self.workspace_id, self.item_id))
+            .detach();
+        self.workspace_id = new_id;
+    }
+
     fn next_blink_epoch(&mut self) -> usize {
         self.blink_epoch += 1;
         self.blink_epoch