On Tue, Jul 8, 2008 at 9:32 PM, Dominique Pelle wrote:
> Hi,
>
> I observe a bug in the omni completion popup menu when doing omni
> completion at the end of a line which is longer than the terminal
> width and with "set nowrap". The menu is not drawn at the correct
> location when pressing <Down> when trying to select an item in the
> popup menu.
>
> Bug happens in both Vim and gvim version 7.2a.19 BETA.
>
> Steps to reproduce:
>
> 1/ Install OmniCppComplete script located at:
>
> http://www.vim.org/scripts/script.php?script_id=1520
>
> 2/ Download the c++ sample source file "bug-omni.cpp":
>
> $ wget http://dominique.pelle.free.fr/bug-omni.cpp
>
> 3/ Build a tag file with:
>
> $ ctags --language-force=c++ --c++-kinds=+p \
> --fields=+iaS --extra=+f+q bug-omni.cpp
>
> 4/ Create a minimalistic simple_vimrc file:
>
> $ cat simple_vimrc
>
> filetype plugin on
> set nocompatible
> set columns=80
> set nowrap
> set tags=tags
>
> 5/ Start vim with:
>
> $ vim -u simple.vimrc bug-omni.cpp
>
> 6/ Append s-> at the end of line 10 (the line with a long comment).
> In Normal mode:
>
> 10GAs->
>
> After pressing -> the omni menu should popup up (at correct location).
>
> 7/ Press <Down> to browse though items in menu and observe that menu is
> then re-drawn at incorrect location, at bottom of screen, corrupting
> the content of the screen) as shown in the following screen shot:
>
> http://dominique.pelle.free.fr/bug-omni.png
>
>
> I can try to debug it when I have time during the weekend but perhaps
> someone can reproduce it and fix it before that.
>
> -- Dominique
The attached patch fixes the bug with the pum misplaced when doing
omni completion on long lines with "set nowrap". I remind that not only
the pum was misplaced, but it could also cause a memory corruption
with crash, which I observed when the pum had many entries.
The problem was probably old: at least vim-7.1.138 and vim-7.2a.19
are affected.
The problem was that depending on where curwin->w_wcol was
set, its value was different. Functions validate_cursor_col() and
curs_columns() set curwin->w_wcol differently: function
curs_columns() removes curwin->w_leftcol while function
validate_cursor_col() did not.
When entering pum_display(), validate_virtcol() is called. If
(curwin->w_valid & VALID_WCOL) was false, then pum was
displayed correctly, if (curwin->w_valid & VALID_WCOL) was
true, then curwin->w_wcol was not recalculted (which is OK)
but if its value came from curs_column(), then it was inconsistent
with the value which would have been set in validate_cursor_col().
I changed the code so that validate_cursor_col() now returns
something consistent with curs_columns().
The pum is now placed correctly. I've tested with "set wrap"
and "set nowrap", "set rightleft" with scrolling horizonally (zl zh)
and the pum was always currectly placed after patch.
"make test" also passes all tests.
What puzzles me a bit, is that the problem seemed to be known
since function validate_cursor_col() has the following comment:
"Only correct when 'wrap' on!"
Please review the patch and test even further.
-- Dominique
--~--~---------~--~----~------------~-------~--~----~
You received this message from the "vim_dev" maillist.
For more information, visit http://www.vim.org/maillist.php
-~----------~----~----~----~------~----~------~--~---
Index: popupmnu.c
===================================================================
RCS file: /cvsroot/vim/vim7/src/popupmnu.c,v
retrieving revision 1.11
diff -c -r1.11 popupmnu.c
*** popupmnu.c 24 Jun 2008 22:00:40 -0000 1.11
--- popupmnu.c 12 Jul 2008 14:02:20 -0000
***************
*** 183,193 ****
/* Calculate column */
#ifdef FEAT_RIGHTLEFT
if (curwin->w_p_rl)
! col = W_WINCOL(curwin) + W_WIDTH(curwin) - curwin->w_wcol -
! curwin->w_leftcol - 1;
else
#endif
! col = W_WINCOL(curwin) + curwin->w_wcol - curwin->w_leftcol;
/* if there are more items than room we need a scrollbar */
if (pum_height < size)
--- 183,192 ----
/* Calculate column */
#ifdef FEAT_RIGHTLEFT
if (curwin->w_p_rl)
! col = W_WINCOL(curwin) + W_WIDTH(curwin) - curwin->w_wcol - 1;
else
#endif
! col = W_WINCOL(curwin) + curwin->w_wcol;
/* if there are more items than room we need a scrollbar */
if (pum_height < size)
Index: move.c
===================================================================
RCS file: /cvsroot/vim/vim7/src/move.c,v
retrieving revision 1.10
diff -c -r1.10 move.c
*** move.c 5 May 2007 17:35:47 -0000 1.10
--- move.c 12 Jul 2008 14:02:21 -0000
***************
*** 876,882 ****
}
/*
! * validate w_wcol and w_virtcol only. Only correct when 'wrap' on!
*/
void
validate_cursor_col()
--- 876,882 ----
}
/*
! * validate w_wcol and w_virtcol only.
*/
void
validate_cursor_col()
***************
*** 899,904 ****
--- 899,907 ----
col = col % (W_WIDTH(curwin) - off + curwin_col_off2());
}
curwin->w_wcol = col;
+ if (curwin->w_wcol > (int)curwin->w_leftcol)
+ curwin->w_wcol -= curwin->w_leftcol;
+
curwin->w_valid |= VALID_WCOL;
}
}
