SirVer / ultisnips

UltiSnips - The ultimate snippet solution for Vim. Send pull requests to SirVer/ultisnips!
GNU General Public License v3.0
7.55k stars 691 forks source link

Add JumpOrExpand support #1542

Closed pilgrimlyieu closed 1 year ago

pilgrimlyieu commented 1 year ago

This is a very small improvement. As far as I know, we can not consider to jump forward instead of expanding snippets first if the expand trigger is as same as the jump forward trigger. So I introduce two new options g:UltiSnipsExpandOrJumpTrigger & g:UltiSnipsJumpOrExpandTrigger to achieve this goal. If none of them is set, it'll still expand snippets first if two triggers remain same, or it'll map separately(that is, same as before, nothing broken).

Besides, I'm wondering how to write the document. I'll appreciate if some one could help me improve the document with adding the introduction of these two new options.

I'm not familiar with this project that I may misunderstand something or make some mistakes. If so, please let me know! Any advice is welcome. Hopefully my PR could be accepted.

SirVer commented 1 year ago

Thanks for your contribution!

You understood correctly that the functionality is not there and I think your approach for implementing it is reasonable. This needs testing and update to the documentation.

As for the documentation, you just change the doc/UltiSnips.txt - it is all manual. For testing see the Contributing document.

pilgrimlyieu commented 1 year ago

I'm not a English native speaker so my expression may be unclear and vague(in this PR & in the documentation).

In addition, I'm sorry that I don't know how to add a test because EX standing for expanding snippets and JF standing for jumping forward in constant.py have been distinguished. Maybe I've missed something? Could you give me more specific details?

SirVer commented 1 year ago

I am not a native speaker either, but your english seems excellent from all that I can tell. The documentation is very well like that, but this still needs a test, otherwise it likely will regress the functionality in the next refactor.

I think you want to add a test in this file. The idea is that you use _extra_vim_config to set one of your new variables, then define two snippets, one that you expand, then another one that creates ambiguity between jumping and expanding, then you check that the right thing happened.

pilgrimlyieu commented 1 year ago

1

I've run the tmux test in WSL and it seems that tests I added have passed. Did I do correctly? What else should I do?

SirVer commented 1 year ago

Absolutely great contribution. Thank you very much for this!

SirVer commented 1 year ago

CI Failures seem to relate to latest Git version, but have nothing to do with this change. Gonna merge. Thanks again for the contributions!

Konfekt commented 1 year ago

Thank you very much! I was ever since missing this feature that e.g. neosnippet had and had to work around it by something like (using Ctrl-E as a trigger)

let g:ulti_expand_or_jump_res = 0
function! UltiSnips_ExpandableOrJumpable()
  call UltiSnips#ExpandSnippetOrJump()
  if g:ulti_expand_or_jump_res > 0
    return ""
  else
    if pumvisible()
      return "\<C-E>"
    else
      return "\<End>"
    endif
  endif
endfunction

inoremap <silent> <C-E> <c-r>=UltiSnips_ExpandableOrJumpable()<cr>
Konfekt commented 1 year ago

From the commit itself it is not obvious to me what happens in case neither a jump nor expansion is possible. Does the mapping fall back to the default operation of the supplied key combo or to no operation?

pilgrimlyieu commented 1 year ago

From the commit itself it is not obvious to me what happens in case neither a jump nor expansion is possible. Does the mapping fall back to the default operation of the supplied key combo or to no operation?

The first case, falling back to the default operation. For example if I set

let g:UltiSnipsJumpOrExpandTrigger = "<Tab>"

It'll jump or exapnd if it could, or it will send Tab.