nixvim
nixvim copied to clipboard
fix(dashboard-nvim): update to new setup schema
Hello, first time contributing to a nix project! I hope this is helpful for users of this distribution of neovim.
When doing my configuration using nixvim I couldn't get dashboard-nvim to work, and when digging deeper I realized that the plugin's configuration hasn't been updated in a bit and it underwent some breaking changes. So this pull request adapts to such breaking changes and gets it back to a working state!
I've tested it both with the default theme hyper:
as well as doom:
Here's with an actual configuration of doom:
I've tried to follow the contribution guidelines and I think I followed it correctly but please do correct me where I may be wrong! Specially code-wise, I am fairly new to the language's specifications!
Thank you very much for your work!
This plugin seems to have been implemented quite a while ago, it does not use the best practices we try to use in the newer implementation. I think it's okay to leave the old options as is (if you don't want to change them), but for all the newer options it would be nice to try and stick with the way we try to do thing:
We want to avoid whenever possible to set a nix default value (default = ....) because this will be added in the user's init.lua, and could slow down neovim. There are a number of helpers in helpers.defaultNullOpts.xxx to create a number of options. It would be best to use them (you can check other plugins for inspiration if needed)
Awesome! I will follow it up with some commits for those changes, thanks for pointing it out.
I've done the adaptions @traxys, not too sure about the cases where I used mkNullable, as ideally you'd want to avoid it, but these properties don't have defaults set by the plugin; please let me know if there is a need for further adaptations!
On another note, there isn't any helper function using mkOption where example is set, yet I've seen a few examples where it's used - however in my cases these are nullable whereas those examples weren't, so how important are examples? Or should I just use mkCompositeOption for these submodules and ditch the example property?
I've done the adaptions @traxys, not too sure about the cases where I used
mkNullable, as ideally you'd want to avoid it, but these properties don't have defaults set by the plugin; please let me know if there is a need for further adaptations!
mkNullable is perfectly good if upstream has no default value!
On another note, there isn't any helper function using
mkOptionwhereexampleis set, yet I've seen a few examples where it's used - however in my cases these are nullable whereas those examples weren't, so how important are examples? Or should I just usemkCompositeOptionfor these submodules and ditch theexampleproperty?
If you want to use a example you would need to skip the helpers wrapper and use the correct mkOption yourself (with the correct type)
On another note, there isn't any helper function using
mkOptionwhereexampleis set, yet I've seen a few examples where it's used - however in my cases these are nullable whereas those examples weren't, so how important are examples? Or should I just usemkCompositeOptionfor these submodules and ditch theexampleproperty?If you want to use a
exampleyou would need to skip thehelperswrapper and use the correctmkOptionyourself (with the correct type)
But how important is it for the document generation? If it doesn't affects it I'll just use mkCompositeOption.
Providing examples can be nice for users, you can do as you wish, I think we don't often provide examples
Made my final changes in regards to submodules, I'm good to go @traxys! Thanks for all the advice!
Could you please rebase this on main ?
Could you please rebase this on
main?
I missed this when working on #1590. This PR is superseded by that one.