-
Notifications
You must be signed in to change notification settings - Fork 9
Clauding #6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Clauding #6
Conversation
|
Caution Review failedThe pull request is closed. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds GitHub templates, CI/workflow and local CI script, linters/formatters, many plugin removals and additions (lualine), extensive NeoTree config and a keymap change, fixes in LSP/Telescope/DAP/util code, lockfile and README updates, plus small formatting tweaks. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant NV as Neovim
participant NT as neo-tree.command
U->>NV: press <space>n
NV->>NT: require("neo-tree.command").execute({ toggle = true })
NT-->>NV: toggle NeoTree UI
NV-->>U: NeoTree shown/hidden
sequenceDiagram
participant LSP as LSP Server
participant NV as Neovim
participant H as InlayHintProcessor
LSP->>NV: send inlay-hints (iterator)
NV->>H: filter by cursor row & insert mode, then .flatten()/.totable()
H-->>NV: concrete table of hints
NV->>NV: publish/display hints via vim.diagnostic.handlers.on_publish_diagnostics
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR performs a significant refactoring of the Neovim configuration, removing complex plugins and adding CI/CD infrastructure for better code quality and maintainability.
- Removes 15+ plugins (wilder, heirline, neotest, colorizer, etc.) to simplify the configuration
- Replaces Heirline statusline with Lualine for a cleaner, more maintainable UI
- Adds comprehensive CI/CD with GitHub Actions and local CI script
- Introduces code quality tools (Luacheck, StyLua) with configuration files
Reviewed Changes
Copilot reviewed 33 out of 34 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
stylua.toml |
Adds StyLua formatter configuration with tabs-based indentation |
scripts/local-ci.sh |
Adds local CI script for pre-push validation with linting, security, and performance checks |
.luacheckrc |
Adds Luacheck linter configuration with custom globals and ignore patterns |
.github/workflows/ci.yml |
Adds GitHub Actions CI pipeline with lint, test, security, and performance jobs |
.github/pull_request_template.md |
Adds PR template for structured change documentation |
.github/ISSUE_TEMPLATE/*.md |
Adds issue templates for bug reports and feature requests |
readme.md |
Updates documentation to reflect statusline change to Lualine and improved performance metrics |
lua/plugins/lualine.lua |
Adds new Lualine statusline configuration with Kanagawa theme |
lua/plugins/neotree.lua |
Expands NeoTree configuration with Git status symbols and file filtering |
lua/plugins/heirline.lua |
Removes complex Heirline statusline implementation (561 lines) |
lua/plugins/wilder.lua |
Removes wilder.nvim command-line completion plugin |
lua/plugins/neotest.lua |
Removes neotest testing framework and adapters |
lua/plugins/*.lua |
Removes 10+ other plugins (colorizer, marks, origami, sort, etc.) |
lua/util/utils.lua |
Condenses formatoptions configuration to single line |
lua/util/functions.lua |
Removes trailing whitespace from treesitter query |
lua/config/telescope/tools.lua |
Reformats vim.iter chain for better readability |
lua/config/lsp/on_attach/ts_ls.lua |
Fixes logical bug in client count check (not #clients == 1 → #clients ~= 1) |
lua/config/lsp/handlers/init.lua |
Reformats vim.iter chain for consistency |
lua/config/dap/go.lua |
Adds error notification for failed terminal start |
lua/config/autocmd.lua |
Uncomments map_q_to_quit function call |
init.lua |
Updates NeoTree toggle keybinding to use function-based approach |
lazy-lock.json |
Updates plugin lock file to reflect removed and added plugins |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "lsp-lens.nvim": { | ||
| "branch": "main", | ||
| "commit": "48bb1a7e271424c15f3d588d54adc9b7c319d977" |
Copilot
AI
Nov 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The lsp-lens.nvim entry remains in lazy-lock.json but the plugin was removed in lua/plugins/lsp-lens.lua. This creates an inconsistency where the lock file references a plugin that's no longer configured. Consider running :Lazy sync to update the lock file.
| "nvim-treesitter-context": { | ||
| "branch": "master", | ||
| "commit": "660861b1849256398f70450afdf93908d28dc945" | ||
| }, |
Copilot
AI
Nov 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The nvim-treesitter-context entry is added to lazy-lock.json, but the plugin file lua/plugins/treesitter-context.lua was deleted (with enabled = false). This is inconsistent - either the plugin should be removed from the lock file or re-enabled in the configuration.
| "nvim-treesitter-context": { | |
| "branch": "master", | |
| "commit": "660861b1849256398f70450afdf93908d28dc945" | |
| }, |
| # StyLua configuration for Neovim config | ||
|
|
||
| # Use tabs for indentation (consistent with your existing style) | ||
| indent_type = "Tabs" | ||
| indent_width = 1 |
Copilot
AI
Nov 14, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment says "Use tabs for indentation (consistent with your existing style)" but this should be verified. If the existing codebase uses spaces, this configuration would introduce inconsistency. Review the actual indentation style used in the Lua files before committing to tabs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (9)
lua/config/autocmd.lua (1)
84-92: Inconsistent reference style for quit handler.Line 89 calls
u.map_q_to_quit(event)directly, while lines 72 and 80 use the local aliasq_to_quit(event). Since line 3 bindsq_to_quit = u.map_q_to_quit, both forms are functionally identical, but the inconsistency reduces code clarity.For consistency, either use the local binding throughout (lines 72, 80, 89) or switch all to the fully qualified form. The local binding is already declared on line 3 and is the clearer, shorter form.
Apply this diff to use the local binding consistently:
autocmd({ "BufEnter" }, { group = quit, pattern = { "*" }, callback = function(event) if bo.buftype == "" and bo.filetype == "" then - u.map_q_to_quit(event) + q_to_quit(event) end end, })lua/util/functions.lua (1)
306-307: Add nil check for LSP client before calling methods.The code retrieves the "templ" LSP client but doesn't verify it exists before calling
:request. If the client isn't running, this will cause a nil dereference error.Apply this diff to add a nil check:
local client = vim.lsp.get_clients({ name = "templ", bufnr = 0 })[1] +if not client then + vim.print("templ client not found") + return +end client:request("textDocument/hover", {.github/ISSUE_TEMPLATE/bug_report.md (1)
29-36: Use proper markdown headings instead of bold text.Lines 29 and 34 use bold text (
**Additional context**and**Plugin Information**) instead of proper markdown headings, which affects document structure and accessibility.Apply this diff:
-**Additional context** +## Additional context - Error messages from `:messages` - Output of `:checkhealth` - Any custom modifications to the config -**Plugin Information** +## Plugin Information - Run `:Lazy` and share any error messages - Include output of `:Mason` if LSP-related.luacheckrc (2)
32-38: File-specific vim global declarations are redundant.The
vimglobal is already declared in the top-levelglobalstable (line 6), so re-declaring it in file-specific overrides (lines 33 and 37) is unnecessary.Apply this diff to simplify:
-- Per-file type overrides -files["lua/plugins/*.lua"] = { - globals = { "vim" } -} - -files["lua/config/**/*.lua"] = { - globals = { "vim" } -} +-- files["lua/plugins/*.lua"] = {} +-- files["lua/config/**/*.lua"] = {}Or remove these sections entirely if no file-specific overrides are needed.
41-42: Redundant unused variable configuration.Setting
unused_args = falseandunused = falseglobally is redundant since codes 212 (unused argument) and 213 (unused loop variable) are already in the ignore list (lines 18-19). The ignore list already suppresses these warnings.Consider removing lines 41-42 or clarifying the intent with a comment if there's a specific reason to keep both approaches.
.github/workflows/ci.yml (3)
35-36: Redundant --globals vim flag in luacheck command.The
--globals vimflag on line 36 is unnecessary since.luacheckrcalready declaresvimas a global (line 6). This redundancy could lead to confusion if the two configurations diverge.Apply this diff:
- - name: Run luacheck - run: luacheck lua/ --globals vim --codes + - name: Run luacheck + run: luacheck lua/ --codes
81-82: HTTP URL check may be too restrictive.The check on line 82 will fail if any
http://URLs are found, but some URLs legitimately use HTTP (e.g.,http://example.comin documentation, local development URLs likehttp://localhost).Consider allowing exceptions for localhost or making this a warning instead:
# Check for suspicious URLs - ! grep -r "http://" --include="*.lua" lua/ || (echo "Insecure HTTP URLs found" && exit 1) + if grep -r "http://" --include="*.lua" lua/ | grep -v "localhost"; then + echo "⚠️ Insecure HTTP URLs found (excluding localhost)" + exit 1 + fi
104-107: bc command may not be available in all environments.The use of
bc -lfor floating-point comparison on line 105 assumesbcis installed, which may not be guaranteed in all GitHub Actions runners.Use a more portable approach with awk:
# Warn if startup is slow (but don't fail) - if (( $(echo "$startup_time > 200" | bc -l) )); then + if awk "BEGIN {exit !($startup_time > 200)}"; then echo "⚠️ Startup time is slower than expected: ${startup_time}ms" fiscripts/local-ci.sh (1)
35-49: Consider using tool configuration files explicitly.Line 36 runs
luacheckwithout specifying the.luacheckrcconfig file mentioned in your summary. Similarly, line 45 runsstylua --checkwhich should auto-detectstylua.toml, but explicit config references improve clarity and portability.Apply this diff to make config file usage explicit:
- luacheck lua/ --globals vim --codes + luacheck lua/ --globals vim --codes --config .luacheckrcFor stylua, you may want to verify it's picking up
stylua.tomlautomatically, or make it explicit if needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (34)
.github/ISSUE_TEMPLATE/bug_report.md(1 hunks).github/ISSUE_TEMPLATE/feature_request.md(1 hunks).github/pull_request_template.md(1 hunks).github/workflows/ci.yml(1 hunks).luacheckrc(1 hunks)init.lua(1 hunks)lazy-lock.json(3 hunks)lua/config/autocmd.lua(1 hunks)lua/config/dap/go.lua(1 hunks)lua/config/lsp/handlers/init.lua(1 hunks)lua/config/lsp/on_attach/ts_ls.lua(1 hunks)lua/config/telescope/tools.lua(1 hunks)lua/plugins/angler.lua(0 hunks)lua/plugins/bmessages.lua(0 hunks)lua/plugins/colorizer.lua(0 hunks)lua/plugins/dashboard.lua(1 hunks)lua/plugins/do-the-needful.lua(0 hunks)lua/plugins/heirline.lua(0 hunks)lua/plugins/init.lua(0 hunks)lua/plugins/leetcode.lua(0 hunks)lua/plugins/lsp-lens.lua(0 hunks)lua/plugins/lualine.lua(1 hunks)lua/plugins/marks.lua(0 hunks)lua/plugins/neotest.lua(0 hunks)lua/plugins/neotree.lua(1 hunks)lua/plugins/origami.lua(0 hunks)lua/plugins/sort.lua(0 hunks)lua/plugins/treesitter-context.lua(0 hunks)lua/plugins/wilder.lua(0 hunks)lua/util/functions.lua(1 hunks)lua/util/utils.lua(1 hunks)readme.md(2 hunks)scripts/local-ci.sh(1 hunks)stylua.toml(1 hunks)
💤 Files with no reviewable changes (14)
- lua/plugins/treesitter-context.lua
- lua/plugins/init.lua
- lua/plugins/marks.lua
- lua/plugins/leetcode.lua
- lua/plugins/origami.lua
- lua/plugins/wilder.lua
- lua/plugins/heirline.lua
- lua/plugins/neotest.lua
- lua/plugins/do-the-needful.lua
- lua/plugins/lsp-lens.lua
- lua/plugins/bmessages.lua
- lua/plugins/angler.lua
- lua/plugins/sort.lua
- lua/plugins/colorizer.lua
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/ci.yml
110-110: shellcheck reported issue in this script: SC2086:info:5:6: Double quote to prevent globbing and word splitting
(shellcheck)
🪛 LanguageTool
.github/pull_request_template.md
[style] ~7-~7: Consider using a different verb for a more formal wording.
Context: ... [ ] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaki...
(FIX_RESOLVE)
🪛 markdownlint-cli2 (0.18.1)
.github/ISSUE_TEMPLATE/bug_report.md
29-29: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
34-34: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
⏰ 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). (1)
- GitHub Check: Agent
🔇 Additional comments (20)
lua/util/functions.lua (1)
249-249: LGTM! Whitespace cleanup in Treesitter query.The removal of trailing whitespace in the query string is a good formatting improvement with no functional impact.
lua/config/dap/go.lua (1)
31-32: LGTM! Improved error handling for terminal startup failure.The addition of error notification for
jobid == 0properly handles the case when the terminal fails to start, complementing the existing handling of the "not executable" case.lua/config/telescope/tools.lua (1)
30-43: LGTM! Proper conversion to concrete table.The iterator is now materialized into a concrete table using
.flatten().totable(), which ensures the command generator returns the expected format forfinders.new_async_job.lua/plugins/dashboard.lua (1)
33-87: LGTM! Cosmetic icon cleanup.The icon values have been cleared while preserving all functional aspects (descriptions, keys, actions). This aligns with the UI simplification theme of the PR.
lua/config/lsp/on_attach/ts_ls.lua (1)
32-32: Excellent fix! Corrected operator precedence bug.The original condition
if not #clients == 1 thenwas incorrectly parsed asif (not #clients) == 1 thendue to operator precedence, comparing a boolean to the number 1 (always false). The corrected formif #clients ~= 1 thenproperly validates that exactly one ts_ls client is present.lua/util/utils.lua (1)
323-323: LGTM! Clean consolidation of formatoptions.The single-line expression using option arithmetic operators (
-and+) is functionally equivalent to the previous multi-line approach and more concise.lua/config/lsp/handlers/init.lua (1)
29-34: LGTM! Proper materialization of inlay hints.The iterator is converted to a concrete table using
.totable()after filtering, ensuring the downstream handler receives the expected format while preserving the filtering logic for cursor row and insert mode..github/pull_request_template.md (1)
1-39: LGTM! Well-structured PR template.The template provides comprehensive sections covering description, change types, testing verification, and impact assessment. This will help standardize PR submissions.
.github/ISSUE_TEMPLATE/feature_request.md (1)
1-25: LGTM! Comprehensive feature request template.The template includes practical sections for problem description, proposed solutions, alternatives, and implementation details specific to Neovim plugin configuration. This will help capture structured feature requests.
readme.md (2)
251-252: Documentation aligns with implementation changes.The UI section accurately reflects the new Lualine statusline and NeoTree configuration added in this PR.
276-278: Remove or verify the specific startup time figures.The claimed startup time of "~30-50ms" cannot be verified from the codebase. While
lua/config/lazy.luashows performance optimizations (lazy loading enabled by default, 14 unnecessary built-in plugins disabled) andscripts/local-ci.shmeasures startup time with a 100ms threshold, there is no evidence of actual measurements achieving 30-50ms, nor any record of improvement from the previously claimed "~50-80ms."The CI script only verifies that startup time stays under 100ms; it does not capture or report the actual measured value. Either:
- Provide actual benchmark measurements showing 30-50ms startup time, or
- Revise the claim to reference the 100ms performance target from AGENT.md instead of unsupported specific figures.
.luacheckrc (1)
17-22: Ignore list appropriately configured.The ignore patterns cover common intentional patterns in Neovim configurations, with clear comments explaining each code.
lua/plugins/neotree.lua (1)
9-51: Enhanced configuration with appropriate defaults.The migration from static
optsto a runtimeconfigfunction with comprehensive Neo-tree setup is well-structured. The configuration includes sensible defaults for git symbols, window positioning, and filesystem filtering.init.lua (1)
33-35: Improved keybinding with descriptive label.The migration from a literal Ex command to a Lua function call with a descriptive label improves code quality and makes the binding more discoverable in tools like which-key.
stylua.toml (1)
1-21: Well-configured formatting rules.The Stylua configuration is appropriate for a Neovim project. The choice to disable
sort_requires(line 21) is wise since require order can have semantic significance in Lua.lua/plugins/lualine.lua (1)
1-31: Clean and well-structured lualine configuration.The statusline setup is comprehensive and follows lualine best practices. The use of
globalstatus = true(line 11) and the Kanagawa theme (line 8) align well with the modern UI goals mentioned in the PR.scripts/local-ci.sh (2)
100-100: Verify grep portability for macOS/BSD systems.The
grep --include="*.lua"flag (lines 100, 106) is a GNU grep extension and won't work with BSD grep (default on macOS). While errors are redirected to/dev/null, this silently skips the security scan on non-Linux systems.If this script needs to support macOS contributors, consider either:
- Adding a fallback pattern:
find lua/ -name "*.lua" | xargs grep "pattern"- Or explicitly checking for GNU grep and warning the user if unavailable
Please clarify the intended target OS for this script.
Also applies to: 106-106
53-54: Verify grep output detection in config loading test.Line 53 pipes
nvimoutput to grep and relies on exit code. Withset -e, if grep doesn't find the string, the script exits, thenprint_statusis called. This should work, but ensure the Lua print output reliably includes the exact string"Config loaded successfully".lazy-lock.json (2)
244-244: Verify nvim-lspconfig commit is valid.The commit for
nvim-lspconfigwas updated. Ensure this commit doesn't introduce breaking changes or regressions to LSP configuration.Given the PR mentions "fixing logic bugs in LSP code," please verify this update is intentional and compatible with any LSP fixes in the PR.
134-137: Both plugins are properly configured; the review contains incorrect assumptions about file naming.Verification confirms both lock file entries have corresponding plugin configurations:
- lualine.nvim →
lua/plugins/lualine.luaexists ✓- nvim-treesitter-context →
lua/plugins/treesitter.luaexists and explicitly configures it (line 183:"nvim-treesitter/nvim-treesitter-context") ✓The review incorrectly assumes the treesitter-context config would be in a file named
lua/plugins/treesitter-context.lua, but the actual configuration resides inlua/plugins/treesitter.luawhere it belongs. Both plugins are properly configured per PR objectives.Likely an incorrect or invalid review comment.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/ci.yml (1)
97-114: Quote variables in shell conditionals to prevent word-splitting issues.Lines 103 and 112 use unquoted variables in test expressions, which triggers shellcheck SC2086 warnings. Although numeric values from
wc -lare unlikely to cause failures, quoting is a shell best practice and will silence the warnings.Apply this diff:
- name: Plugin count check run: | plugin_count=$(find lua/plugins/ -name "*.lua" 2>/dev/null | wc -l) echo "Plugin configurations: $plugin_count" # Check for reasonable plugin count - if [ $plugin_count -gt 100 ]; then + if [ "$plugin_count" -gt 100 ]; then echo "⚠️ High plugin count: $plugin_count configurations" fi - name: Configuration size check run: | total_lines=$(find lua/ -name "*.lua" -exec wc -l {} + | tail -1 | awk '{print $1}') echo "Total configuration lines: $total_lines" - if [ $total_lines -gt 10000 ]; then + if [ "$total_lines" -gt 10000 ]; then echo "⚠️ Large configuration: $total_lines lines" filua/plugins/neotree.lua (1)
16-28: Unicode symbols require Nerd Fonts.The git status symbols use Unicode/Nerd Font characters that require proper font support. Ensure users have a Nerd Font installed for correct rendering.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/ci.yml(1 hunks).luacheckrc(1 hunks)lua/config/dap/go.lua(1 hunks)lua/plugins/neotree.lua(1 hunks)scripts/local-ci.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- .luacheckrc
- scripts/local-ci.sh
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/ci.yml
98-98: shellcheck reported issue in this script: SC2086:info:5:6: Double quote to prevent globbing and word splitting
(shellcheck)
108-108: shellcheck reported issue in this script: SC2086:info:4:6: Double quote to prevent globbing and word splitting
(shellcheck)
🔇 Additional comments (3)
.github/workflows/ci.yml (1)
78-87: Secret scanning pattern significantly improved.The refined patterns (
password\s*=,secret\s*=,api_key\s*=,auth_token\s*=) are much more specific than the overly broad grep approach in previous versions, effectively addressing false positive concerns. Separating HTTP checks and excluding localhost is appropriate.lua/plugins/neotree.lua (2)
9-51: LGTM! Well-structured neo-tree configuration.The config function properly initializes neo-tree with comprehensive settings. The structure follows lazy.nvim conventions and neo-tree best practices.
39-44: Clarification on the filtered_items configuration.The configuration is correct and not contradictory. The
visible = falseflag controls whether filtered items are shown, not all items. Sincehide_dotfiles = falseandhide_gitignored = false, dotfiles and git-ignored files are not in the filtered set and will remain visible. Onlynode_modules(specified inhide_by_name) will be filtered and hidden.The past review comment appears to misunderstand how
filtered_items.visibleworks—it's a toggle for showing/hiding the filtered set, not a master visibility flag.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.luacheckrc (1)
17-23: Inconsistent unused variable handling.The ignore list globally disables all unused variable warnings (211, 212, 213), but the comment on line 41 suggests only underscore-prefixed unused variables should be ignored. Consider implementing the underscore pattern instead:
Remove codes 211, 212, 213 from the ignore list and add this configuration after line 23:
ignore = { - "211", -- Unused variable (common in snippet utility files) - "212", -- Unused argument - "213", -- Unused loop variable "512", -- Loop is executed at most once (intentional early returns) "631", -- Line is too long (we'll use stylua for formatting) } + +-- Allow unused variables/arguments starting with underscore +unused_args = false +unused_secondaries = falseThen update line 41 to actually implement the pattern:
--- Allow unused variables starting with underscore +-- Allow unused variables starting with underscore +self = false
🧹 Nitpick comments (1)
.luacheckrc (1)
32-39: Remove redundant per-file overrides.The
vimglobal is already defined on line 6, making these per-file overrides redundant. Unless you need to restrictvimelsewhere and re-enable it only for these files, these blocks can be removed.- --- Per-file type overrides -files["lua/plugins/*.lua"] = { - globals = { "vim" } -} - -files["lua/config/**/*.lua"] = { - globals = { "vim" } -}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/ci.yml(1 hunks).luacheckrc(1 hunks)lua/config/lsp/handlers/init.lua(2 hunks)lua/plugins/grapple.lua(1 hunks)lua/util/project.lua(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- lua/util/project.lua
🚧 Files skipped from review as they are similar to previous changes (2)
- .github/workflows/ci.yml
- lua/config/lsp/handlers/init.lua
🔇 Additional comments (3)
.luacheckrc (3)
1-2: LGTM!The
std = "luajit"setting is appropriate for Neovim, which uses LuaJIT.
4-14: LGTM!The global definitions are appropriate for a Neovim configuration with testing support.
25-30: LGTM!The excluded paths are appropriate for a Neovim configuration (generated snippets, package manager, and plugin manager directories).
| local function switch(target_scope, dirx) | ||
| return function() | ||
| vim.cmd(("silent! Grapple use_scope %s"):format(_scope or "git_branch")) | ||
| vim.cmd(("silent! Grapple use_scope %s"):format(target_scope or "git_branch")) | ||
| vim.cmd(("silent! Grapple cycle_tags %s"):format(dirx or "next")) | ||
| vim.g.catgoose_grapple_scope = _scope | ||
| vim.g.catgoose_grapple_scope = target_scope | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix state inconsistency when target_scope is nil.
The global variable vim.g.catgoose_grapple_scope should reflect the actual scope used by Grapple. Currently, line 25 defaults to "git_branch" when target_scope is nil, but line 27 sets the global to the raw target_scope value (which could be nil). This creates an inconsistency.
Apply this diff to ensure the global variable always reflects the actual scope:
local function switch(target_scope, dirx)
return function()
- vim.cmd(("silent! Grapple use_scope %s"):format(target_scope or "git_branch"))
+ local actual_scope = target_scope or "git_branch"
+ vim.cmd(("silent! Grapple use_scope %s"):format(actual_scope))
vim.cmd(("silent! Grapple cycle_tags %s"):format(dirx or "next"))
- vim.g.catgoose_grapple_scope = target_scope
+ vim.g.catgoose_grapple_scope = actual_scope
end
end🤖 Prompt for AI Agents
In lua/plugins/grapple.lua around lines 23 to 28, the function uses "git_branch"
as the fallback when target_scope is nil for the Grapple commands but sets
vim.g.catgoose_grapple_scope to the raw target_scope (which can be nil), causing
state inconsistency; fix by computing a single resolved_scope = target_scope or
"git_branch", use resolved_scope in both vim.cmd calls and set
vim.g.catgoose_grapple_scope = resolved_scope so the global always matches the
actual scope used.
Summary by CodeRabbit
New Features
Performance
Bug Fixes
Chores