-
Notifications
You must be signed in to change notification settings - Fork 4
more healthcheck #75
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
more healthcheck #75
Conversation
WalkthroughThe changes introduce health checks for LSP configurations in addition to existing program checks, refactor sorting and reporting logic into reusable functions, and enhance program registration to support filetype-based dynamic requirements. The Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant HealthModule
participant Lspconfig
participant Vim
User->>HealthModule: M.register_lsp(lsp_name)
HealthModule->>Lspconfig: require("lspconfig")[lsp_name]
alt Config exists and valid
HealthModule->>HealthModule: register_program(program, filetypes)
HealthModule->>Vim: autocmd FileType for filetypes
else Config missing/invalid
HealthModule->>Vim: notify("LSP configuration for ... not found", WARN)
end
User->>HealthModule: M.check()
HealthModule->>HealthModule: check_lspconfigs()
HealthModule->>HealthModule: check_programs()
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
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.
Actionable comments posted: 3
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
plugins/nobbz/lua/nobbz/health.lua(6 hunks)plugins/nobbz/lua/nobbz/lsp/nil.lua(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`plugins/**/*.lua`: Please ensure all of the following: - Plugins are lazyloaded if possible through `nobbz.lazy` - `local` has to be used - coding follows the [luarocks styleguide...
plugins/**/*.lua: Please ensure all of the following:
- Plugins are lazyloaded if possible through
nobbz.lazylocalhas to be used- coding follows the luarocks styleguide, but with 2 spaces indentation
plugins/nobbz/lua/nobbz/lsp/nil.luaplugins/nobbz/lua/nobbz/health.lua
🔇 Additional comments (8)
plugins/nobbz/lua/nobbz/health.lua (6)
4-4: LGTM!The addition of
lsp_configstable to track LSP configurations is well-structured and follows the existing pattern used forprograms.
16-38: Excellent refactoring of sorting and reporting logic.The extraction of
health_cmp,health_sort, andreport_tableinto reusable helper functions is a great improvement that eliminates code duplication and makes the codebase more maintainable.
41-57: Well-implemented LSP configuration health checks.The
check_lspconfigsfunction properly integrates with the existing health check infrastructure and provides clear feedback on LSP configuration status. The sorting and reporting using the helper functions maintains consistency with program checks.
86-101: Innovative filetype-based program registration enhancement.The enhancement to support both boolean and filetype array parameters is well-designed. The autocmd approach for dynamically marking programs as required when specific filetypes are opened is elegant and efficient.
114-124: Well-integrated LSP registration function.The
register_lspfunction properly extracts LSP configuration details and integrates with the enhancedregister_programfunction. The approach of checkingvim.lsp.config[lsp]availability and registering the command with associated filetypes is sound.
142-142: Proper integration of LSP checks into main health function.The addition of
check_lspconfigs()beforecheck_programs()provides a logical flow in the health check report.plugins/nobbz/lua/nobbz/lsp/nil.lua (2)
2-2: Proper module import following coding guidelines.The health module import uses
localdeclaration and follows the established pattern in the file.
4-4: Excellent integration with the enhanced health system.The registration of "alejandra" with the "nix" filetype leverages the new filetype-based program registration feature introduced in the health module. This ensures the formatter is only marked as required when working with Nix files, which is the intended behavior.
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 enhances the healthcheck framework by adding LSP configuration checks alongside existing program checks and refactors sorting/reporting logic.
- Introduces tracking and reporting of LSP server configuration status.
- Refactors sorting and reporting into shared helper functions (
health_sort,report_table). - Extends
register_programto support filetype-based dynamic requirements and addsregister_lsp.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| plugins/nobbz/lua/nobbz/lsp/nil.lua | Registers the “alejandra” program for nix files |
| plugins/nobbz/lua/nobbz/health.lua | Adds lsp_configs, refactors sorting/reporting, updates register_program, and implements check_lspconfigs |
Comments suppressed due to low confidence (5)
plugins/nobbz/lua/nobbz/health.lua:43
- There are no tests covering
check_lspconfigs; adding unit or integration tests would ensure both configured and missing LSP scenarios are validated.
local function check_lspconfigs()
plugins/nobbz/lua/nobbz/health.lua:88
- [nitpick] Mixing boolean and table types in the
required_or_filetypesparameter can be confusing; consider separating into two distinct functions or using a more explicit options table.
M.register_program = function(program, required_or_filetypes)
plugins/nobbz/lua/nobbz/health.lua:113
- [nitpick] Update the docblock for
register_lspto mention that it now also registers filetype-based program requirements and populateslsp_configs.
---@param lsp string the name of the LSP server as defined in lspconfig
plugins/nobbz/lua/nobbz/lsp/nil.lua:4
- Registering the
alejandraprogram in thenil.lua(Nil LSP) module seems misplaced—this should live in the Nix-related plugin or configuration module to avoid confusion.
health.register_program("alejandra", { "nix", })
plugins/nobbz/lua/nobbz/health.lua:5
- The
donevariable is declared but never used; consider removing it to clean up dead code.
local done = false
FIXES #56
Summary by CodeRabbit
New Features
Refactor
Chores