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
This commit is contained in:
1 parent
b8649f3464
commit
7d6722f9f4
4 files changed
+153
-52
No files matched your search
@@ -3898,6 +3898,8 @@ impl Project {
|
||||
}),
|
||||
Err(_) => {}
|
||||
},
|
||||
SettingsObserverEvent::GlobalTasksUpdated(_)
|
||||
| SettingsObserverEvent::GlobalDebugScenariosUpdated(_) => {}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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<PathBuf, InvalidSettingsError>),
|
||||
LocalTasksUpdated(Result<PathBuf, InvalidSettingsError>),
|
||||
LocalDebugScenariosUpdated(Result<PathBuf, InvalidSettingsError>),
|
||||
GlobalTasksUpdated(Result<PathBuf, InvalidSettingsError>),
|
||||
GlobalDebugScenariosUpdated(Result<PathBuf, InvalidSettingsError>),
|
||||
}
|
||||
|
||||
impl EventEmitter<SettingsObserverEvent> 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<dyn Fs>,
|
||||
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();
|
||||
}
|
||||
|
||||
@@ -742,7 +742,9 @@ pub fn to_pretty_json(
|
||||
|
||||
pub fn parse_json_with_comments<T: DeserializeOwned>(content: &str) -> Result<T> {
|
||||
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::<Vec<Value>>(valid).unwrap();
|
||||
|
||||
let trailing = r#"
|
||||
[{"label": "a"}]
|
||||
[{"label": "b"}]
|
||||
"#;
|
||||
parse_json_with_comments::<Vec<Value>>(trailing).unwrap_err();
|
||||
}
|
||||
}
|
||||
+110
-1
@@ -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<AppState>, 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::<SettingsObserverEvent>(|_, 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);
|
||||
|
||||
Reference in new issue
Block a user