kickstart.nvim & try zed #2
Conversation
Reviewer's Guide by SourceryThis pull request replaces the existing Neovim configuration with the kickstart.nvim setup, and adds configuration for the Zed editor. The changes involve a complete overhaul of the Neovim configuration structure, adopting a more standardized and modular approach provided by kickstart.nvim. Additionally, it introduces basic configuration for the Zed editor. File-Level Changes
Tips
|
|
@sourcery-ai review |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @thaodt - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider reviewing the removed custom plugins and configurations to ensure no critical functionality is lost in the transition to kickstart.nvim.
- The addition of Zed editor configuration alongside Neovim is interesting. Consider documenting the reasons for maintaining multiple editor setups if this is intended for long-term use.
Here's what I looked at during the review
- 🟡 General issues: 3 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟡 Documentation: 2 issues found
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
| -- | ||
| --]] | ||
|
|
||
| local check_version = function() |
There was a problem hiding this comment.
issue: Resolve duplication between health.lua files
There are two nearly identical health.lua files added in different locations. Consider consolidating these into a single file or ensuring they are intentionally different and clearly documented as such.
| dependencies = { 'hrsh7th/nvim-cmp' }, | ||
| config = function() | ||
| require('nvim-autopairs').setup {} | ||
| -- If you want to automatically add `(` after selecting a function or method |
There was a problem hiding this comment.
suggestion: Consider making the autopairs configuration more flexible
The current setup always enables the automatic addition of parentheses after function selection. Consider making this configurable, allowing users to opt-in or opt-out of this feature easily.
| }, | ||
| cmd = 'Neotree', | ||
| keys = { | ||
| { '\\', ':Neotree reveal<CR>', { desc = 'NeoTree reveal' } }, |
There was a problem hiding this comment.
suggestion: Reconsider the keybinding for NeoTree reveal
The backslash key might not be the most intuitive choice for revealing NeoTree. Consider using a more standard key combination that aligns with common Neovim practices, such as e or .
| { '\\', ':Neotree reveal<CR>', { desc = 'NeoTree reveal' } }, | |
| { '<leader>e', ':Neotree reveal<CR>', { desc = 'NeoTree reveal' } }, |
|
|
||
| You likely want to remove `lazy-lock.json` from your fork's `.gitignore` file | ||
| too - it's ignored in the kickstart repo to make maintenance easier, but it's | ||
| [recommmended to track it in version control](https://lazy.folke.io/usage/lockfile). |
There was a problem hiding this comment.
suggestion (documentation): Fix typo: 'recommmended' should be 'recommended'
| [recommmended to track it in version control](https://lazy.folke.io/usage/lockfile). | |
| [recommended to track it in version control](https://lazy.folke.io/usage/lockfile). |
| ### Install External Dependencies | ||
|
|
||
| External Requirements: | ||
| - Basic utils: `git`, `make`, `unzip`, C Compiler (`gcc`) |
There was a problem hiding this comment.
suggestion (documentation): Consider mentioning that some dependencies may already be installed
It might be helpful to note that some of these dependencies, especially basic utils, might already be present on many systems. This could prevent unnecessary installations.
| - Basic utils: `git`, `make`, `unzip`, C Compiler (`gcc`) | |
| Basic utils: `git`, `make`, `unzip`, C Compiler (`gcc`) (Note: Some of these may already be installed on your system) |
There was a problem hiding this comment.
Hey @thaodt - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 3 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟡 Documentation: 2 issues found
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
| P.S. You can delete this when you're done too. It's your config now! :) | ||
| --]] | ||
|
|
||
| -- Set <space> as the leader key |
There was a problem hiding this comment.
suggestion: Consider adding a comment explaining the 'leader key' concept
-- The 'leader key' is a prefix key used for custom shortcuts
-- It's a way to create a namespace for personal mappings
-- Set <space> as the leader key
vim.g.mapleader = ' '
vim.g.maplocalleader = ' '
|
|
||
| -- [[ Install `lazy.nvim` plugin manager ]] | ||
| -- See `:help lazy.nvim.txt` or https://github.com/folke/lazy.nvim for more info | ||
| local lazypath = vim.fn.stdpath 'data' .. '/lazy/lazy.nvim' |
There was a problem hiding this comment.
suggestion: Add a brief comment explaining the purpose of lazy.nvim
-- Define the path for lazy.nvim, a modern plugin manager for Neovim
local lazypath = vim.fn.stdpath 'data' .. '/lazy/lazy.nvim'
| -- | ||
| --]] | ||
|
|
||
| local check_version = function() |
There was a problem hiding this comment.
issue: Resolve duplication between health.lua files
This health.lua file is very similar to the one in .config/nvim/lua/kickstart/health.lua, but with some differences in the version checking logic. Consider unifying these files or clearly documenting why they are different to avoid confusion and maintenance issues.
|
|
||
| You likely want to remove `lazy-lock.json` from your fork's `.gitignore` file | ||
| too - it's ignored in the kickstart repo to make maintenance easier, but it's | ||
| [recommmended to track it in version control](https://lazy.folke.io/usage/lockfile). |
There was a problem hiding this comment.
suggestion (documentation): Fix typo: 'recommmended' should be 'recommended'
| [recommmended to track it in version control](https://lazy.folke.io/usage/lockfile). | |
| [recommended to track it in version control](https://lazy.folke.io/usage/lockfile). |
| ### Install External Dependencies | ||
|
|
||
| External Requirements: | ||
| - Basic utils: `git`, `make`, `unzip`, C Compiler (`gcc`) |
There was a problem hiding this comment.
suggestion (documentation): Consider mentioning that some dependencies may already be installed
It might be helpful to note that some of these dependencies, especially basic utils, might already be present on many systems. This could prevent unnecessary installations.
| - Basic utils: `git`, `make`, `unzip`, C Compiler (`gcc`) | |
| Basic utils: `git`, `make`, `unzip`, C Compiler (`gcc`) (Note: Some of these may already be installed on your system) |
Summary by Sourcery
Switch to kickstart.nvim for Neovim configuration, replacing the previous custom setup. Add configuration files for the Zed editor, including settings and keymap. Introduce documentation and licensing for the new setup.
New Features:
Enhancements:
Documentation: