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

refactor (user custom): Proposal for enhancing user customization experience #885

Closed
wants to merge 12 commits into from

Conversation

misumisumi
Copy link
Collaborator

@misumisumi misumisumi commented Jul 22, 2023

Hello. Thank you for the configuration that starts up superbly fast!

I want to propose about improvement of user customization experience.
According to the wiki, new plugins are modified by creating files, while existing plugins and keymap and options are modified by editing the files directly.
I think that user customization increases the likelihood of conflicts with upstream changes.
Plugin settings can be overridden by using opts for lazynvim.
keymap and options are handled by creating merge functions.
Therefore, I propose to place the user settings under the user directory and merge them with the existing settings.

LSP configuration is still incomplete, for example, lua/modules/completion/{servers,formatters} needs to be edited.

@ayamir
Copy link
Owner

ayamir commented Jul 22, 2023

Thanks for your PR first, but what you proposed is a big change.
It's better to open a issue first and set todoes for it like what we did in #627.
Split your commits to several parts first. Then we can discuss and improve them one by one.

@misumisumi
Copy link
Collaborator Author

Sorry for not standing up for the issue
I will raise an issue.

Copy link
Collaborator

@Jint-lzxy Jint-lzxy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the thorough PR! This has long been on our todo list :D

In addition to what @ayamir said, the major reason that blocked us from adopting this structure in #458 is the opts key would completely replace parent specs when passed as a function. We made quite a few adjustments to the default options of most plugins, and sometimes users may only want to change a few of them (rather than doing a complete copy & paste; this (complete copy & paste) also indirectly makes user configs vulnerable to upstream updates); But if we pass opts as tables, some plugins will be loaded prematurely b/c they require() themselves (or other dependencies), leading to performance overhead (since a plugin will be loaded once it's required).

Maybe we can consider several alternatives? Rather than letting lazy.nvim handle these specs, we could directly incorporate user's specs when setting up plugins (wrap this as an API call like require("...").load_plugin("<name-used-for-require>", current_spec) where user configs will be automatically merged with parent specs, and if the passed user config is a function, it will completely replace its parent spec). This way, most code in this PR can be reused while keeping maximum backward compatibility. And as all initialization requests are wrapped in functions, all modules will only be loaded when they are needed.

EDIT: lsps and daps can also be setup this way.

@misumisumi misumisumi mentioned this pull request Jul 22, 2023
2 tasks
@ayamir
Copy link
Owner

ayamir commented Jul 23, 2023

Maybe we can consider several alternatives? Rather than letting lazy.nvim handle these specs, we could directly incorporate user's specs when setting up plugins (wrap this as an API call like require("...").load_plugin("", current_spec) where user configs will be automatically merged with parent specs, and if the passed user config is a function, it will completely replace its parent spec). This way, most code in this PR can be reused while keeping maximum backward compatibility. And as all initialization requests are wrapped in functions, all modules will only be loaded when they are needed.

Agree.

@aarnphm
Copy link
Collaborator

aarnphm commented Jul 23, 2023

I think having a merge layer may add too much complexity to the structure. Fwiw nvimdots itself is already very complex and modular, which allows easily merge or update new config.

Even though I have migrated to single init.lua for a while now, I think the current structure is already pretty complex and does what it needs to do.

Not entirely sure if this merge capabilities would even make the newcomer to be afraid of the complexity. Just my two cents

@Jint-lzxy
Copy link
Collaborator

I think having a merge layer may add too much complexity to the structure. Fwiw nvimdots itself is already very complex and modular, which allows easily merge or update new config.

@aarnphm I explained the new structure (briefly) in #888 (comment). load_plugin is pretty much like an "implementation detail" - ordinary users don't need to care much about that :)

@aarnphm
Copy link
Collaborator

aarnphm commented Jul 24, 2023

I think having a merge layer may add too much complexity to the structure. Fwiw nvimdots itself is already very complex and modular, which allows easily merge or update new config.

@aarnphm I explained the new structure (briefly) in #888 (comment). load_plugin is pretty much like an "implementation detail" - ordinary users don't need to care much about that :)

Got it. I will take a look. Not to familiar with lazyvim structure, but it looks good

@Jint-lzxy Jint-lzxy linked an issue Jul 25, 2023 that may be closed by this pull request
7 tasks
@Jint-lzxy Jint-lzxy marked this pull request as draft July 25, 2023 09:11
@misumisumi misumisumi closed this Aug 6, 2023
@misumisumi misumisumi deleted the better-user-override branch August 28, 2023 20:26
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.

About improvement of user customization experience.
4 participants