misc: optimize health check info #46
Conversation
There was a problem hiding this comment.
Pull request overview
This PR optimizes the health check output by adding system information (OS platform and Neovim version), clarifying status descriptions, and consolidating notebook status information into a single info call instead of multiple separate calls.
- Adds OS platform and Neovim version information to the version section
- Restructures status output to display all notebook information in a single consolidated message
- Updates README to clarify Neovim version requirements
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| lua/neopyter/health.lua | Adds platform/version info, refactors notebook status display to build a single message string instead of multiple health.info calls |
| README.md | Clarifies that both stable and nightly Neovim versions are supported |
| health.start("neopyter: version") | ||
| local nvim_plugin_ver = jupyter.jupyterlab:get_nvim_plugin_version() | ||
| health.info(string.format("os platform of neovim: %s", vim.uv.os_uname().sysname)) | ||
| health.info(string.format("neovim version: %s", vim.version())) |
There was a problem hiding this comment.
The vim.version() function returns a table (with major, minor, and patch fields), not a string. When passed to string.format, it will be converted to a string representation like "table: 0x..." which is not user-friendly. Consider using tostring(vim.version()) or formatting it properly with something like string.format("%d.%d.%d", vim.version().major, vim.version().minor, vim.version().patch).
| health.info(string.format("neovim version: %s", vim.version())) | |
| health.info(string.format("neovim version: %d.%d.%d", vim.version().major, vim.version().minor, vim.version().patch)) |
| ) | ||
| else | ||
| msg = string.format("%s %-30s %-10s false", select_mark, notebook.local_path, notebook:is_attached(), nbconnect) | ||
| msg = msg .. "\n" .. string.format("%s %-30s %-10s false", select_mark, notebook.local_path, notebook:is_attached(), nbconnect) |
There was a problem hiding this comment.
The string.format call has 3 format specifiers (%s) but 4 arguments are provided. The nbconnect parameter is unused in the format string. Either add a format specifier for it or remove it from the arguments list.
| msg = msg .. "\n" .. string.format("%s %-30s %-10s false", select_mark, notebook.local_path, notebook:is_attached(), nbconnect) | |
| msg = msg .. "\n" .. string.format("%s %-30s %-10s false", select_mark, notebook.local_path, notebook:is_attached()) |
| health.start("neopyter: status") | ||
| health.info("attach=ready, connect=syncing\n") | ||
| health.info(string.format(" %-30s %-10s %-10s %s", "file", "attach", "connect", "remote_path")) | ||
| health.info("attach: tracking with neopyter\nconnect: synchronizing with jupyter lab\n") |
There was a problem hiding this comment.
Consider clarifying these definitions. The word "synchronizing" might be unclear - does it mean "currently syncing" or "configured to sync"? Also, "tracking with neopyter" could be more specific. For example: "attach: notebook is tracked by neopyter" and "connect: notebook is synchronized with jupyter lab".
| health.info("attach: tracking with neopyter\nconnect: synchronizing with jupyter lab\n") | |
| health.info("attach: notebook is tracked by neopyter\nconnect: notebook is synchronized with jupyter lab\n") |
| local msg = string.format(" %-30s %-10s %-10s %s", "file", "attach", "connect", "remote_path") | ||
| for _, notebook in pairs(jupyter.jupyterlab.notebook_map) do | ||
| local select_mark = " " | ||
| if jupyter.notebook == notebook then | ||
| select_mark = "*" | ||
| end | ||
| local msg = "" | ||
| local nbconnect = notebook:is_connecting() | ||
|
|
||
| if nbconnect then | ||
| msg = string.format( | ||
| "%s %-30s %-10s %-10s %s", | ||
| select_mark, | ||
| notebook.local_path, | ||
| notebook:is_attached(), | ||
| nbconnect, | ||
| notebook.remote_path | ||
| ) | ||
| msg = msg | ||
| .. "\n" | ||
| .. string.format( | ||
| "%s %-30s %-10s %-10s %s", | ||
| select_mark, | ||
| notebook.local_path, | ||
| notebook:is_attached(), | ||
| nbconnect, | ||
| notebook.remote_path | ||
| ) | ||
| else | ||
| msg = string.format("%s %-30s %-10s false", select_mark, notebook.local_path, notebook:is_attached(), nbconnect) | ||
| msg = msg .. "\n" .. string.format("%s %-30s %-10s false", select_mark, notebook.local_path, notebook:is_attached(), nbconnect) | ||
| end | ||
|
|
||
| health.info(msg) | ||
| end | ||
| health.info(msg) |
There was a problem hiding this comment.
If jupyter.jupyterlab.notebook_map is empty, only the header row will be displayed without any notebook entries. Consider adding a check to display a message like "No notebooks currently tracked" when the map is empty to make the output clearer.
No description provided.