debugger beta: Fix bug where debug Rust main running action failed (#31291)

Anthony Eid created 6 months ago

@osiewicz @SomeoneToIgnore If you guys have time to look this over it
would be greatly appreciated. I wanted to move the bug fix into the task
resolution code but wasn't sure if there was a reason that we didn't
already.

The bug is caused by an env variable being empty when we send it as a
terminal command. When the shell resolves all the env variables there's
an extra space that gets added due to the empty env variable being
placed between two other variables.

Closes #31240

Release Notes:

- debugger beta: Fix a bug where debug main Rust runner action wouldn't
work

Change summary

crates/debugger_ui/src/session/running.rs |  9 +----
crates/task/src/task_template.rs          | 41 ++++++++++++++++++++++++
2 files changed, 42 insertions(+), 8 deletions(-)

Detailed changes

crates/debugger_ui/src/session/running.rs

@@ -874,6 +874,7 @@ impl RunningState {
                     args,
                     ..task.resolved.clone()
                 };
+
                 let terminal = project
                     .update_in(cx, |project, window, cx| {
                         project.create_terminal(
@@ -918,12 +919,6 @@ impl RunningState {
             };
 
             if config_is_valid {
-                // Ok(DebugTaskDefinition {
-                //     label,
-                //     adapter: DebugAdapterName(adapter),
-                //     config,
-                //     tcp_connection,
-                // })
             } else if let Some((task, locator_name)) = build_output {
                 let locator_name =
                     locator_name.context("Could not find a valid locator for a build task")?;
@@ -942,7 +937,7 @@ impl RunningState {
 
                 let scenario = dap_registry
                     .adapter(&adapter)
-                    .ok_or_else(|| anyhow!("{}: is not a valid adapter name", &adapter))
+                    .context(format!("{}: is not a valid adapter name", &adapter))
                     .map(|adapter| adapter.config_from_zed_format(zed_config))??;
                 config = scenario.config;
                 Self::substitute_variables_in_config(&mut config, &task_context);

crates/task/src/task_template.rs

@@ -237,6 +237,18 @@ impl TaskTemplate {
             env
         };
 
+        // We filter out env variables here that aren't set so we don't have extra white space in args
+        let args = self
+            .args
+            .iter()
+            .filter(|arg| {
+                arg.starts_with('$')
+                    .then(|| env.get(&arg[1..]).is_some_and(|arg| !arg.trim().is_empty()))
+                    .unwrap_or(true)
+            })
+            .cloned()
+            .collect();
+
         Some(ResolvedTask {
             id: id.clone(),
             substituted_variables,
@@ -256,7 +268,7 @@ impl TaskTemplate {
                     },
                 ),
                 command,
-                args: self.args.clone(),
+                args,
                 env,
                 use_new_terminal: self.use_new_terminal,
                 allow_concurrent_runs: self.allow_concurrent_runs,
@@ -703,6 +715,7 @@ mod tests {
             label: "My task".into(),
             command: "echo".into(),
             args: vec!["$PATH".into()],
+            env: HashMap::from_iter([("PATH".to_owned(), "non-empty".to_owned())]),
             ..TaskTemplate::default()
         };
         let resolved_task = task
@@ -715,6 +728,32 @@ mod tests {
         assert_eq!(resolved.args, task.args);
     }
 
+    #[test]
+    fn test_empty_env_variables_excluded_from_args() {
+        let task = TaskTemplate {
+            label: "My task".into(),
+            command: "echo".into(),
+            args: vec![
+                "$EMPTY_VAR".into(),
+                "hello".into(),
+                "$WHITESPACE_VAR".into(),
+                "$UNDEFINED_VAR".into(),
+                "$WORLD".into(),
+            ],
+            env: HashMap::from_iter([
+                ("EMPTY_VAR".to_owned(), "".to_owned()),
+                ("WHITESPACE_VAR".to_owned(), "   ".to_owned()),
+                ("WORLD".to_owned(), "non-empty".to_owned()),
+            ]),
+            ..TaskTemplate::default()
+        };
+        let resolved_task = task
+            .resolve_task(TEST_ID_BASE, &TaskContext::default())
+            .unwrap();
+        let resolved = resolved_task.resolved;
+        assert_eq!(resolved.args, vec!["hello", "$WORLD"]);
+    }
+
     #[test]
     fn test_errors_on_missing_zed_variable() {
         let task = TaskTemplate {