Skip to content

[audit] Drop vim.lsp.stop_client dead branch + fix PackChanged User event - #162

Closed
stanfish06 wants to merge 1 commit into
masterfrom
claude/festive-goodall-PZARB
Closed

stanfish06 wants to merge 1 commit into
masterfrom
claude/festive-goodall-PZARB

Conversation

@stanfish06

Copy link
Copy Markdown
Owner

What

Two mechanical fixes in lua/config/plugin_config.lua.


1. Remove dead vim.lsp.stop_client() branch in :LspToggle

Where: lua/config/plugin_config.lua — LspToggle user command

Change: Removed the vim.fn.has('nvim-0.12') version check and else vim.lsp.stop_client(client.id) branch. Now unconditionally calls client:stop().

Why: vim.lsp.stop_client() is deprecated in Neovim 0.12. The config already requires 0.12+ (vim.pack, vim.lsp.config, vim.lsp.enable are all 0.12 APIs), so the else branch was unreachable dead code pointing at a deprecated API. The previous audit/lsp-stop-client branch had added the check for backward compat; since that compat window is closed, this removes the dead path.


2. Fix PackChanged autocmd — use User event with pattern (closes #117)

Where: lua/config/plugin_config.lua — fff.nvim PackChanged handler

Change: nvim_create_autocmd("PackChanged", ...) → nvim_create_autocmd("User", { pattern = "PackChanged", ... }).

Why: vim.pack fires lifecycle notifications as User events, not top-level Neovim events. The old registration created an autocmd that never fired, so fff.download.download_or_build_binary() was never called after :SyncPkgs. The fff.nvim native binary was never built, causing find_files() / live_grep() to fail silently at runtime.

Test plan

  • :LspToggle <name> on attached client → stops; run again → re-enables
  • :SyncPkgs → fff.nvim binary built/downloaded; :lua require("fff").find_files() works

Generated by Claude Code

vim.lsp.stop_client() is deprecated in Neovim 0.12. The config already
requires 0.12+ (uses vim.pack), making the else-branch unreachable.
Remove the version check and always call client:stop().

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d1c2e8e975

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +170 to +171
vim.api.nvim_create_autocmd("User", {
pattern = "PackChanged",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Use the PackChanged event directly

On Neovim 0.12, vim.pack emits PackChanged as a dedicated autocmd event, and the upstream examples register it with nvim_create_autocmd('PackChanged', ...); it is not a User event with pattern = 'PackChanged'. With this registration the fff build/download hook still will not run after :SyncPkgs, so installs/updates of fff.nvim can leave the native binary missing.

Useful? React with 👍 / 👎.

@stanfish06 stanfish06 closed this Jun 9, 2026
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.

[audit] PackChanged autocmd in fff.nvim setup is a top-level event — should be User PackChanged — binary never downloaded on install

1 participant