nvim-neorocks / nvim-best-practices

Collection of DOs and DON'Ts for modern Neovim Lua plugin development
Creative Commons Zero v1.0 Universal
232 stars 3 forks source link

upstream to :help lua-guide #5

Open justinmk opened 1 month ago

justinmk commented 1 month ago

Your notes here are quite good. Would you be willing to upstream (most of) it to :help lua-guide :help lua-plugin (in a new lua-plugin.txt help file)? This would allow us to close https://github.com/neovim/neovim/issues/22366

Some of the controversial parts could be skipped for now, but 85% of what you have would give a great starting point for :help lua-plugin.

mrcjkb commented 1 month ago

Thanks for the nice words 😄

I'd be more than happy to.

mrcjkb commented 1 month ago

@justinmk would you like me to open a PR to Neovim? If so, let's clarify here (or in https://github.com/neovim/neovim/issues/22366) which points to include first (and how best to structure them).

justinmk commented 1 month ago

yes! here's a rough idea:

Note: historically the discussion about setup() has been controversial, but I think you wording is good, because you leave the door open for judgement.

include these

skip these for now (to unblock the PR / avoid controversy)

justinmk commented 1 month ago

btw, were' thinking this should go in a new help file called lua-plugin.txt instead of directly in :help lua-guide

echasnovski commented 1 month ago

## User Commands (all)

I don't think suggesting to prefer one umbrella command to several targeted commands is an enough universal advice to be included in core. To me this is a more question of taste. "Pollute the command namespace" is obviously a bad thing if formulated this way, while "provide more narrowly scoped commands" is not .

## Keymaps (all)

I don't think suggesting to create a <Plug> for users to map is a good design for "modern Neovim". If anything, suggesting to export a Lua function which can be used in both "<Cmd>lua xxx()<CR>" and function() xxx() end right hand side is a more straightforward approach with less overhead for plugin author. I have read the "benefits of <Plug> mappings over exposing a lua function", yet most of them are not (per se) benefits of preferring <Plug> over Lua functions.

And "If a user has to call a setup function to set keymaps, it will cause an error if your plugin is not installed or disabled." is not necessarily a bad side effect.

## Initialization (all)

Do not agree at all that this is worded as to "leave the door open for judgement". The "Automatically initialize your plugin (smartly), with minimal impact on startup time." might sound good on paper, yet:

Having setup() be a single entry point for plugin to create any side effects feels like the easiest to grasp concept for both new users and new plugin authors. Yes, it goes against "installed plugin should work right away" mentality in favor of "installing plugin is just downloading code, enable it with setup()" approach.

## Versioning and releases (all)

"Don't use 0ver" suggestion is a bit ironic to be included in Neovim itself and is not a universally good advice. Many successful projects work just fine with 0ver.

mrcjkb commented 1 month ago

@echasnovski thanks for the input. It's definitely great to get feedback from someone with strongly differing views than me, as it sheds some more light on what might be considered too controversial for core :smile:

I don't think suggesting to prefer one umbrella command to several targeted commands is an enough universal advice to be included in core. To me this is a more question of taste.

Just to be clear, I don't advocate for a single command with subcommands per plugin. For example, in rustaceanvim, I provide more than one command.

It's definitely not universal advice, and depends on the use case. I do think it's worth recommending to think carefully about whether one might be polluting the command namespace.

yet most of them are not (per se) benefits of preferring <Plug> over Lua functions.

Could you please elaborate on this?

Being able to enforce things like expr = true (and thus preventing people from opening issues because they forgot to do so), or exposing different functionality for different modes, are very clear benefits.

Sure, these are not always needed, which is why I also have a DO section for just exposing a Lua API. Again, this is something that depends on the use case.

with less overhead for plugin author

A bit of overhead can be a massive time saver if it results in less time wasted supporting people who have issues with your plugin.

And "If a user has to call a setup function to set keymaps, it will cause an error if your plugin is not installed or disabled." is not necessarily a bad side effect.

If it weren't forced on the user, I would agree.

"Minimal impact" is still an impact which would make extreme startup optimization not possible (or again needing some hacky plugin manager). As it is a big part of what makes Neovim fun to use for many people, I'd be careful to not make it harder.

This seems like a slippery slope argument. I don't see how these steps are necessarily connected. In fact, your next point, 'Initialization itself might need some configuration,' suggests a way to make 'extreme startup optimization' possible. And a setup function is not the only thing that makes configuring initialization possible.

Having setup() be a single entry point for plugin to create any side effects feels like the easiest to grasp concept for [...] new users

That perspective is highly subjective and likely influenced by the status quo. Not everyone prefers to be forced into a single entry point. For instance, some may prefer separate files for keymaps, highlight groups, and other configurations.

In fact, the only 'complaint' I've ever heard about one of my plugins not providing a setup function would have been satisfied if I had provided a setup function that does absolutely nothing but set a vim.g. table.

for [...] new plugin authors

Bad practices tend to be easier to grasp.

"Don't use 0ver" suggestion is a bit ironic to be included in Neovim itself

I fully agree with this.

Although personally, I do believe "SemVer to properly communicate bug fixes, new features, and breaking changes." is the best approach, and it would be beneficial if it were consistent across the ecosystem.

echasnovski commented 1 month ago

Just to be clear, I don't advocate for a single command with subcommands per plugin. For example, in rustaceanvim, I provide more than one command.

* `RustLsp {subcmd}` for interacting with the LSP client

* `RustAnalyzer [start|stop|restart]`.

* ...

This looks like violating the advice as it is written, because it should instead be Rust lsp {subcmd} and :Rust analyzer [start|stop|restart]. If it is not treated as "one command per plugin" or at least "one command per exported module", then it does not have much sense as "Don't do that, do this instead" type of recommendation. As an advice that both types of commands are possible - sure, that's great.

Could you please elaborate on this?

Being able to enforce things like expr = true (and thus preventing people from opening issues because they forgot to do so), or exposing different functionality for different modes, are very clear benefits.

Forcing mapping options is the sole benefit here, yes.

"Expose functionality for specific modes" and "Expose different behaviour for different modes with a single <Plug> mapping." are not specific to <Plug> approach and only lead to more code without clear benefit. Both of them still require code to handle different modes, which can be directly exposed to the user. So "one <Plug> for different modes" can easily be "one Lua function for different modes".

And "If a user has to call a setup function to set keymaps, it will cause an error if your plugin is not installed or disabled." is not necessarily a bad side effect.

If it weren't forced on the user, I would agree.

Suggesting instead to force on the user to create mappings themselves does not look like a clear advantage in this aspect.

That perspective is highly subjective and likely influenced by the status quo. Not everyone prefers to be forced into a single entry point. For instance, some may prefer separate files for keymaps, highlight groups, and other configurations.

As not everyone prefers things to be forcefully executed. Separate files for keymaps, highlight groups, and other configurations are certainly possible with "single entry point setup()" approach.

for [...] new plugin authors

Bad practices tend to be easier to grasp.

As is bad advice.


If anything, my main issues with most of the suggestions here can probably be summarized as:

justinmk commented 1 month ago

Officially recommending Vimscript-esque approaches like <Plug> and g: is a serious step backwards.

<Plug> is a mapping feature, it's separate from vimscript. Neither is a "serious step backwards".

I don't want this to be blocked on nitpicks or concerns about style. Plugin authors need a place to start, and in all of the discussion above I don't see any concrete "bad advice". Style preferences are not "bad advice", and plugin authors can choose to diverge if they want. But we need to give a starting point.

echasnovski commented 1 month ago

Style preferences are not "bad advice", and plugin authors can choose to diverge if they want. But we need to give a starting point.

Having "don't do this approach" with this exact wording inside core help is a bit more than "it is just a style preferences".

Modifying the text to contain only "do"s and mentioning other approaches without shedding bad light will go a long way.

mrcjkb commented 1 month ago

for [...] new plugin authors

Bad practices tend to be easier to grasp.

As is bad advice.

@echasnovski In case of misunderstanding: My response aimed to clarify that being easy to grasp does not necessarily equate to "good practice."

Your response did not address the substance of my point and seemed more focused on confrontation rather than constructive discussion.

This approach is counterproductive and detracts from a respectful and meaningful exchange. I would like to point you to our Code of conduct.

If we cannot keep this discussion civil and focused on the topic, I will close this issue as "not planned" due to concerns about negative interactions and will block further communication with you. The Neovim core team will still have my consent to use this document freely.

To address your other concerns:

This looks like violating the advice as it is written

This seems like a misinterpretation of the advice on your part. Perhaps it would help if I added a tip box to the section to clarify?

"one <Plug> for different modes" can easily be "one Lua function for different modes"

Yes, that is possible, but it would involve adding side effects and increasing the complexity of the Lua function.

Suggesting instead to force on the user to create mappings themselves

We're discussing <Plug> mappings versus Lua functions, not forcing users to create mappings themselves (this is a straw man). If you're shifting to the question of why I don't advocate for keymap DSLs, my reasoning is outlined in the document.

As not everyone prefers things to be forcefully executed.

This is another straw man. I never advocated for things to be forcefully executed. As I mentioned in my previous response:

In fact, your next point, 'Initialization itself might need some configuration,' suggests a way to make 'extreme startup optimization' possible. And a setup function is not the only thing that makes configuring initialization possible.

Separate files for keymaps, highlight groups, and other configurations are certainly possible with "single entry point setup()" approach.

Some plugin managers provide abstractions for doing this. However, I imagine doing this manually could be quite tedious. But maybe I'm mistaken?

Lua is a fully featured programming language with first class support in Neovim

Vimscript, too, has first class support in Neovim.

Officially recommending Vimscript-esque approaches like <Plug> and g: is a serious step backwards.

My document does not make any recommendations to use vim.g (I do in my blog post, which is more opinionated). I simply state that this is my preference and that there is no real downside to using it (I'm aware of the theoretical downsides). To the contrary, recommending against Vimscript compatibility without good reason would be a step in the wrong direction.

official suggestions should either advocate for maximum flexibility approach (to me this feels like a current single setup() approach)

I have yet to be presented with evidence that a single setup approach is more flexible than (configurable) smart initialization.

Modifying the text to contain only "do"s and mentioning other approaches without shedding bad light will go a long way.

I agree that some of the DON'Ts could probably be excluded from core. Given how strongly you feel about setup, even if I strongly disagree with you, I do feel that perhaps the official help docs shouldn't make any recommendations against it.

But some of the DON'Ts (e.g. don't create keymaps automatically) are quite uncontroversial, and not immediately obvious to newcomers.

echasnovski commented 1 month ago

for [...] new plugin authors

Bad practices tend to be easier to grasp.

As is bad advice.

@echasnovski In case of misunderstanding: My response aimed to clarify that being easy to grasp does not necessarily equate to "good practice."

Your response did not address the substance of my point and seemed more focused on confrontation rather than constructive discussion.

This approach is counterproductive and detracts from a respectful and meaningful exchange. I would like to point you to our Code of conduct.

If we cannot keep this discussion civil and focused on the topic, I will close this issue as "not planned" due to concerns about negative interactions and will block further communication with you. The Neovim core team will still have my consent to use this document freely.

@mrcjk, in case of misunderstanding: no confrontation was intended. My response aimed to clarify that being easy to grasp should be one of the main goals of built-in help for creating Lua plugins, while avoiding bad advice at the same time.