xavierd / clang_complete

Vim plugin that use clang for completing C/C++ code.
http://www.vim.org/scripts/script.php?script_id=3302
1.96k stars 308 forks source link

Pressing enter inserts HandlePossibleSelectionEnter signature after second completion #431

Closed chaserhkj closed 9 years ago

chaserhkj commented 9 years ago

I'm using supertab together with clang_complete and encountered this issue.

After using <Tab>, <c-x><c-u> or <c-x><c-o> key bindings to perform completion in some C/C++ file 2 times, all enter key stokes in the buffer in the insertion mode would lead to the insertion of string <SNR>(some number)_HandlePossibleSelectionEnter() rather than the insertion of a new line.

I'm running on vim 7.4.527 with gentoo patch set.

When no completion is performed, my verbose imap <CR> gives output No mapping found

When one completion has been performed, my verbose imap <CR> gives output i <CR> @<SNR>(some number)_HandlePossibleSelectionEnter(), and all thing works well

When two completions have been performed, my verbose imap <CR> gives output i <CR> &@<SNR>(some number)_HandlePossibleSelectionEnter(), and this issue begins to appear

Any additional information could be provided if requested.

wilywampa commented 9 years ago

I'm not the plugin maintainer but I ran into this as well. The problem is that the script saves the <CR> map using maparg() but when it goes to restore the mapping, it doesn't check whether the original map was an expression map (or several other options available by using maparg('<cr>', 'i', 0, 1) instead of just maparg('<cr>', 'i')). Unfortunately it's not an easy fix. For example, here is SuperTab's code for restoring maps: https://github.com/ervandew/supertab/blob/master/plugin/supertab.vim#L735

I don't use snippets in this plugin so I just disabled the <C-y> and <CR> maps in my fork.

tl;dr - need to remap as <expr> if original map was made using <expr>

chaserhkj commented 9 years ago

@wilywampa After blaming, I found that this issue and related <CR> mapping handling behavior was introduced by pull request #427 and #428, more precisely commit 2f2fdd7 1330d0c. Reverting to the version before this may make it work, though the conflict with auto-pairs mentioned in the pull requests above is still not solved.

drichardson commented 9 years ago

Thanks @wilywampa, reverting to 6a7ad824 worked for me.

michaellindon commented 9 years ago

@drichardson seconded

miquelbeltran commented 9 years ago

I have the same problem and reverting to the commit pointed by @drichardson worked for me.

I use supertab too.

linluk commented 9 years ago

hi, i have the same issue. reverting to 6a7ad82 fixed it, thanks.

i don't use supertab but i have two <cr> mappings in my vimrc: nnoremap <silent> <CR> :nohl<CR><CR> and nnoremap A<CR> <nop> if this helps.

my vimscript skills are very very low, but i could help reproducing or so if needed.

happy vimming!

xaizek commented 9 years ago

Hi @linluk, I doubt that those are mappings that cause conflicts. Notice that they are for Normal mode, while clang_complete remaps Enter in Insert mode.

linluk commented 9 years ago

hi @xaizek , i also don't think that my mappings cause conflicts. i just posted them to give (more or less) complete information.

and to give more than just more or less information: i am on a debian system with vim installed from the official packages.

:version
VIM - Vi IMproved 7.3 (2010 Aug 15, compiled Feb 10 2013 02:28:47)
Inklusive der Korrekturen: 1-547
Verändert von pkg-vim-maintainers@lists.alioth.debian.org
Übersetzt von jamessan@debian.org
Riesige Version ohne GUI. Ein- (+) oder ausschließlich (-) der Eigenschaften:
+arabic +autocmd -balloon_eval -browse ++builtin_terms +byte_offset +cindent -clientserver -clipboard +cmdline_compl +cmdline_hist +cmdline_info +comments +conceal +cryptv +cscope +cursorbind
+cursorshape +dialog_con +diff +digraphs -dnd -ebcdic +emacs_tags +eval +ex_extra +extra_search +farsi +file_in_path +find_in_path +float +folding -footer +fork() +gettext -hangul_input +iconv
+insert_expand +jumplist +keymap +langmap +libcall +linebreak +lispindent +listcmds +localmap +lua +menu +mksession +modify_fname +mouse -mouseshape +mouse_dec +mouse_gpm -mouse_jsbterm
+mouse_netterm -mouse_sysmouse +mouse_xterm +mouse_urxvt +multi_byte +multi_lang -mzscheme +netbeans_intg +path_extra +perl +persistent_undo +postscript +printer +profile +python -python3
+quickfix +reltime +rightleft +ruby +scrollbind +signs +smartindent -sniff +startuptime +statusline -sun_workshop +syntax +tag_binary +tag_old_static -tag_any_white +tcl +terminfo +termresponse
 +textobjects +title -toolbar +user_commands +vertsplit +virtualedit +visual +visualextra +viminfo +vreplace +wildignore +wildmenu +windows +writebackup -X11 -xfontset -xim -xsmp
-xterm_clipboard -xterm_save
          System-vimrc-Datei: "$VIM/vimrc"
        Benutzer-vimrc-Datei: "$HOME/.vimrc"
         Benutzer-exrc-Datei: "$HOME/.exrc"
     Voreinstellung für $VIM: "/usr/share/vim"
Übersetzt: gcc -c -I. -Iproto -DHAVE_CONFIG_H     -g -O2 -fstack-protector --param=ssp-buffer-size=4 -Wformat -Werror=format-security -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1     -I/usr/include/tcl
8.5  -D_REENTRANT=1  -D_THREAD_SAFE=1  -D_LARGEFILE64_SOURCE=1
Linken: gcc   -L. -Wl,-z,relro -rdynamic -Wl,-export-dynamic  -Wl,-E  -Wl,-z,relro -Wl,--as-needed -o vim       -lm -ltinfo -lnsl  -lselinux  -lacl -lattr -lgpm  -L/usr/lib -llua5.1  -Wl,-E  -fs
tack-protector -L/usr/local/lib  -L/usr/lib/perl/5.14/CORE -lperl -ldl -lm -lpthread -lcrypt -L/usr/lib/python2.7/config -lpython2.7 -lpthread -ldl -lutil -lm -Xlinker -export-dynamic -Wl,-O1 -W
l,-Bsymbolic-functions  -L/usr/lib -ltcl8.5 -ldl -lpthread -lieee -lm -lruby-1.9.1 -lpthread -lrt -ldl -lcrypt -lm  -L/usr/lib
Betätigen Sie die EINGABETASTE oder geben Sie einen Befehl ein

and here are all the plugins i use:

set rtp+=~/.vim/bundle/Vundle.vim
call vundle#begin()
Plugin 'gmarik/Vundle.vim'
Plugin 'Rip-Rip/clang_complete'
Plugin 'vim-scripts/CRefVim'
Plugin 'sjl/gundo.vim'
Plugin 'jondkinney/dragvisuals.vim'
Plugin 'bling/vim-airline'
Plugin 'scrooloose/syntastic'
Plugin 'xolox/vim-misc'
Plugin 'xolox/vim-session'
Plugin 'flazz/vim-colorschemes'
 call vundle#end()

and my vimrc file can be found here: https://github.com/linluk/my-dot-files/blob/master/vimrc

wilywampa commented 9 years ago

I personally don't have any <CR> imap so this isn't terribly well tested, but I tried to fix it in https://github.com/Rip-Rip/clang_complete/pull/439.

xaizek commented 9 years ago

Thanks, that might work and we should have that fix, but bad news is that at least in your case it fixes consequences rather than the issue. After first completion there should be no <cr> mappings left, it must be unmapped. For some reason it doesn't happen for you and that is what's strange, I couldn't reproduce it yet so I don't know if it's function that is not called or s:old_cr gets overwritten somehow.

wilywampa commented 9 years ago

Thanks for testing.

It looks like the problem is that <CR> never gets unmapped unless a snippet is chosen. It also looks <CR> is unmapped the first time the cursor moves after it is mapped so I don't see how it can be used to trigger a snippet at all.

I moved the code that unmaps <CR> and <C-y> into a separate function which is called when a snippet is triggered, on InsertLeave, and on CompleteDone if the CompleteDone event exists (it was introduced somewhere in Vim 7.3 - I wish it were easier to track down which version introduced a given feature). I installed UltiSnips and tested with no <CR> imap, a non-buffer <CR> imap and a buffer <CR> imap and it all seems to be working right now. Please give it another try.

I had never tried UltiSnips before because I use NeoComplete and NeoSnippet. The insertion of function arguments is pretty nifty so I might keep UltiSnips if I can make it play nice with other plugins.

xaizek commented 9 years ago

Thanks for the updates. Changes seem to be correct, don't break anything for me and also fix similar conflict with vim-endwise. I'd appreciate if @chaserhkj, @drichardson, @michaellindon, @miquelbeltran or @linluk could try #439 out to get it tested in at least one more environment (just trying to be careful, I don't like this situation with conflicting mappings and completion functions in Vim), then it should be OK to merge.

linluk commented 9 years ago

for me it seems to work (after a very quick test), but as i mentioned before i have no supertap plugin installed and i don't use snippets so imho there should at least be one more who has tested it.

drichardson commented 9 years ago

I just tried out your old_cr branch at commit 94f6b570325 but got the following error when completing:

Error detected while processing function ClangComplete:
line   27:
E118: Too many arguments for function: maparg
Press ENTER or type command to continue
Error detected while processing function ClangComplete:
line   27:
E15: Invalid expression: maparg('<CR>', 'i', 0, 1)
Press ENTER or type command to continue

Note that after pressing ENTER to continue, the command did complete successfully.

I believe the error I'm seeing above is a regression, as I never saw that error before.

xaizek commented 9 years ago

Thanks. Looks like maparg() was introduced in Vim 7.3.032, you're probably using older version. Maybe we could emulate newer maparg() after catching E118, but I don't think that it will be the same (older version just doesn't expose those details on mappings), so maybe it would be better to simply do not try to restore anything in this case. Please see old_cr branch in this repository for a try to disable new code on Vim < 7.3.032.

drichardson commented 9 years ago

Just tried commit a888c44d98 but got the same result. And yes, I am running an older version of vim, 7.3, the version that seems to have shipped with OS X Yosemite (it's the vim at /usr/bin/vim).

The same commit on ArchLinux with VIM 7.4 worked fine.

xaizek commented 9 years ago

Forced it as c5d8cc7 after testing this time (Monday morning... I could just add an extra argument, no need to downgrade). I forgot that Vim adds prefix for exceptions.

Thanks, I guess if @wilywampa doesn't have any better idea or comments for old maparg() workaround, we can just disable it as I did, behaviour will somewhat differ (no <cr> remapping on older Vim), but it seems to be acceptable solution as it should give same result as reverting to 6a7ad82.

wilywampa commented 9 years ago

There is no way to tell if a mapping is <expr> or not without the dict version of maparg. Both maparg("\<CR>", "i") and :imap <CR> have identical output for imap <CR> rhs and imap <expr> <CR> rhs. The other attributes can be parsed from the output of :imap <CR> but that's tricky and still doesn't fix the <expr> problem.

I can think of two options:

  1. Add an option g:clang_map_cr supplied by the user (only for Vim without dictionary maparg) used to restore the <CR> map with simply execute g:clang_map_cr
  2. Add a User autocmd that allows the user to execute some command when StopMonitoring() is called by adding e.g. doautocmd User ClangCompleteDone instead of using old_cr (again only if dict maparg is not available because these options require manual setup).

I can implement whichever is more popular.

wilywampa commented 9 years ago

I will use the g:clang_map_cr option. That way I can set it to 'silent! iunmap <buffer> <CR>' by default and not have to add logic to check if it has been set.

wilywampa commented 9 years ago

I implemented and documented it in https://github.com/wilywampa/clang_complete/commit/5beaff1d386ed08de56fc378d040e7ba06cea957 and tested in 7.3.0. The default behavior for 7.3.31 and below is to just iunmap it, but I wrote it so you can even put <SID> in g:clang_restore_cr_imap and it will replace it with the original <SNR> code.