Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Snippets should be passed to snippet managers instead of completion plugins #379

Closed
andreyorst opened this issue Apr 14, 2018 · 45 comments
Closed

Comments

@andreyorst
Copy link

Summary

LC Detects UltiSnips snippet manager

function! s:hasSnippetSupport() abort
" https://github.com/SirVer/ultisnips
if exists('g:did_plugin_ultisnips')
return 1
endif

and creates snippets, that are listed in completion popup via deoplete, but upon completion it just inserts placeholders as text. See screenshots below:
Just text completed function name:
image
Now I selected [LC] source in the list:
image
After completion confirm I left with plaintext placeholders inside function definition:
image

Reproduction

  • neovim/vim version: NVIM v0.2.3-987-g2cbeb7ca5
  • This plugin version: d690d2b
  • This plugin's binary version: languageclient 0.1.68
  • Minimal vimrc: minimalrc.txt
  • File to try to reproduce: main.rs.txt
  • Language server link and version: rls-preview 0.125.1-stable (cebf188 2018-03-19)
  • Steps to reproduce the issue from clean state
    1. Download minimalrc.txt and main.rs.txt (you will need vundle to install plugins (sorry), and :UpdateRemotePlugins)
    2. Rename main.rs.txt to main.rs (damn github)
    3. nvim -u /path/to/minimalrc.txt /path/to/main.rs
    4. Try to call vaiv() inside main's body
      • type va, completion menu by deoplete pops up.
      • press Tab to complete vaiv's body
      • look at placeholders: vaiv(${1:a}, ${2:b})
  • Logs. Paste or attach /tmp/LanguageClient.log and /tmp/LanguageServer.log.
    LanguageClient.log
    LanguageServer.log

Current Behavior

image

Expected Behavior

image

There were some disscussion about it here: Shougo/deoplete.nvim#724 (comment)
There currently discussion abut supporting LSP snippets in ultisnips here: SirVer/ultisnips#964

@andreyorst
Copy link
Author

andreyorst commented Apr 15, 2018

Also happens with cquery v20180302-6 (2, 1.35) build from AUR
image

I'm even not sure that such snippet even make sence, because it should be just vaiv(${1:a}, ${2:b})$0, because having types inside braces isn't make any sence on function call.


Edit:
I've seen bunch of issues here, that are referring to neovim-completion-manager, neosnippet, etc. But they are all closed, however the issue is still presists

@andreyorst
Copy link
Author

andreyorst commented Apr 15, 2018

With fresh install of nvim-completion-manager I got even more interesting results:
image
image

However I won't plan to use ncm anyway, but still. This need fixing.

@autozimu
Copy link
Owner

I believe in all those cases, you can jump to snippet placeholders with your defined shortcut <c-j>, right?

@andreyorst
Copy link
Author

No. It's plain text. Not a snippet placeholder.

@YaLTeR
Copy link
Contributor

YaLTeR commented Apr 17, 2018

It works with nvim-completion-manager and UltiSnips but it requires a rather complex setup.

@andreyorst
Copy link
Author

andreyorst commented Apr 17, 2018

@YaLTeR what you refering to as "works" is just a workaround of the problem. LC inserts snippet placeholders as text, instead of integrating with snippet managers. I already have a complex setup to create seampless completion and jumping via one key for 3 plugins (deoplete, ultisnips, delimitmate), and I'd better avoid another one for another plugin. The main Idea that if plugin searches for snippet managers, means that plugin supports them. Without complex configurations. Like NCM + NCM Clang + Ultisnips do. It just works. Parameter expansion like no problem. Plug and play.

But I don't want to use NCM because it has LOT of problems. For example it perceives c style // comments as path completion. Wich is stupid. Plain text completion has lesser priority then file completion, and if I want for snippet the first item will be lost + found found somewhere in the path,
image
wich is stupid too. And many other stupid bugs like displaying completion on scrolling. Even fuzzy matching algorithms are poor. None of theese problems occur when using deoplete, so I prefer to stick with good tools. Sorry for offtopic.

I'd like to replace ALE in my current configuration with LC because LC is much faster, and has more features, like renaming symbols, good semantic completion, etc etc, but I don't want to loose snippets too.

@autozimu
Copy link
Owner

Right now, the right completion (kind of a dynamic snippet) is provided to completion engine, completion engine indeed inserts requested completion item in the right place, and one can jump to snippet placeholder(s) with predefined shortcut(s) after completion.

I do appreciate if the whole experience is smoother, i.e., it should automatically jump to the first placehoder after completion if the completed item is a snippet. But there are a few concerns about making the described behaviour happen in this repo,

  1. The jumping after completion is more closely related to completion engine, such as deoplete and NCM. They should be able to recognize if the completed item is a snippet and act accordingly.
  2. There are multiple completion engines and multiple snippet managers out there.

To be frank, I don't feel too motivated to work on this area because of those previously mentioned concerns.

Maybe https://github.com/Shougo/deoppet.nvim will make the situation better.

@autozimu autozimu changed the title LC + Deoplete + UltiSnips bundle results wrong parameter expansion Snippet placeholder is not activated after completion if completed item is a snippet Apr 20, 2018
@andreyorst
Copy link
Author

andreyorst commented Apr 20, 2018

@autozimu You can define a snippet and pass it to snippet manager instead of completion manager. Because snippets that are provided by snippet manager are displayed in completion manager popup, you can expand it via snippet manager plugin mappings, because it is it's work. Not a work of completion plugin.

So there is no need to pass snippet to completion system. You need to pass a trigger only to completion engine (or even nothing, because trigger is contained in snippet definition, so it will be listed in completion list automatically), wich in this case will be function name only (vaiv in this case). Then completion engine will find a snippet that you have passed to snippet engine. And user will desire, does he want to trigger a snippet with his snippet manager, or does he want just complete the name of the function. That's how it done in NCM for example.

So

The jumping after completion is more closely related to completion engine

Wrong. It is related to Snippet engine only.

There are multiple completion engines and multiple snippet managers out there

And most completion managers are already well integrated with most of snippet managers.

So only implementation of passing snippets to snippet managers is needed. Most of snippet engines provide a way to pass a snippet definition to them in realtime. And in case that your plugin already searches for snippet managers, and defines a snippet, you need only to change where to pass it. Because passing it to completion system is wrong in first place.

@autozimu
Copy link
Owner

As I said, I won't be actively working on this.

PR is always welcome.

@andreyorst
Copy link
Author

andreyorst commented Apr 20, 2018

Then disable searching for snippet plugins because it impossible to use LCN with it. Or make a setting to disable it, because It is absolutely unusable when you need to edit every completion to get rid of placeholders that are just plain text.

@andreyorst
Copy link
Author

andreyorst commented Apr 20, 2018

IIUUC your current implementation looks like this:

Lang Server creates a snippet → LCN → Completion plugin → Snippet plugin

So if user doesn't use any completion plugin, but uses snippet plugin (as it doesn't need a completion system, because it only displays the triggers in it) User simply can't use snippets because they are passed to nowhere.

If your plugin will pass it directly to snippet plugin (wich is make sence, because snippet should be passed to snippet manager, and completion should be passed to completion manager) any user, with or without completion plugin will be able to use such snippets. I'm proposing a change to:

Lang Server creates a snippet → LCN → Snippet Manager

The rest is up to snippet manager, it will pass snippet's trigger to completion system automatically, because it is usual behavior. And user will decide if he want to expand it, or just plain complete the trigger. I'ts not only makes it easier to everyone to implemet and maintain this, but more logical too, because each plugin will to the work it was designed to do.

You may wonder why you would decide to expand function name only? There are some situations like so:

// fancy function pointer typedef
typedef int (*lambda)();

// lets use this function via pointer in main()
int vaiv(int a, int b, int c)
{
    return a + b + c;
}

So for example in main i would like to have function name only, to pass it's address to function pointer:

int main()
{
    lambda ptr_to_vaiv = vaiv; // Imagine I've completed name only
    return ptr_to_vaiv(1, 2, 3); // Now I expand the snippet defined by Lang Server
}

But with your current realisation I will have:

int main()
{
    lambda ptr_to_vaiv = vaiv(${1:int a}, ${2:int b}, ${3:int c})$0| // Jeez...
}

Where | is cursor position at the end of the string right after completion.

@andreyorst andreyorst changed the title Snippet placeholder is not activated after completion if completed item is a snippet Snippets should be passed to snippet managers instead of completion plugins Apr 20, 2018
@andreyorst
Copy link
Author

i.e., it should automatically jump to the first placehoder after completion

And this is not what i;m asking for at all. Because it is snippet manager work too. And it will work out of the box with proposed change, because user will trigger a snippet himself. Even without completion plugin.

@YaLTeR
Copy link
Contributor

YaLTeR commented Apr 20, 2018

This does make sense actually, typing the whole name of the function and hitting the expand snippet key doesn't seem to involve a completion manager plugin in any way. It's kinda tied together though because language servers return snippets as part of completion responses, not by themselves.

@andreyorst
Copy link
Author

andreyorst commented Apr 20, 2018

@YaLTeR Yes, you even don't need to type the whole name of the function, because you can complete it with vim's builtin completion, wich doesn't even need to integrate with LC, and then hit expand snippet button, because basicaly you completed a trigger's text

It's kinda tied together though because language servers return snippets as part of completion responses, not by themselves.

Well I guess that language client, wich recieves the completion from language server, should decide where to pass it, as it client's job to handle interfaces between vim, pugins and lang server. If it is snippet - to snippet manager. If it is semantic completion - to completion manager.

@andreyorst
Copy link
Author

andreyorst commented Apr 22, 2018

@autozimu @YaLTeR I've made a simple snippet manager, and developed a function that should allow other plugins to communicate with it. Take a look:
adding flash snippet

Here I define a snippet by hand, by calling a function:
SimpleSnippets#addFlashSnippet('some_func', 'some_func(${1:a}, ${0:b})')
which accepts a trigger and a snippet's body as a parameters (i've removed last line count parameter, as I decided that it is function who need to calculate that). Current example shows, that completion plugin doesn't involved at all.

I'm not asking of implementation of usage of this function in LCN, but just want to illustrate how I think it should work:

  • LCN calls a SimpleSnippets#addFlashSnippet() function
  • LCN tells completion plugin that there is a trigger avalible (optional step, most snippet managers also support this. If not, LCN can help here)
  • User is typing a trigger and hits his expand key - SimpleSnippets looks for defined Flash Snippets, and expands one if found.

@chemzqm
Copy link

chemzqm commented Apr 29, 2018

I think there's need to clarify the responsibility, LCN should better not bundled to specified function/plugin, it could detect the complete item text format from server response and just pass the complete item info with text format to complete plugin. It's complete plugin's job to make the snippet works.

@chemzqm
Copy link

chemzqm commented Apr 29, 2018

@autozimu snippetSupport should better not set true for now with initialize params

03:16:25 INFO Handler-main src/vim.rs:135 => {"jsonrpc":"2.0","method":"initialize","params":{"capabilities":{"textDocument":{"completion":{"completionItem":{"commitCharactersSupport":null,"documentationFormat":null,"snippetSupport":true},"dynamicRegistration":null}}},"initializationOptions":null,"processId":83753,"rootPath":"/Users/chemzqm/wechat-dev/xinjian","rootUri":"file:///Users/chemzqm/wechat-dev/xinjian","trace":"off"},"id":9}

since it's not working.

autozimu added a commit that referenced this issue Apr 30, 2018
@andreyorst
Copy link
Author

It's complete plugin's job to make the snippet works.

Snippet managers don't need completion plugins at all. You're adding an unneeded route in snippet path from LCN to snippet manager, wich will make snippets unawailible for people who simply not use completion plugins, and are just use lang server for linting and other features like renaming symbol.

how it works now without LCN in UltiSnips for example:
Ultisnips sends deoplete information about trigger, and user can complete it and then expand. So the path is:

UltiSnips->deoplete->user

lets add LCN. I'm proposing this path:
LCN sends snippet to Ultisnips,Ultisnipssends triggerto deoplete,usercompletes trigger, and expands snippet. The path is:

LCN->Ultisnips->deoplete->user

Now your proposed way of doing it:
LCN passes snippet to deoplete, deoplete passes snippet to ultisnips, ultosnips passes trigger to deoplete, user completes trigger, and expands snippet. Deoplete involved 2 times. Without deopletr snippet will never arrave to ultisnips.

LCN->deoplete->ultisnips->deoplete->user

Everyone can use ultisnips without completion plugin. Why someone should add a completion plugin as a dependency to LCN snippets?

@chemzqm
Copy link

chemzqm commented Apr 30, 2018

@andreyorst the snippet of LSP is one kind of complete item in LSP side, it could comes with other plain text complete items, how could you make user to choose complete item if different kinds of complete item goes to different plugin?
The snippet plugin could just provide an extra API for complete plugin to eval the snippet complete item, or the complete plugin eval the snippet itself, since the snippet complete item contains every snippet information needed.

Everyone can use ultisnips without completion plugin. Why someone should add a completion plugin as a dependency to LCN snippets?

It's not LCN snippets, snippets of LSP comes from complete, but snippet manager doesn't provide complete feature

@YaLTeR
Copy link
Contributor

YaLTeR commented Apr 30, 2018

I suppose one way of handling this would be to feed the snippets from the language server to the snippet plugin, then strip them from any snippet parts and feed to the completion plugin. This way the user would select the completion from the completion plugin (if they have one) and then expand it with the snippet plugin (if they have one).

@chemzqm
Copy link

chemzqm commented Apr 30, 2018

@YaLTeR that could work, but then LanguageClient-neovim would have to implement adapters of each complete plugin for feed the snippet, and the user have to manually trigger complete shortcut to complete the snippet instead of just <C-y> or <enter> to complete the snippet.

For user experience and simplification, the snippet support of LSP item should comes from completion plugin or vim/neovim natively.

@chrisduerr
Copy link

I don't feel like putting this job on completion managers is the right choice. I'd love to see this working with LCN because otherwise the placeholders are kinda useless, but I don't think it will work out nicely if we just expect completion engines to suddenly handle snippets too.

@chemzqm
Copy link

chemzqm commented Apr 30, 2018

There's no easy way for vim plugin to support this feature, so I created a feature request for neovim:
neovim/neovim#8334

@chrisduerr
Copy link

Thanks @chemzqm, I'd love to see this work with LCN. It would truly step it up to another level of convenience for me.

@lisongmin
Copy link

I workaround the problem in UltiSnips side as follow in vimrc. Hope it can help somebody who urgently need a temporary solution.

function! ExpandLspSnippet()
    call UltiSnips#ExpandSnippetOrJump()
    if !pumvisible() || empty(v:completed_item)
        return ''
    endif

    " only expand Lsp if UltiSnips#ExpandSnippetOrJump not effect.
    let l:value = v:completed_item['word']
    let l:kind = v:completed_item['kind']
    let l:abbr = v:completed_item['abbr']

    " remove inserted chars before expand snippet
    let l:end = col('.')
    let l:line = 0
    let l:start = 0
    for l:match in [l:abbr . '(', l:abbr, l:value]
        let [l:line, l:start] = searchpos(l:match, 'b', line('.'))
        if l:line != 0 || l:start != 0
            break
        endif
    endfor
    if l:line == 0 && l:start == 0
        return ''
    endif

    let l:matched = l:end - l:start
    if l:matched <= 0
        return ''
    endif

    exec 'normal! ' . l:matched . 'x'

    if col('.') == col('$') - 1
        " move to $ if at the end of line.
        call cursor(l:line, col('$'))
    endif

    " expand snippet now.
    call UltiSnips#Anon(l:value)
    return ''
endfunction

imap <C-k> <C-R>=ExpandLspSnippet()<CR>

I'm not familiar with vimL and UltiSnips, it should much simple than what i written. I hope there is an official solution, either LCN or snippet side.

@butterflyfish
Copy link

butterflyfish commented Jun 2, 2018

@lisongmin, this workaround duplicated func name. example,
exec.Command(${1:name string}, ${2:arg ...string})$0
after kick it will be changed to
exec.CommandCommand(name string, arg ...string)

@lisongmin
Copy link

How to reproduce? the patch work fine with me:

  1. ctrl + n selecet complettion
  2. ctrl + k will auto snippet like this:

20180602t160817_1130x78_scrot

by the way, I just test it under linux.

@butterflyfish
Copy link

steps are the same as yours, but language is golang.
I found it works well in c function, e.g.

    fcntl(${1:int}, ${2:int, ...})$0
will be changed to 
    fcntl(int, int, ...)

is it possible caused by prefix exec. ?

@lisongmin
Copy link

@butterflyfish can your try this version? I remove the inserted chars with len() now, and also work fine with me.

function! ExpandLspSnippet()
    call UltiSnips#ExpandSnippetOrJump()
    if !pumvisible() || empty(v:completed_item)
        return ''
    endif

    " only expand Lsp if UltiSnips#ExpandSnippetOrJump not effect.
    let l:value = v:completed_item['word']
    let l:matched = len(l:value)
    if l:matched <= 0
        return ''
    endif

    " remove inserted chars before expand snippet
    if col('.') == col('$')
        let l:matched -= 1
        exec 'normal! ' . l:matched . 'Xx'
    else
        exec 'normal! ' . l:matched . 'X'
    endif

    if col('.') == col('$') - 1
        " move to $ if at the end of line.
        call cursor(line('.'), col('$'))
    endif

    " expand snippet now.
    call UltiSnips#Anon(l:value)
    return ''
endfunction

imap <C-k> <C-R>=ExpandLspSnippet()<CR>

@butterflyfish
Copy link

perfectly. works now. thank you very much!

@butterflyfish
Copy link

@lisongmin how about sending MR to upstream UltiSnips?

@lisongmin
Copy link

I'd like to do so, but i don't known how to remove inserted candidate words in UltiSnips way. It should simpler than the workaround.

@hkmix
Copy link

hkmix commented Jun 22, 2018

Here's a slight tweak to @lisongmin's tweak to allow the use of the Enter key:

Change the initial return to return a newline:

    if !pumvisible() || empty(v:completed_item)
        return '^M'
    endif

where ^M is the output of <C-v><C-m>. Then you can map <CR>:

imap <silent> <CR> <C-r>=ExpandLspSnippet()<CR>

And after selecting a completion you can hit Enter to start the expansion.

@languitar
Copy link

Is there a chance to get first placeholder to work as well? If I use the proposed "hack", the cursor is correctly placed on the first placeholder, but it is not selected and thus cannot be replace by starting to type.

@butterflyfish
Copy link

it's recommended to use https://github.com/ncm2/ncm2-ultisnips

@languitar
Copy link

@butterflyfish How does that relate to deoplete, which I have been using so far?

@andreyorst
Copy link
Author

@autozimu the conversation went off topic. Should I close the issue, or you'll lock the conversation?

@TylerHorth
Copy link

I made an autocommand based on the CompleteDone event which some of you may find useful.

let g:ulti_expand_res = 0 "default value, just set once
function! CompleteSnippet()
  if empty(v:completed_item)
    return
  endif

  call UltiSnips#ExpandSnippet()
  if g:ulti_expand_res > 0
    return
  endif
  
  let l:complete = type(v:completed_item) == v:t_dict ? v:completed_item.word : v:completed_item
  let l:comp_len = len(l:complete)

  let l:cur_col = mode() == 'i' ? col('.') - 2 : col('.') - 1
  let l:cur_line = getline('.')

  let l:start = l:comp_len <= l:cur_col ? l:cur_line[:l:cur_col - l:comp_len] : ''
  let l:end = l:cur_col < len(l:cur_line) ? l:cur_line[l:cur_col + 1 :] : ''

  call setline('.', l:start . l:end)
  call cursor('.', l:cur_col - l:comp_len + 2)

  call UltiSnips#Anon(l:complete)
endfunction

autocmd CompleteDone * call CompleteSnippet()

These are the mappings I use, although any mappings should work with the autocommand:

imap <silent><expr> <tab> pumvisible() ? "\<c-y>" : "\<tab>"

let g:UltiSnipsExpandTrigger="<NUL>"
let g:UltiSnipsListSnippets="<NUL>"
let g:UltiSnipsJumpForwardTrigger="<tab>"
let g:UltiSnipsJumpBackwardTrigger="<s-tab>"

@yshui
Copy link
Contributor

yshui commented Aug 16, 2018

Why are we parsing the placeholders in vimL when we can do it in LanguageClient-neovim....

@MaskRay
Copy link
Contributor

MaskRay commented Oct 14, 2018

g:LanguageClient_hasClientSupport should just default to 0 as LSP-style snippets are not supported.

For these completion/snippet manager, when they claim they have LSP/LanguageClient-neovim integration, they should make clear if

https://microsoft.github.io/language-server-protocol/specification

InsertTextFormat.Snippet is supported and if they can interpret

interface CompletionItem insertText?: string; (deprecated) or textEdit?: TextEdit; (used by ccls and clangd)

@roxma
Copy link
Contributor

roxma commented Jan 21, 2019

FWIW, there're some differences between ultisnips/neosnippet/snipmate syntax and lsp snippet syntax.

The best way for integration, I believe, is to first parse the lsp snippet, and then convert it to your native snippet code. I have written a small parser, and as a result, ultisnips/neosnippet/snipmate can integrate perfectly with ncm2.

https://github.com/ncm2/ncm2-neosnippet
https://github.com/ncm2/ncm2-ultisnips
https://github.com/ncm2/ncm2-snipmate

The parser and the plugins listed above are all written from scratch, and MIT licensed. Feel free to reuse some of the code for your own use case.

@Shougo
Copy link

Shougo commented Jan 28, 2019

It is fixed in the latest version of neosnippet.

@autozimu The issue can be closed.

@chrisduerr
Copy link

I just tested this with Rust and it works amazingly well.

Thanks for your work @Shougo!

@autozimu
Copy link
Owner

@Shougo Thanks!

@autozimu autozimu mentioned this issue Jan 29, 2019
@autozimu
Copy link
Owner

Verified. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests