Fix configuring shell in project settings (#39795)

Conrad Irwin created

I mistakenly broke this when refactoring settings

Closes #39479

Release Notes:

- Fixed a bug where you could no longer configure `terminal.shell` in
project settings

Change summary

crates/settings/src/settings_content/project.rs  |  7 +
crates/settings/src/settings_content/terminal.rs | 62 ++++++++++++++---
crates/terminal/src/terminal_settings.rs         | 62 +++++++++--------
3 files changed, 88 insertions(+), 43 deletions(-)

Detailed changes

crates/settings/src/settings_content/project.rs 🔗

@@ -7,7 +7,9 @@ use serde_with::skip_serializing_none;
 use settings_macros::MergeFrom;
 use util::serde::default_true;
 
-use crate::{AllLanguageSettingsContent, ExtendingVec, SlashCommandSettings};
+use crate::{
+    AllLanguageSettingsContent, ExtendingVec, ProjectTerminalSettingsContent, SlashCommandSettings,
+};
 
 #[skip_serializing_none]
 #[derive(Debug, PartialEq, Clone, Default, Serialize, Deserialize, JsonSchema, MergeFrom)]
@@ -29,6 +31,9 @@ pub struct ProjectSettingsContent {
     #[serde(default)]
     pub lsp: HashMap<Arc<str>, LspSettings>,
 
+    #[serde(default)]
+    pub terminal: Option<ProjectTerminalSettingsContent>,
+
     /// Configuration for Debugger-related features
     #[serde(default)]
     pub dap: HashMap<Arc<str>, DapSettingsContent>,

crates/settings/src/settings_content/terminal.rs 🔗

@@ -9,9 +9,8 @@ use settings_macros::MergeFrom;
 
 use crate::FontFamilyName;
 
-#[skip_serializing_none]
 #[derive(Clone, Debug, PartialEq, Default, Serialize, Deserialize, JsonSchema, MergeFrom)]
-pub struct TerminalSettingsContent {
+pub struct ProjectTerminalSettingsContent {
     /// What shell to use when opening a terminal.
     ///
     /// Default: system
@@ -20,6 +19,24 @@ pub struct TerminalSettingsContent {
     ///
     /// Default: current_project_directory
     pub working_directory: Option<WorkingDirectory>,
+    /// Any key-value pairs added to this list will be added to the terminal's
+    /// environment. Use `:` to separate multiple values.
+    ///
+    /// Default: {}
+    pub env: Option<HashMap<String, String>>,
+    /// Activates the python virtual environment, if one is found, in the
+    /// terminal's working directory (as resolved by the working_directory
+    /// setting). Set this to "off" to disable this behavior.
+    ///
+    /// Default: on
+    pub detect_venv: Option<VenvSettings>,
+}
+
+#[skip_serializing_none]
+#[derive(Clone, Debug, PartialEq, Default, Serialize, Deserialize, JsonSchema, MergeFrom)]
+pub struct TerminalSettingsContent {
+    #[serde(flatten)]
+    pub project: ProjectTerminalSettingsContent,
     /// Sets the terminal's font size.
     ///
     /// If this option is not included,
@@ -45,11 +62,6 @@ pub struct TerminalSettingsContent {
     pub font_features: Option<FontFeatures>,
     /// Sets the terminal's font weight in CSS weight units 0-900.
     pub font_weight: Option<f32>,
-    /// Any key-value pairs added to this list will be added to the terminal's
-    /// environment. Use `:` to separate multiple values.
-    ///
-    /// Default: {}
-    pub env: Option<HashMap<String, String>>,
     /// Default cursor shape for the terminal.
     /// Can be "bar", "block", "underline", or "hollow".
     ///
@@ -92,12 +104,6 @@ pub struct TerminalSettingsContent {
     ///
     /// Default: 320
     pub default_height: Option<f32>,
-    /// Activates the python virtual environment, if one is found, in the
-    /// terminal's working directory (as resolved by the working_directory
-    /// setting). Set this to "off" to disable this behavior.
-    ///
-    /// Default: on
-    pub detect_venv: Option<VenvSettings>,
     /// The maximum number of lines to keep in the scrollback history.
     /// Maximum allowed value is 100_000, all values above that will be treated as 100_000.
     /// 0 disables the scrolling.
@@ -348,3 +354,33 @@ pub enum ActivateScript {
     PowerShell,
     Pyenv,
 }
+
+#[cfg(test)]
+mod test {
+    use serde_json::json;
+
+    use crate::{ProjectSettingsContent, Shell, UserSettingsContent};
+
+    #[test]
+    fn test_project_settings() {
+        let project_content =
+            json!({"terminal": {"shell": {"program": "/bin/project"}}, "option_as_meta": true});
+
+        let user_content =
+            json!({"terminal": {"shell": {"program": "/bin/user"}}, "option_as_meta": false});
+
+        let user_settings = serde_json::from_value::<UserSettingsContent>(user_content).unwrap();
+        let project_settings =
+            serde_json::from_value::<ProjectSettingsContent>(project_content).unwrap();
+
+        assert_eq!(
+            user_settings.content.terminal.unwrap().project.shell,
+            Some(Shell::Program("/bin/user".to_owned()))
+        );
+        assert_eq!(user_settings.content.project.terminal, None);
+        assert_eq!(
+            project_settings.terminal.unwrap().shell,
+            Some(Shell::Program("/bin/project".to_owned()))
+        );
+    }
+}

crates/terminal/src/terminal_settings.rs 🔗

@@ -10,6 +10,7 @@ pub use settings::AlternateScroll;
 use settings::{
     CursorShapeContent, SettingsContent, ShowScrollbar, TerminalBlink, TerminalDockPosition,
     TerminalLineHeight, TerminalSettingsContent, VenvSettings, WorkingDirectory,
+    merge_from::MergeFrom,
 };
 use task::Shell;
 use theme::FontFamilyName;
@@ -73,13 +74,16 @@ fn settings_shell_to_task_shell(shell: settings::Shell) -> Shell {
 
 impl settings::Settings for TerminalSettings {
     fn from_settings(content: &settings::SettingsContent) -> Self {
-        let content = content.terminal.clone().unwrap();
+        let user_content = content.terminal.clone().unwrap();
+        // Note: we allow a subset of "terminal" settings in the project files.
+        let mut project_content = user_content.project.clone();
+        project_content.merge_from_option(content.project.terminal.as_ref());
         TerminalSettings {
-            shell: settings_shell_to_task_shell(content.shell.unwrap()),
-            working_directory: content.working_directory.unwrap(),
-            font_size: content.font_size.map(px),
-            font_family: content.font_family,
-            font_fallbacks: content.font_fallbacks.map(|fallbacks| {
+            shell: settings_shell_to_task_shell(project_content.shell.unwrap()),
+            working_directory: project_content.working_directory.unwrap(),
+            font_size: user_content.font_size.map(px),
+            font_family: user_content.font_family,
+            font_fallbacks: user_content.font_fallbacks.map(|fallbacks| {
                 FontFallbacks::from_fonts(
                     fallbacks
                         .into_iter()
@@ -87,29 +91,29 @@ impl settings::Settings for TerminalSettings {
                         .collect(),
                 )
             }),
-            font_features: content.font_features,
-            font_weight: content.font_weight.map(FontWeight),
-            line_height: content.line_height.unwrap(),
-            env: content.env.unwrap(),
-            cursor_shape: content.cursor_shape.map(Into::into),
-            blinking: content.blinking.unwrap(),
-            alternate_scroll: content.alternate_scroll.unwrap(),
-            option_as_meta: content.option_as_meta.unwrap(),
-            copy_on_select: content.copy_on_select.unwrap(),
-            keep_selection_on_copy: content.keep_selection_on_copy.unwrap(),
-            button: content.button.unwrap(),
-            dock: content.dock.unwrap(),
-            default_width: px(content.default_width.unwrap()),
-            default_height: px(content.default_height.unwrap()),
-            detect_venv: content.detect_venv.unwrap(),
-            max_scroll_history_lines: content.max_scroll_history_lines,
+            font_features: user_content.font_features,
+            font_weight: user_content.font_weight.map(FontWeight),
+            line_height: user_content.line_height.unwrap(),
+            env: project_content.env.unwrap(),
+            cursor_shape: user_content.cursor_shape.map(Into::into),
+            blinking: user_content.blinking.unwrap(),
+            alternate_scroll: user_content.alternate_scroll.unwrap(),
+            option_as_meta: user_content.option_as_meta.unwrap(),
+            copy_on_select: user_content.copy_on_select.unwrap(),
+            keep_selection_on_copy: user_content.keep_selection_on_copy.unwrap(),
+            button: user_content.button.unwrap(),
+            dock: user_content.dock.unwrap(),
+            default_width: px(user_content.default_width.unwrap()),
+            default_height: px(user_content.default_height.unwrap()),
+            detect_venv: project_content.detect_venv.unwrap(),
+            max_scroll_history_lines: user_content.max_scroll_history_lines,
             toolbar: Toolbar {
-                breadcrumbs: content.toolbar.unwrap().breadcrumbs.unwrap(),
+                breadcrumbs: user_content.toolbar.unwrap().breadcrumbs.unwrap(),
             },
             scrollbar: ScrollbarSettings {
-                show: content.scrollbar.unwrap().show,
+                show: user_content.scrollbar.unwrap().show,
             },
-            minimum_contrast: content.minimum_contrast.unwrap(),
+            minimum_contrast: user_content.minimum_contrast.unwrap(),
         }
     }
 
@@ -160,7 +164,7 @@ impl settings::Settings for TerminalSettings {
         // TODO: handle arguments
         let shell_name = format!("{platform}Exec");
         if let Some(s) = vscode.read_string(&name(&shell_name)) {
-            current.shell = Some(settings::Shell::Program(s.to_owned()))
+            current.project.shell = Some(settings::Shell::Program(s.to_owned()))
         }
 
         if let Some(env) = vscode
@@ -169,15 +173,15 @@ impl settings::Settings for TerminalSettings {
         {
             for (k, v) in env {
                 if v.is_null()
-                    && let Some(zed_env) = current.env.as_mut()
+                    && let Some(zed_env) = current.project.env.as_mut()
                 {
                     zed_env.remove(k);
                 }
                 let Some(v) = v.as_str() else { continue };
-                if let Some(zed_env) = current.env.as_mut() {
+                if let Some(zed_env) = current.project.env.as_mut() {
                     zed_env.insert(k.clone(), v.to_owned());
                 } else {
-                    current.env = Some([(k.clone(), v.to_owned())].into_iter().collect())
+                    current.project.env = Some([(k.clone(), v.to_owned())].into_iter().collect())
                 }
             }
         }