[READY] Do not depend on UltiSnips internals to fetch snippets

[micbou]

Use the UltiSnips#SnippetsInCurrentScope public function instead of the UltiSnips_Manager internal object to fetch snippets. Fixes #2320.

Give a workaround in the FAQ for snippets not suggested as candidates if added with the :UltiSnipsAddFiletypes command.

---

This change is

[vheon]

I think that unless we provide an API that allow to add additional identifier like snippets, hence delegating the "when to do it" to the external source, the FAQ is the best way. This way we cover the majority of the cases anyway.

---

Reviewed 3 of 3 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[puremourning]

Reviewed 2 of 3 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

python/ycm/youcompleteme.py, line 689 [r1] (raw file):
According to the docs:

This function simply returns a vim dictionary with the snippets whose trigger
matches the current word. If you need all snippets information of current
buffer, you can simply pass 1 (which means all) as first argument of this
function, and use a global variable g:current_ulti_dict_info to get the
result (see example below).

It's the last bit that I'm surprised by. It says to use g:current_ulti_dict_info not the return value

the example in question:

function! GetAllSnippets()
  call UltiSnips#SnippetsInCurrentScope(1)
  let list = []
  for [key, info] in items(g:current_ulti_dict_info)
...

---

Comments from Reviewable

[puremourning]

Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

python/ycm/youcompleteme.py, line 689 [r1] (raw file):

Previously, puremourning (Ben Jackson) wrote…

According to the docs:

This function simply returns a vim dictionary with the snippets whose trigger
matches the current word. If you need all snippets information of current
buffer, you can simply pass 1 (which means all) as first argument of this
function, and use a global variable g:current_ulti_dict_info to get the
result (see example below).

It's the last bit that I'm surprised by. It says to use g:current_ulti_dict_info not the return value

the example in question:

function! GetAllSnippets()
  call UltiSnips#SnippetsInCurrentScope(1)
  let list = []
  for [key, info] in items(g:current_ulti_dict_info)
...

Additionally, won't this break if the user doesn't have UltiSnips installed? We used to detect it by catching the import exception.

---

Comments from Reviewable

[micbou]

Reviewed 3 of 3 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

python/ycm/youcompleteme.py, line 689 [r1] (raw file):

It's the last bit that I'm surprised by. It says to use g:current_ulti_dict_info not the return value

This function also returns the g:current_ulti_dict variable, which contains what we want.

Additionally, won't this break if the user doesn't have UltiSnips installed? We used to detect it by catching the import exception.

This is what the try/except block is for. In fact, it is better this way because it supports lazy-loading of UltiSnips.

---

Comments from Reviewable

[puremourning]

Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

python/ycm/youcompleteme.py, line 689 [r1] (raw file):

This function also returns the g:current_ulti_dict variable, which contains what we want.

OK makes sense, but feels a tad like relying on undocumented (and thus unstable) behaviour.

This is what the try/except block is for. In fact, it is better this way because it supports lazy-loading of UltiSnips

OK sure. Thanks makes sense

---

Comments from Reviewable

[micbou]

Review status: 2 of 3 files reviewed at latest revision, 1 unresolved discussion.

---

python/ycm/youcompleteme.py, line 689 [r1] (raw file):

OK makes sense, but feels a tad like relying on undocumented (and thus unstable) behaviour.

Good point. I've decided to follow the documentation.

---

Comments from Reviewable

[kal444]

Does the try/except block inform the user in anyway? (I don't know enough about python/vim interaction to know if vim.error is reported regardless. If not, would be a good idea to do a vimsupport.PostVimMessage().

When I was debugging this, it took a while to track down the issue, an error at the status line would have helped.

Thanks for the quick fix!

[micbou]

Does the try/except block inform the user in anyway? (I don't know enough about python/vim interaction to know if vim.error is reported regardless. If not, would be a good idea to do a vimsupport.PostVimMessage().

Printing a message on the status line would be annoying for those that don't use UltiSnips. This is where a logfile would be useful.

---

Reviewed 1 of 1 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

Comments from Reviewable

[puremourning]

---

Reviewed 1 of 3 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

Comments from Reviewable

[puremourning]

Reviewed 1 of 1 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

Comments from Reviewable

[budi]

---

Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

Comments from Reviewable

[homu]

☔ The latest upstream changes (presumably #2324) made this pull request unmergeable. Please resolve the merge conflicts.

[micbou]

PR rebased.

A lot of users seem to be affected by this so let's merge the PR.

@homu r+

---

Reviewed 2 of 2 files at r3.
Review status: all files reviewed at latest revision, all discussions resolved.

---

Comments from Reviewable

[homu]

📌 Commit 5dca552 has been approved by micbou

[homu]

⚡ Test exempted - status