vim*: organize #392471
Merged
vim*: organize #392471
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
6.topic: vim
8.has: documentation
khaneliman
force-pushed
the
vim-refactor
branch
3 times, most recently
from
2941782 to
dab2bba
Compare
Comment on lines
3519
to
3488
| vim2nix = buildVimPlugin { | ||
| pname = "vim2nix"; | ||
| version = "1.0"; | ||
| src = ./vim2nix; | ||
| src = ./non-generated/vim2nix; | ||
| dependencies = [ self.vim-addon-manager ]; | ||
| }; |
There was a problem hiding this comment.
Should package definitions that aren't "overriding" anything be move out of overrides.nix and into another file?
I wonder if we could take a semi-automated approach, similar in principal to other by-name trees, where each directory in non-generated gets imported into vimPlugins via callPackage, using the directory name as the plugin attr name.
There was a problem hiding this comment.
Yeah, I think we should basically have a non-generated.nix or something that houses them instead of being inside overrides.
EDIT: But, the second idea you had was better.
There was a problem hiding this comment.
Started implementing it. Didn't realize how many we had in there that were new plugin definitions in overrides.nix
I just wanted to double check on what we wanted to do with handling those overrides that might be unnecessary. Was going to either:
Got distracted with some home-manager stuff while I was working on this. |
khaneliman
force-pushed
the
vim-refactor
branch
from
dab2bba to
9bdbe9b
Compare
khaneliman
force-pushed
the
vim-refactor
branch
9 times, most recently
from
353bf28 to
fe42e5e
Compare
I'm hoping nix-community/nixvim#3099 will fix the nixvim issue. I've tested it against e8d0b02 and it seems to be working. EDIT: more thorough testing revealed it is not working |
Want me to drop it from here and then test against this branch? |
I'm conscious that this PR is growing quickly in scope, but sure. I guess specific parts of the refactoring can be split into separate PRs if needed. |
MattSturgeon
added a commit
to MattSturgeon/nixvim
that referenced
this pull request
Mar 24, 2025
NixOS/nixpkgs#392471 flake.lock: Update Flake lock file updates: • Updated input 'nixpkgs': 'github:NixOS/nixpkgs/0740f6f238767d4caf9afe774d3e88105766dfc6?narHash=sha256-NAxwF5cjgh8o5aylhePXWNQETCWYaTpNvdO2bMfINpQ%3D' (2025-03-22) → 'github:nixos/nixpkgs/498dcd13190e85fd593948cf64590eb74a6b40b8?narHash=sha256-H69B2F4oCsWgbmhzD1S%2BasLgHpY2OH0R4QNVgMshcdg%3D' (2025-03-24) flake/dev/flake.lock: Update Flake lock file updates: • Updated input 'nixpkgs': 'github:NixOS/nixpkgs/0740f6f238767d4caf9afe774d3e88105766dfc6?narHash=sha256-NAxwF5cjgh8o5aylhePXWNQETCWYaTpNvdO2bMfINpQ%3D' (2025-03-22) → 'github:nixos/nixpkgs/498dcd13190e85fd593948cf64590eb74a6b40b8?narHash=sha256-H69B2F4oCsWgbmhzD1S%2BasLgHpY2OH0R4QNVgMshcdg%3D' (2025-03-24)
Yeah, I can definitely split stuff out if we need to. Just helps to see the final organization while doing it all. |
github-actions
bot
added
10.rebuild-darwin: 11-100
10.rebuild-linux: 11-100
and removed
10.rebuild-darwin: 1-10
10.rebuild-linux: 1-10
labels
khaneliman
force-pushed
the
vim-refactor
branch
2 times, most recently
from
09944ef to
018224c
Compare
|
I think I'm pleased with overall organization and most of the cleanup. Main things that need to be verified is testing the luaPackages that had the overrides removed. |
|
|
There was a problem hiding this comment.
Thank you @khaneliman! That's more than welcomed.
Also thanks to @MattSturgeon and @teto for the thorough review.
I have looked at each commit and I am fine with the changes made.
All good on my end :)
Tests:
-
nixpkgs-reviewon all platforms -
nixpkgs-review -p vimPluginsonx86_64-linux - Nixvim test suite
GaetanLepage
approved these changes
Mar 25, 2025
|
It's a nice PR but I am still unsure about moving the files to "non-generated" or to "utils/". Is it a problem to leave them where they are ? especially if you dont like the name either. |
This is moving a file that was missed in the last move to
A file rename isn't a hard conflict to resolve, if it triggers a conflict. That's an entire rewrite anyways that has conflicts anytime we do any updates. This is just a minor file path change. Just wanted the folders to not feel cluttered and confusing when trying to find what you're looking for. I don't like the I hoped it would help contributors if we tucked away some of the implementation details away from cluttering the
|
Add some organization
Move the non-generated plugins to using a function call that iterates over the folders in the non-generated directory so we don't need them in overrides.nix
Old repo referenced was removed, referencing currently active repo.
coc is breaking out packages into top-level, but I don't know if we really need separate folders for each of these. Can just create another set to merge in.
Attempt to remove overrides that provide dependencies that should be handled from the luaPackages derivation and buildNeovimPlugin.
Move to actively maintained repo and move to non-generated since plugin updater can't handle sub directories.
khaneliman
force-pushed
the
vim-refactor
branch
from
018224c to
06422e6
Compare
|
I apologize I had not noticed we already had a "non-generated" folder. I thought you were introducing one. Ok for those. |
This was referenced Mar 25, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Things done
Follow up to #390702
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.