-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Fix information logs getting logged as debug in VSCode #78522
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
| /// TODO - Switch this to call LogInformation once appropriate callers have been changed to LogDebug. | ||
| /// </summary> | ||
| public override void LogInformation(string message, params object[] @params) => _hostLogger.LogDebug(message, @params); | ||
| public override void LogInformation(string message, params object[] @params) => _hostLogger.LogInformation(message, @params); |
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.
we were previously downgrading all info logs to debug in vscode because information logs were too verbose. That was because originally the lowest LSP logger level was info, and we ended up logging everything as info.
this fixes it by logging info as info, and adjusting the usages of info logging to a more appropriate level.
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.
Oh thank you for finally fixing this!
src/LanguageServer/Protocol/Handler/Completion/CompletionResolveHandler.cs
Show resolved
Hide resolved
| // what to skip, and what files we have to tell the client have been removed. | ||
| var previousResults = GetPreviousResults(diagnosticsParams) ?? []; | ||
| context.TraceInformation($"previousResults.Length={previousResults.Length}"); | ||
| context.TraceDebug($"previousResults.Length={previousResults.Length}"); |
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.
Not really related to this, but whenever I saw this in the logs it was very unclear what this was related to. Not sure if it should also be prefixed with handlerName or something.
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.
src/LanguageServer/Protocol/Handler/RelatedDocuments/RelatedDocumentsHandler.cs
Outdated
Show resolved
Hide resolved
|
/azp run roslyn-integration-CI |
|
Azure Pipelines successfully started running 1 pipeline(s). |

Updates LSP logging to log using debug more appropriately, and fixes LSP server logging to not downgrade information logs to debug.