Conversation
Refactor/vim.pack
fix: deprecated diagnostic jumping config
By: Ori Perry from: oriori1703/kickstart-modular.nvim.git commit: 6aa660c test/modular
|
Can you please review this, I made some adjustments to your script to better match the existing kickstart-modular structure. Oh and btw for the script to work properly, it depends on your renumbering of sections from here: So if everything looks ok it would be good to eventually integrate that section renumbering in the upstream kickstart too. |
…e more closer A couple of notable changes: - renamed section file names to match existing files in kickstart-modular - split ui section to separate plugins to match existing files in kickstart-modular - append modeline - merge gitsigns from init.lua and existing gitsigns.lua plugin This is so that the resulting file names match existing kickstart-modular files, which should make it easier to review changes, and also to merge these changes to anyone that is forking kickstart-modular
|
I have now rebased my personal config on top of this and it seems to work fine. |
There was a problem hiding this comment.
I commented on some minor code readability improvements, but otherwise the code seems to function fine (though I kinda skimmed the script because I don't have a lot of time until the weekend).
Though I think I prefer something closer to my earlier version because I think parsing the vim.pack calls and splitting the files / combining with the extras is really fragile and adds a lot of complexity to the script.
I get that you want each plugin to be in it's own file, but this is already not the case with blink, telescope, and lsp.
Also most of the plugins in the split file have a really small config and I don't think really warrant their own file.
If this helps I can also merge the 2 gitsigns configs in the upstream repo (and also include 3e17d38) which gives one less reason to split that section.
Thank you so much, many of the suggestion make sense and I will apply them.
I thought about this a lot but in the end decided to keep the current structure. Having an easier path of migrating forward for people that merge in the changes is a compelling argument I think. At least I know it made my migration much easier, because in my personal config I make many small changes all around. Having to deal with the plugin manager change and a restructuring at the same time would be a bit too much of a hassle to even consider the migration. Regarding the script complexity, I wouldn't worry about it, with the accessibility of vibe coding adapting the script is very easy and quick to do so - all my changes were vibe coded (and it shows :) ), but it gets the job done. It's true that some larger sections are not split to separate plugins but in that case it makes sense to keep them as is because they are dealing with the dependencies on the actual plugin and the dependent plugins were kept together already in the original modular structure - because the lazy plugin manager treats that that way - a plugin spec contains also the dependent plugins. Also I find that having the name of the plugins be reflected in the filename instead of a section name is an advantage, because let's say you want to replace telescope with snacks, if the plugin is called by the section name - search, you would be rewriting the whole section file and would not have an easy path back. While if the plugin is called telescope, you can just comment out the We might reconsider restructuring this in the future, as I see the package manager change and the restructuring as two separate issues. (sorry for the lengthy post, just wanted to put out the rationale behind it)
If you can merge the 3e17d38 changes to the section names (split foundation to options and keymaps and renumber) that would be great, yes please. As for gitsigns I'll leave the decision up to you. I guess it would make sense to just fold the existing gitsigns.lua plugin of the original kickstart into init.lua, but at the time that was suggested the hesitation was that it would increase the line count of init.lua where the wish was to keep it short. |
While I agree that it would be easier to fix with the help of LLMs, it would be the easiest if nothing breaks at all :)
I merged added the changes from 3e17d38 in 7031a09. Regarding the gitsigns change, I'm unsure in which direction I want to make the merge (into Let me know if I can help in any other way :) |
aff225a to
64e7f1b
Compare
Co-authored-by: Ori Perry <48057913+oriori1703@users.noreply.github.com>
|
All comments resolved, I think this looks OK now, will merge. |
Based on PR:
#95
And script:
https://github.com/oriori1703/personal-kickstart.nvim/blob/test/modular/scripts/split_init.py
Which was mentioned in:
#94
I applied some changes to split_init.py: