b0o / incline.nvim

🎈 Floating statuslines for Neovim, winbar alternative
MIT License
764 stars 14 forks source link

[Bug] Winhighlight is passed on to quickfix window #4

Closed akinsho closed 2 years ago

akinsho commented 2 years ago

Hi 👋🏾,

Big fan on your work on this plugin, been really enjoying using it 💯 Today I noticed a weird bug which I have seen once or twice before but repeated several times for me today. So I have incline configured as

    use {
      'b0o/incline.nvim',
      config = function()
        require('incline').setup {
          hide = {
            focused_win = true,
          },
          window = {
            options = {
              winhighlight = 'Normal:Search',
            },
          },
        }
      end,
    }

and have also set up nvim-bqf using

use {
      'kevinhwang91/nvim-bqf',
      ft = 'qf',
    }

I've noticed today that when I run a grep command which opens the quickfix list, it takes on the winhighlight of incline. I don't know whether the magic window bqf creates trips up incline somehow or what. It's only really noticeable for me now because I changed the winhiglight to quite an unmissable highlight colour.

Let me know if this isn't enough to reproduce it. I'm hoping it's really just running any quickfix command with both plugins setup and the winhighlight will be enough to trigger the issue

b0o commented 2 years ago

This is very interesting. It seems to be a Neovim bug. I can reproduce it with no plugins, like so:

  1. nvim --clean
  2. :lua vim.api.nvim_win_set_option(vim.api.nvim_open_win(vim.api.nvim_create_buf(false, true), false, { relative = "editor", width = 1, height = 1, row = 1, col = 1 }), "winhighlight", "Normal:Search")
  3. :copen

I've opened a neovim issue: https://github.com/neovim/neovim/issues/18283

b0o commented 2 years ago

By the way, I will be pushing quite a few changes soon, including better highlight support so that you won't need to use winhl. And thank you very much for the kind words!

sindrets commented 2 years ago

@b0o like you've figured out, the problem is caused by the fact that both nvim_win_set_option and nvim_buf_set_option do not behave like :setlocal, even though that's how they're intended to work. I was fighting with this bug for a while in my plugin diffview. Essentially, contrary to when using :setlocal, setting options in a window A with nvim_win_set_option and then creating a new window B from that window, B will inherit all window options from A including those set with nvim_win_set_option.

I found that the simplest way to get around this is to just create a wrapper around :setlocal. I've been using this wrapper in both my plugins, and I've had no problems with this bug ever since (this code uses some ugly polymorphic parameters, but I just really enjoy the convenience of overload signatures).

b0o commented 2 years ago

Hey @sindrets, thank you for sharing! I just wrote a wrapper inspired by yours, it seems to have fixed the issue for me.

Before I merge those changes into main, I am curious to know what the purpose of no_win_event_call is, and if it is necessary in this case? I just tested the following to see if nvim_win_call triggers those events, but when I run these two commands I don't see any "event" messages:

sindrets commented 2 years ago

@b0o Ah, it seems you're right: setlocal doesn't seem to trigger any of these events, and it's probably only there due to an oversight. Thank you for pointing that out!

Finding out what does and doesn't trigger these events can be a bit surprising sometimes, so I've probably used this function a bit too liberally. It has notably improved performance for me in instances where I'm changing buffers multiple times in multiple windows over a short time span.

b0o commented 2 years ago

@sindrets Cool, just wanted to make sure!

@akinsho I've merged the fix into main, would you mind giving it a try for me?

akinsho commented 2 years ago

@b0o thanks for looking into this and raising the issue in neovim core 👍🏿 , just tried out the new API, and it works great, doesn't leak into the quickfix any more 💯. Will close this out now then.