From d06406f7d809a8f22dee3a947e1b18e3fca1d595 Mon Sep 17 00:00:00 2001 From: Om Chillure Date: Wed, 22 Apr 2026 00:55:21 +0530 Subject: [PATCH] Fix debugger start ignoring save property (#53353) ## Summary When `debugger::Start` runs a build task (via the `"build"` field in `debug.json`), it was bypassing the task's `"save"` property and calling `project.create_terminal_task()` directly. This meant unsaved files were never saved before the build ran, even when `"save": "all"` was set unlike `task: spawn` which correctly saved first. **Changes:** - Extracted `Workspace::save_for_task(workspace, strategy, cx)` as a shared async helper in `workspace/src/tasks.rs` - Refactored `schedule_resolved_task` to call this helper (no behavior change, removes inline duplication) - Called the same helper in `RunningState::resolve_scenario` before `create_terminal_task`, so the debugger build path now respects the task's `save` strategy - Added `test_save_for_task_all`, `test_save_for_task_current`, and `test_save_for_task_none` tests covering the extracted helper directly ## Self-Review Checklist - [x] I've reviewed my own diff for quality, security, and reliability - [ ] Unsafe blocks (if any) have justifying comments - [x] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Closes #53284 ## Video after fix [Screencast from 2026-04-08 08-04-18.webm](https://github.com/user-attachments/assets/b0c20164-3670-440a-b43a-8457fb57bfbd) ## Release Notes: - Fixed debugger not saving files before running a build task when `"save": "all"` is set in the task definition --- crates/debugger_ui/src/session/running.rs | 3 + crates/workspace/src/tasks.rs | 112 ++++++++++++++++++---- 2 files changed, 94 insertions(+), 21 deletions(-) diff --git a/crates/debugger_ui/src/session/running.rs b/crates/debugger_ui/src/session/running.rs index 836f76a73fe69aa5dfdacf3359be34946d8c3740..c273778ec3852788c8a6fef5b10f2bea7ebeefb0 100644 --- a/crates/debugger_ui/src/session/running.rs +++ b/crates/debugger_ui/src/session/running.rs @@ -1145,6 +1145,9 @@ impl RunningState { args, ..task.resolved.clone() }; + + Workspace::save_for_task(&weak_workspace, task_with_shell.save, cx).await; + let terminal = project .update(cx, |project, cx| { project.create_terminal_task( diff --git a/crates/workspace/src/tasks.rs b/crates/workspace/src/tasks.rs index 98421365532a8fdd4fc36f0f5c68e83b0814ae8e..3ea356788655537f1f7fe32b6c8fe006a23cc608 100644 --- a/crates/workspace/src/tasks.rs +++ b/crates/workspace/src/tasks.rs @@ -2,7 +2,7 @@ use std::process::ExitStatus; use anyhow::Result; use collections::HashSet; -use gpui::{AppContext, Context, Entity, Task}; +use gpui::{AppContext, AsyncWindowContext, Context, Entity, Task, WeakEntity}; use language::Buffer; use project::{TaskSourceKind, WorktreeId}; use remote::ConnectionState; @@ -78,26 +78,7 @@ impl Workspace { if self.terminal_provider.is_some() { let task = cx.spawn_in(window, async move |workspace, cx| { - let save_action = match spawn_in_terminal.save { - SaveStrategy::All => { - let save_all = workspace.update_in(cx, |workspace, window, cx| { - let task = workspace.save_all_internal(SaveIntent::SaveAll, window, cx); - // Match the type of the other arm by ignoring the bool value returned - cx.background_spawn(async { task.await.map(|_| ()) }) - }); - save_all.ok() - } - SaveStrategy::Current => { - let save_current = workspace.update_in(cx, |workspace, window, cx| { - workspace.save_active_item(SaveIntent::SaveAll, window, cx) - }); - save_current.ok() - } - SaveStrategy::None => None, - }; - if let Some(save_action) = save_action { - save_action.log_err().await; - } + Self::save_for_task(&workspace, spawn_in_terminal.save, cx).await; let spawn_task = workspace.update_in(cx, |workspace, window, cx| { workspace @@ -132,6 +113,32 @@ impl Workspace { } } + pub async fn save_for_task( + workspace: &WeakEntity, + save_strategy: SaveStrategy, + cx: &mut AsyncWindowContext, + ) { + let save_action = match save_strategy { + SaveStrategy::All => { + let save_all = workspace.update_in(cx, |workspace, window, cx| { + let task = workspace.save_all_internal(SaveIntent::SaveAll, window, cx); + cx.background_spawn(async { task.await.map(|_| ()) }) + }); + save_all.ok() + } + SaveStrategy::Current => { + let save_current = workspace.update_in(cx, |workspace, window, cx| { + workspace.save_active_item(SaveIntent::SaveAll, window, cx) + }); + save_current.ok() + } + SaveStrategy::None => None, + }; + if let Some(save_action) = save_action { + save_action.log_err().await; + } + } + pub fn start_debug_session( &mut self, scenario: DebugScenario, @@ -417,6 +424,69 @@ mod tests { item } + #[gpui::test] + async fn test_save_for_task_all(cx: &mut TestAppContext) { + let (fixture, cx) = create_fixture(cx, SaveStrategy::All).await; + let workspace = fixture.workspace.downgrade(); + cx.run_until_parked(); + + assert!(cx.read(|cx| fixture.item.read(cx).is_dirty)); + fixture.workspace.update_in(cx, |_workspace, window, cx| { + cx.spawn_in(window, { + let workspace = workspace.clone(); + async move |_this, cx| { + Workspace::save_for_task(&workspace, SaveStrategy::All, cx).await; + } + }) + .detach(); + }); + cx.run_until_parked(); + assert!(cx.read(|cx| !fixture.item.read(cx).is_dirty)); + } + + #[gpui::test] + async fn test_save_for_task_none(cx: &mut TestAppContext) { + let (fixture, cx) = create_fixture(cx, SaveStrategy::None).await; + let workspace = fixture.workspace.downgrade(); + cx.run_until_parked(); + + assert!(cx.read(|cx| fixture.item.read(cx).is_dirty)); + fixture.workspace.update_in(cx, |_workspace, window, cx| { + cx.spawn_in(window, { + let workspace = workspace.clone(); + async move |_this, cx| { + Workspace::save_for_task(&workspace, SaveStrategy::None, cx).await; + } + }) + .detach(); + }); + cx.run_until_parked(); + assert!(cx.read(|cx| fixture.item.read(cx).is_dirty)); + } + + #[gpui::test] + async fn test_save_for_task_current(cx: &mut TestAppContext) { + let (fixture, cx) = create_fixture(cx, SaveStrategy::Current).await; + let inactive = add_test_item(&fixture.workspace, "file2.txt", false, cx); + let workspace = fixture.workspace.downgrade(); + cx.run_until_parked(); + + assert!(cx.read(|cx| fixture.item.read(cx).is_dirty)); + assert!(cx.read(|cx| inactive.read(cx).is_dirty)); + fixture.workspace.update_in(cx, |_workspace, window, cx| { + cx.spawn_in(window, { + let workspace = workspace.clone(); + async move |_this, cx| { + Workspace::save_for_task(&workspace, SaveStrategy::Current, cx).await; + } + }) + .detach(); + }); + cx.run_until_parked(); + assert!(cx.read(|cx| !fixture.item.read(cx).is_dirty)); + assert!(cx.read(|cx| inactive.read(cx).is_dirty)); + } + struct TestTerminalProvider { item: Entity, dirty_before_spawn: Arc>>,