-
Notifications
You must be signed in to change notification settings - Fork 351
fix(language-server): handle non-baml-src baml files #2506
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
🔒 Entelligence AI Vulnerability Scanner ✅ No security vulnerabilities found! Your code passed our comprehensive security analysis. |
|
LGTM 👍 |
|
🌿 Preview your docs: https://boundary-preview-9db42254-7f3e-4609-a0c6-f22f612ddbc5.docs.buildwithfern.com |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Codex Review: Here are some suggestions.
Reply with @codex fix comments to fix any unresolved comments.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
| if params.changes.iter().any(|change| { | ||
| let Ok(path) = change.uri.to_file_path() else { | ||
| return true; | ||
| }; | ||
| session.get_or_create_project(&path).is_err() | ||
| }) { | ||
| return Ok(()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P1] Avoid dropping watched-file batches with mixed paths
The new guard returns early whenever any change in the batch cannot be resolved to a baml_src project. This skips the reload and diagnostic publishing for the entire batch, even when the same batch contains valid baml_src files. The previous code only bailed out when no baml_src paths were present. A watcher that reports both BAML and non-BAML file events (e.g. a generator output alongside a source change) will now leave diagnostics stale for the actual BAML file. Filter out non-BAML entries instead of aborting the whole handler.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think this is valid no?
|
🌿 Preview your docs: https://boundary-preview-4d9769c5-1754-48a9-af0d-cff1b49dff4f.docs.buildwithfern.com |
9762467 to
9fcf85c
Compare
9fcf85c to
dfce4bd
Compare
|
🌿 Preview your docs: https://boundary-preview-77ac7d03-771e-4af3-b325-0269e080469e.docs.buildwithfern.com |
|
🌿 Preview your docs: https://boundary-preview-08faac3a-c892-44f0-befa-128e6d4418b0.docs.buildwithfern.com |
|
🌿 Preview your docs: https://boundary-preview-5b7dc2ea-0c6a-472e-ab6e-325a73e7c9b4.docs.buildwithfern.com |
Roll forward of #2486, with deadlocks fixed.
Deadlocks were present because
session.reload()needs to grab locks on every project, but its API does not make this obvious, and in #2486 there were implicit scope changes that resulted in MutexGuard destructors not running before session.reload() got called.