nvim revamp - #360
Conversation
…nsions list - Included 'github.vscode-github-actions' in the proprietary extensions to enhance the sync script's functionality and prevent sync errors.
- Updated bashrcExtra and initContent to include additional binary paths for improved accessibility. - Consolidated Go and Bun configurations while ensuring compatibility with other tools like Foundry and Nix.
…nfiguration - Updated nvim-pack-lock.json to ensure proper formatting. - Enhanced flake.lock with new inputs and locked versions for neovim-nightly-overlay and related dependencies. - Added neovim-nightly-overlay to flake.nix inputs for better package management. - Configured home-manager to utilize neovim-nightly-overlay overlays. - Updated neovim default.nix to specify the neovim package from the new overlay. - Modified darwin default.nix to include neovim-nightly-overlay in the package imports and overlays.
…d remove obsolete nvim commands from help
- Updated the Neovim configuration to replace the gruvbox color scheme with dracula. - Adjusted related settings in the lualine plugin to reflect the new theme.
- Updated the Makefile to rename nvim-update and nvim-sync to neovim-update and neovim-sync, respectively, for improved clarity and consistency in naming.
…status line settings - Removed outdated comments related to previous color schemes. - Ensured status line highlight settings are clear and concise.
…sure lua attribute compatibility for neovim-unwrapped
…s and improve modularity
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 26 out of 29 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| vim.api.nvim_buf_set_option(buf, "filetype", "help") | ||
| vim.api.nvim_buf_set_option(buf, "modifiable", false) |
There was a problem hiding this comment.
The function vim.api.nvim_buf_set_option is deprecated in Neovim 0.10+. Use vim.bo[buf].filetype = "help" and vim.bo[buf].modifiable = false instead, or use vim.api.nvim_set_option_value("filetype", "help", {buf = buf}) for consistency with the pattern used elsewhere in the file (lines 426, 434).
| vim.api.nvim_buf_set_option(buf, "filetype", "help") | |
| vim.api.nvim_buf_set_option(buf, "modifiable", false) | |
| vim.api.nvim_set_option_value("filetype", "help", {buf = buf}) | |
| vim.api.nvim_set_option_value("modifiable", false, {buf = buf}) |
…d for improved environment setup
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
nix-darwin/config/system.nix (1)
64-73: Update the message to reflect both skip conditionsThe else-branch message only mentions "runner", but the condition also skips when
NIX_OFFLINE="1". On non-runner systems in offline mode, the message is inaccurate. Update it to reflect both reasons:- echo "Skipping activation scripts for runner"; + echo "Skipping activation scripts (runner mode or NIX_OFFLINE=1)";
♻️ Duplicate comments (9)
home-manager/programs/bash/default.nix (1)
38-45: Consolidate PATH mutations to avoid growth and clarify precedenceEach
export PATH=...here prepends/appends to the existing PATH, so if this snippet is ever sourced more than once, PATH will accumulate duplicates; it also mixes append ($PATH:$GOPATH/bin) with prepend ($HOME/...:$PATH), making ordering harder to reason about. Consider a single, explicit assignment that defines the intended priority:- export PATH="$PATH:$GOPATH/bin" - export PATH="$HOME/.bun/bin:$PATH" - export PATH="$HOME/.foundry/bin:$PATH" - export PATH="$HOME/.local/bin:$PATH" - export PATH="$HOME/.nix-profile/bin:$PATH" - export PATH="/nix/var/nix/profiles/default/bin:$PATH" - export PATH="/opt/homebrew/bin:$PATH" + export PATH="$HOME/.bun/bin:$HOME/.foundry/bin:$HOME/.local/bin:$HOME/.nix-profile/bin:/nix/var/nix/profiles/default/bin:/opt/homebrew/bin:$PATH:$GOPATH/bin"This keeps ordering predictable and avoids repeated growth.
config/nvim/init.lua (5)
70-70: Remove unused plugins or add configuration.The plugins
nvim-colorizer.lua(line 70) andconform.nvim(line 82) are installed but never configured. Consider removing them or adding their setup calls.Also applies to: 82-82
28-28: Missing path separator in undodir configuration.The path is missing a "/" between the data directory and "undodir" subdirectory.
Apply this diff:
-vim.opt.undodir = vim.fn.stdpath("data") .. "undodir" +vim.opt.undodir = vim.fn.stdpath("data") .. "/undodir"
350-350: Fix typo in gitsigns command.The command should be
:Gitsigns preview_hunk_inline<cr>(with an 's' at the end), not:Gitsign.Apply this diff:
-keymap("n", "<leader>gd", ":Gitsign preview_hunk_inline<cr>", opts) +keymap("n", "<leader>gd", ":Gitsigns preview_hunk_inline<cr>", opts)
912-913: Remove duplicate treesitter parser entry.The "http" parser appears twice in the
ensure_installedlist.Apply this diff to remove the duplicate on line 913:
"html", "http", - "http", "ini",
1319-1320: Replace deprecated buffer option API.The
vim.api.nvim_buf_set_option()function is deprecated in Neovim 0.10+.Apply this diff:
- vim.api.nvim_buf_set_option(buf, "filetype", "help") - vim.api.nvim_buf_set_option(buf, "modifiable", false) + vim.bo[buf].filetype = "help" + vim.bo[buf].modifiable = falseMakefile (2)
364-365: Remove empty section header.The "##@ Offline Mode" section header at lines 364-365 is empty with no targets beneath it. The offline mode targets are already grouped under "##@ Nix Offline Mode" starting at line 326.
460-462: Handle existing directory in neovim-dev cleanup.The check only tests for symlinks (
-L) but doesn't handle the case where~/.config/nvimexists as a regular directory.Apply this diff:
- @if [ -L "$(HOME)/.config/nvim" ]; then \ - rm "$(HOME)/.config/nvim"; \ - fi + @if [ -L "$(HOME)/.config/nvim" ] || [ -d "$(HOME)/.config/nvim" ]; then \ + rm -rf "$(HOME)/.config/nvim"; \ + ficonfig/nvim/nvim-pack-lock.json (1)
52-55: Remove unused plugins from lock file.The plugins
gruvbox.nvim(lines 52-55) andnvim-lspconfig(lines 76-79) are present in the lock file but not declared ininit.luaor used in the configuration.Also applies to: 76-79
🧹 Nitpick comments (4)
home-manager/programs/bash/default.nix (1)
35-37: Avoid duplicating GOPATH definition in bothsessionVariablesandbashrcExtraYou already define
GOPATH = "$HOME/go";insessionVariables, and then re‑export it here. That duplication can drift over time (one gets changed, the other doesn’t). Consider relying on the Home ManagersessionVariablesexport only and dropping the explicitexport GOPATHhere.nix-darwin/config/homebrew.nix (2)
4-8: Confirm intent to fully disable Homebrew whenNIX_OFFLINE=1(and consider factoring the check)Tying
homebrew.enableto!isRunner && (builtins.getEnv "NIX_OFFLINE" != "1")means any host withNIX_OFFLINE=1will have Homebrew disabled at the module level, not just skip updates. That’s a stronger behavior change than only gatingautoUpdate/upgrade, but it’s consistent with the idea of “no networked package management when offline.”If that’s intended, the logic is sound. To reduce duplication and make the policy clearer, you could factor the env check:
{ isRunner }: let offline = builtins.getEnv "NIX_OFFLINE" == "1"; in { homebrew = { enable = !isRunner && !offline; onActivation = { autoUpdate = !offline; upgrade = !offline; cleanup = "zap"; extraFlags = [ "--force" ]; }; # ... }; }
81-81: Addingvscodiumalongsidevisual-studio-code— verify you want both installedYou now install both the official VS Code cask and VSCodium. That’s fine if you actively use both, but it will pull in and manage two separate editor apps (and their updates). If you only want one, consider removing the other entry.
.gitignore (1)
4-6: Correct ignore for Home Manager backups; consider grouping Nix rulesIgnoring
*.hm-backupis useful and safe. For readability, you might optionally fold this under the existing# Nixblock further down so all Nix-related patterns live in one place.
📜 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 ignored due to path filters (2)
bun.lockis excluded by!**/*.lockflake.lockis excluded by!**/*.lock
📒 Files selected for processing (27)
.github/workflows/lua.yml(1 hunks).gitignore(1 hunks)Makefile(13 hunks)config/default.nix(1 hunks)config/nvim/.gitignore(1 hunks)config/nvim/default.nix(1 hunks)config/nvim/init.lua(1 hunks)config/nvim/nvim-pack-lock.json(1 hunks)flake.nix(2 hunks)home-manager/default.nix(0 hunks)home-manager/modules/codex/default.nix(1 hunks)home-manager/modules/opencode/opencode.jsonc(1 hunks)home-manager/packages/default.nix(1 hunks)home-manager/programs/bash/default.nix(1 hunks)home-manager/programs/neovim/config/init.lua(0 hunks)home-manager/programs/neovim/default.nix(1 hunks)home-manager/programs/zsh/default.nix(1 hunks)home-manager/services/code-syncer/sync.sh(1 hunks)hosts/darwin/default.nix(3 hunks)hosts/linux/default.nix(1 hunks)hosts/nixos/default.nix(2 hunks)lib/nixpkgs-config.nix(1 hunks)nix-darwin/config/homebrew.nix(2 hunks)nix-darwin/config/nix.nix(0 hunks)nix-darwin/config/system.nix(1 hunks)overlays/default.nix(1 hunks)package.json(1 hunks)
💤 Files with no reviewable changes (3)
- nix-darwin/config/nix.nix
- home-manager/programs/neovim/config/init.lua
- home-manager/default.nix
🧰 Additional context used
🪛 checkmake (0.2.2)
Makefile
[warning] 413-413: Target body for "rekey" exceeds allowed length of 5 (7).
(maxbodylength)
[warning] 546-546: Missing required phony target "all"
(minphony)
[warning] 546-546: Missing required phony target "clean"
(minphony)
[warning] 546-546: Missing required phony target "test"
(minphony)
⏰ 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). (11)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: Mesa Review
- GitHub Check: Mesa Description
- GitHub Check: e2e-run (Ubuntu, ubuntu-latest)
- GitHub Check: e2e-run (NixOS, ubuntu-latest)
- GitHub Check: e2e-run (MacOS, macos-latest)
- GitHub Check: nix-nixos
- GitHub Check: nix-linux
- GitHub Check: nix-darwin
- GitHub Check: docker-build-push (linux/arm64, arm64, ubuntu-24.04-arm)
- GitHub Check: docker-build-push (linux/amd64, amd64, ubuntu-latest)
🔇 Additional comments (21)
home-manager/modules/opencode/opencode.jsonc (1)
27-32: Clean configuration addition with consistent structure.The two new models (
openai/gpt-oss-20bandopenai/gpt-oss-120b) are properly added to the lmstudio provider with correct JSON structure and naming conventions matching existing entries. The changes are isolated and non-breaking.Please verify that both model identifiers (
openai/gpt-oss-20bandopenai/gpt-oss-120b) are actually available and configurable within LM Studio before merging. You may check the LM Studio documentation or test locally.config/default.nix (1)
1-8: Wire-up of./nvimmodule looks goodAdding
./nviminto this config list cleanly integrates the new Neovim module; ordering with other modules is consistent and there are no structural issues.config/nvim/.gitignore (1)
1-1: Scoped ignore for Neovim init backups is appropriateIgnoring
init.lua.backup.*keeps editor/backup artifacts out of version control while preserving the main config and lock file.home-manager/services/code-syncer/sync.sh (1)
19-36: Consistent handling of GitHub Actions VS Code extensionAdding
"github.vscode-github-actions"toPROPRIETARY_EXTENSIONSaligns with how other GitHub/MS extensions are treated and ensures it’s filtered from sync/uninstall logic as intended.package.json (1)
21-29: Trusting Biome alongside other CLI tools makes senseAdding
"@biomejs/biome"totrustedDependenciesis consistent with the other tooling entries here and avoids trust warnings without affecting runtime deps.home-manager/modules/codex/default.nix (1)
3-6: Confirm that forcibly overwriting.codex/config.tomlis intendedSetting
force = truehere means Home Manager will overwrite any existing~/.codex/config.tomlon each activation, even if it was created or modified by Codex or manual edits. That’s fine if this file is meant to be 100% HM-managed, but could be surprising otherwise—worth double‑checking that no tooling expects to write back to this path.home-manager/programs/zsh/default.nix (1)
20-41: PATH ordering for additional bin directories looks reasonableAppending
$GOPATH/binthen prepending user, Nix, and Homebrew bins results in those toolchains taking precedence over the existing PATH, which matches common macOS + Nix setups. No functional concerns from these additions.lib/nixpkgs-config.nix (1)
1-11: I'll verify the nixpkgs configuration semantics to determine whether the review comment's concern is valid.Let me search for more explicit documentation on how these options interact.
Let me search for more specific information about how nixpkgs evaluates these options.
Let me search for one more clarification on whether setting
allowUnfree = truebypasses the predicate.Based on my verification through nixpkgs documentation and real-world configuration examples, the review comment's core claim is not substantiated by evidence.
The documentation frames
allowUnfree = trueandallowUnfreePredicateas alternative approaches, and real-world configurations set both together without documentation indicating that one defeats the other. Additionally, the precedence pattern for similar config options (likeallowInsecurePredicate) is that the predicate is checked if specified, and alternatives are checked only if the predicate is not specified—suggesting they work together rather than one overriding the other.While the suggestion to use only
allowUnfreePredicatefor a strict allowlist is valid advice (more explicit and restrictive intent), the specific claim thatallowUnfree = true"may effectively enable all unfree packages and make the predicate moot" lacks support. No authoritative nixpkgs documentation states this override behavior occurs when both are set.Likely an incorrect or invalid review comment.
home-manager/programs/neovim/default.nix (1)
1-10: LGTM! Clean simplification.The module has been appropriately simplified by removing unused parameters and delegating Neovim configuration to dedicated modules. The explicit
packagefield provides clear package sourcing.Makefile (1)
491-543: LGTM! Well-structured Lua validation targets.The new Lua validation targets are well-designed with proper error handling, availability checks, and clear messaging. The separation of Neovim and Hammerspoon checks provides good granularity.
hosts/nixos/default.nix (1)
10-17: LGTM! Centralized nixpkgs configuration.The addition of centralized
nixpkgsConfigandoverlaysprovides consistent package resolution across the system. TheuseGlobalPkgsflag ensures Home Manager uses the same package set.Also applies to: 123-123
hosts/linux/default.nix (1)
4-17: LGTM! Consistent nixpkgs configuration.The addition of
overlays,nixpkgsConfig, and centralizedpkgsbinding aligns with the PR's objective to centralize nixpkgs management across host configurations.hosts/darwin/default.nix (1)
14-21: LGTM! Unified package management.The migration to centralized
pkgswith overlays and config, along withuseGlobalPkgs, ensures consistent package resolution across the Darwin system and Home Manager.Also applies to: 40-40, 43-43, 57-58
flake.nix (2)
32-35: LGTM! Input follows established pattern.The neovim-nightly-overlay input is configured correctly with nixpkgs.follows to avoid dependency duplication.
93-93: Refactor verified and working correctly.The hosts/linux/default.nix (and related host configs) properly accept the system attribute and use it to resolve pkgs and lib internally. The pattern is consistent and idiomatic—each homeConfiguration passes the appropriate system value, which the host config destructures and uses to import nixpkgs with the correct architecture.
config/nvim/default.nix (2)
13-17: Good approach for writable lock file.Copying the lock file via activation script rather than symlinking allows Neovim to modify it, which is appropriate for a plugin lock file that may be updated during plugin management operations.
The activation ordering (
entryAfter ["writeBoundary"]) and permissions (644) are correct.
6-9: No actionable issues identified; workflow choice confirmed.The file
init.luaexists inconfig/nvim/, confirming the symlink target is available. The review comment accurately describes the trade-offs ofmkOutOfStoreSymlink(editable during development vs. loss of portability). The choice is intentional and the approach is sound for an active development workflow. No code changes required.overlays/default.nix (1)
5-16: I need to gather more specific information about neovim-nightly-overlay and how home-manager's neovim wrapper actually uses the lua attribute. Let me search for additional details.Based on the evidence, I can now provide a comprehensive answer. The web search revealed critical information about how home-manager expects to access the lua attribute.
The dual-exposure pattern is correct and necessary for home-manager compatibility.
Home-manager's neovim module accesses the lua attribute via
cfg.finalPackage.unwrapped.lua.pkgs, which means the overlay must exposeluaat the top level ofneovim-unwrapped. Nixpkgs' own neovim wrapper exposes lua the same way (lua = neovim-unwrapped.lua), confirming this is the expected pattern.The
// { lua = prev.lua5_4; }merge at lines 13-15 is necessary. However, thepassthru.luaat lines 9-10 may be redundant—you can verify by checking ifprev.neovim-unwrappedalready includes a lua attribute in its passthru from the upstream nixpkgs neovim package. If nixpkgs already provides it, only the top-level merge is needed.To simplify: consider removing the passthru assignment if the base package already exposes lua that way, keeping only the final
// { lua = ... }merge.home-manager/packages/default.nix (3)
90-96: Good practice: excluding heavy GUI packages from CI.The conditional logic properly excludes GUI applications and heavy multimedia tools from CI environments, which improves build performance and avoids unnecessary dependencies in automated environments.
11-11: No issues found—NUR overlay is properly configured.The refactor is valid and correct. The NUR overlay is included in
overlays/default.nixand properly applied topkgs, which is then passed to thehome-manager/packages/default.nixmodule (line 11:packages = import ./packages { inherit pkgs inputs; }). The simplified referencepkgs.nur.repos.charmbracelet.crushworks as intended.
77-89: No action required — all packages are available in nixpkgs-unstable.All four packages mentioned in the Linux-specific block (claude-code, codex, gemini-cli, opencode) are confirmed to be available in nixpkgs-unstable, which your flake.nix pins to. claude-code is available on nixpkgs-unstable, codex is available in nixpkgs-unstable, gemini-cli is packaged in nixpkgs, and opencode is packaged in nixpkgs. The code at lines 77–89 will resolve correctly and will not cause build failures.
Likely an incorrect or invalid review comment.
| jobs: | ||
| lua-neovim: | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 300 |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Reduce excessive timeout for validation jobs.
The 300-minute (5-hour) timeout is excessive for Lua configuration validation. Syntax checks and basic validation should complete within minutes. Consider reducing to 10-15 minutes to provide faster feedback and avoid resource waste.
Apply this diff:
- timeout-minutes: 300
+ timeout-minutes: 15Also apply the same change to line 33.
Also applies to: 33-33
🤖 Prompt for AI Agents
.github/workflows/lua.yml lines 15 and 33: the workflow timeout is set to 300
minutes which is excessive for Lua validation; change both occurrences to a
shorter timeout such as 10 or 15 minutes (e.g., set timeout-minutes to 10 or 15)
to speed feedback and reduce resource usage.
![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 fc54c6a...2472732
Analysis
-
Dual Configuration State: Both the old Nix-managed Neovim configuration and new vim.pack-based Lua configuration exist simultaneously. The old configuration is stripped but still imported, creating potential confusion with no clear migration path or documentation.
-
Reproducibility Compromises: Moving from Nix's hermetic builds to vim.pack shifts reproducibility boundaries. The treesitter configuration with
auto_install = trueenables runtime-dependent installations, breaking Nix's reproducibility guarantees. The lock file lacks cryptographic integrity checks. -
Incomplete Migration: Despite 1330 lines of new configuration, there's no LSP setup despite including nvim-lspconfig in the lock file. The migration appears unfinished without clear documentation about the transition plan.
-
Unexplained Overlay Usage: The PR adds neovim-nightly-overlay but still uses
pkgs.neovim(not neovim-nightly). The overlay includes a lua passthru fix without documentation explaining its necessity. -
PATH Management Redundancy: Shell configurations add multiple Nix-related PATH entries that should already be set by Nix daemon scripts, suggesting potential integration issues or unnecessary defensive programming.
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
29 files reviewed | 3 comments | Edit Agent Settings • Read Docs
| - name: Install Neovim | ||
| run: nix profile install nixpkgs#neovim | ||
| - name: Validate Neovim Configuration | ||
| run: make lua-check-neovim |
There was a problem hiding this comment.
The make lua-check-neovim target symlinks the config and runs headless validation, but doesn't actually test plugin installation via vim.pack. The plugins defined with vim.pack.add() only queue installation - actual git clones don't happen in a headless validation run. Consider enhancing this CI step to explicitly trigger plugin installation with nvim --headless +"lua vim.pack.update()" +qa and verify that expected plugin directories exist in ~/.local/share/nvim/pack/ to catch installation issues early.
Agent: 🏛 Architecture
| scope_incremental = "<noop>", | ||
| }, | ||
| }, | ||
| auto_install = true, |
There was a problem hiding this comment.
Setting auto_install = true breaks Nix's reproducibility guarantees. Treesitter parsers will be installed at runtime when encountering unknown filetypes, which is network-dependent and non-deterministic. Consider setting this to false and pre-installing required parsers via the Makefile or a Nix derivation to maintain reproducible builds across environments.
Agent: 🏛 Architecture
| }, | ||
| }, | ||
| sources = { | ||
| default = { "lsp", "path", "snippets", "buffer", "copilot" }, |
There was a problem hiding this comment.
The blink.cmp configuration includes "lsp" as a completion source, but there's no LSP configuration in this 1330-line file. The nvim-pack-lock.json includes nvim-lspconfig, but no LSP servers are configured with require('lspconfig').*.setup(). This suggests the migration is incomplete. Where is LSP configured? Is it: (1) still managed by Nix somehow, (2) in a separate file not shown in this PR, or (3) not set up yet? The absence of LSP setup in a comprehensive Neovim config is architecturally significant and needs clarification.
Agent: 🏛 Architecture
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
6 issues found across 29 files
Prompt for AI agents (all 6 issues)
Understand the root cause of the following 6 issues and fix them.
<file name="config/nvim/init.lua">
<violation number="1" location="config/nvim/init.lua:28">
The undo directory path is missing a path separator, so persistent undo tries to write to a non-existent `…/nvimundodir` directory and fails. Use `… .. "/undodir"` (and ensure it exists) so undo files can be created.</violation>
<violation number="2" location="config/nvim/init.lua:46">
'winborder' is not a valid Neovim option, so this assignment throws “Unknown option: winborder” during startup and prevents the rest of init.lua from loading.</violation>
<violation number="3" location="config/nvim/init.lua:350">
The `<leader>gd` mapping calls `:Gitsign …`, but the plugin’s command is `:Gitsigns`. As written the mapping always errors with “Not an editor command” and never previews hunks.</violation>
<violation number="4" location="config/nvim/init.lua:719">
The BufWinEnter callback checks `vim.o.filetype`, but `filetype` is buffer-local; the global value stays empty, so help buffers are never detected and their window positioning logic never runs. Use `vim.bo.filetype` (or the autocmd’s buffer arg) instead.</violation>
</file>
<file name="Makefile">
<violation number="1" location="Makefile:351">
`nix-switch-offline` runs the Home Manager activation under sudo, which writes dotfiles as root and breaks permissions.</violation>
<violation number="2" location="Makefile:509">
`lua-check-neovim` overwrites the user's ~/.config/nvim files via forced symlinks, causing data loss just to run a syntax check.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| vim.api.nvim_create_autocmd("BufWinEnter", { | ||
| pattern = { "*.txt" }, | ||
| callback = function() | ||
| if vim.o.filetype == "help" then |
There was a problem hiding this comment.
The BufWinEnter callback checks vim.o.filetype, but filetype is buffer-local; the global value stays empty, so help buffers are never detected and their window positioning logic never runs. Use vim.bo.filetype (or the autocmd’s buffer arg) instead.
Prompt for AI agents
Address the following comment on config/nvim/init.lua at line 719:
<comment>The BufWinEnter callback checks `vim.o.filetype`, but `filetype` is buffer-local; the global value stays empty, so help buffers are never detected and their window positioning logic never runs. Use `vim.bo.filetype` (or the autocmd’s buffer arg) instead.</comment>
<file context>
@@ -0,0 +1,1330 @@
+vim.api.nvim_create_autocmd("BufWinEnter", {
+ pattern = { "*.txt" },
+ callback = function()
+ if vim.o.filetype == "help" then
+ vim.cmd.wincmd("L")
+ end
</file context>
| if vim.o.filetype == "help" then | |
| if vim.bo.filetype == "help" then |
| -- @keymap <F9>: Open Git mergetool in new tab | ||
| keymap("n", "<F9>", ":tab Git mergetool<cr>", opts) | ||
| -- @keymap <leader>gd: Preview hunk inline | ||
| keymap("n", "<leader>gd", ":Gitsign preview_hunk_inline<cr>", opts) |
There was a problem hiding this comment.
The <leader>gd mapping calls :Gitsign …, but the plugin’s command is :Gitsigns. As written the mapping always errors with “Not an editor command” and never previews hunks.
Prompt for AI agents
Address the following comment on config/nvim/init.lua at line 350:
<comment>The `<leader>gd` mapping calls `:Gitsign …`, but the plugin’s command is `:Gitsigns`. As written the mapping always errors with “Not an editor command” and never previews hunks.</comment>
<file context>
@@ -0,0 +1,1330 @@
+-- @keymap <F9>: Open Git mergetool in new tab
+keymap("n", "<F9>", ":tab Git mergetool<cr>", opts)
+-- @keymap <leader>gd: Preview hunk inline
+keymap("n", "<leader>gd", ":Gitsign preview_hunk_inline<cr>", opts)
+
+-- ====================================================================================
</file context>
| keymap("n", "<leader>gd", ":Gitsign preview_hunk_inline<cr>", opts) | |
| keymap("n", "<leader>gd", ":Gitsigns preview_hunk_inline<cr>", opts) |
| vim.opt.termguicolors = true | ||
| vim.opt.shortmess:append("c") | ||
| vim.opt.timeoutlen = 300 | ||
| vim.opt.winborder = "none" |
There was a problem hiding this comment.
'winborder' is not a valid Neovim option, so this assignment throws “Unknown option: winborder” during startup and prevents the rest of init.lua from loading.
Prompt for AI agents
Address the following comment on config/nvim/init.lua at line 46:
<comment>'winborder' is not a valid Neovim option, so this assignment throws “Unknown option: winborder” during startup and prevents the rest of init.lua from loading.</comment>
<file context>
@@ -0,0 +1,1330 @@
+vim.opt.termguicolors = true
+vim.opt.shortmess:append("c")
+vim.opt.timeoutlen = 300
+vim.opt.winborder = "none"
+
+vim.hl.priorities.semantic_tokens = 10
</file context>
| vim.opt.swapfile = false | ||
| vim.opt.backup = true | ||
| vim.opt.undofile = true | ||
| vim.opt.undodir = vim.fn.stdpath("data") .. "undodir" |
There was a problem hiding this comment.
The undo directory path is missing a path separator, so persistent undo tries to write to a non-existent …/nvimundodir directory and fails. Use … .. "/undodir" (and ensure it exists) so undo files can be created.
Prompt for AI agents
Address the following comment on config/nvim/init.lua at line 28:
<comment>The undo directory path is missing a path separator, so persistent undo tries to write to a non-existent `…/nvimundodir` directory and fails. Use `… .. "/undodir"` (and ensure it exists) so undo files can be created.</comment>
<file context>
@@ -0,0 +1,1330 @@
+vim.opt.swapfile = false
+vim.opt.backup = true
+vim.opt.undofile = true
+vim.opt.undodir = vim.fn.stdpath("data") .. "undodir"
+vim.opt.hlsearch = false
+vim.opt.ignorecase = true
</file context>
| elif [ "$(NIX_CONFIG_TYPE)" = "nixosConfigurations" ]; then \ | ||
| NIX_OFFLINE=1 sudo $(NIX_ALLOW_UNFREE) $(NIX_EXEC) run $(NIX_FLAGS) --impure nixpkgs#nixos-rebuild -- switch --flake .#runner --no-update-lock-file --offline || exit 0; \ | ||
| elif [ "$(NIX_CONFIG_TYPE)" = "homeConfigurations" ]; then \ | ||
| NIX_OFFLINE=1 USER=$(NIX_USERNAME) sudo $(NIX_ALLOW_UNFREE) $(NIX_EXEC) run $(NIX_FLAGS) --impure .#$(NIX_CONFIG_TYPE)."$(NIX_USERNAME)@$(NIX_SYSTEM)".activationPackage --offline; \ |
There was a problem hiding this comment.
nix-switch-offline runs the Home Manager activation under sudo, which writes dotfiles as root and breaks permissions.
Prompt for AI agents
Address the following comment on Makefile at line 351:
<comment>`nix-switch-offline` runs the Home Manager activation under sudo, which writes dotfiles as root and breaks permissions.</comment>
<file context>
@@ -328,28 +323,68 @@ nix-switch-vm:
+ elif [ "$(NIX_CONFIG_TYPE)" = "nixosConfigurations" ]; then \
+ NIX_OFFLINE=1 sudo $(NIX_ALLOW_UNFREE) $(NIX_EXEC) run $(NIX_FLAGS) --impure nixpkgs#nixos-rebuild -- switch --flake .#runner --no-update-lock-file --offline || exit 0; \
+ elif [ "$(NIX_CONFIG_TYPE)" = "homeConfigurations" ]; then \
+ NIX_OFFLINE=1 USER=$(NIX_USERNAME) sudo $(NIX_ALLOW_UNFREE) $(NIX_EXEC) run $(NIX_FLAGS) --impure .#$(NIX_CONFIG_TYPE)."$(NIX_USERNAME)@$(NIX_SYSTEM)".activationPackage --offline; \
+ else \
+ echo "Unsupported OS $(OS) for offline switch"; \
</file context>
| NIX_OFFLINE=1 USER=$(NIX_USERNAME) sudo $(NIX_ALLOW_UNFREE) $(NIX_EXEC) run $(NIX_FLAGS) --impure .#$(NIX_CONFIG_TYPE)."$(NIX_USERNAME)@$(NIX_SYSTEM)".activationPackage --offline; \ | |
| NIX_OFFLINE=1 USER=$(NIX_USERNAME) $(NIX_ALLOW_UNFREE) $(NIX_EXEC) run $(NIX_FLAGS) --impure .#$(NIX_CONFIG_TYPE)."$(NIX_USERNAME)@$(NIX_SYSTEM)".activationPackage --offline; \ |
| fi | ||
| @echo "📝 Validating Neovim configuration syntax..." | ||
| @mkdir -p ~/.config/nvim | ||
| @ln -sf "$(PWD)/config/nvim/init.lua" ~/.config/nvim/init.lua |
There was a problem hiding this comment.
lua-check-neovim overwrites the user's ~/.config/nvim files via forced symlinks, causing data loss just to run a syntax check.
Prompt for AI agents
Address the following comment on Makefile at line 509:
<comment>`lua-check-neovim` overwrites the user's ~/.config/nvim files via forced symlinks, causing data loss just to run a syntax check.</comment>
<file context>
@@ -407,20 +442,109 @@ shell-install:
+ fi
+ @echo "📝 Validating Neovim configuration syntax..."
+ @mkdir -p ~/.config/nvim
+ @ln -sf "$(PWD)/config/nvim/init.lua" ~/.config/nvim/init.lua
+ @if [ -f "$(PWD)/config/nvim/nvim-pack-lock.json" ]; then \
+ ln -sf "$(PWD)/config/nvim/nvim-pack-lock.json" ~/.config/nvim/nvim-pack-lock.json; \
</file context>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 26 out of 29 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -19,6 +19,7 @@ | |||
| "open-composer": "^0.8.23" | |||
| }, | |||
There was a problem hiding this comment.
Missing @biomejs/biome from dependencies. It's listed in trustedDependencies but not in dependencies or devDependencies. Based on bun.lock showing it as a devDependency, this should be added to the devDependencies section of package.json.
| }, | |
| }, | |
| "devDependencies": { | |
| "@biomejs/biome": "*" | |
| }, |
| vim.opt.swapfile = false | ||
| vim.opt.backup = true | ||
| vim.opt.undofile = true | ||
| vim.opt.undodir = vim.fn.stdpath("data") .. "undodir" |
There was a problem hiding this comment.
Missing path separator in undodir configuration. Should be vim.fn.stdpath("data") .. "/undodir" to create a proper path.
| vim.opt.undodir = vim.fn.stdpath("data") .. "undodir" | |
| vim.opt.undodir = vim.fn.stdpath("data") .. "/undodir" |
| -- @keymap <F9>: Open Git mergetool in new tab | ||
| keymap("n", "<F9>", ":tab Git mergetool<cr>", opts) | ||
| -- @keymap <leader>gd: Preview hunk inline | ||
| keymap("n", "<leader>gd", ":Gitsign preview_hunk_inline<cr>", opts) |
There was a problem hiding this comment.
Invalid command "Gitsign" - the correct command for gitsigns.nvim is "Gitsigns" (with an 's'). Should be :Gitsigns preview_hunk_inline<cr>.
| keymap("n", "<leader>gd", ":Gitsign preview_hunk_inline<cr>", opts) | |
| keymap("n", "<leader>gd", ":Gitsigns preview_hunk_inline<cr>", opts) |
| allowUnfreePredicate = | ||
| pkg: | ||
| builtins.elem (nixpkgsLib.getName pkg) [ | ||
| "claude-code" | ||
| "qwen-code" | ||
| "crush" | ||
| ]; |
There was a problem hiding this comment.
Conflicting configuration: allowUnfree = true on line 3 makes the allowUnfreePredicate on lines 4-10 redundant. When allowUnfree is true, ALL unfree packages are allowed regardless of the predicate. Either remove allowUnfree = true to rely solely on the predicate, or remove the predicate if all unfree packages should be allowed.
| allowUnfreePredicate = | |
| pkg: | |
| builtins.elem (nixpkgsLib.getName pkg) [ | |
| "claude-code" | |
| "qwen-code" | |
| "crush" | |
| ]; |
| vim.api.nvim_buf_set_option(buf, "filetype", "help") | ||
| vim.api.nvim_buf_set_option(buf, "modifiable", false) |
There was a problem hiding this comment.
Deprecated API usage: vim.api.nvim_buf_set_option is deprecated in Neovim 0.10+. Use vim.bo[buf].filetype = "help" and vim.bo[buf].modifiable = false instead.
| vim.api.nvim_buf_set_option(buf, "filetype", "help") | |
| vim.api.nvim_buf_set_option(buf, "modifiable", false) | |
| vim.bo[buf].filetype = "help" | |
| vim.bo[buf].modifiable = false |
Summary by cubic
Revamps Neovim with a new Lua config and a modern plugin stack, plus nightly Neovim via Nix for a faster, smoother editor experience. Adds Make targets, Lua checks, and Nix offline helpers to streamline local dev and plugin management.
Written for commit dd932a1. Summary will update automatically on new commits.