Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The popup functions in Vim lack a function to change the buffer in popup #15006

Closed
Yggdroot opened this issue Jun 15, 2024 · 14 comments
Closed

Comments

@Yggdroot
Copy link

Yggdroot commented Jun 15, 2024

Is your feature request about something that is currently impossible or hard to do? Please describe the problem.
Neovim has a function nvim_win_set_buf, which can change the buffer in the specified window, but vim's popup functions do
not have a function like it.
If I want to change the content in the popup, I have to use popup_settext, if a file is too large, reading from the file and popup_settext would be too costly. It would be better if vim's popup has a function like nvim_win_set_buf.

Describe the solution you'd like

Describe alternatives you've considered

Additional context

@chrisbra
Copy link
Member

If I want to change the content in the popup, I have to use popup_settext, if a file is too large, reading from the file and popup_settext would be too costly.

I don't understand that comment. If the file is large and hasn't been loaded as a buffer yet, it will be exactly as costly because you need to load this as a buffer. It may even be more costly, since we have to create a buffer for that file. Using popup_settext() can even be better, since you can use readfile() directly and do not need to handle the overhead of opening that file in a buffer yet.

If the buffer has been already loaded, you may be right, but the same overhead has happened eventually, but might have happened a bit earlier, when the file was read into a buffer.

So I don't think this benefits us anything here.

@Yggdroot
Copy link
Author

I meant the buffer has already been loaded.
Yes, the overhead is inevitable when loading the buffer. however,this only happens once.
Actually, the popup feature is used only when we write a plugin. For example, if there is a plugin to preview the opening buffers in a popup window, I may switch the buffer back and forth, if I switch to a large buffer 3 times, I have to call popup_settext() 3 times.
Notice that just popup_settext() without readfile() is also very costly.

1

@Shane-XB-Qian
Copy link
Contributor

do you use matchstr() etc funcs for ptn search in your code, if yes, maybe that's the cause of slowness,
i faced some times, maybe that's a flaw of perf, if you (any of you) can reproduce it as well, wish can be fixed.

@bfrg
Copy link
Contributor

bfrg commented Jun 15, 2024

For example, if there is a plugin to preview the opening buffers in a popup window, I may switch the buffer back and forth, if I switch to a large buffer 3 times, I have to call popup_settext() 3 times.

You don't have to use popup_settext() for displaying a buffer in a popup window. You can use popup_create() and pass a buffer number, see :h popup_create(). Once the buffers are loaded, you can display them as often as you like in a popup window with popup_create() without any performance issues. But unfortunately, you cannot show a different buffer in the same popup window, so you need to draw another popup window on top of the current one, and then remove the one below the new one. I have been doing this for quite some time without any issues. There's definitively no flickering because Vim won't redraw the screen while a function is being executed. Unless there's an explicit :redraw somewhere within the function.

Here's a script that I've written a few years ago (and eventually rewritten in vim9script). Hit CTRL-P to preview the next buffer in your buffer list in a popup window. Once the popup is displayed, you can scroll the popup window with j/k/d/u/f/b/g/G and jump to the next or previous buffer in the buffer list with n and p, respectively.

vim9script

# Return next or previous buffer number
def Bufnext(bufnr: number, step: number): number
    const buflist: list<number> = range(1, bufnr('$'))->filter((_, i: number): bool => buflisted(i))
    const idx: number = index(buflist, bufnr)

    if idx < 0
        return 0
    elseif idx + step >= len(buflist)
        return buflist[idx + step - len(buflist)]
    elseif idx + step < 0
        return buflist[idx + step + len(buflist)]
    else
        return buflist[idx + step]
    endif
enddef

# NOTE when using normal-mode commands for scrolling, popup window is not
# redrawn, see https://github.com/vim/vim/issues/6663#issuecomment-671060713
def Win_scroll(winid: number, key: string): bool
    if key == 'j'
        win_execute(winid, "normal! \<c-e>")
    elseif key == 'k'
        win_execute(winid, "normal! \<c-y>")
    elseif key == 'g'
        win_execute(winid, 'normal! gg')
    elseif key == 'G'
        win_execute(winid, 'normal! G')
    elseif key == 'd'
        win_execute(winid, "normal! \<c-d>")
    elseif key == 'u'
        win_execute(winid, "normal! \<c-u>")
    elseif key == 'f'
        win_execute(winid, "normal! \<c-f>")
    elseif key == 'b'
        win_execute(winid, "normal! \<c-b>")
    elseif key == 'q'
        popup_close(winid)
    else
        return false
    endif
    return true
enddef

def Preview_filter(winid: number, key: string): bool
    if key == 'n'
        Open(Bufnext(winbufnr(winid), 1))
        popup_close(winid)
    elseif key == 'p'
        Open(Bufnext(winbufnr(winid), -1))
        popup_close(winid)
    elseif key == 'o' || key == "\<cr>"
        execute 'buffer' winbufnr(winid)
        popup_close(winid)
    elseif key == 's'
        execute 'sbuffer' winbufnr(winid)
        popup_close(winid)
    elseif key == 'v'
        execute 'vertical sbuffer' winbufnr(winid)
        popup_close(winid)
    elseif key == 't'
        execute 'tab sbuffer' winbufnr(winid)
        popup_close(winid)
    elseif key =~ '[jkdufbgGq]'
        return Win_scroll(winid, key)
    else
        return false
    endif
    return true
enddef

# Preview listed buffers in a popup window: scroll, cycle, open
def Open(buf: number = 0): number
    const bufnr: number = buf != 0 ? buf : Bufnext(bufnr(), 1)
    const bufs: list<number> = range(1, bufnr('$'))->filter((_, i: number): bool => buflisted(i))
    const curidx: number = index(bufs, bufnr)
    const title1: string = $'({curidx + 1}/{len(bufs)}) '
    const title2: string = bufnr->bufname()->fnamemodify(':~:.')
    var title: string

    # Truncate long bufnames
    if len(title1 .. title2) > 77
        title = title1 .. '…' .. title2[-73 :]
    else
        title = title1 .. title2
    endif

    const winid: number = popup_create(bufnr, {
        minwidth: 80,
        maxwidth: 80,
        minheight: 20,
        maxheight: 20,
        title: title,
        padding: [0, 1, 1, 1],
        border: [1, 0, 0, 0],
        borderchars: [' '],
        close: 'none',
        drag: true,
        scrollbar: 1,
        filter: Preview_filter,
        filtermode: 'n',
        mapping: false
    })

    setwinvar(winid, '&number', 1)
    return winid
enddef

nnoremap <C-P> <ScriptCmd>Open()<CR>

This is what it looks like:
screenshot-2024-06-16_004538

But I agree, not being able to preview another buffer in an already displayed popup window is annoying. It would certainly be helpful.

@Yggdroot
Copy link
Author

do you use matchstr() etc funcs for ptn search in your code, if yes, maybe that's the cause of slowness, i faced some times, maybe that's a flaw of perf, if you (any of you) can reproduce it as well, wish can be fixed.

No, the search is not slow, to preview a large buffer is slow.

@Yggdroot
Copy link
Author

You don't have to use popup_settext() for displaying a buffer in a popup window. You can use popup_create() and pass a buffer number,

I knew this method and tried it. It's slower than using popup_settext() directly. popup_settext() is slow only when the buffer is very large.

Yggdroot referenced this issue in Yggdroot/LeaderF Jun 16, 2024
chrisbra added a commit to chrisbra/vim that referenced this issue Jun 16, 2024
related: vim#15006

Problem:  cannot change buffer in a popup
Solution: add popup_setbuf() function
chrisbra added a commit to chrisbra/vim that referenced this issue Jun 16, 2024
related: vim#15006

Problem:  cannot change buffer in a popup
Solution: add popup_setbuf() function
chrisbra added a commit to chrisbra/vim that referenced this issue Jun 16, 2024
related: vim#15006

Problem:  cannot change buffer in a popup
Solution: add popup_setbuf() function
chrisbra added a commit to chrisbra/vim that referenced this issue Jun 16, 2024
related: vim#15006

Problem:  cannot change buffer in a popup
Solution: add popup_setbuf() function
chrisbra added a commit to chrisbra/vim that referenced this issue Jun 16, 2024
related: vim#15006

Problem:  cannot change buffer in a popup
Solution: add popup_setbuf() function
chrisbra added a commit to chrisbra/vim that referenced this issue Jun 16, 2024
related: vim#15006

Problem:  cannot change buffer in a popup
Solution: add popup_setbuf() function
@zhyzky
Copy link

zhyzky commented Jun 17, 2024

I noticed the describtion of popup_settext() explicitly limit the second arg not the buffer number, while popup_create() allowed the first arg. Would it be more unified to remove this limitation instead of creating a new interface?

chrisbra added a commit to chrisbra/vim that referenced this issue Jun 17, 2024
related: vim#15006

Problem:  cannot change buffer in a popup
Solution: add popup_setbuf() function
chrisbra added a commit to chrisbra/vim that referenced this issue Jun 17, 2024
related: vim#15006

Problem:  cannot change buffer in a popup
Solution: add popup_setbuf() function
@chrisbra
Copy link
Member

Honestly, I don't understand that remark from popup_settext(). It says:

{text} is the same as supplied to |popup_create()|, except that a buffer number is not allowed.

But at popup_create() we don't have the {text} argument. I suppose it refers to the {what} of popup_create? I guess we could change popup_settext() and make {text} also take a buffer number (however not a buffer name). That could work.

@Yggdroot
Copy link
Author

That sounds good to me.

@chrisbra
Copy link
Member

what exactly?

@Yggdroot
Copy link
Author

{text} is the same as {what} except that a buffer number is not allowed.

@Yggdroot
Copy link
Author

Yggdroot commented Jun 19, 2024

👍

@Shane-XB-Qian
Copy link
Contributor

@Yggdroot so how's this new func, was it solving your perf problem now?

@Yggdroot
Copy link
Author

I haven't tried yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants