-
Notifications
You must be signed in to change notification settings - Fork 63
Add LSP logger #1162
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
Add LSP logger #1162
Conversation
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.
Pull request overview
This PR adds a proper LSP logger to improve debugging visibility in LSP clients that don't display console output. The implementation introduces a logging system that sends messages via the LSP window/logMessage protocol, replacing direct console.log/error calls in the incremental compilation and project configuration caching code paths.
Key Changes:
- Added a new
logger.tsmodule with a Logger interface and LSP-compliant implementation that supports configurable log levels (error, warn, info, log) - Replaced conditional debug logging with unconditional logger calls that respect the configured log level
- Added
logLevelconfiguration option withincrementalTypechecking.debugLoggingas a legacy override
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| server/src/logger.ts | New file implementing Logger interface with LSP-compliant logging via window/logMessage protocol |
| server/src/server.ts | Added logger initialization, applyUserConfiguration function to handle log level settings, and migrated console.log/error calls to logger |
| server/src/incrementalCompilation.ts | Removed debug() function and migrated all debug-gated console logs to logger calls |
| server/src/config.ts | Exported initialConfiguration and added logLevel to extensionConfiguration interface |
| server/src/bsc-args/rewatch.ts | Migrated debug-gated console.log calls to logger |
| package.json | Added rescript.settings.logLevel configuration with enum values including error, warn, info, log, and debug |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export interface Logger { | ||
| error(message: string): void; | ||
| warn(message: string): void; | ||
| info(message: string): void; | ||
| log(message: string): void; | ||
| } |
Copilot
AI
Dec 17, 2025
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.
The Logger interface and its implementations lack documentation. Consider adding JSDoc comments to explain the purpose of the logger, the log level hierarchy (log is most verbose, error is least verbose), and when each log level should be used. This will help other developers understand how to use the logger correctly.
zth
left a comment
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.
Looks good to me. Just a few questions.
Writing to the console does not show up in all LSP clients.
I was pretty blind in Zed when something did not work as expected.
Turned out my problem was that we do not override the default settings when the client provides them (see 7334e1e).
configuration.incrementalTypechecking?.debugLoggingenables the highest log level.This is to semantically close enough to what we have today. Existing config won't break.