echasnovski / mini.nvim

Library of 40+ independent Lua modules improving overall Neovim (version 0.8 and higher) experience with minimal effort
MIT License
4.54k stars 175 forks source link

Statusline flicker with `laststatus=3` #842

Closed wroyca closed 2 months ago

wroyca commented 2 months ago

Contributing guidelines

Module(s)

statusline

Description

When switching between buffers, mini.statusline resets to Neovim's default statusline and then redraws itself, resulting in significant flickering. This can be solved by removing the following autocommand: https://github.com/echasnovski/mini.nvim/blob/f75b954473b11dc98a82475d8d1cf4f84560791b/lua/mini/statusline.lua#L473

Neovim version

NVIM v0.10.0-dev+3027-gc3061a40f

Steps to reproduce

  1. nvim -nu minimal.lua
  2. Swap with w

Expected behavior

No flickering should happen, redraw should be extremely fast, e.g.:

https://github.com/echasnovski/mini.nvim/assets/91024200/4d5def24-6772-4c86-aca9-71758b92328f

Actual behavior

https://github.com/echasnovski/mini.nvim/assets/91024200/1e91f40a-2f93-4206-b5eb-d4ad432d2252

echasnovski commented 2 months ago

Thanks for the issue!

Unfortunately, I can not reproduce. Even after (what looks like a) more precise identification of a problem: temporarily shown text seems to be from inactive content. It is visible and perceived as flicker because a global statusline (laststatus = 3) is used. Even with global statusline I can not reproduce:

https://github.com/echasnovski/mini.nvim/assets/24854248/233b3b94-ca14-4f4f-8651-4d35e4d68907

My guess is that there is some plugin which not appropriately triggers event(s) on which 'mini.statusline' relies. Removing entirely those autocommands in a plugin is certainly not a good idea, as they power the main laststatus = 2 case.

Could you try only with 'mini.statusline' enabled?

wroyca commented 2 months ago

Thanks for the issue!

Unfortunately, I can not reproduce. Even after (what looks like a) more precise identification of a problem: temporarily shown text seems to be from inactive content. It is visible and perceived as flicker because a global statusline (laststatus = 3) is used. Even with global statusline I can not reproduce:

https://github.com/echasnovski/mini.nvim/assets/24854248/233b3b94-ca14-4f4f-8651-4d35e4d68907

My guess is that there is some plugin which not appropriately triggers event(s) on which 'mini.statusline' relies. Removing entirely those autocommands in a plugin is certainly not a good idea, as they power the main laststatus = 2 case.

Could you try only with 'mini.statusline' enabled?

Thank for looking into it! I'm at work for the whole day (busy day), but I'll take a closer look asap tomorrow or this week depending on how days goes :)

wroyca commented 2 months ago

Okay @echasnovski, got home early, here's what I observed:

I can actually reproduce with only 'mini.statusline' enabled, but it's racy - that is, when the line has its default component, it happens so fast that it "looks" like it isn't happening at all when in fact, it is. This is really hard, nearly impossible to observe. Now, once we start providing more custom components, it starts to take more time to draw everything and from there the flickering really shows itself. Perhaps we don't need %{%v:lua.MiniStatusline.inactive()%}?

If you want to stress-test a heavier component, here's one that integrates with Coc diagnostics and LSP Progress (disclaimer: not optimized):

function COC_DIAGNOSTICS_LIST()
  if pcall(vim.api.nvim_buf_get_var, 0, [[coc_diagnostic_info]]) then
    vim.cmd [[CocList diagnostics]]
  end
end

local function coc_diagnostics()
  local coc_initialized = pcall(vim.api.nvim_get_var, [[coc_service_initialized]])
  local has_info, info = pcall(vim.api.nvim_buf_get_var, 0, [[coc_diagnostic_info]])
  local diagnostics = [[]]

  -- Helper function to append diagnostics
  ---@diagnostic disable-next-line: redefined-local
  local function append_diagnostics(diagnostics, text)
    return diagnostics == [[]] and text or diagnostics .. [[ ]] .. text
  end

  -- Check for each type of diagnostic
  diagnostics = append_diagnostics(diagnostics, has_info and info["error"] and "%@v:lua.COC_DIAGNOSTICS_LIST@%#MiniStatuslineDiagnosticsError# " .. info["error"] or "")
  diagnostics = append_diagnostics(diagnostics, has_info and info["warning"] and "%@v:lua.COC_DIAGNOSTICS_LIST@%#MiniStatuslineDiagnosticsWarning# " .. info["warning"] or "")
  diagnostics = append_diagnostics(diagnostics, has_info and info["information"] and "%@v:lua.COC_DIAGNOSTICS_LIST@%#MiniStatuslineDiagnosticsInfo# " .. info["information"] or "")
  diagnostics = append_diagnostics(diagnostics, has_info and info["hint"] and "%@v:lua.COC_DIAGNOSTICS_LIST@%#MiniStatuslineDiagnosticsHint# " .. info["hint"] or "")

  -- If no diagnostics are present, handle CoC initialization status
  if diagnostics == "" then
    local coc_status = coc_initialized and vim.api.nvim_call_function('coc#status', {}) or ""

    -- lua-language-server sends two workspace/configuration requests to the
    -- LSP client: one with scopeUri being the workspace folder, and another
    -- with scopeUri being absent (or nil). The latter is for the fallback
    -- scope settings. Unfortunately CoC's LSP implementation completely
    -- ignores scopeUri and responds with the same settings to both, which
    -- causes luals to show two progress indicator.
    --
    -- The strategy here is transform duplicate characters into whitespace and
    -- then identify consecutive whitespace characters to disregard anything
    -- that appears after, assuming such would be from the fallback.
    --
    --   https://github.com/LuaLS/lua-language-server/issues/1596
    --
    local sanitize = function(input)
      local dup = {}
      local out = input:gsub("%S+", function(word)
        -- Checks if the current element word already exists in the dup data
        -- structure.
        if dup[word] then return ""

        -- If condition was not met, adds the element to the dup data
        -- structure, marking it as encountered.
        end dup[word] = ""
      end)

      -- Identify consecutive whitespace characters and disregard anything that
      -- appears after.
      return out:match("^(.-%s%s)")
    end

    vim.g.timer = coc_initialized and -1 or 0
    diagnostics = coc_initialized and (vim.bo.filetype == [[lua]] and sanitize(coc_status) or coc_status) or ""
    if coc_initialized then
      vim.cmd [[call timer_start(100, {-> execute(':let &stl=&stl')}, {'repeat': g:timer })]]
    end
  else
    vim.g.timer = 0
  end

  return diagnostics
end

return {
  [[mini.statusline]],
  lazy = false,
  opts = function()
    local MiniStatusline = require [[mini.statusline]]
    return {
      set_vim_settings = false,
      content = {
        active = function()
          local mode, mode_hl = MiniStatusline.section_mode({ trunc_width = 120 })
          local git = MiniStatusline.section_git({ trunc_width = 75 })
          local diagnostics = coc_diagnostics()
          local filename = MiniStatusline.section_filename({ trunc_width = 140 })
          local fileinfo = MiniStatusline.section_fileinfo({ trunc_width = 120 })
          local location = [[%2v|%-2{virtcol("$") - 1}]]
          local search = MiniStatusline.section_searchcount({ trunc_width = 75 })

          return MiniStatusline.combine_groups({
            { hl = mode_hl, strings = { mode } },
            "%<", -- Mark general truncate point
            { hl = [[MiniStatuslineDevinfo]], strings = { git, diagnostics } },
            "%=", -- End left alignment
            { hl = mode_hl, strings = { search, location } },
          })
        end
      },
    }
  end
}
echasnovski commented 2 months ago

If you want to stress-test a heavier component, here's one that integrates with Coc diagnostics and LSP Progress (disclaimer: not optimized):

I see. I did not test this particular case, but I just added vim.loop.sleep(20) to one of the components and can see slight flicker with laststatus = 3.

The following patch seems to fix the issue:

diff --git a/lua/mini/statusline.lua b/lua/mini/statusline.lua
index 9fc290c..f196388 100644
--- a/lua/mini/statusline.lua
+++ b/lua/mini/statusline.lua
@@ -511,9 +511,9 @@ end
 H.ensure_content = vim.schedule_wrap(function()
   -- NOTE: Use `schedule_wrap()` to properly work inside autocommands because
   -- they might temporarily change current window
-  local cur_win_id = vim.api.nvim_get_current_win()
+  local cur_win_id, is_global_stl = vim.api.nvim_get_current_win(), vim.o.laststatus == 3
   for _, win_id in ipairs(vim.api.nvim_list_wins()) do
-    vim.wo[win_id].statusline = win_id == cur_win_id and '%{%v:lua.MiniStatusline.active()%}'
+    vim.wo[win_id].statusline = (win_id == cur_win_id or is_global_stl) and '%{%v:lua.MiniStatusline.active()%}'
       or '%{%v:lua.MiniStatusline.inactive()%}'
   end
 end)

Would you be comfortable applying it to confirm if it works for you?

wroyca commented 2 months ago

If you want to stress-test a heavier component, here's one that integrates with Coc diagnostics and LSP Progress (disclaimer: not optimized):

I see. I did not test this particular case, but I just added vim.loop.sleep(20) to one of the components and can see slight flicker with laststatus = 3.

The following patch seems to fix the issue:

diff --git a/lua/mini/statusline.lua b/lua/mini/statusline.lua
index 9fc290c..f196388 100644
--- a/lua/mini/statusline.lua
+++ b/lua/mini/statusline.lua
@@ -511,9 +511,9 @@ end
 H.ensure_content = vim.schedule_wrap(function()
   -- NOTE: Use `schedule_wrap()` to properly work inside autocommands because
   -- they might temporarily change current window
-  local cur_win_id = vim.api.nvim_get_current_win()
+  local cur_win_id, is_global_stl = vim.api.nvim_get_current_win(), vim.o.laststatus == 3
   for _, win_id in ipairs(vim.api.nvim_list_wins()) do
-    vim.wo[win_id].statusline = win_id == cur_win_id and '%{%v:lua.MiniStatusline.active()%}'
+    vim.wo[win_id].statusline = (win_id == cur_win_id or is_global_stl) and '%{%v:lua.MiniStatusline.active()%}'
       or '%{%v:lua.MiniStatusline.inactive()%}'
   end
 end)

Would you be comfortable applying it to confirm if it works for you?

Yes, give me a moment :+1:

wroyca commented 2 months ago

If you want to stress-test a heavier component, here's one that integrates with Coc diagnostics and LSP Progress (disclaimer: not optimized):

I see. I did not test this particular case, but I just added vim.loop.sleep(20) to one of the components and can see slight flicker with laststatus = 3.

The following patch seems to fix the issue:

diff --git a/lua/mini/statusline.lua b/lua/mini/statusline.lua
index 9fc290c..f196388 100644
--- a/lua/mini/statusline.lua
+++ b/lua/mini/statusline.lua
@@ -511,9 +511,9 @@ end
 H.ensure_content = vim.schedule_wrap(function()
   -- NOTE: Use `schedule_wrap()` to properly work inside autocommands because
   -- they might temporarily change current window
-  local cur_win_id = vim.api.nvim_get_current_win()
+  local cur_win_id, is_global_stl = vim.api.nvim_get_current_win(), vim.o.laststatus == 3
   for _, win_id in ipairs(vim.api.nvim_list_wins()) do
-    vim.wo[win_id].statusline = win_id == cur_win_id and '%{%v:lua.MiniStatusline.active()%}'
+    vim.wo[win_id].statusline = (win_id == cur_win_id or is_global_stl) and '%{%v:lua.MiniStatusline.active()%}'
       or '%{%v:lua.MiniStatusline.inactive()%}'
   end
 end)

Would you be comfortable applying it to confirm if it works for you?

LGTM, I don't see flickering anymore :partying_face:

echasnovski commented 2 months ago

LGTM, I don't see flickering anymore 🥳

Great! I have the fix which I intend to re-review tomorrow and merge if everything is OK.

echasnovski commented 2 months ago

This should now be the part of main. Thanks for the early testing!