tzachar / cmp-tabnine

TabNine plugin for hrsh7th/nvim-cmp
MIT License
287 stars 28 forks source link

cmp-tabnine did not work on Windows for me. #29

Closed vzytoi closed 2 years ago

vzytoi commented 2 years ago

Windows support was recently added #21 but I noticed that it wasn't working on my Windows machine. Whenever I ran nvim, I always got the error telling me to run install.sh even though I had already done so.
So I wanted to check where TabNine was downloaded on my machine:

$version = iwr https://update.tabnine.com/bundles/version
$version = $version.content

if([environment]::Is64bitOperatingSystem){
    $platform = 'x86_64-pc-windows-gnu'
}
else{
    $platform = 'i686-pc-windows-gnu'
}

$path = "$version/$platform"

iwr "https://update.tabnine.com/bundles/$path/TabNine.zip" -OutFile TabNine.zip

if(!(Test-Path ./binaries)){
    mkdir binaries
}

if(!(Test-Path "./binaries/$path")){
    mkdir "binaries/$path"
}

expand-archive Tabnine.zip -destinationpath "./binaries/$path" -force
Remove-Item -Recurse "./TabNine.zip"

So here, a binaries/4.0.23/x86_64-pc-windows-gnu/... folder is created next to an install.sh file
But for I then went to see where source.lua was looking for my TabNine installations:

local binaries_folder = get_parent_dir(get_parent_dir(script_path())) .. 'binaries'
print(binaries_folder)
--[[ output :
    C:\Users\Cyprien\AppData\Local\nvim-data\site\pack\packer\binaries
]]

So in my case, the binaries folder is not downloaded to the same place where it is searched.
I'm not a big fan of my modifications but I only propose a solution to make the plugin work for other users in my case. Good day to all ! (:

vzytoi commented 2 years ago

I forgot to mention but this PR also solve this issue : #24

tzachar commented 2 years ago

There are several problems with this PR. First, please make sure you do not change the white spaces or the indentation. This create many spurious changes which make reviewing harder. Second, the issue is a bit more complicated, as the directories are not consistent between different setups. No once has given a satisfactory explanation yet, so your solution will break the plugin for other users.

If you can find a better explanation of why script_path() returns different directories under different setups this would be the way forward.

Maybe a better temporary solution would be to add an option in the configuration to override the binaries install directory.

vzytoi commented 2 years ago

Is script_path supposed to returns the path of the current file (in this case source.lua) ?

local function script_path()
  local str = debug.getinfo(2, "S").source:sub(2)
  return str:match("(.*" .. get_path_separator() .. ")")
end
print(script_path())
--[[ output : 
    C:\Users\Cyprien\AppData\Local\nvim-data\site\pack\packer\start\
]]
local function script_path()
  local str = debug.getinfo(2, "S").source:sub(2)
  return str
  -- return str:match("(.*" .. get_path_separator() .. ")")
end
--[[ output : 
    C:\Users\Cyprien\AppData\Local\nvim-data\site\pack\packer\start\cmp-tabnine/lua/cmp_tabnine/source.lua
]]
local function script_path()
  local str = debug.getinfo(2, "S").source:sub(2)
  if is_win() then
    str = str:gsub("/", "\\")
  end
  return str:match("(.*" .. get_path_separator() .. ")")
end
--[[ output : 
    C:\Users\Cyprien\AppData\Local\nvim-data\site\pack\packer\start\cmp-tabnine\lua\cmp_tabnine\
]]

I think this is is once again a separator issue ...

vzytoi commented 2 years ago

I manage to make it work for me by changing these two things

local function script_path()
  local str = debug.getinfo(2, "S").source:sub(2)
  if is_win() then
    str = str:gsub("/", "\\")
  end
  return str:match("(.*" .. get_path_separator() .. ")")
end
$version = iwr https://update.tabnine.com/bundles/version
$version = $version.content

if([environment]::Is64bitOperatingSystem){
    $platform = 'x86_64-pc-windows-gnu'
}
else{
    $platform = 'i686-pc-windows-gnu'
}

iwr "https://update.tabnine.com/bundles/$version/$platform/TabNine.zip" -OutFile TabNine.zip

$path = "binaries/$version/$platform"

if(!(Test-Path $path)){
    mkdir -p $path
}

expand-archive Tabnine.zip -destinationpath $path -force
Remove-Item -Recurse "./TabNine.zip"
tzachar commented 2 years ago

So it works only with these 2 changes? If so, can you either update your pull request or create a new one?

tzachar commented 2 years ago

can you try this patch?

diff --git a/lua/cmp_tabnine/source.lua b/lua/cmp_tabnine/source.lua
old mode 100644
new mode 100755
index 51e400c..3ceec5d
--- a/lua/cmp_tabnine/source.lua
+++ b/lua/cmp_tabnine/source.lua
@@ -17,8 +17,12 @@ local function json_decode(data)
        end
 end

+local function is_win()
+  return fn.has('win64') == 1 or fn.has('win32') == 1
+end
+
 local function get_path_separator()
-    if ((fn.has('win64') == 1) or (fn.has('win32') == 1)) then return '\\' end
+    if is_win() then return '\\' end
     return '/'
 end

@@ -36,8 +40,11 @@ local function build_snippet(prefix, placeholder, suffix, add_final_tabstop)
 end

 local function script_path()
-   local str = debug.getinfo(2, "S").source:sub(2)
-   return str:match("(.*" .. get_path_separator() .. ")")
+  local str = debug.getinfo(2, "S").source:sub(2)
+  if is_win() then
+    str = str:gsub("/", "\\")
+  end
+  return str:match("(.*" .. get_path_separator() .. ")")
 end

 local function get_parent_dir(path)
tzachar commented 2 years ago

it looks to be enough

vzytoi commented 2 years ago

That's exactly what I did in the second PR that I made, i'm closing this one