Skip to content

Commit 784f1b1

Browse files
authored
[Rust LSP] Support generateCodeOnSave setting, clean up error messages (#1881)
- **Respect on-save notifications** - **clean up error panel when theres diangostic errors and you try to generate** - **obey the generateCodeOnSave vscode setting** <!-- ELLIPSIS_HIDDEN --> ---- > [!IMPORTANT] > Improves configuration handling, error management, and respects on-save notifications in the language server. > > - **Behavior**: > - `BamlProject::run_generators_native` in `baml_project/mod.rs` now handles errors more gracefully, logging specific error messages and returning appropriate errors. > - `DidSaveTextDocument` in `did_save_text_document.rs` respects `generate_code_on_save` setting, skipping code generation if set to "never". > - **Configuration Handling**: > - `DidChangeConfiguration` in `did_change_configuration.rs` updates session BAML settings and requests latest settings if params are null. > - `DidOpenTextDocument` in `did_open.rs` requests workspace configuration on file open. > - **Error Handling**: > - `Hover` in `hover.rs` swallows errors to prevent user-facing notifications on hover failures. > - **Session Management**: > - `Session` in `session.rs` now includes `baml_settings` and updates them via `update_baml_settings()`. > - `Session::reload` reloads projects and updates runtime, logging errors if they occur. > - **Misc**: > - Minor logging improvements in `server.rs` and `session.rs`. > > <sup>This description was created by </sup>[<img alt="Ellipsis" src="https://img.shields.io/badge/Ellipsis-blue?color=175173">](https://www.ellipsis.dev?ref=BoundaryML%2Fbaml&utm_source=github&utm_medium=referral)<sup> for d0ce84c. You can [customize](https://app.ellipsis.dev/BoundaryML/settings/summaries) this summary. It will automatically update as commits are pushed.</sup> <!-- ELLIPSIS_HIDDEN -->
1 parent e6df552 commit 784f1b1

8 files changed

Lines changed: 120 additions & 28 deletions

File tree

engine/language_server/src/baml_project/mod.rs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,17 @@ impl BamlProject {
156156
.collect();
157157
let start_time = Instant::now();
158158

159-
let runtime = self.runtime(env)?;
159+
let runtime = self.runtime(env);
160+
if let Err(e) = runtime {
161+
if (e.has_errors()) {
162+
tracing::error!("Failed to run codegen: {:?}", e);
163+
return Err(anyhow::anyhow!("Project has errors."));
164+
} else {
165+
tracing::error!("Failed to run codegen: {:?}", e);
166+
return Err(e.into());
167+
}
168+
}
169+
let runtime = runtime.unwrap();
160170

161171
let generated = match runtime.run_codegen(&all_files, no_version_check.unwrap_or(false)) {
162172
Ok(gen) => {

engine/language_server/src/server.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ impl Server {
7777
mut workspace_settings,
7878
} = AllSettings::from_value(
7979
init_params
80+
.clone()
8081
.initialization_options
8182
.unwrap_or_else(|| serde_json::Value::Object(serde_json::Map::default())),
8283
);
@@ -107,7 +108,7 @@ impl Server {
107108
);
108109

109110
let workspaces = init_params
110-
.workspace_folders
111+
.workspace_folders.clone()
111112
.filter(|folders| !folders.is_empty())
112113
.map(|folders| folders.into_iter().filter_map(|folder| {
113114
let baml_src_dir = find_baml_src(&PathBuf::from(folder.uri.path()))?;
@@ -130,6 +131,8 @@ impl Server {
130131
anyhow::anyhow!("Failed to get the current working directory while creating a default workspace.")
131132
})?;
132133

134+
// tracing::info!("init params: {:?}", init_params);
135+
133136
// for some reason tracing logs are not available before this point
134137
tracing::info!("Starting server with {} worker threads", worker_threads);
135138

@@ -229,6 +232,7 @@ impl Server {
229232
let mut scheduler =
230233
schedule::Scheduler::new(&mut session, worker_threads, connection.make_sender());
231234
Self::try_register_capabilities(&_client_capabilities, &mut scheduler);
235+
232236
for msg in connection.incoming() {
233237
if connection.handle_shutdown(&msg)? {
234238
break;

engine/language_server/src/server/api/notifications/did_change_configuration.rs

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
use crate::server::api::ResultExt;
22
use crate::server::client::{Notifier, Requester};
3-
use crate::server::Result;
3+
use crate::server::{Result, Task};
44
use crate::session::Session;
5-
use lsp_types as types;
65
use lsp_types::notification as notif;
6+
use lsp_types::{self as types, ConfigurationItem, ConfigurationParams};
77

88
pub(crate) struct DidChangeConfiguration;
99

@@ -15,14 +15,15 @@ impl super::SyncNotificationHandler for DidChangeConfiguration {
1515
fn run(
1616
_session: &mut Session,
1717
notifier: Notifier,
18-
_requester: &mut Requester,
18+
requester: &mut Requester,
1919
params: types::DidChangeConfigurationParams,
2020
) -> Result<()> {
21-
tracing::info!("*** DID CHANGE CONFIGURATION");
21+
tracing::info!("*** DID CHANGE CONFIGURATION: {:?}", params);
2222

2323
// Extract the BAML configuration from the params
2424
if let Some(settings) = params.settings.as_object() {
2525
if let Some(baml_settings) = settings.get("baml") {
26+
tracing::info!("BAML settings: {:?}", baml_settings);
2627
// Send the BAML settings as a notification
2728
notifier
2829
.0
@@ -34,9 +35,29 @@ impl super::SyncNotificationHandler for DidChangeConfiguration {
3435
))
3536
.internal_error()?;
3637
tracing::info!("Sent baml_settings_updated notification");
38+
_session.update_baml_settings(baml_settings.clone());
3739
}
3840
}
3941

42+
// Also manually schedule a request for latest settings since sometimes the above params just have Null (not sure why)
43+
// note that the task will run after this current task is done.
44+
requester.request::<types::request::WorkspaceConfiguration>(
45+
ConfigurationParams {
46+
items: vec![types::ConfigurationItem {
47+
scope_uri: None,
48+
section: Some("baml".to_string()),
49+
}],
50+
},
51+
|response| {
52+
Task::local(move |session, _, _, _| {
53+
tracing::info!("Workspace configuration request received: {:?}", response);
54+
if let Some(first_response) = response.first() {
55+
session.update_baml_settings(first_response.clone());
56+
}
57+
})
58+
},
59+
);
60+
4061
Ok(())
4162
}
4263
}

engine/language_server/src/server/api/notifications/did_open.rs

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,16 @@
1-
use lsp_types::notification::DidOpenTextDocument;
2-
use lsp_types::{DidOpenTextDocumentParams, PublishDiagnosticsParams, TextDocumentItem};
3-
41
use crate::server::api::diagnostics::publish_session_lsp_diagnostics;
52
use crate::server::api::notifications::baml_src_version::BamlSrcVersionPayload;
63
use crate::server::api::traits::{NotificationHandler, SyncNotificationHandler};
74
use crate::server::api::ResultExt;
85
use crate::server::client::{Notifier, Requester};
9-
use crate::server::Result;
6+
use crate::server::{Result, Task};
107
use crate::session::Session;
118
use crate::{DocumentKey, TextDocument};
12-
9+
use lsp_types::notification::DidOpenTextDocument;
10+
use lsp_types::{
11+
self as types, ConfigurationItem, ConfigurationParams, DidOpenTextDocumentParams,
12+
PublishDiagnosticsParams, TextDocumentItem,
13+
};
1314
pub(crate) struct DidOpenTextDocumentHandler;
1415

1516
impl NotificationHandler for DidOpenTextDocumentHandler {
@@ -20,13 +21,32 @@ impl SyncNotificationHandler for DidOpenTextDocumentHandler {
2021
fn run(
2122
session: &mut Session,
2223
notifier: Notifier,
23-
_requester: &mut Requester,
24+
requester: &mut Requester,
2425
params: DidOpenTextDocumentParams,
2526
) -> Result<()> {
2627
tracing::info!("DidOpenTextDocumentHandler");
2728

2829
let url = params.text_document.uri;
2930

31+
// TODO: do this when server initializes instead of every time a file is opened
32+
// note this just schedules the task. It will run after the current task is done.
33+
requester.request::<types::request::WorkspaceConfiguration>(
34+
ConfigurationParams {
35+
items: vec![types::ConfigurationItem {
36+
scope_uri: None,
37+
section: Some("baml".to_string()),
38+
}],
39+
},
40+
|response| {
41+
Task::local(move |session, _, _, _| {
42+
tracing::info!("Workspace configuration request received: {:?}", response);
43+
if let Some(first_response) = response.first() {
44+
session.update_baml_settings(first_response.clone());
45+
}
46+
})
47+
},
48+
);
49+
3050
let file_path = url
3151
.to_file_path()
3252
.internal_error_msg(&format!("Could not convert URL '{}' to file path", url))?;

engine/language_server/src/server/api/notifications/did_save_text_document.rs

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
use crate::server::api::notifications::baml_src_version::BamlSrcVersionPayload;
22
use crate::server::api::ResultExt;
33
use crate::server::client::{Notifier, Requester};
4-
use crate::server::Result;
4+
use crate::server::{Result, Task};
55
use crate::session::{DocumentSnapshot, Session};
6-
use lsp_types as types;
76
use lsp_types::notification as notif;
7+
use lsp_types::request::Request;
8+
use lsp_types::{self as types, ConfigurationParams};
89
use std::borrow::Cow;
910

1011
pub struct DidSaveTextDocument;
@@ -27,6 +28,14 @@ impl super::SyncNotificationHandler for DidSaveTextDocument {
2728
.internal_error_msg("Could not convert URL to path")?;
2829
session.clear_unsaved_files();
2930
session.reload(Some(notifier.clone())).internal_error()?;
31+
32+
if let Some(generate_code_on_save) = session.baml_settings.clone().generate_code_on_save {
33+
if generate_code_on_save == "never" {
34+
tracing::info!("Skipping generator because generate_code_on_save is false");
35+
return Ok(());
36+
}
37+
}
38+
3039
tracing::info!("About to run generator. URL path: {:?}", path);
3140
let project = session
3241
.get_or_create_project(&path)
@@ -59,9 +68,7 @@ impl super::SyncNotificationHandler for DidSaveTextDocument {
5968
},
6069
|e| {
6170
tracing::error!("Error generating: {e}");
62-
notifier
63-
.notify_baml_error(&format!("Error generating: {e}"))
64-
.unwrap_or(())
71+
notifier.notify_baml_error(&format!("{e}")).unwrap_or(())
6572
},
6673
);
6774

@@ -102,9 +109,7 @@ impl super::BackgroundDocumentNotificationHandler for DidSaveTextDocument {
102109
},
103110
|e| {
104111
tracing::error!("Error generating: {e}");
105-
notifier
106-
.notify_baml_error(&format!("Error generating: {e}"))
107-
.unwrap_or(())
112+
notifier.notify_baml_error(&format!("{e}")).unwrap_or(())
108113
},
109114
);
110115
} else {

engine/language_server/src/server/api/requests/hover.rs

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -52,11 +52,18 @@ impl SyncRequestHandler for Hover {
5252
}
5353
.internal_error()?;
5454
let position = params.text_document_position_params.position;
55-
let hover = project
56-
.lock()
57-
.unwrap()
58-
.handle_hover_request(&text_document_item, &position, notifier)
59-
.internal_error()?;
55+
// Just swallow the error here, we dont want hover failures to show error notifs for a user.
56+
let hover = match project.lock().unwrap().handle_hover_request(
57+
&text_document_item,
58+
&position,
59+
notifier,
60+
) {
61+
Ok(hover) => hover,
62+
Err(e) => {
63+
tracing::error!("Error handling hover request: {}", e);
64+
None
65+
}
66+
};
6067
Ok(hover)
6168
}
6269
}

engine/language_server/src/session.rs

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
use anyhow::Context;
44
use index::DocumentController;
55
use itertools::any;
6+
use serde_json::Value;
67
use std::collections::HashMap;
78
use std::ops::{Deref, DerefMut};
89
use std::path::{Path, PathBuf};
@@ -21,6 +22,7 @@ use crate::{PositionEncoding, TextDocument};
2122
pub(crate) use self::capabilities::ResolvedClientCapabilities;
2223
pub use self::index::DocumentQuery;
2324
pub(crate) use self::settings::AllSettings;
25+
pub use self::settings::BamlSettings;
2426
pub use self::settings::ClientSettings;
2527
use crate::server::client::Notifier;
2628

@@ -44,6 +46,8 @@ pub struct Session {
4446
pub position_encoding: PositionEncoding,
4547
/// Tracks what LSP features the client supports and doesn't support.
4648
pub resolved_client_capabilities: Arc<ResolvedClientCapabilities>,
49+
50+
pub baml_settings: BamlSettings,
4751
}
4852

4953
impl Clone for Session {
@@ -53,6 +57,7 @@ impl Clone for Session {
5357
baml_src_projects: self.baml_src_projects.clone(),
5458
position_encoding: self.position_encoding.clone(),
5559
resolved_client_capabilities: self.resolved_client_capabilities.clone(),
60+
baml_settings: self.baml_settings.clone(),
5661
}
5762
}
5863
}
@@ -65,7 +70,7 @@ impl Session {
6570
workspace_folders: &[(Url, ClientSettings)],
6671
) -> anyhow::Result<Self> {
6772
let mut projects = HashMap::new();
68-
let index = index::Index::new(global_settings);
73+
let index = index::Index::new(global_settings.clone());
6974

7075
for (url, _) in workspace_folders {
7176
let workspace_path = url
@@ -99,9 +104,21 @@ impl Session {
99104
resolved_client_capabilities: Arc::new(ResolvedClientCapabilities::new(
100105
client_capabilities,
101106
)),
107+
baml_settings: BamlSettings::default(),
102108
})
103109
}
104110

111+
pub fn update_baml_settings(&mut self, settings: Value) {
112+
match serde_json::from_value(settings) {
113+
Ok(parsed_settings) => {
114+
self.baml_settings = parsed_settings;
115+
}
116+
Err(err) => {
117+
tracing::error!("Failed to parse BAML settings: {}", err);
118+
}
119+
}
120+
}
121+
105122
/// Gets or creates a project for the given path.
106123
///
107124
/// This is the primary method for working with projects, replacing the multiple

engine/language_server/src/session/settings.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,16 @@ use serde::Deserialize;
77
/// Maps a workspace URI to its associated client settings. Used during server initialization.
88
pub(crate) type WorkspaceSettingsMap = FxHashMap<Url, ClientSettings>;
99

10+
#[derive(Debug, Deserialize, Default, Clone)]
11+
#[cfg_attr(test, derive(PartialEq, Eq))]
12+
#[serde(rename_all = "camelCase")]
13+
pub struct BamlSettings {
14+
pub(crate) cli_path: Option<String>,
15+
pub(crate) generate_code_on_save: Option<String>,
16+
}
17+
1018
/// This is a direct representation of the settings schema sent by the client.
11-
#[derive(Debug, Deserialize, Default)]
19+
#[derive(Debug, Deserialize, Default, Clone)]
1220
#[cfg_attr(test, derive(PartialEq, Eq))]
1321
#[serde(rename_all = "camelCase")]
1422
pub struct ClientSettings {
@@ -20,7 +28,7 @@ pub struct ClientSettings {
2028

2129
/// Settings needed to initialize tracing. These will only be
2230
/// read from the global configuration.
23-
#[derive(Debug, Deserialize, Default)]
31+
#[derive(Debug, Deserialize, Default, Clone)]
2432
#[cfg_attr(test, derive(PartialEq, Eq))]
2533
#[serde(rename_all = "camelCase")]
2634
pub(crate) struct TracingSettings {

0 commit comments

Comments
 (0)