refactor: optimize popup menu display with direct rendering #17081
Conversation
Problem: Popup menu had memory alloc and incorrect truncation logic. Solution: Replaced string allocation with direct screen rendering and fixed RTL/LTR truncation calculations.
|
@glepnir |
|
Note: Tested on the latest master branch, not this PR. Is this Bug? $ cat test.vim
func Omni_test(findstart, base)
if a:findstart
return col(".")
endif
return [
\ #{word: "foo", menu: "fooMenu", kind: "fooKind"},
\ #{word: "bar", menu: "barMenu", kind: "barKind"},
\ #{word: "baz", menu: "bazMenu", kind: "bazKind"},
\ ]
endfunc
set omnifunc=Omni_test
set pummaxwidth=20
$ vim --clean -S test.vim -c 'call feedkeys("i\<C-X>\<C-O>")'Result:
|
|
This is not reproducible on this PR. So why don't you test it with PR.
Please search for the English meaning of about you mean |
Please take a moment to calm down. You are not correctly understanding what I said. That's why it feels like you are wasting your time.
I was not opposed to adding 'trunc' to the 'fcs' option. In fact, I am in favor of it. Again, I want to emphasize that I never said to hardcode it. I brought up the implementation location for the ellipsis character in textproperty only as reference information, and I do not recommend hardcoding.
Also, it was my initial suggestion that the ellipsis character in textproperty should be able to use 'trunc' from the 'fcs' option.
@chrisbra and ALL, |
|
What I am asserting in the comment above is as follows:
|
Maybe I'm also using translation software so the tone may be ambiguous lol ？
This is borrowed from the existing code. If the screen space is not enough, use >. RL uses <. So the default trunc symbol for fcs is >.. The truncation logic is here so truncation at a col can be handled directly by screen_put. Isn't it more troublesome for win_line to handle this? Can wlv handle pum code? rendering of pum is handled in the popupmenu. If you have better suggestions for the default value of trunc can change it in the subsequent PR. I have no opinion. |
|
@glepnir There are times when your PR execution or specifications seem a bit off, so I comment to initiate a discussion. However, you often misunderstand my intent. I don’t think this is a matter of my English being unclear or anything like that. I'll consult with other maintainers, so if you don’t understand, there’s no need to force yourself to reply. |
Yes, the translation is strange and it seems that you don't understand after all. |
|
Please calm down 🙏 |
To be honest, I really don't understand why the win_line function is not responsible for rendering the pum code. What can I do in drawline? pum is handled in popupmenu.c.
There is no inversion for the text property of a single cell. It's just that we use it by default |
|
@chrisbra Example: :set fillchars=eob:~,trunc:> norightleft
:set fillchars? rightleft?
fillchars=eob:~,trunc:>
norightleft
:set rightleft
:set fillchars?
fillchars=eob:~,trunc:< |
|
I thought about it, but we would need to update fillchars whenever we set rightleft option for any window. And when we set fillchars option, need to check if rightleft needs is set. And need to make sure we handle it correctly if it is setup for one window but not for another. And I am still not sure, if mirroring the |
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
|
Thanks all, especially for trying to stay professional 🙇 |
Problem: completion: incorrect truncation logic (after: v9.1.1284)
Solution: replace string allocation with direct screen rendering and
fixe RTL/LTR truncation calculations (glepnir)
closes: vim#17081
Signed-off-by: glepnir <glephunter@gmail.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>
And from #17006 (comment)
This may raise another question, because there are two types of truncation:
In drawline.c, apart from type 1 truncation of text properties, there is type 2 truncation of double-width chars, where // If a double-width char doesn't fit at the left side display
// a '<' in the first column. Don't do this for unprintable
// characters.
if (skip_cells > 0 && mb_l > 1 && wlv.n_extra == 0)
{
wlv.n_extra = 1;
wlv.c_extra = MB_FILLER_CHAR;
wlv.c_final = NUL; // If a double-width char doesn't fit at the left side display
// a '<' in the first column. Don't do this for unprintable
// characters.
if (skip_cells > 0 && mb_l > 1 && wlv.n_extra == 0)
{
wlv.n_extra = 1;
wlv.c_extra = MB_FILLER_CHAR;
wlv.c_final = NUL;However, here the truncation character is not flipped when And in screen.c, there is also if (col + mbyte_cells > screen_Columns)
{
// Only 1 cell left, but character requires 2 cells:
// display a '>' in the last column to avoid wrapping.
c = '>';
mbyte_cells = 1;
}Now, the question is whether the same
|
|
makes sense. It's much more useful than some people's ridiculous remarks. |
Problem: completion: incorrect truncation logic (after: v9.1.1284)
Solution: replace string allocation with direct screen rendering and
fixe RTL/LTR truncation calculations (glepnir)
closes: vim/vim#17081
vim/vim@d4dbf82
Co-authored-by: glepnir <glephunter@gmail.com>
Problem: completion: incorrect truncation logic (after: v9.1.1284)
Solution: replace string allocation with direct screen rendering and
fixe RTL/LTR truncation calculations (glepnir)
closes: vim/vim#17081
vim/vim@d4dbf82
Co-authored-by: glepnir <glephunter@gmail.com>
Problem: completion: incorrect truncation logic (after: v9.1.1284)
Solution: replace string allocation with direct screen rendering and
fixe RTL/LTR truncation calculations (glepnir)
closes: vim/vim#17081
vim/vim@d4dbf82
Co-authored-by: glepnir <glephunter@gmail.com>
Problem: Popup menu had memory alloc and incorrect truncation logic.
Solution: Replaced string allocation with direct screen rendering and fixed RTL/LTR truncation calculations.