feat: recalculate picker layout on VimResized

[l-kershaw]

• implements Picker:recalculate_layout function
• adds an autocommand to call this function on the VimResized event
• relies on feat: allow Borders and popups to be resized nvim-lua/plenary.nvim#180 and WIP: allow popups to be resized nvim-lua/popup.nvim#15

Currently has issues when the results window changes in height.

Demo:

As shown in the demo, the picker still works fine when the results window is the same height, but when the height increases, you can no longer change the selection. In other tests, when the height decreases, you can change the selection but the displayed list is in the wrong position and updates as you select each one.

I will have a go at fixing the results window issues, but thought it would be good to get feedback on what is done so far 🙂

[Conni2461]

My first thought that all we need to do is run picker:__on_lines(nil, nil, nil, 0, 1) after picker:recalculate_layout() but that did not work :| i will take a closer look at it later today. :) But looks already promising good job

[l-kershaw]

I think this may require some fiddling with EntryManager and LinkedList as we would want to increase the number of entries that we have sorted when the height of the results window increases. It will probably require updating things like entry_manager.max_results, entry_manager.worst_acceptable_score, linked_list.track_at, linked_list._tracked_node, linked_list.tracked.

@Conni2461 could you have a look through #927 and make sure that I haven't misunderstood anything before I change stuff there? Don't want to start implementing stuff with errors in my understanding 😅

[tjdevries]

It could be possible that some of the optimizations in EntryManager are no longer necessary given that we've switched to more async design. There is one step I'd like to do before removing those which is switchign to new async style for both plenary jobs & a few things in telescope made by ober in plenary.

After we do that, it's possible that this could be greatly simplified because we could just always sort the entries and just refresh the display that way.

(this may not make a lot of sense, I am just typing fast 😆 )

[l-kershaw]

I'm happy to wait to finalise the implementation for this until after the async changes.

On a related note, could you point me in the direction of where I can find information about the new async stuff? I've seen a few people referring to "async2" but I can't figure out where to find out about it 😅

[Conni2461]

This is async await 2: nvim-lua/plenary.nvim#148

tj just suggested. Lets put the new async into plenary.async rather than plenary.async_lib2 https://github.com/nvim-lua/plenary.nvim/tree/master/lua/plenary/async

and we keep the original async, till all plugins moved to the new variant. For example gitsigns already switched to the new version

[KodyVB]

Sorry if this isn't the place to ask, but could someone give me a rough estimate of when this would get merged to the project? I've never gotten experience with big projects like this before, so I just don't know if I should be expecting it anywhere from "any day now" to "maybe in a few months". Anyways, keep up the good work, y'all.

[l-kershaw]

I am currently waiting for the big async rewrite that TJ is working on here.
"Asyncifying" the handling of the results list will likely make it much easier to fix the issues when the results window changes height. Therefore its not worth writing a solution for these problems until after the rewrite is complete.

In terms of timeline, I'm not really sure sorry. I would expect this PR to take less than a week once the async rewrite is done, but I can't really tell how long that will take.

@KodyVB I hope this answers your question 🙂

[KodyVB]

@l-kershaw I see, thanks for the reply!

[l-kershaw]

Updated demo for this PR:

I think I have now fixed all the issues I am aware of for this.
There are a few places where the implementation might not be 100% efficient, but there were lots of edge cases that came up that were hard to cover.

If people want to test this PR and give feedback that would be great 😊
Make sure you update your plenary as well first, as you need the popup.move implementation that was merged earlier today.

---

@Conni2461 @tjdevries I would be particularly interested if either of you guys have any thoughts on this PR 🙂

[fdschmidt93]

Just a few clean up comments; otherwise, great job :) looking forward to this!

E: I'll stress test this latest on Sunday :)

[fdschmidt93]

I think entry adding can be simplified quite a bit when thinking of my experience from caching pickers.

Have a look at this segment here:

telescope.nvim/lua/telescope/pickers.lua

Lines 451 to 466 in d1e9655

	local index = 1
	for entry in self.manager:iter() do
	self:entry_adder(index, entry, _, true)
	index = index + 1
	end
	self.cache_picker.is_cached = false
	-- if text changed, required to set anew to restart finder; otherwise hl and selection
	if self.cache_picker.cached_prompt ~= self.default_text then
	self:reset_prompt()
	self:set_prompt(self.default_text)
	else
	-- scheduling required to apply highlighting and selection appropriately
	await_schedule(function()
	self:highlight_displayed_rows(self.results_bufnr, self.cache_picker.cached_prompt)
	if self.cache_picker.selection_row ~= nil then
	self:set_selection(self.cache_picker.selection_row)

[l-kershaw]

I have refactored find and recalculate_layout slightly to use a helper function called _create_window.
This helper function is based on helper functions in #1305 by @johnybx (e.g. _create_results_window), but tweaked so that we only need one function instead of three (thanks for the inspiration johny 🙂).

This makes it easier to standardise the creation of picker windows in both find and recalculate_layout.
@tjdevries is this sufficient in terms of your request for shared code between the two?

---

I have also added a vague plan for toggle_padding to be used when you need to maximise the size of the picker.
This would probably be a good first PR for anyone who wants to try it; I'd be more than happy to mentor someone on this 🙂

[fdschmidt93]

I have also added a vague plan for toggle_padding to be used when you need to maximise the size of the picker.
This would probably be a good first PR for anyone who wants to try it; I'd be more than happy to mentor someone on this

You can add this to #1190 if you want :)

[l-kershaw]

You can add this to #1190 if you want :)

Done! Thanks for the reminder @fdschmidt93 🙂

[johnybx]

@l-kershaw in meantime I found out that you can even pass these highlight directly to popup.create

function Picker:_get_window_options()
  local line_count = vim.o.lines - vim.o.cmdheight
  if vim.o.laststatus ~= 0 then
    line_count = line_count - 1
  end

  local popup_opts = self:get_window_options(vim.o.columns, line_count)

  popup_opts.results.minheight = popup_opts.results.height
  popup_opts.results.highlight = "TelescopeNormal"
  popup_opts.results.borderhighlight = "TelescopeResultsBorder"
  popup_opts.prompt.minheight = popup_opts.prompt.height
  popup_opts.prompt.highlight = "TelescopeNormal"
  popup_opts.prompt.borderhighlight = "TelescopePromptBorder"
  if popup_opts.preview then
    popup_opts.preview.minheight = popup_opts.preview.height
    popup_opts.preview.highlight = "TelescopePreviewNormal"
    popup_opts.preview.borderhighlight = "TelescopePreviewBorder"
  end

  return popup_opts
end

function Picker:_create_window(bufnr, opts)
  local what = bufnr or ""
  local win, win_opts = popup.create(what, opts)

  a.nvim_win_set_option(win, "wrap", false)
  a.nvim_win_set_option(win, "winblend", self.window.winblend)
  return win, win_opts
end

I think that could be even better as all the highlight can be specified once in one place 🤔

[l-kershaw]

I made some changes to simplify how highlights are set, as suggested by @johnybx.
I seem to have messed up somewhere and broken something for the tests in the process.
I'll have an in depth look at this later.

[johnybx]

@l-kershaw probably the problem with tests is caused by my suggestion - I didn't realize that borderhighlight cannot be specified while border=false so there need to be check like this -> bd6f6ae

[l-kershaw]

I have pushed a change to plenary that fixes that issue that @johnybx explained above.
This check is now handled on the plenary side, and behavior around this for popups matches that of vim now.
This means that we can keep the highlighting options in the simplified form we have here.

[l-kershaw]

Actually, realised that I didn't need all of the rerendering bit thanks to the scrolling PR, as there are like 1000 results in the buffer, and no one is going to have a window that large😅

Also factored out the scrolling position updater to make it easier to manage and to handle different sorting strategies differently.

---

@tjdevries @Conni2461 (or @fdschmidt93 when you're back), could one of you test this please?
Would be good to merge this relatively soon so that there is the infrastructure in place, so that people can start implementing "preview toggle", "padding toggle", etc...

[tjdevries]

Seems to work well for me -- code seems much less repeated now. We can always clean up a bit more later if we think of anything good.

LGTM (could get tested by maybe one more person and then let's merge)

[Conni2461]

Works great for me 👍 feel free to merge this as if you are done :) Thanks for the PR :)

[l-kershaw]

Thanks for all the help everyone ☺️