Skip to content

make sure to use node on js file - #23

Closed
Chanakan5591 wants to merge 1 commit into
zbirenbaum:masterfrom
Chanakan5591:master
Closed

make sure to use node on js file#23
Chanakan5591 wants to merge 1 commit into
zbirenbaum:masterfrom
Chanakan5591:master

Conversation

@Chanakan5591

Copy link
Copy Markdown

A very simple fix for this to work on Windows. Since windows don't understand shebang, we need to make sure it use node from the Path environment variable. (There might be a better way to do this but I'm pretty new to Lua)

@zbirenbaum

zbirenbaum commented May 2, 2022

Copy link
Copy Markdown
Owner

Technically I already implemented (and later fixed!) overriding the command so that windows users can do so, but since passing node as an additional arg works fine on all systems I'll just do that without a check. While normally I am happy to take contributions, to be honest the code here is a bit redundant and the fix is pretty easy so I'll just merge it myself. Thank you for pointing this out to me though, and I wish you luck learning lua!

Just for your reference, if the check were necessary the way you would probably want to do this is just by changing the main statement to be the following:

M.merge_server_opts = function (params)
  return vim.tbl_deep_extend("force", {
    cmd = {
        vim.fn.has("win32") and "node" or nil,
        require("copilot.util").get_copilot_path(params.plugin_manager_path)
    },
    name = "copilot",
    trace = "messages",
    root_dir = vim.loop.cwd(),
    autostart = true,
    on_init = function(_, _)
      vim.schedule(M.buf_attach_copilot)
      vim.schedule(register_autocmd)
    end,
    on_attach = function()
      vim.schedule_wrap(params.on_attach())
    end,
  }, params.server_opts_overrides or {})
end

@zbirenbaum zbirenbaum closed this May 2, 2022
@Chanakan5591

Copy link
Copy Markdown
Author

I learnt new thing about lua today, thanks for that :D

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants