-
Notifications
You must be signed in to change notification settings - Fork 4
add lspsaga #40
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
add lspsaga #40
Conversation
📝 WalkthroughWalkthroughThis pull request updates the source management system and plugin configuration in the Nix environment. It revises function signatures in Changes
Sequence Diagram(s)sequenceDiagram
participant U as User Request
participant F as mkGitSource Function
participant R as Repository Resolver
participant G as Git Fetcher
U->>F: Call mkGitSource({repository, revision, url, submodules, hash, branch})
F->>F: Check if url is provided and submodules flag is set
alt Tarball fetch
F->>R: Resolve tarball URL
else Git fetch
F->>R: Determine repository URL (GitHub, GitLab, etc.)
F->>G: Fetch repository with submodules if requested
end
G--)F: Return fetched repository
F--)U: Return source specification
sequenceDiagram
participant E as DeferredUIEnter Event
participant L as lspsaga Setup
participant P as Plugin Manager
E->>L: Trigger lspsaga initialization
L->>P: Register lspsaga key mappings and configurations
P--)L: Confirm registration
L--)E: lspsaga setup complete
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 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: 0
🧹 Nitpick comments (4)
plugins/nobbz/lua/nobbz/lspsaga.lua (1)
5-5: Debug print statement left in code.There is a
vim.printdebug statement in the code that should be removed before merging to production.- vim.print("loading lspsaga")plugins/nobbz/lua/nobbz/lsp/helpers.lua (1)
45-45: Potential performance impact of frequent codelens refresh.
Triggeringrefresh_codelensonTextChanged,InsertLeave,CursorHold, andLspAttachcould introduce overhead in large files or during rapid edits. Consider limiting refreshes to events likeCursorHoldandInsertLeaveor adjusting the interval if performance issues arise.npins/default.nix (2)
27-48: Override logic is straightforward, but consider trace verbosity.
Reading an environment variable to override fetched sources is convenient and mirrors patterns in Niv. Be mindful that extensive usage ofbuiltins.tracemay clutter logs in large builds. Switching to a less verbose diagnostics approach in production might be safer.
69-115: Support forsubmodulesinmkGitSourceis well-structured, but address the SRI TODO.
The flow forurl != null && !submodulesvs. fetchGit with submodules is coherent. However, the# FIXME: check nix version & use SRI hashesremains. If you'd like to adopt SRI, updating the hash usage to thesha256-<base32>format is recommended.I can help finalize the SRI approach and verify any minimal Nix version requirements—would you like me to open a follow-up PR or provide the modifications here?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
npins/default.nix(1 hunks)npins/sources.json(33 hunks)plugins/default.nix(2 hunks)plugins/nobbz/lua/nobbz/init.lua(1 hunks)plugins/nobbz/lua/nobbz/lsp/helpers.lua(3 hunks)plugins/nobbz/lua/nobbz/lspsaga.lua(1 hunks)
🔇 Additional comments (19)
npins/sources.json (3)
11-11: Consistent submodules property added.The addition of
"submodules": falseto all repository entries standardizes the Git fetching behavior, explicitly specifying that submodules should not be included when fetching repositories. This change provides better clarity and control over dependency management.Also applies to: 24-24, 37-37, 52-52, 66-66, 79-79, 92-92, 105-105, 118-118, 131-131, 144-144, 157-157, 183-183, 196-196, 209-209, 222-222, 235-235, 248-248, 261-261, 274-274, 289-289, 303-303, 316-316, 331-331, 345-345, 358-358, 371-371, 384-384, 397-397, 410-410, 423-423, 436-436, 449-449
162-174: New LSP Saga plugin added to sources.The LSP Saga plugin has been added to the sources JSON. This plugin enhances LSP-related UI interactions with functionality like peeking at definitions, navigating diagnostics, and finding references.
455-455: Version number updated from 3 to 5.The version number has been updated to 5, which likely indicates a significant change in the overall structure or schema of the sources file. This aligns with the standardized addition of the
submodulesproperty to all entries.plugins/nobbz/lua/nobbz/lspsaga.lua (2)
1-15: New LSP Saga plugin configuration.The configuration loads the LSP Saga plugin on the "DeferredUIEnter" event, which is a good practice for deferring plugin loading until the UI is ready. The setup enables the symbol display in the window bar and configures the outline to close after jump, enhancing usability.
17-24: Comprehensive key mappings for LSP Saga.A well-organized set of key mappings has been added for LSP Saga functionality. The mappings follow a logical pattern (
<leader>lpfor peek operations,<leader>l<and<leader>l>for navigating diagnostics), making them intuitive for users.plugins/nobbz/lua/nobbz/init.lua (2)
32-32: Added LSP Saga module loading.The LSP Saga module is now loaded after the LSP module, which is the correct sequence since LSP Saga extends LSP functionality. This ensures that LSP is fully initialized before LSP Saga attempts to enhance it.
28-28:Details
✅ Verification successful
Changed order of module loading.
The
blinkmodule (foundations for completions) now loads afteraugment. This reordering may impact the initialization sequence. Ensure that there are no dependencies between these modules that could be affected by this change.
🏁 Script executed:
#!/bin/bash # Check for potential dependencies between augment and blink grep -r "require.*augment" --include="*.lua" . | grep -v "init.lua" grep -r "require.*blink" --include="*.lua" . | grep -v "init.lua"Length of output: 294
Action: Module Loading Order Verified
The dependency check indicates no direct dependencies between the
augmentandblinkmodules. Theblinkmodule’s imports (e.g.,require("blink.cmp")) do not rely onaugment, so the reordering inplugins/nobbz/lua/nobbz/init.luaappears safe.
- No
requirecalls that linkaugmenttoblinkwere found outside of initialization files.- The usage of
blink.cmpin bothblink.luaandlsp.luaconfirms that the dependency remains isolated.plugins/default.nix (2)
9-9: Code formatting improvement.The space has been removed before the closing brace in the
overrideCheckfunction definition, making the code more compact without changing functionality.
18-18: LSP Saga added as an optional plugin.LSP Saga has been added to the
optionalPluginsset with a value oftrue, indicating it should be included. This aligns with the new LSP Saga configuration added in the other files.plugins/nobbz/lua/nobbz/lsp/helpers.lua (2)
4-8: Confirm correctness of new parameter usage inrefresh_codelens.
The addedbufferparameter and the usage ofclient:supports_method("textDocument/codelens", buffer)appear functionally sound. However, this deviates from the typical Neovim LSP client API, which usually checksclient.supports_method("textDocument/codelens")without a second parameter. Ensure that this custom approach is intentional and recognized by any upstream or downstream code.Would you like me to generate a script to search the codebase for other references to
supports_methodto confirm consistency with this new signature?
54-54: Alias addition is clear and improves type clarity.
The newly introduced---@alias Handler nobbz.lsp.helpers.Handlerhelps unify references and improves readability.npins/default.nix (8)
1-9: MIT license header looks valid.
Including the full MIT license text at the start of the file is clear and follows best practices.
15-18: Range function implementation is correct.
Usingbuiltins.genList (n: first + n)aligns with standard Nix patterns, and referencing the official link to Nixpkgs clarifies provenance.
19-26: String utilities align with Nixpkgs references.
The newly addedstringToCharacters,stringAsChars,concatMapStrings, andconcatStringsare standard building blocks for string manipulation. This modular approach improves maintainability.
49-68:mkSourcefunction extension looks appropriate.
Handling new source types like "Tarball" expands usability. Failing fast viabuiltins.throwon unknown types is a good practice.
117-123:mkPyPiSourceadheres to standard fetch usage.
Fetching PyPI packages viabuiltins.fetchurlusing straightforward parameters is consistent with typical Nix derivation patterns.
124-130:mkChannelSourcetarball fetching is standard.
Invokingbuiltins.fetchTarballfor channels remains a reliable approach.
131-141:mkTarballSourcesupports locked URL usage.
Acceptinglocked_urlgracefully decouples the user-specified URL from the verified, locked version, which is beneficial for reproducible builds.
143-147: Version check updated to 5.
Throwing an error on unsupported versions encourages users to update, aligning with the note insources.json.
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
plugins/nobbz/lua/nobbz/lsp/lua.lua(1 hunks)plugins/nobbz/lua/nobbz/lspsaga.lua(1 hunks)plugins/nobbz/lua/nobbz/startuptime.lua(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- plugins/nobbz/lua/nobbz/startuptime.lua
🚧 Files skipped from review as they are similar to previous changes (1)
- plugins/nobbz/lua/nobbz/lspsaga.lua
Summary by CodeRabbit
New Features
lspsagaplugin with new key mappings for improved functionality.Bug Fixes
Refactor