-
Notifications
You must be signed in to change notification settings - Fork 4
augment #34
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
Conversation
📝 WalkthroughWalkthroughThis pull request introduces new environment configurations, plugin specifications, and lazy loading improvements. A new environment variable ( Changes
Sequence Diagram(s)sequenceDiagram
participant Env as .envrc
participant Nvim as nvim.nix
participant Augment as Augment Plugin (augment.lua)
participant Lazy as Lazy Loader (nobbz.lazy)
participant User as Neovim User
Env->>Nvim: Set AUGMENT_WORKSPACE
Nvim->>Nvim: Configure AUGMENT_NODE with nodejs
Augment->>Env: Retrieve environment variables
Augment->>Lazy: Register augment plugin spec and key mappings
Lazy->>Augment: Load and finish lazy configuration
User->>Augment: Trigger plugin commands via key mappings
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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 (4)
plugins/default.nix (1)
16-24: Well-structured approach for optional plugins managementThe introduction of the
optionalPluginsattribute set and themakeOptionalfunction provides a clean way to handle plugin optionality. This is a good architectural improvement that increases flexibility in the plugin system.One small suggestion: consider changing the warning message from "assuming mandatoryness" to "assuming mandatory status" for better clarity and standard English usage.
- optional = optionalPlugins.${name} or (lib.warn "${name} has no explicit optionality, assuming mandatoryness" false); + optional = optionalPlugins.${name} or (lib.warn "${name} has no explicit optionality, assuming mandatory status" false);plugins/nobbz/lua/nobbz/lazy/specs.lua (1)
1-4: Add documentation for better maintainabilityThis module appears to be laying the groundwork for a plugin specification system, but lacks documentation. Consider adding comments explaining the purpose of this module and how it interacts with the broader system.
---@type lz.n.Spec +--- Module for storing lazy-loaded plugin specifications +--- This module is populated through nobbz.lazy.add_specs and used by the lazy loading system local M = {} return Mnvim.nix (1)
115-117: Add a comment explaining the purpose of AUGMENT_NODEThe environment variable setup looks good, but consider adding a brief comment explaining what this variable is used for and why Node.js is needed.
"--set" "NOBBZ_NVIM_PATH" "${placeholder "out"}/bin/nvim" + # Set Node.js path for the augment plugin "--set" "AUGMENT_NODE" "${lib.getExe nodejs}"plugins/nobbz/lua/nobbz/lazy/init.lua (1)
19-19: Remove or improve the debug notification.The
vim.notify(k)call appears to be a debugging statement that should be removed or enhanced with more context for production code.- vim.notify(k) + -- Remove this line if it's just for debugging + -- Or improve it with context if it's intended for users + -- vim.notify("Loading plugin " .. v, vim.log.levels.INFO)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.envrc(1 hunks)npins/sources.json(1 hunks)nvim.nix(2 hunks)plugins/default.nix(1 hunks)plugins/nobbz/lua/nobbz/augment.lua(1 hunks)plugins/nobbz/lua/nobbz/init.lua(3 hunks)plugins/nobbz/lua/nobbz/lazy/init.lua(1 hunks)plugins/nobbz/lua/nobbz/lazy/specs.lua(1 hunks)
🔇 Additional comments (14)
plugins/default.nix (1)
30-30: LGTM - Good integration with the existing plugin pipelineThe integration of
makeOptionalinto the existing plugin processing pipeline is clean and non-disruptive..envrc (1)
5-5: LGTM - Good addition of workspace environment variableThe export of
AUGMENT_WORKSPACEis straightforward and properly sets the current working directory for the augment plugin.nvim.nix (1)
9-9: LGTM - Node.js dependency added correctlyAdding Node.js as a dependency is appropriate for supporting the augment plugin functionality.
plugins/nobbz/lua/nobbz/init.lua (4)
1-1: Module dependency change looks good.The change from using the external
lz.nmodule directly to a custom wrapper modulenobbz.lazyis a good architectural decision that provides more control over the lazy loading interface.
28-28: Well-placed AI module integration.This addition loads the new augment module which is properly documented with a clear comment about its AI functionality.
49-52: API update is consistent with the new module approach.The replacement of
lz_n.load({...})withlazy.add_specs({...})maintains the same functionality while using the new wrapper interface.
53-53: Proper finalization of the lazy loading process.The addition of
lazy.finish()ensures proper completion of the lazy loading setup, which is consistent with the new architecture.plugins/nobbz/lua/nobbz/lazy/init.lua (3)
4-6: Clean implementation of the finish function.The function correctly completes the configuration by loading specifications from the dedicated specs module.
8-11: Effective plugin loading wrapper.The
packaddfunction provides a clean abstraction over the Neovim API for adding plugins.
28-31: Well-implemented specs extension function.The function correctly extends the existing specifications using Vim's list extension utility.
plugins/nobbz/lua/nobbz/augment.lua (3)
1-3: Good environment variable handling.The code properly retrieves environment variables that will be used for conditional plugin loading.
4-14: Well-structured plugin specification with proper conditional loading.The augment plugin specification is well-defined with appropriate conditional enabling based on environment variables. The before function correctly sets up global variables required by the plugin.
16-22: Comprehensive key mappings for the augment plugin.The key mappings are well-organized with a dedicated leader key group and descriptive commands. The usage of WhichKey for mapping definition provides good discoverability for users.
npins/sources.json (1)
39-53: Well-configured augment plugin source.The new entry for "nvim-augment" is properly configured with all necessary fields and pinned to a specific version (v0.26.0) for stability. The configuration aligns well with the augment module implementation.
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: 0
🧹 Nitpick comments (2)
plugins/nobbz/lua/nobbz/lazy/init.lua (2)
13-24: Consider adding error handling for more robustness.The
load_oncefunction might benefit from some validation to handle cases where the plugin name doesn't exist in the state.M.load_once = function(name) local state = require("lz.n.state").plugins + if not name or type(name) ~= "string" then + vim.notify("Invalid plugin name provided", vim.log.levels.ERROR) + return + end require("lz.n").trigger_load(name) for k, v in ipairs(state) do if v == name then table.remove(state, k) break end end + -- Optional: Notify if plugin wasn't found in state + if not found then + vim.notify("Plugin '" .. name .. "' not found in state", vim.log.levels.DEBUG) + end end
3-6: Consider enhancing documentation for better maintainability.While the current documentation is adequate, more detailed descriptions would help future maintainers understand the purpose and behavior of each function.
---finishes the configuration of `lz.n`. +---Completes the configuration by loading all specifications. +---This should be called after all specs have been added. M.finish = function() require("lz.n").load(require("nobbz.lazy.specs")) end
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
nvim.nix(2 hunks)plugins/default.nix(1 hunks)plugins/nobbz/lua/nobbz/lazy/init.lua(1 hunks)plugins/nobbz/lua/nobbz/lazy/specs.lua(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- plugins/nobbz/lua/nobbz/lazy/specs.lua
- plugins/default.nix
- nvim.nix
🔇 Additional comments (5)
plugins/nobbz/lua/nobbz/lazy/init.lua (5)
1-31: Overall structure looks good, with clear module organization.The module provides a clean interface for lazy loading plugins with well-defined functions for configuration, loading, and management.
3-6: LGTM: Finish function implementation is straightforward.The function correctly loads specifications from the required module.
8-11: LGTM: Packadd wrapper is well implemented.Good use of Neovim API to expose the packadd functionality.
13-24: LGTM: Load_once implementation is correct.This function correctly handles triggering the load and updating the plugin state. I see the previous issue with table.remove has been fixed by using the index parameter correctly.
26-29: LGTM: Add_specs function is simple and effective.The implementation correctly extends the specifications list.
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: 0
🧹 Nitpick comments (3)
plugins/nobbz/lua/nobbz/lazy/init.lua (3)
7-9: Consider adding error handling to thefinishfunction.The function loads specifications from an external module but doesn't handle potential errors if the required module is missing or contains syntax errors.
M.finish = function() + local ok, specs = pcall(require, "nobbz.lazy.specs") + if not ok then + vim.notify("Failed to load specifications: " .. tostring(specs), vim.log.levels.ERROR) + return + end - require("lz.n").load(require("nobbz.lazy.specs")) + require("lz.n").load(specs) end
37-40: Guard against adding specs afterfinishis called.The documentation for
finishstates that "there is no way to effectively add more specs" after it has been called, but there's no mechanism to prevent this. Consider adding a state flag to track whetherfinishhas been called.local M = {} +local _finished = false M.finish = function() + _finished = true require("lz.n").load(require("nobbz.lazy.specs")) end ---@param specs lz.n.Spec[] M.add_specs = function(specs) + if _finished then + vim.notify("Cannot add specs after finish() has been called", vim.log.levels.WARN) + return + end vim.list_extend(require("nobbz.lazy.specs"), specs) end
16-35: Ensure proper handling of plugins that don't exist in state.The function correctly handles non-existent plugin names with a warning notification, but consider returning a boolean to indicate success/failure for better programmatic usage.
---@param name string +---@return boolean success Whether the plugin was found and loaded M.load_once = function(name) if not name or type(name) ~= "string" then vim.notify("Invalid plugin name provided", vim.log.levels.ERROR) - return + return false end local state = require("lz.n.state").plugins require("lz.n").trigger_load(name) for k, v in ipairs(state) do if v == name then table.remove(state, k) - return + return true end end -- Only reached if `name` was not in `state`: vim.notify("Plugin " .. name .. " not found in state", vim.log.levels.WARN) + return false end
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
plugins/nobbz/lua/nobbz/lazy/init.lua(1 hunks)
🔇 Additional comments (1)
plugins/nobbz/lua/nobbz/lazy/init.lua (1)
11-14: LGTM: Thepackaddfunction looks good.This is a simple and clean wrapper around the Neovim API command, with proper type annotation.
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: 0
🧹 Nitpick comments (1)
plugins/nobbz/lua/nobbz/lazy/init.lua (1)
1-52: Consider adding unit tests for this module.This module handles critical plugin loading functionality. Adding unit tests would help ensure continued reliability, especially for edge cases like invalid plugin names or handling of the
_finishedstate.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
plugins/nobbz/lua/nobbz/lazy/init.lua(1 hunks)plugins/nobbz/lua/nobbz/lazy/specs.lua(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- plugins/nobbz/lua/nobbz/lazy/specs.lua
🔇 Additional comments (5)
plugins/nobbz/lua/nobbz/lazy/init.lua (5)
1-3: Good attribution practice.Properly acknowledging the source of the borrowed implementation with a specific commit link is excellent practice for open source collaboration.
7-14: Well-documented function with clear constraints.The documentation clearly explains the purpose and limitations of the
finishfunction. The implementation correctly sets a completion flag and loads the specifications.
16-19: Clean implementation of packadd wrapper.The function provides a convenient wrapper around Neovim's command API with appropriate type annotation.
21-40: Fixed implementation of load_once function.The function now correctly uses the index
kwhen removing an item from the state table, addressing the previously reported issue.
42-50: Well-designed spec management with validation.The
add_specsfunction properly validates the module state before attempting to modify specs and provides helpful error messaging.
Summary by CodeRabbit
New Features
AUGMENT_WORKSPACEvariable.Chores
nodejsto enhance Neovim setup.