Skip to content

fix: address bugs found in config review - #172

Merged
stanfish06 merged 2 commits into
masterfrom
fix/repo-review-bugs
Jun 9, 2026
Merged

stanfish06 merged 2 commits into
masterfrom
fix/repo-review-bugs

Conversation

@stanfish06

Copy link
Copy Markdown
Owner

Fixes the bugs identified in a full review of the config. No behavior changes beyond the fixes themselves.

Fixes

  1. Visual-mode format hit the wrong range (plugin_config.lua) — the '</'> marks only update after leaving visual mode, so the visual <leader>lf mapping formatted the previous selection. conform computes the live selection itself (getpos("v")/getpos(".")) when no explicit range is passed, so the two mappings collapse into one { "n", "v" } mapping.
  2. LSP progress timer could spin forever (statusline.lua) — a client that exits with a progress token in flight never sends kind == "end", so lsp_progress stayed non-empty and the 100ms redrawstatus timer never stopped. Dead clients' entries are now pruned in the timer callback.
  3. Splits showed the focused window's statusline everywhere — %! is evaluated in the current window's context (:h stl-%!), so with laststatus=2 every split displayed the focused buffer's file/branch/diagnostics. Now uses a single global statusline (laststatus=3), matching the existing design.
  4. current_file Lua-pattern matching (the in-code BUG comment) — home_path:find(root_dir) treated dir names as patterns: my-project never matched (- is a quantifier) and basenames like config matched .config first. Replaced with a plain string prefix compare against the window's cwd.
  5. % in file/branch names corrupted the statusline — statusline_escape existed but was only applied to LSP progress text; now also applied to file paths and branch names (e.g. a file named 50%.md).
  6. Unknown-mode fallback produced %#nil# — the [?] mode entry was missing hl_alt.
  7. <leader>la leaked into a global mapping — LspAttach re-registered it globally on every attach; now buffer-local like gd.
  8. Headless SyncPkgs never applied updates — vim.pack.update() opens a confirmation buffer that the README's nvim --headless -c 'SyncPkgs' -c 'qa' flow silently discarded. Now passes force = true when no UI is attached (verified update() blocks until checkout completes, so -c qa is safe).
  9. Obsidian deprecation warning on every launch — opted into legacy_commands = false (commands are now :Obsidian <subcommand>).
  10. fff_ok didn't actually test fff — it was pcall(nvim_create_autocmd), which always succeeds. The PackChanged rebuild hook is now registered unconditionally and the keymaps degrade to a warning if fff is missing. Also fixes the "skiped" typo.

Verification

  • nvim --headless +messages +q — clean startup, obsidian warning gone
  • current_file: file under cwd renders nvim/lua/config/options.lua; file outside cwd renders absolute/~ path; /tmp/50%.md renders escaped (50%%.md raw)
  • <leader>lf mapped in both n and v modes; laststatus is 3
  • <leader>la confirmed buffer-local after luals attach
  • Nerd-font glyphs in statusline.lua untouched (diff context verified)

Not included (from the same review, left as follow-ups)

cnoreabbrev vimgrep expanding in :help, :make vs :make! in Compile, DescribeKey register clobbering, per-buffer syntax fallback, server.lua float polish, dead code in image.lua/lib/ui.lua, startup ldd shell-out, vim.pack.add at startup, scopeline virt_text_win_col, and the async lib's sequential execution.

🤖 Generated with Claude Code

- conform: visual-mode format used stale '< '> marks (they only update
  on leaving visual mode), so it formatted the previous selection; let
  conform derive the live range itself
- statusline: prune progress entries of dead LSP clients so the 100ms
  redraw timer can stop (a client that exits mid-progress never sends
  an "end" event, keeping the timer alive forever)
- statusline: set laststatus=3 — %! evaluates in the focused window's
  context, so per-split statuslines all showed the focused buffer
- statusline: replace Lua-pattern cwd matching in current_file with a
  plain prefix compare (dir names with '-' or '.' broke the old match)
- statusline: escape '%' in file paths and git branch names; add the
  missing hl_alt to the unknown-mode fallback
- lsp: make <leader>la buffer-local like gd instead of re-registering
  a global map on every LspAttach
- pack: pass force=true to vim.pack.update() in headless mode, where
  the confirmation buffer would be silently discarded (README flow)
- obsidian: set legacy_commands=false to silence the startup
  deprecation warning
- fff: register the PackChanged rebuild hook unconditionally and guard
  the keymaps against fff being missing; fix "skiped" typo

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@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: ae461de2d1

ℹ️ 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".

end
require("fff.download").download_or_build_binary()
-- rebuild the rust binary whenever vim.pack installs/updates fff.nvim
vim.api.nvim_create_autocmd("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.

P1 Badge Guard PackChanged on older Neovim

When this config runs on a Neovim build without vim.pack support, the old install path in plugins.lua is still intended to work, but plugin_config.lua now unconditionally registers the PackChanged event. On those versions the event is unknown, so nvim_create_autocmd('PackChanged', ...) raises during startup before any keymaps or plugin setup can continue. Keep this autocmd behind the same version/feature guard or restore the protective pcall so the non-vim.pack fallback remains usable.

Useful? React with 👍 / 👎.

@stanfish06
stanfish06 merged commit 64e5fed into master Jun 9, 2026
1 check passed
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.

1 participant