fix: (terminal) buffer previewer

[fdschmidt93]

Would close #714 by introducing a new previewer that reuses existing buffers.

@Conni2461 Idk, personally a bit torn on this. While previewers.new_buffer_previewer feels backward for builtin.buffers that we already have, though now ofc the buffer for actions.delete has to be replaced prior to deletion and the solution generally feels bloat. I'm not sure there are other side effects (that's why I abstained touching new_buffer_previewer, as we'd have conflicting logic ala creating buffers vs using existing ones).

I'm personally fine with terminal buffers not being highlighted ;)

[Conni2461]

I wouldn't touch new_buffer_previewer for this, i had something like this in mind when mentioning that i need to restructure it from in #714, then i ignored it for 4 and half months because i dont use previewer for builtin.buffers 🤣
Originally i was to lazy to write another base Previewer so i just used vimgrep and it worked till someone mentioned "special" buffers and i just introduced a silly workaround. We can remove those which probably will make PR smaller, so i dont actually have a problem doing this PR 👍
We can delete this

telescope.nvim/lua/telescope/previewers/term_previewer.lua

Lines 297 to 300 in f1a27ba

	if entry.bufnr and (p == "[No Name]" or vim.api.nvim_buf_get_option(entry.bufnr, "buftype") ~= "") then
	return
	end

and this

telescope.nvim/lua/telescope/previewers/buffer_previewer.lua

Lines 373 to 378 in f1a27ba

	-- Workaround for unnamed buffer when using builtin.buffer
	if entry.bufnr and (p == "[No Name]" or vim.api.nvim_buf_get_option(entry.bufnr, "buftype") ~= "") then
	local lines = vim.api.nvim_buf_get_lines(entry.bufnr, 0, -1, false)
	vim.api.nvim_buf_set_lines(self.state.bufnr, 0, -1, false, lines)
	jump_to_line(self, self.state.bufnr, entry.lnum)
	else

[Conni2461]

otherwise LGTM thanks for picking up my slack :)

[fdschmidt93]

There is a crazy stupid issue with this I've so far just overlooked in my tests (please confirm if you have the time). If we use actual buffers for the preview, the fact that the popup style is minimal messes the buffer look once attached to the new window.

This only happens if you select a buffer that was not yet attached to the window the picker was launched from.. Pretty tricky, I haven't yet figured out how to work around this in a non-hacky way.

E: is this an upstream issue?

[Conni2461]

There is a crazy stupid issue with this I've so far just overlooked in my tests (please confirm if you have the time). If we use actual buffers for the preview, the fact that the popup style is minimal messes the buffer look once attached to the new window.

Good old weird bugs, i have not missed you

[fdschmidt93]

Actually, it's intended behavior.. Just not really nice one as it's been probably set without floats in mind.

When editing a buffer that has been edited before, the options from the window
that was last closed are used again. If this buffer has been edited in this
window, the values from back then are used. Otherwise the values from the
last closed window where the buffer was edited last are used.
It's possible to set a local window option specifically for a type of buffer.
When you edit another buffer in the same window, you don't want to keep
using these local window options. Therefore Vim keeps a global value of the
local window options, which is used when editing another buffer. Each window
has its own copy of these values. Thus these are local to the window, but
global to all buffers in the window.

:h local-options

[fdschmidt93]

Well, that escalated quite a lot. It's done though it's quite complicated for fixing terminal buffer preview... While I cannot think of anything wrong with this right now, I'll test this more tomorrow or next week I suppose.

Complexity mostly stems from two things:

• Low-level api vim.api.nvim_set_decoration_provider needed to set buffer-window local extmarks -- it's unclear to me from docs whether this is adequately cleared but I think I cannot do more to assure this per spec I think I'm adequately clearing the provider
• :h local-options -- if a buffer hasn't yet been attached to a window, it will carry over the minimal layout from the popup -- doing my best to recover from this, but maybe fillchars (which seem fine in my tests) or EndOfBuffer (doesn't seem to be cleared as per :h vim.api.nvim_open_win) are still acting up?

[Conni2461]

LGTM but its still broken if previewer = false see one of the old conversations are opened it again.

Thanks :)

[fdschmidt93]

Don't merge this yet.. if previewer is false, the window options are not applied 🤦

[Conni2461]

Don't merge this yet.. if previewer is false, the window options are not applied facepalm

Window options are applied just fine for me and i dont even see why this should be a problem. previewer = false hasn't changed in this PR really besides the additional action doing the a second jump. Works in master and this PR just fine

[Conni2461]

Oh but buffer deletion is broken 😆

[fdschmidt93]

Window options are applied just fine for me and i dont even see why this should be a problem. previewer = false hasn't changed in this PR really besides the additional action doing the a second jump. Works in master and this PR just fine

Yes, you're right. It cannot be an issue because the preview window is never created which could overwrite the buffer-window local options. But in one state (E: yeah it was a faulty state) I saw the options not applied and I cannot reproduce this now 😆 I'm too fried.

[fdschmidt93]

action.delete_buffer is now fine.

E: sorry for the rodeo, I've grown to despise this PR 😆

[fdschmidt93]

I've also tested whether nvim_set_decoration_provider is cleared by nvim_buf_clear_namespace and it's not. So if a user would ever create a window with the same id as preview window (s)he would have the cursorline highlighted with TelescopePreviewLine. Not even sure if this can even happen, but something to check
Travelling this weekend, so I'll look into it thereafter.

[fdschmidt93]

@Conni2461 could you please give this another whirl? previewer=false and action.delete_buffer are fixed. Sorry, couldn't test myself before you've took a crack at it 😅

I've asked on nvim gitter about clearing decoration provider and didn't get an answer (don't wanna ping bfredl, but I'm sure the circle of people to answer this definitively is very exclusive right now 😆 )

I opened the buffer previewer 30 times in a session and virtually nothing happened in htop. I've minimized callbacks from set_decoration_provider as you'll see in diff of final commit via returning false in on_start once previewer is closed.

[Conni2461]

I think this is good. Thanks :)

And i also think this might fix #1145 which happens since today but not with this PR, no idea why or how this PR should affect this

[Conni2461]

Oh wait it doesn't happen here because this branch is not yet rebased. So 1145 is a actual bug we need to address. Looking into it

Just ignore my comments about 1145. You can merge this if its done