Skip to content
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

lib: function for custom plugin setup() arguments #181

Merged
merged 1 commit into from
Jan 3, 2024

Conversation

horriblename
Copy link
Collaborator

@horriblename horriblename commented Nov 11, 2023

closes #133

I started playing around with the idea a bit more. will try to convert the bigger plugins first to get a grasp on how it would go

progress (by directories in modules/)

  • assistant

    • copilot
    • tabnine
  • autopairs

    • nvim-autopairs
  • basic**

  • comments

    • comment-nvim
  • completion

    • nvim-cmp (need rawLua)
  • core**

  • [-] dashboard

    • alpha
    • dashboard-nvim
    • startify
  • debugger

    • nvim-dap
  • dap-ui

  • filetree

    • nvimtree
  • git

  • languages (need rawLua)

    • bash
    • dart
    • elixir
    • markdown
    • tidal
    • ...
  • lsp

    • lightbulb*
    • lspconfig**
    • lspkind**
    • lsplines*
    • lspsaga**
    • lsp-signature
    • null-ls (need rawLua)
    • nvim-code-action-menu**
    • nvim-docs-view
    • trouble**
  • minimap

    • codewindow*
    • minimap-vim **
  • notes

    • mind-nvim *
    • obsidian
    • orgmode
    • todo-comments
  • projects

    • project-nvim
  • rich-presence

    • [x]neocord
  • session

    • nvim-session-manager
  • snippets

    • vsnip
  • statusline

    • lualine
  • tabline

    • nvim-bufferline
  • terminal

    • toggleterm
  • theme

  • treesitter

  • ui

    • borders
    • breadcrumbs
    • colorizer
    • illuminate
    • modes
    • noice
    • notifications
    • nvim-notify
      • smartcolumn
  • utility

    • binds
         - [ ] cheatsheet
         - [ ] filetree
         - [ ] nvimtree-lua
    • which-key
    • ccc
    • diffview
    • gestures
    • gesture-nvim
    • icon-picker
    • motion
         - [ ] hop
         - [ ] leap
    • treesitter-textobjects
    • preview
         - [ ] glow
    • markdown-preview
    • surround
    • telescope
    • wakatime
  • visuals

    • fidget-nvim

*no breaking changes: I won't add it for now, since they can be easily added later
**nothing to do
random bugs I found:

  1. telescope: all the options should be under defaults but most are not
  2. lualine: extensions should not be under options

@horriblename horriblename force-pushed the feat-plugin-setup-config branch 4 times, most recently from b98889a to aff4f4b Compare December 12, 2023 00:49
@NotAShelf
Copy link
Owner

@horriblename I'd like to lend a hand with this, as my schedule is finally clearing up. Which modules would you like me to take a look at?

@horriblename
Copy link
Collaborator Author

thanks! been kinda busy lately so I couldn't put much into this

you can start with any directory in modules/; we should probably merge this first then add other plugins in separate PRs


right now the conversion process is something like:

  1. add new option

    setupOpts = lib.nvim.types.mkPluginSetupOption "Plugin Name" {
      optionName = mkOption {...};
    };
  2. change the lua script to

    ''
    require("...").setup(${nvim.lua.toLuaObject cfg.setupOpts})
    ''
  3. add back old options for compat

    config.vim.statusline.lualine.setupOpts = {
      option_name = cfg.optionName;
    };

some problems I haven't worked out:

  • right now I use extractDefault in lib/types/plugins.nix , which feels bad, I should look at how other projects use freeform modules
  • should we deprecate old options? there's 146 of them that needs to be renamed in nvim-tree alone, deprecating would definitely make someone unhappy but having two versions of the same option is confusing and clutters the doc

@NotAShelf
Copy link
Owner

NotAShelf commented Dec 27, 2023

right now I use extractDefault in lib/types/plugins.nix , which feels bad, I should look at how other projects use freeform modules

I get the idea behind extractDefault (I think) but it looks like a clever abuse of functional programming. As you said, it "feels" bad. Pretty sure we can run up to some very abstract errors through that. I'll see if I can get around that.

In other news, I'm still not even sure how freeform modules are meant to work. Other projects (the ones that I'm aware of) are either raw lua, or they mkOption for each plugin option like we do. That's a solution, but it gets out of hand in time.

should we deprecate old options? there's 146 of them that needs to be renamed in nvim-tree alone, deprecating would definitely make someone unhappy but having two versions of the same option is confusing and clutters the doc

I'm not sure how many of the users actually use all 146 of the nvim-tree options. I know I don't. We can go around deprecating those with a major release (0.7) to give people the option to remain on 0.6 until they get around changing the old options. A "migration guide" of sorts would definitely not hurt.

@horriblename
Copy link
Collaborator Author

turns out I don't need extractDefault, home-manager just leaves the default as {}

maybe we can use mkRenamedOptionModule instead of writing a migration guide - example.

@NotAShelf
Copy link
Owner

we already use mkRenamedOptionModule as of v0.6 release - it has a few problems that I don't know how to work out (i.e the file location is not always reported correctly) but should work for the most part, though it's still a problem for us to rename all the options we got rid of

@NotAShelf
Copy link
Owner

@horriblename what do you think about backporting the library changes of this plugin into main, and creating a new branch for individually updating each major pluigins? I've been meaning to add a few plugins, but avoiding adding more lua configuration until we get this one through - it'd make adding quick plugins faster, and we can tackle large plugins on our own accord.

@horriblename
Copy link
Collaborator Author

what do you mean backport? we can merge this now if thats what you mean

@NotAShelf
Copy link
Owner

I'm talking about extracting specifically the lib changes and merging them into main, keeping plugin config translations in this branch for the foreseeable future. We can rebase this, and it should contain only plugin changes.

@horriblename horriblename force-pushed the feat-plugin-setup-config branch 2 times, most recently from 9f8626d to 59b2e6b Compare January 2, 2024 23:57
@horriblename horriblename changed the title Feat custom arguments to pass to plugin setup() add lib function for custom plugin setup() arguments Jan 2, 2024
@horriblename
Copy link
Collaborator Author

I've removed the changes to nvim-tree, lualine and telescope. You can merge this first, then I'll make a new PR for the plugins

@NotAShelf NotAShelf changed the title add lib function for custom plugin setup() arguments lib: function for custom plugin setup() arguments Jan 3, 2024
@NotAShelf NotAShelf marked this pull request as ready for review January 3, 2024 00:03
@NotAShelf NotAShelf self-requested a review as a code owner January 3, 2024 00:03
@NotAShelf NotAShelf merged commit 39dd12f into NotAShelf:main Jan 3, 2024
5 of 7 checks passed
bloxx12 pushed a commit to bloxx12/nvf that referenced this pull request Sep 29, 2024
…config

lib: function for custom plugin `setup()` arguments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

extraConfig to pass into builtin plugin's setup()
2 participants