Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 27, 2025

Implementation Complete ✅

Successfully implemented default configuration_map for all built-in LSP clients to automatically enable extra features.

Summary of Changes

  1. Client ABC Enhancement (src/lsp_client/client/abc.py)

    • Added create_default_configuration_map() method with comprehensive documentation
    • Auto-initialize configuration in __asynccontextmanager__ if not already set
  2. Default Configurations Implemented (7 clients)

    • Rust Analyzer: Inlay hints (all types), diagnostics, completion, check on save, code lenses
    • Gopls: Inlay hints (all types), code lenses, analyses, completion, semantic tokens
    • Pyright: Inlay hints, auto-import completions, type checking, diagnostics, indexing
    • TypeScript: Inlay hints (TS & JS), suggestions, import preferences
    • Deno: Inlay hints, linting, unstable features, code lenses, import suggestions
    • Pyrefly: Inlay hints, diagnostics, auto-imports
    • Ty: Diagnostics, auto-imports
  3. ConfigurationMap Enhancement (src/lsp_client/utils/config.py)

    • Added has_global_config() public method for better encapsulation
  4. Comprehensive Testing (tests/test_default_configuration.py)

    • 37 tests covering all clients (increased from 35)
    • Added specific tests for TyClient and PyreflyClient configurations
    • Method presence, type validation, content validation
    • Client-specific feature tests
    • All tests passing ✓
  5. Documentation (docs/configuration.md)

    • Default configuration overview with examples
    • Per-client configuration details
    • Customization instructions
    • Public API usage examples
  6. Example (examples/default_configuration.py)

    • Demonstrates using default configurations
    • Shows how to override defaults
    • Inspects configurations for all clients
    • Added comment explaining TyClient uses diagnostics instead of inlay hints

Recent Updates (addressing PR feedback)

  • ✅ Added test_pyrefly_default_config_has_inlay_hints test
  • ✅ Added test_ty_default_config_has_diagnostics test
  • ✅ Updated example to include ty.diagnostics in the configuration list with explanatory comment

Test Results

37 tests passed in 0.51s (increased from 35)

Security Scan

CodeQL: No alerts found ✓

Benefits

  • Zero Configuration: Users can start immediately with all features enabled
  • Consistent Experience: All clients provide sensible defaults
  • Customizable: Users can override defaults as needed
  • Well Tested: Comprehensive test coverage (37 tests)
  • Well Documented: Clear documentation and examples
  • Encapsulated: Public API prevents direct access to private attributes
  • Secure: No security vulnerabilities detected
Original prompt

为 clients 提供 configuration_map 的默认值,默认将对应 server 的额外功能(比如 inlay hint)全部打开


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI self-assigned this Dec 27, 2025
Copilot AI changed the title [WIP] Add default values to configuration_map for clients Add default configuration maps for LSP clients to enable extra features Dec 27, 2025
Copilot AI requested a review from observerw December 27, 2025 20:38
@observerw observerw requested a review from Copilot December 28, 2025 04:58
Copy link
Contributor

Copilot AI left a 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 default configuration maps to LSP clients, enabling inlay hints, diagnostics, auto-imports, and other advanced features automatically without requiring manual configuration. The implementation adds a create_default_configuration_map() method to the base client class that subclasses override to provide language-server-specific defaults.

Key changes:

  • Base client auto-initializes configuration on startup if not explicitly set
  • Six client implementations provide tailored defaults (rust-analyzer, gopls, pyright, typescript, deno, pyrefly/ty)
  • New has_global_config() method added to ConfigurationMap for public API access

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/lsp_client/client/abc.py Adds create_default_configuration_map() method and auto-initialization logic in __asynccontextmanager__
src/lsp_client/utils/config.py Adds has_global_config() public method to check if global configuration exists
src/lsp_client/clients/rust_analyzer.py Implements default config with inlay hints, diagnostics, completion, check-on-save, and code lenses
src/lsp_client/clients/gopls.py Implements default config with hints, code lenses, analyses, completion, and semantic tokens
src/lsp_client/clients/pyright.py Implements default config with inlay hints, auto-imports, type checking, and diagnostics
src/lsp_client/clients/typescript.py Implements default config with inlay hints and suggestions for both TypeScript and JavaScript
src/lsp_client/clients/deno/client.py Implements default config with inlay hints, linting, unstable features, code lens, and import suggestions
src/lsp_client/clients/pyrefly.py Implements default config with inlay hints, diagnostics, and auto-imports
src/lsp_client/clients/ty.py Implements default config with diagnostics and auto-imports (no inlay hints)
tests/test_default_configuration.py Adds 35 tests covering method presence, type validation, content validation, and client-specific features
examples/default_configuration.py Provides example demonstrating default usage, overrides, and inspection of configurations
docs/configuration.md Documents default configurations feature with examples and per-client capability listings

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +138 to +147
def test_deno_default_config_has_inlay_hints():
"""Test that deno default config enables inlay hints."""
from lsp_client.clients.deno import DenoClient

client = DenoClient()
config_map = client.create_default_configuration_map()

assert config_map is not None
config = config_map.get(None, "deno.inlayHints")
assert config is not None, "deno should have inlay hints configuration"
Copy link

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing test coverage for TyClient default configuration. While the parametrized tests provide general coverage, there's no specific test validating TyClient's configuration content like there is for other clients (rust_analyzer, gopls, pyright, typescript, deno). Consider adding a test_ty_default_config test to ensure the ty-specific configuration keys are properly set.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added test_ty_default_config_has_diagnostics test that validates TyClient's diagnostics and completion configuration. (661ad01)

Comment on lines +138 to +147
def test_deno_default_config_has_inlay_hints():
"""Test that deno default config enables inlay hints."""
from lsp_client.clients.deno import DenoClient

client = DenoClient()
config_map = client.create_default_configuration_map()

assert config_map is not None
config = config_map.get(None, "deno.inlayHints")
assert config is not None, "deno should have inlay hints configuration"
Copy link

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing test coverage for PyreflyClient default configuration. While the parametrized tests provide general coverage, there's no specific test validating PyreflyClient's configuration content like there is for other clients (rust_analyzer, gopls, pyright, typescript, deno). Consider adding a test_pyrefly_default_config test to ensure the pyrefly-specific configuration keys are properly set.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added test_pyrefly_default_config_has_inlay_hints test that validates PyreflyClient's inlay hints configuration. (661ad01)

Comment on lines 96 to 103
for section in [
"rust-analyzer.inlayHints",
"gopls.hints",
"python.analysis.inlayHints",
"typescript.inlayHints",
"deno.inlayHints",
"pyrefly.inlayHints",
]:
Copy link

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The inlay hints section list in the example is missing "ty.inlayHints". However, upon checking the ty.py implementation, TyClient does not configure inlayHints - it only configures diagnostics and completion. While this is technically correct, it would be helpful to add a comment or include "ty.diagnostics" or "ty.completion" in the list to demonstrate that TyClient is also being checked, so it's clear to readers that all clients are covered.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added "ty.diagnostics" to the configuration list with a comment explaining that TyClient uses diagnostics instead of inlay hints. This makes it clear all clients are covered. (661ad01)

@observerw
Copy link
Collaborator

@copilot apply changes based on the comments in this thread

@observerw observerw marked this pull request as ready for review December 28, 2025 05:13
@observerw observerw merged commit 9a5851d into main Dec 28, 2025
1 of 3 checks passed
@observerw observerw deleted the copilot/add-default-values-to-configuration-map branch December 28, 2025 18:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants