fix: health check for ultisnips source

[ybc37]

Hey there,

I noticed the health check for UltiSnips failed for me, but it was correctly installed. The health check looks for ".*ultisnips.*" in rtp, but I think it's case-sensitive and my rtp contained UltiSnips. Does this makes sense?

Thanks!

[haorenW1025]

Hi and sorry for the long wait! Actually my rtp path is ultisnips, so I think matching both upper case and lower case character may be a better solution.

[ybc37]

Thanks for checking :)

I see, it depends on how the repo (plugin) was cloned. I used sirver/UltiSnips with vim-plug (for no reason, just copied it from somewhere).

I wonder if it's worth "fixing" the health check. At least it's a more general issue than related to ultisnips.

We could support both variants:

diff --git a/lua/completion/health.lua b/lua/completion/health.lua
index 1c43d1e..b3f4577 100644
--- a/lua/completion/health.lua
+++ b/lua/completion/health.lua
@@ -21,3 +21,3 @@ local checkSnippetSource = function()
   elseif snippet_source == 'UltiSnips' then
-    if string.match(api.nvim_get_option("rtp"), ".*ultisnips.*") then
+    if string.match(api.nvim_get_option("rtp"), ".*[Uu]lti[Ss]nips.*") then
       health_ok("You are using UltiSnips as your snippet source")

Or better make the string matching case insensitive (also for the other checks; not included in the diff):

diff --git a/lua/completion/health.lua b/lua/completion/health.lua
index 1c43d1e..4dd51d6 100644
--- a/lua/completion/health.lua
+++ b/lua/completion/health.lua
@@ -21,3 +21,4 @@ local checkSnippetSource = function()
   elseif snippet_source == 'UltiSnips' then
-    if string.match(api.nvim_get_option("rtp"), ".*ultisnips.*") then
+    local rtp = string.lower(api.nvim_get_option("rtp"))
+    if string.match(rtp, ".*ultisnips.*") then
       health_ok("You are using UltiSnips as your snippet source")

What do you think? Happy to change the PR :) Of course, feel free to close this PR, if you think it's not worth it.

[haorenW1025]

Hmm I guess having matching all case insensitive is a better solution. Also maybe we should do that for other health check component as well.

[ybc37]

I refactored the check a bit to address DRY. But tbh, I wasn't sure if you want that additional complexity?

[haorenW1025]

The extra complexity would be fine since users aren't going to call healthcheck all the time and slowing down healthcheck is not quite a big deal in my opinion. One small things you probably need is to rebase this PR since we already have snippets.nvim support and you might need to add that as well.

[ybc37]

Sure, rebased it on master :) Includes snippets.nvim now.

[haorenW1025]

You should use health_ok or health_error instead of print to integrate with healthcheck.

[ybc37]

Yes, sorry for the confusion. I tested the code snippet in a separate lua script and unfortunately left print in there when integrating it. Fixed it when rebasing on master and hoped you haven't seen it yet 😉

[haorenW1025]

LGTM! Thanks for taking the time fixing this. Highly appreciate:)