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

popup_create "fixed" option when soft wrapping is on #16231

Closed
bstaletic opened this issue Dec 16, 2024 · 7 comments
Closed

popup_create "fixed" option when soft wrapping is on #16231

bstaletic opened this issue Dec 16, 2024 · 7 comments

Comments

@bstaletic
Copy link
Contributor

Currently, popup_create() has an option called fixed, which defaults to FALSE.
It basically does what it says on the tin - if there's not enough room, shifts the popup to the left.
However, it stops working if wrap is on in the popup window.

That's somewhat hostile to plugin maintainers, because occasionally fully shifted popups still have lines that don't fit on the screen (python signatures can get rather nasty).

I have recently implemented in YouCompleteMe a combination of soft-wrapping and popup shifting, all along avoiding vim's ability to shift the popup for me.
The way I have implemented it is such that YouCompleteMe will first shift the popup to the left and then, if there's still not enough space, turn wrap on.
It would be nice if vim could do this on its own.

The YouCompleteMe pull request, for reference: ycm-core/YouCompleteMe#4252
I wouldn't mind working on this feature, if there are no objections.

@chrisbra
Copy link
Member

I am not sure I follow. Can you please show an example?

@bstaletic
Copy link
Contributor Author

bstaletic commented Dec 17, 2024

Of course.

First recording - fixed is not set for the popup, wrap is not set:
https://asciinema.org/a/eRDJ8oce2VT7MXdgzAm5nsT7R
Result: Popup is shifted to the left, but the text still does not fit on the screen. Users eventually start to complain about loss of information.

Second recording - fixed is not set for the popup, wrap is set:
https://asciinema.org/a/dW6qUszsKOKPRKUFxxpVblxsR
Result: Because of set wrap, vim does no longer shift the popup to the left, despite fixed still having its default value.

Both of those are clearly documented in :help popup_create-arguments.

Desired behaviour, that I have implemented in vimscript:
https://asciinema.org/a/H2xNQQk8lZOdSWOUNkdrXonVR
The popup is both shifted and has line wrapping turned on.

Ideally, this would be taken care of by setting fixed to off in the popup option and then set wrap on the popup window.

@chrisbra
Copy link
Member

thanks for clarification. So basically, always move the popup window to the left when fixed is set. When text wrapped, it would just resize the popup to make but keep the popup at the same fixed position?

Yes I think this change would make sense. A PR would be definitely welcome.

@bstaletic
Copy link
Contributor Author

when fixed is set.

That was my bad. fixed should be not set. Even today setting it prevents shifting to the left. I'll fix my previous comment as well.

So move the popup to the left when fixed is not set. When text gets wrapped, resize the popup. Allow both to happen.

A PR would be definitely welcome.

Noted.

@bfrg
Copy link
Contributor

bfrg commented Dec 18, 2024

I ran into the same issue a few years ago. This is how I solved it basically:

vim9script

def OpenPopup()
    # " Example text
    const text: list<string> =<< trim EOF
        0123456789 0123456789 0123456789 0123456789 0123456789 
        0123456789 0123456789 0123456789 
        0123456789 0123456789 0123456789 0123456789 0123456789 0123456789 0123456789 0123456789 0123456789 0123456789 0123456789 0123456789 0123456789 0123456789 0123456789 0123456789 0123456789 0123456789 0123456789 0123456789
    EOF

    # " If set to a positive value, this will be the popup's maximum width (text will
    # " be wrapped); if 0, popup will either have max screen width or max text width
    const maxtextwidth = 0

    # " Whether to enable popup borders: [above, right, below, left] (must be 0 or 1)
    const border: list<number> = [0, 0, 0, 0]

    # " Popup padding: [above, right, below, left] (positive numbers)
    const padding: list<number> = [0, 1, 0, 1]

    # " Maximum text width _in_ the popup window
    # " NOTE: strdisplaywidth() uses &tabstop of _current_ window; &tabstop could
    # " be different in the popup window. If that's the case, we need a different
    # " method to obtain the display width of the text in the popup window
    const textwidth: number = maxtextwidth > 0
        ? maxtextwidth
        : text
            ->len()
            ->range()
            ->map((_, i: number): number => strdisplaywidth(text[i]))
            ->max()

    # " Total padding
    const pad: number = border[1] + border[3] + padding[1] + padding[3]

    # " Popup width (padding and border excluded)
    const width: number = textwidth + pad > &columns ? &columns - pad : textwidth

    # " Column position for popup window
    const screenpos: dict<number> = screenpos(win_getid(), line('.'), col('.'))
    const col: number = &columns - screenpos.curscol <= width
        ? &columns - width - 1
        : screenpos.curscol

    # " Open popup window at cursor position, shift to left, if popup too wide to
    # " fit to the right of the cursor; wrap is always enabled
    const popup_id: number = popup_atcursor(text, {
        moved: 'any',
        col: col,
        minwidth: width,
        maxwidth: width,
        padding: padding,
        border: border,
    })
enddef

nnoremap gh <ScriptCmd>OpenPopup()<CR>

Note that the # " comments are just for GitHub because it still doesn't support vim9script.

@bstaletic
Copy link
Contributor Author

A very brief testing seems to indicate that this works:

bstaletic@945ff05

I've probably broken some tests, so not yet making a pull request.

@bstaletic
Copy link
Contributor Author

Tests are fixed. New tests added.
I will open the pull request once CI is fixed.

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

3 participants