plugins/which-key: migrate to mkNeovimPlugin

[khaneliman]

Working on converting this and there are a few breaking changes for data types of config options https://github.com/folke/which-key.nvim/pull/624/files#diff-a8347a13956b70b044d3e091cd3115e2fcfb44d4ebde516ff03223b2e84c7a7b.

- One being the triggers option which used to be string or list of strings, but is now a list of attribute sets like the registrations change. What should we do in that case?
Very niche setting, continuing with breaking change.
- Another issue I was hoping for clarification on is nested properties moving to settings and moving to a different path within settings.
Resolved, most can be just parent level rename. Manually specified those that wouldn't be handled properly for nested.
- plugins -> presets -> textObjects moved and changed from camel to snake case. Does that get handled from the normal optionsRenamedToSettings?
^
- windows -> winblend moved to win -> wo -> winblend. i'm unable to make a mkRenamedOption for that while also using the optionsRenamedToSettings for the parent object because of the order of evaluation, it seems. Will I need to explicitly remap all of those or just handle the deprecation like registrations and support forwarding the previous config through, for now?
Created a deprecation warning for the window setting, dont need to worry about the child setting since it's mentioned in the warning that the options have changed.

Lots of TODO notes and some temporary test stuff for testing out renaming and warnings that can be ignored/questions answered

[MattSturgeon]

Working on converting this and there are a few breaking changes for data types of config options folke/which-key.nvim#624 (files).

TBH, 90% of users probably don't have much config in which-key, other than registrations. I don't think we have to gracefully transition all changes to the v3 scheme. Rather mkRemovedOptionModule with clear migration instructions would be enough for lesser used options or anything particularly complex.

• One being the triggers option which used to be string or list of strings, but is now a list of attribute sets like the registrations change. What should we do in that case?

IMO this can be a mkRemovedOptionModule assertion, unless it is trivial to take the same approach as cfg.registrations.

• Another issue I was hoping for clarification on is nested properties moving to settings and moving to a different path within settings.

• plugins -> presets -> textObjects moved and changed from camel to snake case. Does that get handled from the normal optionsRenamedToSettings?

optionsRenamedToSettings can handle either string entries or listof-string entries (for nested options). It assumes all renames should use snake_case, so it converts any camelCase option names in the old path to snake_case in the new path.

Here's one example:

nixvim/plugins/utils/indent-blankline.nix

Lines 19 to 112 in 5922a48

	optionsRenamedToSettings = [
	"debounce"
	[
	"viewportBuffer"
	"min"
	]
	[
	"viewportBuffer"
	"max"
	]
	[
	"indent"
	"char"
	]
	[
	"indent"
	"tabChar"
	]
	[
	"indent"
	"highlight"
	]
	[
	"indent"
	"smartIndentCap"
	]
	[
	"indent"
	"priority"
	]
	[
	"whitespace"
	"highlight"
	]
	[
	"whitespace"
	"removeBlanklineTrail"
	]
	[
	"scope"
	"enabled"
	]
	[
	"scope"
	"char"
	]
	[
	"scope"
	"showStart"
	]
	[
	"scope"
	"showEnd"
	]
	[
	"scope"
	"showExactScope"
	]
	[
	"scope"
	"injectedLanguages"
	]
	[
	"scope"
	"highlight"
	]
	[
	"scope"
	"priority"
	]
	[
	"scope"
	"include"
	"nodeType"
	]
	[
	"scope"
	"exclude"
	"language"
	]
	[
	"scope"
	"exclude"
	"nodeType"
	]
	[
	"exclude"
	"filetypes"
	]
	[
	"exclude"
	"buftypes"
	]
	];

Note: it's preferred to rename each of the old options, instead of a group of them as otherwise the camelCase/snake_case transformation won't happen for options nested within the group.

If you want anything more bespoke, such as a completely different name, you can manually use mkRenamedOptionModule.

• windows -> winblend moved to win -> wo -> winblend. I'm unable to make a mkRenamedOption for that while also using the optionsRenamedToSettings for the parent object because of the order of evaluation, it seems.

This is another reason why it's usually better to rename each option rather than a common path containing a group of options.

EDIT: To clarify, renaming a group of options is fine if none of the sub-options are camelCase and you don't need a bespoke rename for any of them.

Will I need to explicitly remap all of those or just handle the deprecation like registrations and support forwarding the previous config through, for now?

Without looking at the actual code (yet), I think renaming all the options is probably better.

Lots of TODO notes and some temporary test stuff for testing out renaming and warnings that can be ignored/questions answered

Thanks for working on this! Don't hesitate to ping as and when you have any questions/issues or would like a code review.

[MattSturgeon]

I've focused this review on the window deprecation. The rest of the changes I've only really skimmed over (so far), but overall they're looking good!

[khaneliman]

Last push is removing the graceful deprecation of window attribute. During testing, the options weren't being used by the actual plugin even though they are listed as just deprecated.

[khaneliman]

Made window.border and window.winblend options renames to gracefully transition users with that config.

[MattSturgeon]

Thanks for your continued contributions!

This is looking pretty good, I've tried to focus more on the detail and pick out any nits with this review. The PR is looking more or less done.

It's up to you, but I still wonder if some of the more "internal" feeling options should be omitted from settingsOptions? As discussed before, you can make the judgement call of which options you wish to support until we come up with a policy agreed on by all maintainers.

[MattSturgeon]

Looks like we're down to the final nits now! 🚀

[MattSturgeon]

No more issues from me!

I'm still uncomfortable with having option declarations for all settings-options, but I'm fine with it in the short term if @khaneliman is comfortable maintaining them.

@GaetanLepage @traxys any final feedback before we merge?

@khaneliman thanks again for all your efforts!

[GaetanLepage]

All good for me. I had a quick browse of the changes and I haven't seen anything too fishy.
I'm OK for declaring all the options, as it is at the maintainer's initiative.

Kudos to both of you for this chunky PR !