[mini.pick] extend MiniPick.default_choose_marked() opts so the user can change window height and position

[felpafel]

Hello, I want to propose a small feature to the MiniPick.default_choose_marked() so the user can control the quickfix/location list position and height/width.

Reasoning

copen behavior

Sometimes, :copen's split behavior is not desired by the user.
For example, at a vertical split :copen will open below the last created window instead of the current window.

---------                ---------               ---------
|       |                |   |   |               |   |   |
|       |                |   |   |               |   | B |
|   A   |  =`:vsplit`=>  | A | B |  =`:copen`=>  | A |---|
|       |                |   |   |               |   |QF |
|       |                |   |   |               |   |   |
---------                ---------               ---------
(The user create a new vertical split "B", move the cursor back to "A" and then call `:copen`, but the quickfix open below "B")

Vim solution to this problem

Vim solve this problem by letting the user combine :copen/:lopen with commands like :vertical, :horizontal and more (the full list of commands start with the tag :h vertical up to :h botright in the :h windows.txt file). It's even possible to combine multiple of those commands, like: :vertical topleft copen 120 (Open a vertical split at the left occupying the full height and width equals 120).

Current limitations using MiniPick.default_choose_marked()

It's not possible to specify the height before opening the quickfix/location list (It's possible to change the height after opened using an autocmd/filetype config).
The window position is always the default behavior of :copen/:lopen.

Solution

I know that is possible to override source.choose_marked with a user written function, but the burden of maintaining a identical function to MiniPick.default_choose_marked() is greater than a 3 lines of code change I want to propose.

Extend the opts parameter in MiniPick.default_choose_marked() so it accepts two more fields. One for the split behavior, and other for the width/height modifier.

Since copen and lopen are called only once in the codebase, the following changes suffice:

split and width_or_height are just a naming suggestion.
The example does not include changes in the documentation.
The tostring() calls are totally optional since string.format('%s', 2) will return "2" (as a string type).

-- From line 1189
opts = vim.tbl_deep_extend('force', { list_type = 'quickfix' }, opts or {})
-- To
opts = vim.tbl_deep_extend('force', { list_type = 'quickfix', split = '', width_or_height = ''}, opts or {})

-- and

-- From line 1225
vim.schedule(function() vim.cmd('lopen') end)
-- To
vim.schedule(function() vim.cmd(string.format('%s lopen %s', opts.split, tostring(opts.width_or_height))) end)

-- From line 1228
vim.schedule(function() vim.cmd('copen') end)
-- To
vim.schedule(function() vim.cmd(string.format('%s copen %s', opts.split, tostring(opts.width_or_height)))

Code usage (user side) comparison:

In the default case (require('mini.pick').setup()) there is no change in behavior and code.

-- Actual
local minipick = require('mini.pick')
minipick.setup({
    source = {
        choose_marked = function(items)
            minipick.default_choose_marked(items, { list_type = 'quickfix' })
        end
      }
  })

-- After changes

-- Make `quickfix` appears always at the bottom in any window layout, like:
-- ---------
-- |   | B |
-- | A |---|
-- |   | C |
-- ---------
-- |  QF   |
-- ---------

local minipick = require('mini.pick')
minipick.setup({
    source = {
        choose_marked = function(items)
            local len = math.min(#items, 5) -- Prevent split to get over 5 lines
            minipick.default_choose_marked(items, { list_type = 'quickfix', split = 'botright', width_or_height = len })
        end
      }
  })

I have tested the proposed solution locally without any issues.
Let me know what you think, and thanks for your time.

[abeldekat]

Hi @felpafel,

I read your post with interest. Thanks!

Sometimes, :copen's split behavior is not desired by the user.
For example, at a vertical split :copen will open below the last created window instead of the current window.

I can't reproduce that behavior using nightly. I tried with MiniMax, and also with an "empty" init.lua. The quickfix always opens below, occupying the full width.

I know that is possible to override source.choose_marked with a user written function, but the burden of maintaining a identical function to MiniPick.default_choose_marked() is greater than a 3 lines of code change I want to propose.

You are comparing maintenance by users versus maintenance by the mini.nvim team. Also, by allowing the user to override choose_marked, the user obtains much more flexibility in customizing the behavior.

Perhaps it is a bit too much to have both the override and the configuration options.

[felpafel]

Hello @abeldekat, thanks for the response.

Yeah, Nightly changed the default behavior compared to nvim stable and vim, but the window modifiers like :vertical copen 120 are still valid, and useful. My point is just: "The user may want to open the qf window in another position and/or size".

vertical_copen_120.mp4

Perhaps it is a bit too much to have both the override and the configuration options.

I didn't understand what you mean. The ability to override and the configuration options are already implemented, for example to send marked items to the location list (instead of the quickfix list) the user must do:

local minipick = require('mini.pick')
minipick.setup({
    source = {
        choose_marked = function(items)
            minipick.default_choose_marked(items, { list_type = 'location' })
        end
      }
  })

My issue is that the function call :copen/:lopen making it impossible to change the qf window position and size. And, from my understanding, the current solution is copying the default_choose_marked source code (with the helper functions used by it) to my config.

[abeldekat]

@drowning-cat

:vertical copen 30
:copen

Yes, I was also thinking about that 2 hours ago...:) We are in sync today...)

@felpafel,

Again, trying not to be rude, but "hacky" solutions are just hacky. Your example doesn't take in consideration, for example, that default_choose_marked() can parse the items arguments and return an empty list {} falling-back to the choose() function and not calling :copen at all

The code fragment shown above has an important implication: You can create a custom dedicated mapping especially for the quickfix/location list. See :h MiniPick-actions. Thus, when you are selecting items for the quickfix, use that mapping instead of builtin <M-CR>. No need to vim.schedule, just open the quickfix before MiniPick.default_choose_marked does.

... Again, trying not to be rude, but "hacky" solutions are just hacky

and also

... I don't want to create a back and forward of replies (or to be rude),

I think that when you post an idea in the discussions section, a discussion can be expected and can also be seen as a token of appreciation. I have had discussions about my ideas containing over 34 comments, with lots of suggestions. Writing "hacky solutions are just hacky" is not very delicate in my opinion...)

Also, MiniPick is one of the larger modules. Extra pickers are defined in mini.extras. One could argue that the job of MiniPick is to pick, not to facilitate advanced window layouts.

[felpafel]

@abeldekat English isn't my first language, I use terms "don't want to be rude" for lack of better terms in my explanations, any way I don't mean to offend anyone or be "that guy" that just want thing their way.

The code fragment shown above has an important implication: You can create a custom dedicated mapping especially for the quickfix/location list. See :h MiniPick-actions. Thus, when you are selecting items for the quickfix, use that mapping instead of builtin . No need to vim.schedule, just open the quickfix before MiniPick.default_choose_marked does.

As previously said, there is no good way to predict if the items argument will be parsed to a valid qflist without implementing the same H.parse_item functions. So pre-opening a qf window, and then checking if default_choose_marked put something in the qflist to decide if the window should remain open or be closed, and having to put in consideration that maybe the function took the choose() path... (I digress)

One could argue that the job of MiniPick is to pick, not to facilitate advanced window layouts.

I don't want advanced window layouts, if the default behavior was just setqflist/setloclist without opening the qf window I would have no issues.

What I'm arguing is, one of the behaviors of default_choose_marked is to open a qf window, if so let the user put a window modifier so its possible to control where the window will be open (and the implementation is just a single string concatenation nothing more, without changes in default behavior or any other side effect).

Another possible idea, is to have the opening (copen/lopen) decoupled from the setqflist/setloclist. On telescope.nvim they have two separated actions, one for populating the qflist and another one for opening it. (actions.send_selected_to_qflist and actions.open_qflist)

Thanks again.

[abeldekat]

I see that @echasnovski posted a better solution.

Just for the record, finishing my thoughts...)

As previously said, there is no good way to predict if the items argument will be parsed to a valid qflist without implementing the

There is, see :h MiniPick.default_choose_marked

The following code provides a custom dedicated mapping to invoke default_choose_marked when the user knows that the quickfix will be invoked:

---@diagnostic disable: undefined-global
vim.pack.add({ "https://github.com/nvim-mini/mini.nvim" })

require("mini.pick").setup({
  mappings = {
    choose_marked_quickfix = {
      char = "<M-q>",
      -- According to :h MiniPick.default_choose_marked(), quickfix is opened if
      -- marked items contain at least a file or buffer
      func = function()
        local pick_error = function(msg) error("(mini.pick) " .. msg, 0) end

        local choose_marked_override = function(items)
          if #items == 0 then return end

          vim.cmd("vertical copen 30")
          MiniPick.default_choose_marked(items)
        end

        local ok, res = pcall(choose_marked_override, MiniPick.get_picker_matches().marked)
        if not ok then vim.schedule(function() pick_error("Error during choose marked quickfix:\n" .. res) end) end
        return not (ok and res)
      end,
    },
  },
})

[felpafel]

The choose() path checks if list is size 0, not items you can see here. The picker can pass items that is { "foo", "bar" } and will not generate a qflist, and will fallback to the choose() function.

[abeldekat]

The choose() path checks if list is size 0, not items you can see here. The picker can pass items that is { "foo", "bar" } and will not generate a qflist, and will fallback to the choose() function.

Yes I know. The idea is that the user knows that the qflist will be invoked, as he marked either files or buffers.

I do also think that it's not ideal.

[drowning-cat]

It's not possible to specify the height before opening the quickfix/location list (It's possible to change the height after opened using an autocmd/filetype config).

While the guys from the team are responding to you, I have a simple question: is there any reason not to change the window properties after opening? Do you see the difference?

P.S. Sorry, I'll wait for your reply so I don't cause more confusion.

[abeldekat]

We are responding at the same time @drowning-cat...)

[abeldekat]

P.S. Sorry, I'll wait for your reply so I don't cause more confusion.

No problem at all!

[echasnovski]

Thanks for the suggestion!

As others noted, adding new configurations to any 'mini.nvim' module is usually a hard decision, but for 'mini.pick' it is especially so due to its size.

My initial reaction is that this might be reasonable for two reasons:

• The code change should indeed be rather small.
• There is a precedent for opts.default_split in help built-in picker.

My second reaction was that I don't think that 'mini.pick' has to even decide how to open quickfix/loclist. Is it reasonable for the user to have 'mini.pick' open quickfix/loclist differently than it is usually done? I think the answer is no. So the solution here should be to configure quickfix to always open as you'd prefer. For example, something like this should fix this problem:

local fix_quickfix = function(ev)
  local qf_data = vim.fn.getqflist({ qfbufnr = true, size = true, winid = true })
  if qf_data.qfbufnr ~= ev.buf then return end
  vim.api.nvim_win_call(qf_data.winid, function()
    vim.cmd('wincmd J')
    vim.api.nvim_win_set_height(0, math.min(qf_data.size, 5))
  end)
end
vim.api.nvim_create_autocmd('FileType', { pattern = 'qf', callback = fix_quickfix, desc = 'Fix quickfix' })

@felpafel, does this solve the issue for you?

[felpafel]

Is it reasonable for the user to have 'mini.pick' open quickfix/loclist differently than it is usually done?

Well, nvim nightly change the default behavior of :copen be in a split below the last opened window (Can be tested on nvim 0.11.6 and Vim) to always be :botright copen. There are arguments from both sides on what should be the behavior (Vim thinks window should be opened below last window), but I think this is a solved problem enabling a window modifier (:vertical, etc..).

There are other 2 solutions presented by me that are worth thinking. One is the solution in telescope.nvim where setqflist is decouple from copen so the user is able to override just the open action. The other solution is to expose the code that generate the qflist like vim.diagnostic.toqflist() so the user can set the qflist and open it be themselves.

@felpafel, does this solve the issue for you?

Sadly, but no. There is a way to know if the qflist was set in the first place? (maybe vim.schedule(function() if vim.bo.filetype == 'qf' then... end)). Second, the code is "fine" to force :botright behavior, but sometimes I want to open vertically right relative to the current cursor position, with make it more trick (Since nightly put it on the :botright position). And on my machine, I can see the window being created on the default location and them moving to the :botright position.

[echasnovski]

There is a way to know if the qflist was set in the first place?

The code is an autocommand that adjusts any new quickfix list. So it only acts when the quickfix list is created.

In short, I'll think about it, but my intuition says that deciding on how quickfix list is opened is not 'mini.pick' responsibility.

And it indeed can be worked around with custom action: call default_choose_marked() and then adjust the split+size manually. Detecting if quickfix was opened can also be done similarly as the above code snippet.

[felpafel]

As discussed previously, moving a window after it's creation isn't easy. I pointed that telescope, for example, handle this by not calling copen on the same function that sets the qflist, so the user can override just the opening action. The parsing of items in MiniPick is behind many helper functions, and is a lot of code to maintain on the user side if the user just want to modify the opening code.

[echasnovski]

As discussed previously, moving a window after it's creation isn't easy.

It should be relatively easy. There is a vim.api.nvim_win_call(qf_win_id, function() end) approach similar to above.

To move the window to be left/right/below/above some target window in split, use nvim_win_set_config(qf_win_id, { win = win_id, split = 'right' }).

[felpafel]

I could make it work using the nvim_win_set_config, the autocmd has to be aware that the target window is the MiniPick.get_picker_state().windows.target and it's value can only be acquired while the picker is opened.

It's not as nice as having proper modifiers as you mentioned the help picker has, but it's fine.

Thanks for you time.