feat: enhance height handling, add view argument to open()/toggle()

[drowning-cat]

1. Add new options

-- Restore the previous quickfix window view parameters, such as height and cursor position, on `quicker.open` and `quicker.toggle`
winrestore = {
  -- Restore the height of the quickfix window
  height = false,
  -- Respect the min_height and max_height arguments for `quicker.open` and `quicker.toggle`
  height_minmax = false,
  -- Restore cursor and scroll positions. See `:h winsaveview()`
  view = true,
},

2. Prevent the scroll position from jumping when the quickfix opens

recording-1.mp4

[drowning-cat]

Warning

There is also a bug to fix: quicker.open and quicker.toggle functions are unusable with the vertical split option due to incompatible properties:

• winrestore.height
• min_height
• max_height

require('quicker').toggle { open_cmd_mods = { vertical = true } }

[stevearc]

This kind of config and feature is likely to grow out of control in the future. It starts here, but then someone will ask for a way to restore the height for only certain quickfixes, or they want to clear the stored height/view when the quickfix changes, or they don't want to clear the height/view when the quickfix changes. I would much rather look at how the user can customize this in their own config, and then see if there are any primitives we can expose to make that simpler/easier.

Right now I think most of this should be easy to do in end-user config. Just set up a WinClosed autocmd, check if the buffer is quickfix, and store the height/view. The toggle and open commands already let you specify a height and min/max height, so that's pretty easy. The only thing that we could maybe do is make it slightly easier to restore the saved win view, and honestly maybe just returning the winid from open and toggle would be enough. What do you think?

[drowning-cat]

What you’re suggesting does make sense, but there are a lot of configurations required to implement saving the state for each window (give me a few more days).

For now, I’d like to point out a few things.

1. Rework height option

height should override min_height, max_height

1.1 Or remove min_height and max_height (optional, !BREAKING CHANGE)

Ignore

I suggest removing min_height and max_height since they don’t align with the command interface (:copen, :lopen, etc.) and must be set along with height.

Instead of this:

require('quicker').toggle { height = 10, max_height = 10, min_height = 10 }

I’d prefer something like this:

require('quicker').toggle { height = 10 }

-- local clamp = require('quicker.util').clamp
-- local minmax = require('quicker.util').minmax
local clamp = function(min, val, max)
  return math.max(min, math.min(val, max))
end

require('quicker').toggle {
  focus = true,
  height = clamp(4, #vim.fn.getqflist(), 10)
}

By default, height should be set using:

{
  height = clamp(4, number_of_entries, 10)
}

---

Or perhaps, as an alternative interface, height could be implemented as a function

height = function(entries)
  return require('quicker.util').clamp(4, #entries, 10)
end

2. Add width option (OUTDATED)

Ignore

I’d also recommend adding a width option that behaves similarly to height but should be ignored if not set (no default value).

For example:

require('quicker').toggle {
  -- height is ignored for full height
  width = 20,
  open_cmd_mods = { vertical = true }
}

3. Manage view restoration via plugin (OUTDATED)

Ignore

I think view restoration should be handled by the plugin itself, restoring the view for each window (btw, my current implementation is stupid).

This should be the default behavior because it’s much easier to press g or gg to navigate within the quickfix list than to manually find a specific position.

It could also lead to exposing utility functions or events to help replicate similar behavior for restoring a window's height.

---

There’s no rush—I just want to recreate this from a user’s perspective and will paste a solution here for further discussion.
Maybe it's not as scary as I think.

[drowning-cat]

Preserve the height and cursor position when toggling the quickfix or location list window:

Code

---@alias WinSaveDict { height: integer, view: vim.fn.winsaveview.ret }

---@type table<integer, WinSaveDict>
-- I believe a table like { 0 = {}, 1000 = {}, 25 =  {} } will not cause any trouble
local win_config = {}

local get_qfid = function(winid)
  -- `getloclist` and `getqflist` both return 0 if no list is defined. However, you cannot open an empty location list.
  -- This means the ID is 0 only for an empty quickfix list.
  -- We may clear it after defining the first quickfix list.

  local wintype = vim.fn.win_gettype()
  if wintype == 'loclist' then
    return vim.fn.getloclist(winid, { id = 0 }).id
  end
  if wintype == 'quickfix' then
    return vim.fn.getqflist({ id = 0 }).id
  end
end

local save_qf_window = function()
  local winid = vim.fn.win_getid()
  local qfid = get_qfid(winid)
  -- Quickfix and location list IDs do not overlap.
  -- Yay, we can simplify the code.
  win_config[qfid] = {
    height = vim.api.nvim_win_get_height(winid),
    view = vim.fn.winsaveview(),
  }
  -- Optional. Remove `win_config[0]` after defining the first quickfix list (not a location list)
  if vim.fn.win_gettype() == 'quickfix' and qfid ~= 0 then
    table.remove(win_config, 0)
  end
end
local restore_qf_window = function()
  local winid = vim.fn.win_getid()
  local qfid = get_qfid(winid)
  if win_config[qfid] then
    vim.api.nvim_win_set_height(winid, win_config[qfid].height)
    vim.fn.winrestview(win_config[qfid].view)
  end
end

-- :cclose
vim.api.nvim_create_autocmd('WinClosed', {
  callback = function()
    if vim.bo.ft == 'qf' then
      save_qf_window()
    end
  end,
})

-- Optional. Prevent restoring the quickfix window twice
local prevent_qf_next_schedule = false

-- :copen; :cnewer, :colder
vim.api.nvim_create_autocmd('BufReadPost', {
  pattern = 'quickfix',
  callback = function()
    -- Scheduling is required. I'm not sure how to eliminate it. It's causing annoying height and cursor jumps.
    -- To prevent such jumping for `quicker.toggle`, I need to write a lot of garbage code (refer to quicker_toggle).
    -- However, I’m unable to resolve the jumping issue for commands such as :copen, :cnewer, and :colder.
    vim.schedule(function()
      if not prevent_qf_next_schedule then
        restore_qf_window()
      end
      prevent_qf_next_schedule = false
    end)
  end,
})

---@param opts quicker.OpenOpts
local quicker_toggle = function(opts)
  local qfwinid = vim.fn.getqflist({ winid = 0 }).winid
  local qfid = vim.fn.getqflist({ id = 0 }).id

  -- Restore height

  local height = (win_config[qfid] or {}).height or 10

  -- height = clamp(4, height, 16)

  opts.min_height = opts.height or height
  opts.max_height = opts.height or height
  opts.height = opts.height or height

  require('quicker').toggle(opts)

  -- Restore view

  local is_open = qfwinid ~= 0
  local view = (win_config[qfid] or {}).view

  if is_open and view then
    if not opts.focus then
      local goback_winid = vim.fn.win_getid()
      vim.fn.win_getid(qfwinid)
      vim.fn.winrestview(view)
      vim.fn.win_gotoid(goback_winid)
    else
      vim.fn.winrestview(view)
    end
  end

  prevent_qf_next_schedule = true
end

return {
  {
    'stevearc/quicker.nvim',
    ft = 'qf',
    opts = {},
    keys = {
      {
        '<leader>q',
        function()
          quicker_toggle { focus = true }
        end,
        desc = 'Toggle [q]quickfix list',
      },
      {
        '<leader>l',
        function()
          quicker_toggle { loclist = true, focus = true }
        end,
        desc = 'Toggle [l]oclist list',
      },
    },
  },
}

Code (refactored, updated)

---@type table<integer, { loclist: boolean, height: integer, view: quicker.WinViewDict }>
local win_config = {}

---@param loclist? boolean
local get_qfid = function(loclist)
  if loclist then
    return vim.fn.getloclist(0, { id = 0 }).id
  end
  return vim.fn.getqflist({ id = 0 }).id
end

---@param winid? integer
local is_loclist = function(winid)
  return vim.fn.win_gettype(winid) == 'loclist'
end

-- :q, :cclose
vim.api.nvim_create_autocmd('WinClosed', {
  callback = function(args)
    if vim.bo[args.buf].ft ~= 'qf' then
      return
    end
    -- NOTE: WinClosed event does not guarantee the window is focused
    local qfwinid = tonumber(args.match) --[[@as integer]]
    local loclist = is_loclist(qfwinid)
    local qfid = get_qfid(loclist)
    -- NOTE: Quickfix and location list IDs do not overlap
    win_config[qfid] = {
      loclist = loclist,
      height = vim.api.nvim_win_get_height(qfwinid),
      view = vim.api.nvim_win_call(qfwinid, vim.fn.winsaveview),
    }
    -- NOTE: Set the same height for every quickfix window.
    -- Remove the loop if you want each quickfix window to have its own unique height.
    for _, wconf in pairs(win_config) do
      -- NOTE: Set the same height within each group: quickfix and loclist
      if loclist == wconf.loclist then
        wconf.height = win_config[qfid].height
      end
    end
  end,
})

-- HACK: Restoring `view` from the event is so slow it may cause the cursor to jump after user input
local prevent_auto_restore = false

-- :copen; :cnewer, :colder
vim.api.nvim_create_autocmd('BufReadPost', {
  pattern = 'quickfix',
  callback = function()
    if prevent_auto_restore then
      return
    end
    local qfwinid = vim.fn.win_getid()
    local qfid = get_qfid(is_loclist())
    local wconf = win_config[qfid]
    if not wconf then
      return
    end
    -- NOTE: Restore `view`
    -- HACK: The `view` (cursor) cannot be restored from the event without `vim.schedule`
    vim.schedule(function()
      -- NOTE: The quickfix window may not be focused when `vim.schedule` is called
      vim.api.nvim_win_call(qfwinid, function()
        vim.fn.winrestview(wconf.view)
      end)
    end)
    -- NOTE: Restore `height`
    -- Skip if `:copen <count>`
    if not vim.fn.histget('cmd', -1):find 'copen? ?%d+' then
      vim.api.nvim_win_set_height(qfwinid, wconf.height)
    end
  end,
})

---@param opts quicker.OpenOpts
local quicker_toggle = function(opts)
  local qfid = get_qfid(opts.loclist)
  local wconf = win_config[qfid] or {}
  prevent_auto_restore = true
  require('quicker').toggle(vim.tbl_extend('keep', opts, {
    focus = true, -- NOTE: false causes scroll jump
    height = wconf.height,
    view = wconf.view,
  }))
  prevent_auto_restore = false
end

return {
  'stevearc/quicker.nvim',
  ft = 'qf',
  version = false,
  ---@module "quicker"
  ---@type quicker.SetupOptions
  opts = {},
  -- stylua: ignore
  keys = {
    { '<leader>q', function() quicker_toggle {} end, { desc = 'Toggle [q]quickfix' } },
    { '<leader>l', function() quicker_toggle { loclist = true } end, { desc = 'Toggle [l]oclist' } },
  },
}

[drowning-cat]

Ok, in addition to the #38 (comment) option, I would also add a view option to open/toggle:

require('quicker').open {
  ---@type vim.fn.winrestview.dict
  view = {
    lnum = 20,
  },
}

Warning

You can also reset the view using vim.fn.winrestview inside vim.api.nvim_win_call(winid, fn) after toggling.
However, if focus=false, a bug occurs where the cursor position is not updated until the window gains focus.
To avoid such behavior, it is recommended to use the view option instead.

[drowning-cat]

I will also rework how the height property is set. It's better to set it via command arguments.
For example:

• :botright 40 copen will set the height
• :botright 30 copen will change the height of the opened window
• :vert botright 40 copen will not only open the window vertically but also set the width (not height)
• :vert botright 30 copen will change the width of the vertically opened window

---

vim.cmd.copen { count = <height>, ... }

[drowning-cat]

You're right; the winrestore feature should not be part of the plugin itself.

---

Here is a list of small changes I will apply instead in this PR:

• Use count to set height in open/toggle.
• height should override min_height and max_height.
  Note: You can use the clamp utility if you want to constrain the height.
• Increase the default value of max_height (from 10 to 16)
  Note: Even for my 14" laptop, 10 is too small. The values are symmetric about 10: 4+6 = 10 and 16-6 = 10.
• Add the view argument to open/toggle.
  Note: on_qf and BufReadPost require vim.schedule to work, which causes a cursor jump.
  Note: Prefer using the view option over calling vim.fn.winrestview inside vim.api.nvim_win_call after a toggle to avoid buggy cursor behavior when focus=false.
• Update the documentation.
• Perform manual testing.

---

Not related to the PR:

• Constrain the cursor on the very first load.
  • Try to remove scheduling from constrain_cursor().
  • Prevent event pollution.

[drowning-cat]

Caution

Function constrain_cursor(), which defines 3 events, is triggered by the FileType event. This results in many events being produced each time the same quickfix list is toggled, even though the buffer for the quickfix list does not change over time.

The same applies to the setup_editor() function.

I don't want to change this logic in this PR.

Can be patched with

---Utility that checks if an event(s) is already defined
---@param aug nil|integer|string
---@param bufnr nil|integer|integer[]
---@param ... string
M.event_defined = function(aug, bufnr, ...)
  return #vim.api.nvim_get_autocmds({
    group = aug,
    buffer = bufnr,
    event = { ... },
  }) > 0
end

local util = require("quicker.util")

---@param bufnr number
function M.constrain_cursor(bufnr)
  -- HACK: we have to defer this call because sometimes the autocmds don't take effect.
  vim.schedule(function()
    if not vim.api.nvim_buf_is_valid(bufnr) then
      return
    end
    local aug = vim.api.nvim_create_augroup("quicker", { clear = false })

    if util.event_defined(aug, bufnr, "InsertEnter", "CursorMoved", "ModeChanged") then
      return
    end
 
    vim.api.nvim_create_autocmd("InsertEnter", {
      desc = "Constrain quickfix cursor position",
      group = aug,
      nested = true,
      buffer = bufnr,
      -- For some reason the cursor bounces back to its original position,
      -- so we have to defer the call
      callback = vim.schedule_wrap(constrain_cursor),
    })
    vim.api.nvim_create_autocmd({ "CursorMoved", "ModeChanged" }, {
      desc = "Constrain quickfix cursor position",
      nested = true,
      group = aug,
      buffer = bufnr,
      callback = constrain_cursor,
    })
  end)
end

Can be patched with (bad)

local defined_events_for = {}

---@param bufnr number
function M.constrain_cursor(bufnr)
  vim.schedule(function()
    -- I would also move these checks outside of `vim.schedule`
    if not vim.api.nvim_buf_is_valid(bufnr) then
      return
    end

    -- Alternatively, it can be done in more reliable ways:
    --  - by deleting events each time before defining
    --  - by checking if the event already exists in the autogroup
    if defined_events_for[bufnr] then
      return
    end

    local aug = vim.api.nvim_create_augroup("quicker", { clear = false })
    vim.api.nvim_create_autocmd("InsertEnter", {
      desc = "Constrain quickfix cursor position",
      group = aug,
      nested = true,
      buffer = bufnr,
      callback = vim.schedule_wrap(constrain_cursor),
    })
    vim.api.nvim_create_autocmd({ "CursorMoved", "ModeChanged" }, {
      desc = "Constrain quickfix cursor position",
      nested = true,
      group = aug,
      buffer = bufnr,
      callback = constrain_cursor,
    })
    
    defined_events_for[bufnr] = true
  end)
end

return M

---

Note

There may also be a check to not run the same logic in higher level events for the same buffer.

[drowning-cat]

The scheduling in constrain_cursor() does shift event order and may cause some troubles with setting the view (for future).

And it also should be named like setup_constrain_cursor() (the same applies for other functions).

[drowning-cat]

The issues #38 (comment) and #38 (comment) are not related to the current PR.

I’m done working on this PR (see #38 (comment)).

Apologies for the lengthy or unrelated comments.

[stevearc]

Current approach LGTM. Just fix the merge conflicts and we're good to go

[drowning-cat]

I am a little bit confused because resolving conflicts (merging master branch) resulted in an empty commit.
However, the merge conflicts have been resolved.

[stevearc]

I think there may be an issue with how you resolve merges, because if all you saw was an empty commit then that's quite wrong. The merge deleted some logic that I added to master to handle the case when opening the quickfix as a vsplit

[drowning-cat]

I changed the merge process. Instead of "resolving" conflicts in the merge commit, I added parts of the code during the merge and then removed them with separate commits (the same result in a different form).

TL;DR
My branch has a diverged history (started one commit before 049def7), and I don't want to rebase because it would require resolving a ton of conflicts to preserve the history of changes. In the end, I would still need to delete all the code merged from upstream/master.

Sorry for wasting so much time on such a trivial PR.

[stevearc]

Great, looks good now. Thanks for the PR!