[vim/vim] completion: popup misplaced when text before it is concealed (PR #20539)
Barrett Ruth
Problem
When the cursor line has concealed text before the start of the completion, the insert-mode completion popup is drawn at the wrong screen column and the cursor no longer lines up with the completed text. The column comes from w_wcol, which is not conceal-aware during completion: the conceal correction win_line() normally applies ends up computed for the temporarily-moved compl_col used to anchor the popup, not the real cursor.
Originating from barrettruth/canola.nvim#359
Solution
pum_display() now subtracts the width of the concealed text before the cursor when positioning the menu, and ins_compl_show_pum() redraws the cursor line afterward so win_line() re-applies the conceal correction to the cursor column. This shows up with buffers that conceal a per-line prefix, including {canola,oil}.nvim
You can view, comment on, or merge this pull request online at:
https://github.com/vim/vim/pull/20539
Commit Summary
- f1d9bd0 completion: popup misplaced when text before it is concealed
File Changes
(5 files)
- M src/insexpand.c (9)
- M src/popupmenu.c (138)
- A src/testdir/dumps/Test_pum_position_with_concealed_match.dump (10)
- A src/testdir/dumps/Test_pum_position_with_concealed_text.dump (10)
- M src/testdir/test_ins_complete.vim (48)
Patch Links:
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Christian Brabandt
chrisbra left a comment (vim/vim#20539)Hm, this is expansive. You trigger a second redraw of the cursor line + iterate one more time over the cursor line again, to calculate the offset which win_line() already does. I wonder if we cannot add a new member to the win_T struct, that remembers the concealing offset at the cursor position. Basically here https://github.com/vim/vim/blob/a80874d9b84a01040e3d1aef2d4a59e1934dafb7/src/drawline.c#L3796-L3801
You just remember the boguscols in the win_T struct and inside pum_display() you can then just retrieve the value:
#ifdef FEAT_CONCEAL // w_wcol does not account for text concealed before the cursor; // shift left so the menu lines up with the visible text. wcol -= curwin->w_wcol_conceal_off; if (wcol < 0) wcol = 0; #endif
That would also avoid the risk of having two similar implementations for calculating the offset, once in win_line() and once in pum_concealed_offset().
It would still require a second redraw of the cursorline, in ins_compl_show_pum(), but that might be acceptable.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Barrett Ruth
barrettruth left a comment (vim/vim#20539)much cleaner that way, thanks! I stash the cul's conceal offset in a win_T field now.
one more note though: boguscols is off by one when the completion starts right after the concealed region, which is the case here. the conceal correction in win_line() lands a vcol early via skip_cells, so I found in my experimentation that boguscols comes out 9 for a 10-cell prefix and the menu is a column off :/. I got around this by storing the equivalent from the columns instead, i.e. virtcol - leftcol - (col - boguscols).
Note that the right-to-left case doesn't work here, although I'm not sure if you care. Happy to formalize it in an issue.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Barrett Ruth
barrettruth left a comment (vim/vim#20539)thanks on all three and forcing me to not punt the rightleft things xd
I redid it to be per-screen-line and direction-independent, so both wrap and rightleft (should) both work. updated the outdated comment as well - all of the above should be addressed.
to avoid churn here I ran the latest code across {ltr, rightleft, nowrap+horizontal-scroll, wrapped} x conceallevel {1,2,3}. let me know how it looks now.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
zeertzjq
zeertzjq left a comment (vim/vim#20539)Hmm, the cursor position when the pum is displayed is still incorrect. Not sure how to solve that without causing flicker...
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.