possible regression / API breakage / lack of deprecation warnings: 'opts.window' not being respected since commit eb3ad2eb

[sleeptightAnsiC]

Did you check docs and existing issues?

• I have read all the which-key.nvim docs
• I have searched the existing issues of which-key.nvim
• I have searched the existing issues of plugins related to this issue

Neovim version (nvim -v)

NVIM v0.10.0 Build type: Release LuaJIT 2.1.1720049189

Operating system/version

6.9.8-arch1-1 x86_64 GNU/Linux

Describe the bug

This might not be a bug but I'll report it anyway as it is a super annoying and possibly a bigger problem. Please, take your time to read it (even though my report may seem rude, I didn't mean it, apologies...)

window options were valid back in v2.1.0 according to readme:

which-key.nvim/README.md

Lines 123 to 130 in 0539da0

	window = {
	border = "none", -- none, single, double, shadow
	position = "bottom", -- bottom, top
	margin = { 1, 0, 1, 0 }, -- extra window margin [top, right, bottom, left]. When between 0 and 1, will be treated as a percentage of the screen size.
	padding = { 1, 2, 1, 2 }, -- extra window padding [top, right, bottom, left]
	winblend = 0, -- value between 0-100 0 for fully opaque and 100 for fully transparent
	zindex = 1000, -- positive value to position WhichKey above other floating windows.
	},

in v3.3.0 all functionality still seems to be there, just a little bit changed:

which-key.nvim/README.md

Lines 128 to 145 in f0e99d4

	win = {
	-- don't allow the popup to overlap with the cursor
	no_overlap = true,
	-- width = 1,
	-- height = { min = 4, max = 25 },
	-- col = 0,
	-- row = math.huge,
	-- border = "none",
	padding = { 1, 2 }, -- extra window padding [top/bottom, right/left]
	title = true,
	title_pos = "center",
	zindex = 1000,
	-- Additional vim.wo and vim.bo options
	bo = {},
	wo = {
	-- winblend = 10, -- value between 0-100 0 for fully opaque and 100 for fully transparent
	},
	},

So before around eb3ad2e using the repro.lua provided in this report was fine, after said patch the options are just being ignored. There is no depreciation message about this, nothing appears in :mess and :checkhealth which-key that would indicate something being wrong with the options, nothing even in ~/wk.log. Latest README tells only about mappings spec changed in v3, nothing about options. Just the the WhichKey prompt appears as the default one.

If this is not intentional regression then I'm happy that I took my time to report it 😃
but if it's NOT, then please take into consideration that I had to:
(a) sandbox the plugin, (b) test which options were broken, (c) see recent changelogs, (d) bisect the git repo, (e) read the documentation yet again, (f) report this.
If I did all of this just to find out that the API has changed, then I have a really bad day today 😢

It would be really nice to prevent such situations in the future. I understand that there were 4 major releases in past month that could potentially break the API but this is just a terrible user experience.

Steps To Reproduce

1. copy paste the attached lua script into ~/repro.lua
2. $ nvim --clean -u ~/repro.lua
3. press any key to invoke whichkey, e.g. v

Expected Behavior

WhichKey should honor opts.window, OR there should be any kind of deprecation warning mentioning that said option has been dropped

Health

==============================================================================
which-key: require("which-key.health").check()

- OK Most of these checks are for informational purposes only.
  WARNINGS should be treated as a warning, and don't necessarily indicate a problem with your config.
  Please |DON't| report these warnings as an issue.
- WARNING |mini.icons| is not installed
- WARNING |nvim-web-devicons| is not installed
- WARNING Keymap icon support will be limited.

Checking for issues with your mappings ~
- OK No issues reported

checking for overlapping keymaps ~
- WARNING In mode `n`, <gc> overlaps with <gcc>:
  - <gc>: Toggle comment
  - <gcc>: Toggle comment line
- OK Overlapping keymaps are only reported for informational purposes.
  This doesn't necessarily mean there is a problem with your config.

Checking for duplicate mappings ~
- OK No duplicate mappings found

Log

Debug Started for v3.3.0
on_key: v
ModeChanged(n:v)
State(start): { "Mode(x)", "Node()", { update = true } }
  continue: { "", "Mode(x)" }
  getchar
  on_key: <Esc>
  got: <Esc>
ModeChanged(v:n)
not safe
on_key: Z
on_key: Z

Repro

local lazy_plugins = {
	"folke/which-key.nvim",
	lazy = false,
	opts = {
		window = {
			border = "shadow",
			position = "top",
			margin = { 3, 3, 3, 3 },
			padding = { 3, 3, 3, 3 },
			winblend = 50,
			zindex = 100,
		},
		debug = true,
	}
}

local lazy_path = vim.fn.stdpath("data") .. "/lazy/lazy.nvim"
if not vim.loop.fs_stat(lazy_path) then
	assert(vim.system({"git", "--version"}):wait().code == 0, "where's git, brah?")
	vim.fn.system({ "git", "clone", "--filter=blob:none", "--branch=stable", "https://github.com/folke/lazy.nvim.git", lazy_path})
end
vim.opt.rtp:prepend(lazy_path)
require("lazy").setup(lazy_plugins)

[folke]

First of all, it does indeed make sense to show a message when using an old option.

But I don't get:

I had to:
(a) sandbox the plugin, (b) test which options were broken, (c) see recent changelogs, (d) bisect the git repo, (e) read the > documentation yet again, (f) report this.
If I did all of this just to find out that the API has changed, then I have a really bad day today 😢

The valid options are all clearly documented in the config section of the readme.
Anything that is not there is not supported.

[sleeptightAnsiC]

Hey @folke,

But I don't get:

I had to:
(a) sandbox the plugin, (b) test which options were broken, (c) see recent changelogs, (d) bisect the git repo, (e) read the > documentation yet again, (f) report this.
If I did all of this just to find out that the API has changed, then I have a really bad day today 😢

The valid options are all clearly documented in the config section of the readme. Anything that is not there is not supported.

This was just to explain why "read the friendly manual" is not always so obvious and simple, and that I DID in fact read it. Due to the lack of deprecation message I thought it's more possible there was a regression than the API has changed. After all (if I understand right) the docs are assembled by CI so this was possible.

Anyway, you've accepted the my ticket, so thanks for taking care of this man!

Fix confirmed on my side:

[max397574]

also the healtheck (:checkhealth which-key) is really useful here

[sleeptightAnsiC]

:checkhealth still doesn't tell about deprecation, at least not on my side.
Even after 81413ef output looks exactly the same as the one attached to this ticket.

[folke]

I'll add it to healthcheck too, but it already shows when opening

[max397574]

:checkhealth still doesn't tell about deprecation, at least not on my side. Even after 81413ef output looks exactly the same as the one attached to this ticket.

well I wasn't referring to this specific thing
just in general healthcheck might be something useful to figure out if something changed
and otherwise just read the new docs 🤷