Add configurable log level across LSP server, VS, and VS Code clients - #22
Merged
Conversation
Every logging path was hardcoded to maximum verbosity with no lever to turn it down (issue #16). The server now accepts --log-level, defaulting to Warning, and drives both its file logging and the OmniSharp protocol logging pipeline from the same value. The VS client passes --log-level Warning when spawning the server; VS Code's existing reqnroll.trace.server setting now also maps onto the server's --log-level instead of only gating the client-side wire trace. While adding filtering tests, found that SynchronousFileLogger.Log() overrode the base class's level check and wrote every message unconditionally regardless of the configured Level — fixed, since it would have made the above plumbing a no-op for file logging. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
runTest.ts pointed extensionTestsPath directly at the compiled extension.test.js instead of a Mocha bootstrapper, so the suite/test BDD globals were never registered — every test threw ReferenceError: suite is not defined before running, and npm test failed with exit code 1 without a single test actually executing. Added index.ts to register Mocha and run the bundled tests; this also surfaced a genuine race in "Language client should start after activation" (a 2000ms sleep against Mocha's default 2000ms timeout), fixed by raising the suite timeout. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #16 — every logging path was hardcoded to maximum verbosity with no way to turn it down and no consistency between client and server.
--log-level <Off|Error|Warning|Info|Verbose>(defaultWarning), driving both file logging and the OmniSharp protocol-logging pipeline from the same value instead of independently hardcodingTrace.--log-level Warningwhen spawning the server.reqnroll.trace.serversetting now also maps onto the server's--log-level, instead of only gating the client-side wire trace.SynchronousFileLoggergained alevelconstructor parameter, with its previous hardcodedVerbosedefault flipped toWarning(quiets ~19 VS extension call sites with no per-file edits).SynchronousFileLogger.Log()overrode the base class and skipped its level filter entirely, writing every message unconditionally regardless of configured level — found while writing the filtering test, fixed since it would have made the level plumbing a no-op for file logging.AsynchronousFileLogger.DeleteOldLogFiles, 10-day retention) — no action needed.Second commit: fixes the VS Code extension's
npm testharness, which was never actually running any tests (extensionTestsPathpointed at a compiled test file instead of a Mocha bootstrapper, sosuite/testglobals were never registered and every test threw before running). Discovered while adding thetraceServerToLogLeveltest. Also fixes a real 2000ms-sleep-vs-2000ms-timeout race this surfaced in an existing test.Not addressed (deliberately out of scope): a VS Options UI to let users override the level at runtime — no existing settings-page infrastructure in the
[VisualStudioContribution]model to hook into; worth its own follow-up if wanted.Test plan
dotnet test— Common (65), LSP Server unit (516), LSP Server specs (131), VS extension (74) — all passnpm test(VS Code) — 16/16 pass (previously 0 tests were actually executing)SynchronousFileLoggerdrops below-threshold messages and writes at-or-above-threshold ones🤖 Generated with Claude Code