fix: LSP diagnostics not refreshing on file changes and config updates#499
fix: LSP diagnostics not refreshing on file changes and config updates#499
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the responsiveness and accuracy of the LSP server by transitioning to a push-based diagnostic model. This change ensures that linting feedback is provided in real-time as files are opened, edited, or saved, and that diagnostics are correctly cleared when files are closed. Furthermore, it improves the server's ability to react to configuration changes, such as modifications to Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
bf9b1ac to
423d8ea
Compare
There was a problem hiding this comment.
Code Review
This pull request significantly refactors the LSP server's diagnostic handling to improve responsiveness and correctness, particularly concerning file changes and configuration updates. Key changes include introducing a refreshCh buffered channel to coalesce and signal diagnostic refreshes, ensuring relinting occurs on the main goroutine. The server now implements handleDidChangeWatchedFiles to process file system events, triggering reloadConfigAndRelint for rslint.json changes and forwarding other file changes to the underlying project.Session. Document lifecycle events (DidOpen, DidChange, DidSave, DidClose) are now integrated with the session and trigger immediate diagnostic pushes via a new pushDiagnostics method, replacing the previous DocumentDiagnostic request handler. New test fixtures and extensive tests for diagnostic lifecycle, rapid edits, and config watching have been added. Review comments highlight a security concern regarding sensitive information leakage due to logging the full params object in handleDidOpen and handleDidSave, suggesting logging only the document URI. Additionally, an efficiency improvement is recommended by moving the config.RegisterAllRules() call to server initialization to avoid redundant executions.
4ad37bb to
37071b1
Compare
- Add debounce (200ms) for didChange to avoid linting on every keystroke - Move RegisterAllRules to handleInitialized for efficiency - Sanitize log output to print URI instead of full params - Clean up pendingLintURIs on document close to prevent stale lints - Add waitForDiagnosticsCount helper in E2E tests for reliable CI
37071b1 to
cd6bd92
Compare
Summary
publishDiagnosticsondidOpen/didChange/didSave)DidOpenFile/DidChangeFile/DidSaveFile/DidCloseFileat correct handler pointsdidCloseby pushing empty diagnostics before closingworkspace/didChangeWatchedFilesRefreshDiagnosticssignals main dispatch loop via buffered channel for goroutine-safe relintwatchEnabledbased on client capabilities so Session can register tsconfig watchersisRslintConfigURIto require path separator (avoid matchingnot-rslint.json)Related Links
Fixes issues where:
rslint.jsondid not trigger config reload and re-linttsconfig.jsonchanges were not detected by the LSP serverChecklist