feat!: support replace mode, new colors and improve performance via caching

[mvllow]

This pull request is an internal refactor of modes, including support for replace mode, more predictable mode/operator detection, a shiny new palette, and builtin documentation.

These changes should not be breaking for most, but we have removed default ignored_filetypes as the detection of unlisted buffers seems to suffice.

Todo

• Port feat: allow disabling entire modes #67
• Move config options to per mode, e.g. visual.color and visual.opacity
• Re-add tests?

Fixes #57

[adriankarlen]

I was playing around using the canary branch but for me this update breaks custom colors.

However it seems to always use the hl-group. so if the hl group is set it will overwrite default colors. but custom colors will still not be used

[fitrh]

I am currently in the middle of migrating to a new machine and I don't have any neovim setup at the moment so I can't test it.

By skimming the changes, everything looks good, regardless of my personal preferences to avoid using:

1. The _G global variable
2. The plugin/ dir pattern
3. The vim.g.*_loaded pattern, in lua we have package.loaded[mod]
4. The vim.validate usage, is known for its poor performance, lately in neovim core there have been several refactor to avoid vim.validate for simple type checking

Of course all of them are personal preference and not a bad thing, so feel free to ignore them

[fitrh]

Regarding documentation, I think it would be better if we could document the available highlight groups (Modes{Scene}{Part}) and which parts of the editor they affect, in my personal setup, the highlight groups are a powerful way to customize the appearance, but this can be done in a separate PR

[mvllow]

Really appreciate the feedback. I'll look into the custom colors not applying, that's something I simply haven't tried manually so thanks for bringing it up.

As far as your feedback, @fitrh, the global shenanigans were inspired by mini.nvim but I'm not attached to anything. I'll definitely remove vim.validate as that doesn't add much value after initial setup.

A large goal of this refactor was to help me regain a grasp of how modes even works because it's been a while since I've looked at it in depth and to allow easier disabling/enabling (especially the autocmds)

[mvllow]

& maybe we don't need to be able to configure colors via modes. We could have an example autocmd in the readme to modify the colors? Less abstraction is always my preferred way but I don't want to take away from UX either.

[adriankarlen]

I agree that using hl-groups is better. Perhaps something like this could be added to the readme

[mvllow]

I think we will end up removing colours then. The only question is what the API should look like.

vim.api.nvim_create_autocmd("ColorScheme", {
  callback = function()
    -- With full control of fg/bg, using blend for the bg to replace `line_opacity`
    vim.api.nvim_set_hl(0, "ModesInsert", { fg = "blue", bg = "blue", blend = 15 })
    -- or only caring about the bg, leaving `line_opacity` as an option
    vim.api.nvim_set_hl(0, "ModesInsert", { bg = "blue" })
  end
})

Also, there seems to be a pretty bad performance regression—probably related to the autocmds. Haven't had time to debug but opening mini.files (presumably any ignored buffer) and navigating around brings nvim to a halt after a few seconds.

[mvllow]

Would package.loaded be a drop-in replacement @fitrh?

diff --git a/plugin/modes.lua b/plugin/modes.lua
index cc97d14..8e95b79 100644
--- a/plugin/modes.lua
+++ b/plugin/modes.lua
@@ -1,7 +1,7 @@
-if vim.g.modes_loaded then
+if package.loaded["modes"] then
 	return
 end
 
-vim.g.modes_loaded = true
+package.loaded["modes"] = true
 
 require("modes").setup()

Edit: after reading :h package.loaded it seems that package.loaded is automatically set

Also, I'm curious why you are against enabling the plugin without explicitly calling setup? I quite like this pattern from standard vim and aside from convention I don't see any downsides—especially with package managers like lazy.nvim having an enable field.

Finally, once the colour handling is addressed/fixed the only remaining point I see is the use of _G. I can understand not wanting to pollute the global namespace and am not set on keeping this.

[fitrh]

Also, I'm curious why you are against enabling the plugin without explicitly calling setup?

There is no technical reason here, I just feel like that kind of behavior limits the user's freedom in how they want to load plugins (personally, for modes.nvim I would load it inside the ModeChanged autocmd and I don't need any plugin manager for that)

especially with package managers like lazy.nvim having an enable field

The dependency on package managers/package manager-oriented stuff is one thing I really don't like in this day neovim plugin ecosystem

It should be noted that I'm fine with all the changes, not against them, it's a matter of personal preference ❤️

[fitrh]

after reading :h package.loaded it seems that package.loaded is automatically set

Yes, we just need to check package.loaded

diff --git a/plugin/modes.lua b/plugin/modes.lua
index cc97d14..8e95b79 100644
--- a/plugin/modes.lua
+++ b/plugin/modes.lua
@@ -1,7 +1,7 @@
-if vim.g.modes_loaded then
+if package.loaded["modes"] then
 	return
 end
 
-vim.g.modes_loaded = true
 
 require("modes").setup()

[mvllow]

There is no technical reason here, I just feel like that kind of behavior limits the user's freedom in how they want to load plugins (personally, for modes.nvim I would load it inside the ModeChanged autocmd and I don't need any plugin manager for that)

That’s fair. This is why I asked you—I only know my workflow so it’s good to hear more/different preferences and I respect yours :)

The dependency on package managers/package manager-oriented stuff is one thing I really don't like in this day neovim plugin ecosystem

Yea I definitely wouldn’t want to rely on a specific package manager. I assume setting package.loaded['modes'] = true could be a workaround to load modes on your own terms? I’m not sure on the load order there.

Thanks again for your input, I really do appreciate it

[kamack38]

Any progress on this?

[mvllow]

After some quick testing, it looks like the colours are working. The line numbers are only highlighted if you set the color scheme after setting up modes, though.

@kamack38 you are welcome to use the canary branch and report any bugs here. If more people confirm that setting colours is working etc. then perhaps we can move this to the main branch.

Edit: Looks like I never got around to fixing the performance regression. This can be reproduced by opening mini.pick.

[mvllow]

Getting closer here. The performance regression should be fixed in da8b76b. I removed the global _G.Modes and am considering removing enable/disable behaviour simply to get this pushed through as that now seems broken. We'll see :)