[vim/vim] CompleteDone fires before listener_add callback when completion is finished (#5860)

25 views
Skip to first unread message

Paul Jolly

unread,
Mar 28, 2020, 11:34:25 AM3/28/20
to vim/vim, Subscribed

Describe the bug

When a keystroke causes a completion to "end" (which, depending on the state might cause the currently selected candidate to be inserted), the CompleteDone event fires before the insertion of the candidate (if applicable) or the character corresponding to the keystroke (if applicable).

To Reproduce

Using repro.vim:

set nocp
set noswapfile
set completeopt=menu,popup
set bs=2

function DoComplete(findstart, base)
  if a:findstart == 1
    return 0
  else
    return [{"abbr":"apple","word":"apple","info":""},{"abbr":"banana","word":"banana","info":"Type"}]
  endif
endfunction

set omnifunc=DoComplete

function s:EchoEvent(msg)
  redir >> /tmp/listener.log | echo a:msg | redir END
endfunction

function HandleDelta(bufnr, start, end, added, changes)
  call s:EchoEvent("HandleDelta")
endfunction

call listener_add("HandleDelta")

au CompleteDone * call s:EchoEvent("CompleteDone")

Then:

vim -u repro.vim

Then:

i<C-X><C-O>

will produce the following output in /tmp/listener.log:

HandleDelta
HandleDelta

and leave the buffer in the following state:

apple_

with the cursor at the position _ and the popup menu showing.

Now type ). Notice the order of the events in /tmp/listener.log:

CompleteDone
HandleDelta

A consequence of this is that any remote plugin (that relies on listener_add deltas to update the plugin's buffer state) that attempts to perform an action in response to CompleteDone (e.g. handling additional text edits received from a language server) will do so with a dirty buffer.

Expected behavior

The CompleteDone event to fire after the listener_add callback.

Screenshots

n/a

Environment (please complete the following information):

  • Vim version
VIM - Vi IMproved 8.2 (2019 Dec 12, compiled Mar 28 2020 14:13:18)
Included patches: 1-397
Compiled by myitcv@myitcv
Huge version with GTK2 GUI.  Features included (+) or not (-):
+acl               -farsi             -mouse_sysmouse    -tag_old_static
+arabic            +file_in_path      +mouse_urxvt       -tag_any_white
+autocmd           +find_in_path      +mouse_xterm       -tcl
+autochdir         +float             +multi_byte        +termguicolors
-autoservername    +folding           +multi_lang        +terminal
+balloon_eval      -footer            -mzscheme          +terminfo
+balloon_eval_term +fork()            -netbeans_intg     +termresponse
+browse            +gettext           +num64             +textobjects
++builtin_terms    -hangul_input      +packages          +textprop
+byte_offset       +iconv             +path_extra        +timers
+channel           +insert_expand     -perl              +title
+cindent           +job               +persistent_undo   +toolbar
+clientserver      +jumplist          +popupwin          +user_commands
+clipboard         +keymap            +postscript        +vartabs
+cmdline_compl     +lambda            +printer           +vertsplit
+cmdline_hist      +langmap           +profile           +virtualedit
+cmdline_info      +libcall           +python/dyn        +visual
+comments          +linebreak         +python3/dyn       +visualextra
+conceal           +lispindent        +quickfix          +viminfo
+cryptv            +listcmds          +reltime           +vreplace
+cscope            +localmap          +rightleft         +wildignore
+cursorbind        +lua               -ruby              +wildmenu
+cursorshape       +menu              +scrollbind        +windows
+dialog_con_gui    +mksession         +signs             +writebackup
+diff              +modify_fname      +smartindent       +X11
+digraphs          +mouse             -sound             -xfontset
+dnd               +mouseshape        +spell             +xim
-ebcdic            +mouse_dec         +startuptime       +xpm
+emacs_tags        -mouse_gpm         +statusline        +xsmp_interact
+eval              -mouse_jsbterm     -sun_workshop      +xterm_clipboard
+ex_extra          +mouse_netterm     +syntax            -xterm_save
+extra_search      +mouse_sgr         +tag_binary        
   system vimrc file: "$VIM/vimrc"
     user vimrc file: "$HOME/.vimrc"
 2nd user vimrc file: "~/.vim/vimrc"
      user exrc file: "$HOME/.exrc"
  system gvimrc file: "$VIM/gvimrc"
    user gvimrc file: "$HOME/.gvimrc"
2nd user gvimrc file: "~/.vim/gvimrc"
       defaults file: "$VIMRUNTIME/defaults.vim"
    system menu file: "$VIMRUNTIME/menu.vim"
  fall-back for $VIM: "/home/myitcv/usr/vim/share/vim"
Compilation: gcc -c -I. -Iproto -DHAVE_CONFIG_H -DFEAT_GUI_GTK  -pthread -I/usr/include/gtk-2.0 -I/usr/lib/x86_64-linux-gnu/gtk-2.0/include -I/usr/include/pango-1.0 -I/usr/include/atk-1.0 -I/usr/include/cairo -I/usr/include/pixman-1 -I/usr/include/gdk-pixbuf-2.0 -I/usr/include/libmount -I/usr/include/blkid -I/usr/include/pango-1.0 -I/usr/include/harfbuzz -I/usr/include/pango-1.0 -I/usr/include/fribidi -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I/usr/include/uuid -I/usr/include/freetype2 -I/usr/include/libpng16   -g -O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1       
Linking: gcc   -L/usr/local/lib -Wl,--as-needed -o vim   -lgtk-x11-2.0 -lgdk-x11-2.0 -lpangocairo-1.0 -latk-1.0 -lcairo -lgdk_pixbuf-2.0 -lgio-2.0 -lpangoft2-1.0 -lpango-1.0 -lgobject-2.0 -lglib-2.0 -lfontconfig -lfreetype -lSM -lICE -lXpm -lXt -lX11 -lXdmcp -lSM -lICE  -lm -ltinfo -lnsl    -ldl  -L/usr/lib/x86_64-linux-gnu -lluajit-5.1         
  • OS: Ubuntu 19.10
  • Terminal: XTerm(348)

Additional context
Add any other context about the problem here.

cc @leitzler


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.

Bram Moolenaar

unread,
Mar 28, 2020, 3:45:53 PM3/28/20
to vim/vim, Subscribed


Paul Jolly wrote:

> **Describe the bug**

>
> When a keystroke causes a completion to "end" (which, depending on the state might cause the currently selected candidate to be inserted), the `CompleteDone` event fires before the insertion of the candidate (if applicable) or the character corresponding to the keystroke (if applicable).
>
> **To Reproduce**
>
> Using `repro.vim`:
>
> ```vim
> Now type `)`. Notice the order of the events in `/tmp/listener.log`:

>
> ```
> CompleteDone
> HandleDelta
> ```
>
> A consequence of this is that any remote plugin (that relies on `listener_add` deltas to update the plugin's buffer state) that attempts to perform an action in response to `CompleteDone` (e.g. handling additional text edits received from a language server) will do so with a dirty buffer.
>
> **Expected behavior**

>
> The `CompleteDone` event to fire after the `listener_add` callback.

That is not expected. The listner callback is called later, before the
screen is redrawn.

CompleteDone is invoked when the completion has been done. When typing
")" this ends the completion, then it adds the ")".

As far as I can see this works as intended. You can perhaps call
listener_flush() somewhere to have the callback invoked earlier.


--
We learn from our mistakes. Politicians don't make mistakes.

/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\
/// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\ an exciting new programming language -- http://www.Zimbu.org ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///

Paul Jolly

unread,
Mar 28, 2020, 4:47:54 PM3/28/20
to vim/vim, Subscribed

That is not expected. The listner callback is called later, before the screen is redrawn.

I think this has important implications for remote plugins. Because a "native" VimScript plugin would see the buffer as updated on receiving CompleteDone, a remote plugin relying on deltas via listener_add callbacks would not.

With my limited understanding of the Vim internals it feels like Vim should be calling listener_flush before any autocommand events are fired. Or does that not make sense?

You can perhaps call listener_flush() somewhere to have the callback invoked earlier.

This works for now, thank you.

Bram Moolenaar

unread,
Mar 28, 2020, 5:38:22 PM3/28/20
to vim/vim, Subscribed


> > That is not expected. The listner callback is called later, before
> > the screen is redrawn.
>
> I think this has important implications for remote plugins. Because a
> "native" VimScript plugin would see the buffer as updated on receiving
> `CompleteDone`, a remote plugin relying on deltas via `listener_add`

> callbacks would not.
>
> With my limited understanding of the Vim internals it feels like Vim
> should be calling `listener_flush` before any autocommand events are
> fired. Or does that not make sense?

I don't see why there would be any relation between triggering an
autocommand and the listener mechanism. They are completely separate
things. The listeners only depend on changes in the buffer text, not
how how those changes are made.


> > You can perhaps call listener_flush() somewhere to have the callback
> > invoked earlier.
>
> This works for now, thank you.

If a plugin depends on listener callbacks to be invoked before something
else happens, the flushing is the right way.

--
There are 10 kinds of people: Those who understand binary and those who don't.


/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\
/// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
\\\ an exciting new programming language -- http://www.Zimbu.org ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///

Paul Jolly

unread,
Mar 29, 2020, 3:31:43 AM3/29/20
to Vim Dev Mailing List, reply+ACY5DGE3DILDSYCUEK...@reply.github.com, vim/vim, Subscribed
> With my limited understanding of the Vim internals it feels like Vim
> should be calling `listener_flush` before any autocommand events are
> fired. Or does that not make sense?

I don't see why there would be any relation between triggering an
autocommand and the listener mechanism. They are completely separate
things. The listeners only depend on changes in the buffer text, not
how how those changes are made.

There isn't a relationship per se, but for remote plugins, as I described in this issue, if there are pending deltas when an autocommand is fired (or a function called for that matter) where there is a remote plugin handling the autocommand (or function) then there is a chance, as we discovered, for effectively performing a "dirty write" back to Vim.

We can, however, put such a call into our VimScript shim (https://github.com/govim/govim/blob/db2f5323507e94805d5d9f2bc4771f29a1e3f8e0/plugin/govim.vim) for now and experiment there.

> > You can perhaps call listener_flush() somewhere to have the callback
> > invoked earlier.
>
> This works for now, thank you.

If a plugin depends on listener callbacks to be invoked before something
else happens, the flushing is the right way.

The plugin relies on events being in the right order. i.e. that the edits happened before the CompleteDone event was triggered (more explicitly, the buffer contents at the time of the event reflect the changes but a listener_add callback does not, at this point know about them)

But as I said above, we can experiment with a workaround in our shim for now.

 

vim-dev ML

unread,
Mar 29, 2020, 3:32:03 AM3/29/20
to vim/vim, vim-dev ML, Your activity

Paul Jolly

unread,
Apr 6, 2020, 12:44:23 AM4/6/20
to vim/vim, vim-dev ML, Comment

FWIW our workaround appears to be working in this case:

https://github.com/govim/govim/blob/80f0949c0a467c2f899d7aa43ff09ed0c2fd2b2d/plugin/govim.vim#L66-L73

Before making any call to govim from Vim (which we usefully have happening through a single function) we call listener_flush unless the call itself is the handling of listener_add callbacks.

@brammool - would you like to keep this issue open, or do you think this workaround is the right approach and hence this can be closed?


You are receiving this because you commented.

Bram Moolenaar

unread,
Apr 6, 2020, 3:00:34 PM4/6/20
to vim/vim, vim-dev ML, Comment

Closed #5860.


You are receiving this because you commented.

Bram Moolenaar

unread,
Apr 6, 2020, 3:00:34 PM4/6/20
to vim/vim, vim-dev ML, Comment

I think that calling listener_flush() for an autocommand event is a normal way to make things work. It is intentional that flushing to listeners is postponed until a series of related changes is done, which usually means when the screen gets updated. If it is required to have it happen earlier, calling the function to initiate the flush is correct.


You are receiving this because you commented.

Reply all
Reply to author
Forward
0 new messages