From 7d6722f9f4e45e3d474a3ba9d2b8ee66a70d8b4f Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 10 Aug 2026 19:42:30 +0300 Subject: [PATCH] Show error toast on startup for malformed tasks (#62387) Closes https://github.com/zed-industries/zed/issues/61208 Before, Zed showed no toasts on startup when tasks.json contained malformed entries, also if there were two top-level arrays, the last one was silently discarded without any toasts too. The PR fixes both. Release Notes: - Fixed error toast not showing for malformed tasks.json --- crates/project/src/project.rs | 2 + crates/project/src/project_settings.rs | 73 +++++--------- crates/settings_json/src/settings_json.rs | 19 +++- crates/zed/src/zed.rs | 111 +++++++++++++++++++++- 4 files changed, 153 insertions(+), 52 deletions(-) diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 447f83ca9c..cb6e88d296 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -3898,6 +3898,8 @@ impl Project { }), Err(_) => {} }, + SettingsObserverEvent::GlobalTasksUpdated(_) + | SettingsObserverEvent::GlobalDebugScenariosUpdated(_) => {} } } diff --git a/crates/project/src/project_settings.rs b/crates/project/src/project_settings.rs index 275e0218d3..e4c806dbe6 100644 --- a/crates/project/src/project_settings.rs +++ b/crates/project/src/project_settings.rs @@ -8,9 +8,9 @@ use git::repository::DEFAULT_WORKTREE_DIRECTORY; use gpui::{AsyncApp, BorrowAppContext, Context, Entity, EventEmitter, Subscription, Task}; use lsp::{DEFAULT_LSP_REQUEST_TIMEOUT_SECS, LanguageServerName}; use paths::{ - EDITORCONFIG_NAME, local_debug_file_relative_path, local_settings_file_relative_path, - local_tasks_file_relative_path, local_vscode_launch_file_relative_path, - local_vscode_tasks_file_relative_path, task_file_name, + EDITORCONFIG_NAME, debug_task_file_name, local_debug_file_relative_path, + local_settings_file_relative_path, local_tasks_file_relative_path, + local_vscode_launch_file_relative_path, local_vscode_tasks_file_relative_path, task_file_name, }; use rpc::{ AnyProtoClient, TypedEnvelope, @@ -814,6 +814,8 @@ pub enum SettingsObserverEvent { LocalSettingsUpdated(Result), LocalTasksUpdated(Result), LocalDebugScenariosUpdated(Result), + GlobalTasksUpdated(Result), + GlobalDebugScenariosUpdated(Result), } impl EventEmitter for SettingsObserver {} @@ -1425,17 +1427,17 @@ impl SettingsObserver { log::error!( "Failed to set local debug scenarios in {path:?}: {message:?}" ); - cx.emit(SettingsObserverEvent::LocalTasksUpdated(Err( + cx.emit(SettingsObserverEvent::LocalDebugScenariosUpdated(Err( InvalidSettingsError::Debug { path, message }, ))); } Err(e) => { - log::error!("Failed to set local tasks: {e}"); + log::error!("Failed to set local debug scenarios: {e}"); } Ok(()) => { - cx.emit(SettingsObserverEvent::LocalTasksUpdated(Ok(directory - .as_std_path() - .join(task_file_name())))); + cx.emit(SettingsObserverEvent::LocalDebugScenariosUpdated(Ok( + directory.as_std_path().join(debug_task_file_name()), + ))); } } } @@ -1484,20 +1486,9 @@ impl SettingsObserver { }) else { return; }; - if let Some(user_tasks_content) = user_tasks_content { - task_store - .update(cx, |task_store, cx| { - task_store - .update_user_tasks( - TaskSettingsLocation::Global(&file_path), - Some(&user_tasks_content), - cx, - ) - .log_err(); - }) - .ok(); - } - while let Some(user_tasks_content) = user_tasks_file_rx.next().await { + let mut user_tasks_contents = + futures::stream::iter(user_tasks_content).chain(user_tasks_file_rx); + while let Some(user_tasks_content) = user_tasks_contents.next().await { let Ok(result) = task_store.update(cx, |task_store, cx| { task_store.update_user_tasks( TaskSettingsLocation::Global(&file_path), @@ -1510,20 +1501,16 @@ impl SettingsObserver { settings_observer .update(cx, |_, cx| match result { - Ok(()) => cx.emit(SettingsObserverEvent::LocalTasksUpdated(Ok( + Ok(()) => cx.emit(SettingsObserverEvent::GlobalTasksUpdated(Ok( file_path.clone() ))), - Err(err) => cx.emit(SettingsObserverEvent::LocalTasksUpdated(Err( - InvalidSettingsError::Tasks { - path: file_path.clone(), - message: err.to_string(), - }, - ))), + Err(err) => cx.emit(SettingsObserverEvent::GlobalTasksUpdated(Err(err))), }) .ok(); } }) } + fn subscribe_to_global_debug_scenarios_changes( fs: Arc, file_path: PathBuf, @@ -1539,20 +1526,9 @@ impl SettingsObserver { }) else { return; }; - if let Some(user_tasks_content) = user_tasks_content { - task_store - .update(cx, |task_store, cx| { - task_store - .update_user_debug_scenarios( - TaskSettingsLocation::Global(&file_path), - Some(&user_tasks_content), - cx, - ) - .log_err(); - }) - .ok(); - } - while let Some(user_tasks_content) = user_tasks_file_rx.next().await { + let mut user_tasks_contents = + futures::stream::iter(user_tasks_content).chain(user_tasks_file_rx); + while let Some(user_tasks_content) = user_tasks_contents.next().await { let Ok(result) = task_store.update(cx, |task_store, cx| { task_store.update_user_debug_scenarios( TaskSettingsLocation::Global(&file_path), @@ -1565,15 +1541,12 @@ impl SettingsObserver { settings_observer .update(cx, |_, cx| match result { - Ok(()) => cx.emit(SettingsObserverEvent::LocalDebugScenariosUpdated(Ok( + Ok(()) => cx.emit(SettingsObserverEvent::GlobalDebugScenariosUpdated(Ok( file_path.clone(), ))), - Err(err) => cx.emit(SettingsObserverEvent::LocalDebugScenariosUpdated( - Err(InvalidSettingsError::Tasks { - path: file_path.clone(), - message: err.to_string(), - }), - )), + Err(err) => { + cx.emit(SettingsObserverEvent::GlobalDebugScenariosUpdated(Err(err))) + } }) .ok(); } diff --git a/crates/settings_json/src/settings_json.rs b/crates/settings_json/src/settings_json.rs index 5198e475af..f5194d8703 100644 --- a/crates/settings_json/src/settings_json.rs +++ b/crates/settings_json/src/settings_json.rs @@ -742,7 +742,9 @@ pub fn to_pretty_json( pub fn parse_json_with_comments(content: &str) -> Result { let mut deserializer = serde_json_lenient::Deserializer::from_str(content); - Ok(serde_path_to_error::deserialize(&mut deserializer)?) + let value = serde_path_to_error::deserialize(&mut deserializer)?; + deserializer.end()?; + Ok(value) } #[cfg(test)] @@ -2632,4 +2634,19 @@ mod tests { }"#; assert_eq!(infer_json_indent_size(json_mixed), 2); } + + #[test] + fn test_parse_json_with_comments_rejects_trailing_content() { + let valid = r#" + // comment + [{"label": "a"}] + "#; + parse_json_with_comments::>(valid).unwrap(); + + let trailing = r#" + [{"label": "a"}] + [{"label": "b"}] + "#; + parse_json_with_comments::>(trailing).unwrap_err(); + } } diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index d1302e75e2..1b5434efe7 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -60,7 +60,10 @@ use paths::{ local_debug_file_relative_path, local_settings_file_relative_path, local_tasks_file_relative_path, }; -use project::{DirectoryLister, DisableAiSettings, ProjectItem}; +use project::{ + DirectoryLister, DisableAiSettings, ProjectItem, + project_settings::{SettingsObserver, SettingsObserverEvent}, +}; use project_panel::ProjectPanel; use quick_action_bar::QuickActionBar; use recent_projects::open_remote_project; @@ -436,6 +439,7 @@ pub fn initialize_workspace(app_state: Arc, cx: &mut App) { init_cursor_hide_mode(cx); init_app_appearance(cx); init_reduce_motion(cx); + init_global_config_error_notifications(cx); cx.observe_new(|_multi_workspace: &mut MultiWorkspace, window, cx| { let Some(window) = window else { @@ -2045,6 +2049,46 @@ fn notify_settings_errors(result: settings::SettingsParseResult, is_user: bool, }; } +fn init_global_config_error_notifications(cx: &mut App) { + cx.observe_new(|_: &mut SettingsObserver, _, cx| { + cx.subscribe_self::(|_, event, cx| { + let (result, file_kind, on_click): (_, _, fn(&mut Window, &mut App)) = match event { + SettingsObserverEvent::GlobalTasksUpdated(result) => { + (result, "tasks", |window, cx| { + window.dispatch_action(OpenTasks.boxed_clone(), cx) + }) + } + SettingsObserverEvent::GlobalDebugScenariosUpdated(result) => { + (result, "debug scenarios", |window, cx| { + window.dispatch_action(OpenDebugTasks.boxed_clone(), cx) + }) + } + _ => return, + }; + let id = NotificationId::Named(format!("invalid-global-{file_kind}-file").into()); + match result { + Ok(_) => dismiss_app_notification(&id, cx), + Err(error) => { + let message = format!("Invalid global {file_kind} file\n{error}"); + show_app_notification(id, cx, move |cx| { + cx.new(|cx| { + MessageNotification::new(message.clone(), cx) + .primary_message("Open File") + .primary_icon(IconName::Settings) + .primary_on_click(move |window, cx| { + on_click(window, cx); + cx.emit(DismissEvent); + }) + }) + }); + } + } + }) + .detach(); + }) + .detach(); +} + #[derive(Copy, Clone, Debug, settings::RegisterSetting)] struct CursorHideModeSetting(gpui::CursorHideMode); @@ -6257,6 +6301,71 @@ mod tests { // If this panics, the test has failed } + #[gpui::test] + async fn test_invalid_global_tasks_file_shows_notification_on_startup( + cx: &mut gpui::TestAppContext, + ) { + let app_state = init_test(cx); + let tasks_file_path = paths::tasks_file().as_path(); + app_state + .fs + .create_dir(tasks_file_path.parent().unwrap()) + .await + .unwrap(); + app_state + .fs + .save( + tasks_file_path, + &r#"[{ "label": "first" }] [{ "label": "trailing garbage" }]"#.into(), + Default::default(), + ) + .await + .unwrap(); + + let project = Project::test(app_state.fs.clone(), [], cx).await; + let window = cx.add_window(|window, cx| MultiWorkspace::test_new(project, window, cx)); + cx.run_until_parked(); + + let workspace = window + .read_with(cx, |multi_workspace, _| multi_workspace.workspace().clone()) + .unwrap(); + let notification_id = NotificationId::Named("invalid-global-tasks-file".into()); + let shown_notifications = workspace.read_with(cx, |workspace, _| { + workspace + .notification_ids() + .into_iter() + .filter(|id| *id == notification_id) + .count() + }); + assert_eq!( + shown_notifications, 1, + "invalid global tasks file at startup should show an app notification" + ); + + app_state + .fs + .save( + tasks_file_path, + &r#"[{ "label": "first", "command": "echo" }]"#.into(), + Default::default(), + ) + .await + .unwrap(); + cx.run_until_parked(); + + let shown_notifications = workspace.read_with(cx, |workspace, _| { + workspace + .notification_ids() + .into_iter() + .filter(|id| *id == notification_id) + .count() + }); + assert_eq!( + shown_notifications, 0, + "fixing the global tasks file should dismiss the notification" + ); + } + #[gpui::test] async fn test_disable_ai_filters_keybindings(cx: &mut gpui::TestAppContext) { let _app_state = init_keymap_test(cx);