feat(neovim): add recommended settings from dmtrKovalenko/dotfiles - #393
Conversation
- Add smart delete function (avoids polluting registers with whitespace-only lines) - Add yank register rotation to preserve yank history (registers 1-9) - Add SSH/OSC52 clipboard handling for remote sessions - Add filetype-specific keyword extensions (TS/JS, CSS, HTML, JSON, YAML, etc.) - Add auto-save on buffer leave and focus loss - Add custom terminal title with file icons - Add close floating windows utility - Add visible whitespace rendering (tabs, trailing spaces, etc.) - Add enhanced diagnostic display configuration Reference: https://github.com/dmtrKovalenko/dotfiles
📝 WalkthroughSummary by CodeRabbitRelease Notes
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThis pull request expands the Neovim configuration with multiple runtime enhancements. Changes include new autocmds for auto-saving, yank highlighting with register rotation, dynamic terminal titles, and per-filetype keyword expansions. Utility functions are introduced for buffer cycling, register manipulation, SSH detection, and floating window management. Settings additions cover SSH-aware clipboard handling, diagnostic display options, and visible whitespace configuration. Two new keymaps modify deletion and floating window behavior. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
Summary of ChangesHello @shunkakinoki, 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 integrates a comprehensive set of recommended Neovim configurations and utility functions, primarily sourced from Highlights
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
|
Mesa DescriptionTL;DRAdded a suite of recommended Neovim settings and features from dmtrKovalenko/dotfiles. What changed?
Description generated by Mesa. Update settings |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces several useful features from dmtrKovalenko/dotfiles. The changes are generally good, but I have a few suggestions to improve maintainability and avoid potential issues. I've pointed out some code duplication that should be addressed, an opportunity to refactor a series of autocommands to be more data-driven and maintainable, and a concern about a keymap override that could have unintended side effects. Please see the detailed comments.
| local function is_ssh() | ||
| return os.getenv("SSH_CLIENT") ~= nil or os.getenv("SSH_TTY") ~= nil | ||
| end | ||
|
|
||
| if is_ssh() then |
There was a problem hiding this comment.
The is_ssh function is duplicated here and in lua/utils.lua. To adhere to the DRY (Don't Repeat Yourself) principle and improve maintainability, you should remove this local implementation and use the one from the utils module. You can require the module inline to avoid adding it at the top of the file.
if require("utils").is_ssh() then
| autocmd("FileType", { | ||
| group = keyword_group, | ||
| pattern = { "typescript", "typescriptreact", "javascript", "javascriptreact" }, | ||
| callback = function() | ||
| -- Add @ and - to keyword characters for better word navigation | ||
| vim.opt_local.iskeyword:append("@-@") | ||
| vim.opt_local.iskeyword:append("-") | ||
| end, | ||
| }) | ||
| autocmd("FileType", { | ||
| group = keyword_group, | ||
| pattern = { "css", "scss", "sass", "less" }, | ||
| callback = function() | ||
| vim.opt_local.iskeyword:append("-") | ||
| vim.opt_local.iskeyword:append("#") | ||
| end, | ||
| }) | ||
| autocmd("FileType", { | ||
| group = keyword_group, | ||
| pattern = { "html", "xml", "vue", "svelte" }, | ||
| callback = function() | ||
| vim.opt_local.iskeyword:append("-") | ||
| vim.opt_local.iskeyword:append(":") | ||
| end, | ||
| }) | ||
| autocmd("FileType", { | ||
| group = keyword_group, | ||
| pattern = { "json", "jsonc" }, | ||
| callback = function() | ||
| vim.opt_local.iskeyword:append("-") | ||
| vim.opt_local.iskeyword:append("$") | ||
| end, | ||
| }) | ||
| autocmd("FileType", { | ||
| group = keyword_group, | ||
| pattern = { "yaml", "toml" }, | ||
| callback = function() | ||
| vim.opt_local.iskeyword:append("-") | ||
| vim.opt_local.iskeyword:append(".") | ||
| end, | ||
| }) | ||
| autocmd("FileType", { | ||
| group = keyword_group, | ||
| pattern = "markdown", | ||
| callback = function() | ||
| vim.opt_local.iskeyword:append("-") | ||
| vim.opt_local.iskeyword:append("#") | ||
| end, | ||
| }) |
There was a problem hiding this comment.
There's a lot of repetition in the creation of FileType autocommands for setting iskeyword. This can be refactored into a more concise, data-driven approach to improve maintainability. You can define a table of configurations and loop through it to create the autocommands.
This also allows for merging the rules for css, scss, sass, less, and markdown since they share the same keyword additions.
Additionally, vim.opt_local.iskeyword:append("@-@") is functionally equivalent to vim.opt_local.iskeyword:append("@") but is less clear. The suggested refactoring simplifies this as well.
local keyword_extensions = {
{
patterns = { "typescript", "typescriptreact", "javascript", "javascriptreact" },
keywords = { "@", "-" },
},
{
patterns = { "css", "scss", "sass", "less", "markdown" },
keywords = { "-", "#" },
},
{
patterns = { "html", "xml", "vue", "svelte" },
keywords = { "-", ":" },
},
{
patterns = { "json", "jsonc" },
keywords = { "-", "$" },
},
{
patterns = { "yaml", "toml" },
keywords = { "-", "." },
},
}
for _, config in ipairs(keyword_extensions) do
autocmd("FileType", {
group = keyword_group,
pattern = config.patterns,
callback = function()
for _, keyword in ipairs(config.keywords) do
vim.opt_local.iskeyword:append(keyword)
end
end,
})
end
| keymap("n", "<Esc>", function() | ||
| utils.close_floating_wins() | ||
| vim.cmd("nohlsearch") | ||
| end, opts) |
There was a problem hiding this comment.
Overriding the <Esc> key to close all floating windows is quite aggressive and may lead to unexpected behavior. For instance, many plugins use floating windows where <Esc> is used to return to normal mode within that window. This mapping would close the window entirely, along with any other floating windows that might be open (like diagnostics). Consider using a more specific keymap (e.g., <leader><Esc>) or making the function less aggressive, for example, by only closing the current window if it's a floating window.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
home-manager/programs/neovim/lua/settings.lua (1)
15-32: Consider using utils.is_ssh() to avoid duplication.The local
is_ssh()function duplicates the same logic already available inutils.is_ssh()(lines 55-57 of utils.lua). The OSC52 clipboard configuration itself is correct.To reduce duplication, import from utils:
+local utils = require("utils") + -- ==================================================================================== -- SSH / OSC52 CLIPBOARD HANDLING -- From: https://github.com/dmtrKovalenko/dotfiles -- Uses OSC 52 protocol for clipboard when running over SSH -- ==================================================================================== -local function is_ssh() - return os.getenv("SSH_CLIENT") ~= nil or os.getenv("SSH_TTY") ~= nil -end - -if is_ssh() then +if utils.is_ssh() then -- Use OSC 52 for clipboard when in SSH session
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
home-manager/programs/neovim/lua/autocmds.lua(1 hunks)home-manager/programs/neovim/lua/keymaps.lua(1 hunks)home-manager/programs/neovim/lua/settings.lua(2 hunks)home-manager/programs/neovim/lua/utils.lua(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
- GitHub Check: Agent
- GitHub Check: cubic · AI code reviewer
- GitHub Check: Mesa Review
- GitHub Check: e2e-run (NixOS, ubuntu-latest)
- GitHub Check: nix-linux
- GitHub Check: nix-nixos
- GitHub Check: docker-build-push (linux/arm64, arm64, ubuntu-24.04-arm)
- GitHub Check: e2e-run (MacOS, macos-latest)
- GitHub Check: docker-build-push (linux/amd64, amd64, ubuntu-latest)
- GitHub Check: e2e-run (Ubuntu, ubuntu-latest)
- GitHub Check: lua-hammerspoon
- GitHub Check: lua-neovim
- GitHub Check: nix-darwin
🔇 Additional comments (13)
home-manager/programs/neovim/lua/keymaps.lua (2)
113-117: LGTM! Smart delete implementation is clean.The expr-mode mapping correctly delegates to
utils.smart_delete(), which will avoid polluting registers with whitespace-only lines while preserving normalddbehavior for content lines.
119-123: LGTM! Escape key enhancement is sensible.This mapping provides a clean UX by closing floating windows and clearing search highlights with a single key. The combination of actions is logical and non-destructive.
home-manager/programs/neovim/lua/settings.lua (2)
76-83: LGTM! Whitespace rendering configuration is standard.The listchars configuration uses sensible symbols for tabs, trailing spaces, and other whitespace elements, providing good visual feedback without being obtrusive.
89-102: LGTM! Diagnostic configuration is well-structured.The diagnostic display settings provide a good balance of visibility and clarity with rounded borders, source attribution, and appropriate virtual text formatting.
home-manager/programs/neovim/lua/autocmds.lua (4)
10-20: LGTM! Yank rotation logic is correctly scoped.The condition
vim.v.event.operator == "y"ensures register rotation only happens on yank operations, not on deletions, which is the correct behavior to preserve yank history separately from delete history.
26-36: LGTM! Auto-save guards are appropriate.The conditions properly filter for regular file buffers that are modified and have a filename, avoiding special buffers and unintentional saves. The
silent!prefix prevents errors on read-only files.
63-112: LGTM! Filetype-specific keyword extensions are well-considered.The keyword character additions are appropriate for each language:
- TS/JS decorators and kebab-case
- CSS properties and color codes
- HTML attributes and namespaces
- JSON keys and variable notation
- YAML/TOML nested paths
- Markdown syntax elements
These will improve word motion navigation (
w,b,e) in their respective contexts.
42-57: BufFilePost is a valid Neovim autocmd event.
BufFilePostis a documented event that fires after the buffer's filename changes via:fileor:saveas. However, consider whether using bothBufEnterandBufFilePostis necessary—BufEnteralone may be sufficient for updating the terminal title on buffer navigation, sinceBufFilePostonly fires when the buffer filename explicitly changes, not on normal buffer switching.Likely an incorrect or invalid review comment.
home-manager/programs/neovim/lua/utils.lua (5)
8-15: LGTM! Smart delete logic is correct.The pattern
^%s*$correctly identifies whitespace-only lines, and the blackhole register ("_) prevents cluttering the default register with blank content.
22-33: LGTM! Register rotation implementation is correct.The backward iteration from 9 to 2 prevents overwrites, and using both
getreg()andgetregtype()ensures the register type (characterwise/linewise/blockwise) is preserved during rotation.
40-49: LGTM! Floating window detection and closure is correct.The check
config.relative ~= ""correctly identifies floating windows, andforce=falsepreserves buffers with unsaved changes, preventing accidental data loss.
55-57: LGTM! SSH detection is standard and correct.Checking both
SSH_CLIENTandSSH_TTYenvironment variables is the standard approach for detecting SSH sessions.
63-70: LGTM! Icon retrieval is safely guarded.The
pcallwrapper ensures the function degrades gracefully whennvim-web-deviconsis unavailable, and the empty string fallback prevents UI breakage.
There was a problem hiding this comment.
Pull request overview
This pull request enhances Neovim configuration with quality-of-life improvements sourced from dmtrKovalenko/dotfiles. The changes add smart register management, remote session support, and improved visual feedback while maintaining the existing codebase structure and conventions.
Key changes:
- Smart delete and yank register rotation to improve clipboard management
- SSH/OSC52 clipboard integration for seamless remote editing
- Auto-save functionality and enhanced terminal titles with file icons
- Filetype-specific keyword extensions for better navigation in TS/JS, CSS, HTML, and other web development files
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
home-manager/programs/neovim/lua/utils.lua |
Adds utility functions for smart delete, yank rotation, SSH detection, file icons, and closing floating windows |
home-manager/programs/neovim/lua/settings.lua |
Configures OSC52 clipboard for SSH sessions, visible whitespace rendering, and enhanced diagnostic display |
home-manager/programs/neovim/lua/keymaps.lua |
Implements smart delete keymap and Esc key override for closing floating windows |
home-manager/programs/neovim/lua/autocmds.lua |
Adds autocmds for yank rotation, auto-save, terminal title updates, and filetype-specific keyword extensions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| callback = function() | ||
| vim.highlight.on_yank() | ||
| -- Rotate registers on yank to preserve yank history | ||
| if vim.v.event.operator == "y" then |
There was a problem hiding this comment.
The register rotation runs on every yank operation, including yanks to named registers (e.g., "ayy), which may not be the desired behavior. This could pollute the numbered register history with intentional named register operations.
Consider checking vim.v.event.regname to only rotate when yanking to the unnamed register:
if vim.v.event.operator == "y" and vim.v.event.regname == "" then
utils.yank_shift()
end| if vim.v.event.operator == "y" then | |
| if vim.v.event.operator == "y" and vim.v.event.regname == "" then |
| for _, win in ipairs(vim.api.nvim_list_wins()) do | ||
| if vim.api.nvim_win_is_valid(win) then | ||
| local config = vim.api.nvim_win_get_config(win) | ||
| if config.relative ~= "" then | ||
| vim.api.nvim_win_close(win, false) | ||
| end | ||
| end | ||
| end |
There was a problem hiding this comment.
The function may fail to close some floating windows if closing one window invalidates others in the iteration. When a floating window is closed, it's removed from the window list, but the iteration continues with indices that may now be stale. This could result in errors or some floating windows not being closed.
Consider collecting window handles first, then closing them, or iterate in reverse order:
function M.close_floating_wins()
local floating_wins = {}
for _, win in ipairs(vim.api.nvim_list_wins()) do
if vim.api.nvim_win_is_valid(win) then
local config = vim.api.nvim_win_get_config(win)
if config.relative ~= "" then
table.insert(floating_wins, win)
end
end
end
for _, win in ipairs(floating_wins) do
if vim.api.nvim_win_is_valid(win) then
vim.api.nvim_win_close(win, false)
end
end
end| for _, win in ipairs(vim.api.nvim_list_wins()) do | |
| if vim.api.nvim_win_is_valid(win) then | |
| local config = vim.api.nvim_win_get_config(win) | |
| if config.relative ~= "" then | |
| vim.api.nvim_win_close(win, false) | |
| end | |
| end | |
| end | |
| local floating_wins = {} | |
| for _, win in ipairs(vim.api.nvim_list_wins()) do | |
| if vim.api.nvim_win_is_valid(win) then | |
| local config = vim.api.nvim_win_get_config(win) | |
| if config.relative ~= "" then | |
| table.insert(floating_wins, win) | |
| end | |
| end | |
| end | |
| for _, win in ipairs(floating_wins) do | |
| if vim.api.nvim_win_is_valid(win) then | |
| vim.api.nvim_win_close(win, false) | |
| end | |
| end |
| local function is_ssh() | ||
| return os.getenv("SSH_CLIENT") ~= nil or os.getenv("SSH_TTY") ~= nil | ||
| end | ||
|
|
||
| if is_ssh() then |
There was a problem hiding this comment.
The is_ssh() function is duplicated - it's defined both here and in utils.lua (line 55). This creates code duplication and potential maintenance issues if the logic needs to be updated.
Consider removing this local definition and using require("utils").is_ssh() instead, or if this file is loaded before utils.lua, keep it here and remove the duplicate from utils.lua.
| local function is_ssh() | |
| return os.getenv("SSH_CLIENT") ~= nil or os.getenv("SSH_TTY") ~= nil | |
| end | |
| if is_ssh() then | |
| if require("utils").is_ssh() then |
| -- From: https://github.com/dmtrKovalenko/dotfiles | ||
| -- ==================================================================================== | ||
| local title_group = augroup("TerminalTitle", { clear = true }) | ||
| autocmd({ "BufEnter", "BufFilePost" }, { |
There was a problem hiding this comment.
BufFilePost is not a standard Neovim autocmd event. This appears to be a typo - the correct event name is likely BufFilePre or BufReadPost. This event will never trigger, meaning the terminal title won't be updated when files are read.
Consider using BufReadPost or BufWinEnter instead, which are standard events that trigger when buffers are loaded or displayed in windows.
| autocmd({ "BufEnter", "BufFilePost" }, { | |
| autocmd({ "BufEnter", "BufReadPost" }, { |
| callback = function() | ||
| -- Only save if buffer is modified and has a filename | ||
| if vim.bo.modified and vim.fn.expand("%") ~= "" and vim.bo.buftype == "" then | ||
| vim.cmd("silent! update") |
There was a problem hiding this comment.
The auto-save logic may fail silently for unwritable files. The silent! command suppresses all errors, which could hide important issues like permission errors or readonly files. Users may not realize their changes aren't being saved.
Consider using pcall to handle errors gracefully while still logging them:
local ok, err = pcall(vim.cmd, "update")
if not ok then
vim.notify("Auto-save failed: " .. err, vim.log.levels.WARN)
end| vim.cmd("silent! update") | |
| local ok, err = pcall(vim.cmd, "update") | |
| if not ok then | |
| vim.notify("Auto-save failed: " .. err, vim.log.levels.WARN) | |
| end |
| -- @keymap <Esc>: Close floating windows | ||
| keymap("n", "<Esc>", function() |
There was a problem hiding this comment.
Overriding the <Esc> key in normal mode could interfere with other plugins or user workflows that expect <Esc> to only clear search highlights. Some plugins use <Esc> as a cancellation key, and closing all floating windows might be too aggressive in certain contexts (e.g., when a user wants to cancel an action without closing all floating windows).
Consider using a different key combination like <leader><Esc> or <C-c> for this functionality, or make the floating window closing conditional.
| -- @keymap <Esc>: Close floating windows | |
| keymap("n", "<Esc>", function() | |
| -- @keymap <leader><Esc>: Close floating windows | |
| keymap("n", "<leader><Esc>", function() |
![mesa-dot-dev[bot]](https://avatars.githubusercontent.com/in/1050077?s=60&v=4){kind=small}
There was a problem hiding this comment.
Performed full review of 38ce9f6...adfab56
Analysis
-
Critical Breaking of Vim Register Semantics - Custom yank history rotation breaks Vim's standard numbered register behavior. The implementation makes registers rotate on EVERY yank operation, conflicting with native behavior and breaking standard Vim workflows where registers 1-9 are for deletions, not yanks.
-
Unconfigurable Auto-save Feature - Implements auto-save on BufLeave/FocusLost events without opt-out options. Could interfere with git workflows, temporary edits, and has error suppression that hinders debugging.
-
Overriding Core Vim Commands - Remapping fundamental commands like
ddand<Esc>changes muscle memory behaviors with no escape hatches for standard operation, potentially breaking user expectations and plugin compatibility. -
Code Duplication and Organization Issues -
is_ssh()function duplicated in two files, vim options placed in incorrect modules, and missing error handling for features like OSC52 clipboard that requires specific Neovim versions. -
Performance Concerns - Frequent trigger points (BufEnter for terminal titles, BufLeave/FocusLost for saves) could cause performance issues with no throttling mechanisms.
Tip
Help
Slash Commands:
/review- Request a full code review/review latest- Review only changes since the last review/describe- Generate PR description. This will update the PR body or issue comment depending on your configuration/help- Get help with Mesa commands and configuration options
4 files reviewed | 6 comments | Edit Agent Settings • Read Docs
| callback = function() | ||
| vim.highlight.on_yank() | ||
| -- Rotate registers on yank to preserve yank history | ||
| if vim.v.event.operator == "y" then |
There was a problem hiding this comment.
This yank register rotation fundamentally breaks Vim's numbered register semantics. In standard Vim, registers 1-9 store the last 9 deletions (not yanks), and Vim manages this automatically. By rotating registers on every yank operation:
- You're overwriting the deletion history that users expect with
"1p,"2p, etc. - This triggers on ALL yank events including plugin-initiated ones, potentially causing unexpected side effects
- It conflicts with standard Vim behavior documented everywhere
If keeping this feature, strongly consider:
- Adding a configuration flag to opt-in (
vim.g.enable_yank_rotation) - Documenting this non-standard behavior prominently in README
- Filtering by
v:event.regnameto avoid affecting special registers - Adding guards against recursive or plugin-initiated yanks
Agent: 🏛 Architecture •
Prompt for Agent
Task: Address review feedback left on GitHub.
Repository: shunkakinoki/dotfiles#393
File: home-manager/programs/neovim/lua/autocmds.lua#L16
Action: Open this file location in your editor, inspect the highlighted code, and resolve the issue described below.
Feedback:
This yank register rotation fundamentally breaks Vim's numbered register semantics. In standard Vim, registers 1-9 store the last 9 **deletions** (not yanks), and Vim manages this automatically. By rotating registers on every yank operation:
1. You're overwriting the deletion history that users expect with `"1p`, `"2p`, etc.
2. This triggers on ALL yank events including plugin-initiated ones, potentially causing unexpected side effects
3. It conflicts with standard Vim behavior documented everywhere
If keeping this feature, strongly consider:
- Adding a configuration flag to opt-in (`vim.g.enable_yank_rotation`)
- Documenting this non-standard behavior prominently in README
- Filtering by `v:event.regname` to avoid affecting special registers
- Adding guards against recursive or plugin-initiated yanks
| callback = function() | ||
| -- Only save if buffer is modified and has a filename | ||
| if vim.bo.modified and vim.fn.expand("%") ~= "" and vim.bo.buftype == "" then | ||
| vim.cmd("silent! update") |
There was a problem hiding this comment.
The silent! suppresses all errors including critical ones like filesystem permission problems, disk full conditions, or network filesystem timeouts. Consider removing silent! and handling errors explicitly, or at minimum add checks for file writability before attempting save. Also consider adding configuration options:
- Global flag:
vim.g.auto_save_enabled - Buffer-local opt-out:
vim.b.disable_auto_save - Per-filetype configuration to avoid interfering with git rebase/merge workflows
Agent: 🏛 Architecture •
Prompt for Agent
Task: Address review feedback left on GitHub.
Repository: shunkakinoki/dotfiles#393
File: home-manager/programs/neovim/lua/autocmds.lua#L33
Action: Open this file location in your editor, inspect the highlighted code, and resolve the issue described below.
Feedback:
The `silent!` suppresses all errors including critical ones like filesystem permission problems, disk full conditions, or network filesystem timeouts. Consider removing `silent!` and handling errors explicitly, or at minimum add checks for file writability before attempting save. Also consider adding configuration options:
- Global flag: `vim.g.auto_save_enabled`
- Buffer-local opt-out: `vim.b.disable_auto_save`
- Per-filetype configuration to avoid interfering with git rebase/merge workflows
| end, { noremap = true, expr = true }) | ||
|
|
||
| -- @keymap <Esc>: Close floating windows | ||
| keymap("n", "<Esc>", function() |
There was a problem hiding this comment.
Remapping <Esc> globally changes fundamental Vim behavior. The floating window close could interfere with:
- Plugin UI elements that need manual dismissal (telescope, which-key, lazy.nvim)
- Intentionally persistent floating windows
- Custom workflows that rely on keeping floating windows open
Consider using a different key like <leader><Esc> or <C-l>, or add checks to exclude certain floating windows from being closed.
Agent: 🏛 Architecture •
Prompt for Agent
Task: Address review feedback left on GitHub.
Repository: shunkakinoki/dotfiles#393
File: home-manager/programs/neovim/lua/keymaps.lua#L120
Action: Open this file location in your editor, inspect the highlighted code, and resolve the issue described below.
Feedback:
Remapping `<Esc>` globally changes fundamental Vim behavior. The floating window close could interfere with:
- Plugin UI elements that need manual dismissal (telescope, which-key, lazy.nvim)
- Intentionally persistent floating windows
- Custom workflows that rely on keeping floating windows open
Consider using a different key like `<leader><Esc>` or `<C-l>`, or add checks to exclude certain floating windows from being closed.
| -- Utility to close all floating windows | ||
| -- From: https://github.com/dmtrKovalenko/dotfiles | ||
| -- ==================================================================================== | ||
| function M.close_floating_wins() |
There was a problem hiding this comment.
This function closes ALL floating windows without discrimination, which could close important plugin UIs (lazy.nvim, mason.nvim, telescope previews, LSP hover docs). Add:
pcallprotection aroundnvim_win_close- Filetype filtering to exclude important windows
- Optional parameter to specify exclude list
Example:
function M.close_floating_wins(opts)
opts = opts or {}
local exclude_fts = opts.exclude_filetypes or {}
for _, win in ipairs(vim.api.nvim_list_wins()) do
if vim.api.nvim_win_is_valid(win) then
local config = vim.api.nvim_win_get_config(win)
if config.relative ~= "" then
local buf = vim.api.nvim_win_get_buf(win)
local ft = vim.bo[buf].filetype
if not vim.tbl_contains(exclude_fts, ft) then
pcall(vim.api.nvim_win_close, win, false)
end
end
end
end
endAgent: 🏛 Architecture •
Prompt for Agent
Task: Address review feedback left on GitHub.
Repository: shunkakinoki/dotfiles#393
File: home-manager/programs/neovim/lua/utils.lua#L40
Action: Open this file location in your editor, inspect the highlighted code, and resolve the issue described below.
Feedback:
This function closes ALL floating windows without discrimination, which could close important plugin UIs (lazy.nvim, mason.nvim, telescope previews, LSP hover docs). Add:
1. `pcall` protection around `nvim_win_close`
2. Filetype filtering to exclude important windows
3. Optional parameter to specify exclude list
Example:
```lua
function M.close_floating_wins(opts)
opts = opts or {}
local exclude_fts = opts.exclude_filetypes or {}
for _, win in ipairs(vim.api.nvim_list_wins()) do
if vim.api.nvim_win_is_valid(win) then
local config = vim.api.nvim_win_get_config(win)
if config.relative ~= "" then
local buf = vim.api.nvim_win_get_buf(win)
local ft = vim.bo[buf].filetype
if not vim.tbl_contains(exclude_fts, ft) then
pcall(vim.api.nvim_win_close, win, false)
end
end
end
end
end
</details>
|
|
||
| -- @keymap dd: Smart delete (uses blackhole register for empty lines) | ||
| -- From: https://github.com/dmtrKovalenko/dotfiles | ||
| keymap("n", "dd", function() |
There was a problem hiding this comment.
Overriding dd (one of Vim's most fundamental commands) is architecturally risky and changes muscle memory for all Vim users. Consider:
- Using a different keymap like
<leader>ddinstead to avoid breaking standard behavior - Making this opt-in via configuration
- Adding a toggle command to enable/disable
- Document this breaking change prominently if you keep it
The expr mapping adds indirection and could interfere with plugins that rely on standard dd behavior.
Agent: 🏛 Architecture •
Prompt for Agent
Task: Address review feedback left on GitHub.
Repository: shunkakinoki/dotfiles#393
File: home-manager/programs/neovim/lua/keymaps.lua#L115
Action: Open this file location in your editor, inspect the highlighted code, and resolve the issue described below.
Feedback:
Overriding `dd` (one of Vim's most fundamental commands) is architecturally risky and changes muscle memory for all Vim users. Consider:
- Using a different keymap like `<leader>dd` instead to avoid breaking standard behavior
- Making this opt-in via configuration
- Adding a toggle command to enable/disable
- Document this breaking change prominently if you keep it
The expr mapping adds indirection and could interfere with plugins that rely on standard `dd` behavior.
| -- From: https://github.com/dmtrKovalenko/dotfiles | ||
| -- Uses OSC 52 protocol for clipboard when running over SSH | ||
| -- ==================================================================================== | ||
| local function is_ssh() |
There was a problem hiding this comment.
Duplicate code: is_ssh() function is defined identically in utils.lua (lines 55-57). This should use require("utils").is_ssh() instead. Additionally, the require("vim.ui.clipboard.osc52") below lacks error handling - OSC52 is only available in Neovim >= 0.10. Add pcall protection:
local utils = require("utils")
if utils.is_ssh() then
local ok, osc52 = pcall(require, "vim.ui.clipboard.osc52")
if not ok then
vim.notify("OSC52 clipboard not available", vim.log.levels.WARN)
return
end
-- ... rest of config
endAgent: 🏛 Architecture •
Prompt for Agent
Task: Address review feedback left on GitHub.
Repository: shunkakinoki/dotfiles#393
File: home-manager/programs/neovim/lua/settings.lua#L15
Action: Open this file location in your editor, inspect the highlighted code, and resolve the issue described below.
Feedback:
Duplicate code: `is_ssh()` function is defined identically in `utils.lua` (lines 55-57). This should use `require("utils").is_ssh()` instead. Additionally, the `require("vim.ui.clipboard.osc52")` below lacks error handling - OSC52 is only available in Neovim >= 0.10. Add `pcall` protection:
```lua
local utils = require("utils")
if utils.is_ssh() then
local ok, osc52 = pcall(require, "vim.ui.clipboard.osc52")
if not ok then
vim.notify("OSC52 clipboard not available", vim.log.levels.WARN)
return
end
-- ... rest of config
end
</details>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Reference: https://github.com/dmtrKovalenko/dotfiles
Summary by cubic
Adopt recommended Neovim settings from dmtrKovalenko/dotfiles to polish editing UX and diagnostics. Improves remote clipboard, auto-save, yank history, and UI details.
Written for commit adfab56. Summary will update automatically on new commits.