task: Spawn static tasks in separate shell (#8827)

Piotr Osiewicz created 2 years ago

That way one can use environment variables in task definitions.

Fixes: #8660

/cc @SomeoneToIgnore it looks like we don't ever set `separate_shell` to
false anymore, it might be worth streamlining?

Release Notes:

- Fixed static tasks not being run under a separate shell.
- Removed `separate_shell` setting from task definitions. It is now a default for tasks defined in tasks.json file.

Change summary

crates/task/src/lib.rs                     |  2 -
crates/task/src/oneshot_source.rs          |  1 
crates/task/src/static_source.rs           |  1 
crates/terminal_view/src/terminal_panel.rs | 32 ++++++++++++-----------
4 files changed, 17 insertions(+), 19 deletions(-)

Detailed changes

crates/task/src/lib.rs

@@ -34,8 +34,6 @@ pub struct SpawnInTerminal {
     pub use_new_terminal: bool,
     /// Whether to allow multiple instances of the same task to be run, or rather wait for the existing ones to finish.
     pub allow_concurrent_runs: bool,
-    /// Whether the command should be spawned in a separate shell instance.
-    pub separate_shell: bool,
 }
 
 /// Represents a short lived recipe of a task, whose main purpose

crates/task/src/oneshot_source.rs

@@ -47,7 +47,6 @@ impl Task for OneshotTask {
             env: Default::default(),
             use_new_terminal: Default::default(),
             allow_concurrent_runs: Default::default(),
-            separate_shell: true,
         })
     }
 }

crates/task/src/static_source.rs

@@ -34,7 +34,6 @@ impl Task for StaticTask {
             command: self.definition.command.clone(),
             args: self.definition.args.clone(),
             env: self.definition.env.clone(),
-            separate_shell: false,
         })
     }
 

crates/terminal_view/src/terminal_panel.rs

@@ -303,23 +303,25 @@ impl TerminalPanel {
             args: spawn_in_terminal.args.clone(),
             env: spawn_in_terminal.env.clone(),
         };
-        if spawn_in_terminal.separate_shell {
-            let Some((shell, mut user_args)) = (match TerminalSettings::get_global(cx).shell.clone()
-            {
-                Shell::System => std::env::var("SHELL").ok().map(|shell| (shell, vec![])),
-                Shell::Program(shell) => Some((shell, vec![])),
-                Shell::WithArguments { program, args } => Some((program, args)),
-            }) else {
-                return;
-            };
+        // Set up shell args unconditionally, as tasks are always spawned inside of a shell.
+        let Some((shell, mut user_args)) = (match TerminalSettings::get_global(cx).shell.clone() {
+            Shell::System => std::env::var("SHELL").ok().map(|shell| (shell, vec![])),
+            Shell::Program(shell) => Some((shell, vec![])),
+            Shell::WithArguments { program, args } => Some((program, args)),
+        }) else {
+            return;
+        };
 
-            let command = std::mem::take(&mut spawn_task.command);
-            let args = std::mem::take(&mut spawn_task.args);
-            spawn_task.command = shell;
-            user_args.extend(["-i".to_owned(), "-c".to_owned(), command]);
-            user_args.extend(args);
-            spawn_task.args = user_args;
+        let mut command = std::mem::take(&mut spawn_task.command);
+        let args = std::mem::take(&mut spawn_task.args);
+        for arg in args {
+            command.push(' ');
+            command.push_str(&arg);
         }
+        spawn_task.command = shell;
+        user_args.extend(["-i".to_owned(), "-c".to_owned(), command]);
+        spawn_task.args = user_args;
+
         let working_directory = spawn_in_terminal.cwd.clone();
         let allow_concurrent_runs = spawn_in_terminal.allow_concurrent_runs;
         let use_new_terminal = spawn_in_terminal.use_new_terminal;