feat: Install server #247

jcs090218 · 2024-01-29T08:58:45Z

copilot.el

jcs090218 · 2024-01-29T09:00:57Z

copilot.el

+(defcustom copilot-version "1.14.0"
+  "Copilot version."
+  :type 'string
+  :group 'copilot)


I've made this variable customizable so the user can choose what version to install.

emil-vdw · 2024-01-31T09:15:07Z

As part of this PR, we would also get rid of the dist folder and current chore workflow no?

zerolfx · 2024-01-31T09:21:48Z

@emil-vdw
We may need two steps:

Prioritize using the agent.js installed by the new method then the old dist folder. If a user is using the agent.js from the dist folder, send a deprecation warning and ask them to install it via the new method.
Remove the dist folder and the chore workflow.

zerolfx · 2024-01-31T09:31:32Z

@jcs090218
Could you also modify copilot--start-agent to load the agent.js from the new installing method?
It would be better if you check the currently installed version of the NPM package and send a warning if it is mismatched with copilot-version.

zerolfx · 2024-01-31T09:51:25Z

Prioritize using the agent.js installed by the new method then the old dist folder. If a user is using the agent.js from the dist folder, send a deprecation warning and ask them to install it via the new method.

@emil-vdw After giving it another thought, I realized that this step can be skipped. It's okay to delete dist after providing a new way to install them.

jcs090218 · 2024-01-31T10:07:23Z

We may need two steps:

Prioritize using the agent.js installed by the new method then the old dist folder. If a user is using the agent.js from the dist folder, send a deprecation warning and ask them to install it via the new method.

Remove the dist folder and the chore workflow.

What if we prompt the user to install the server when it's not yet installed in the new location? This is much more straightforward IMO. 🤔 Then we can remove dist and chore workflow in this PR (or later).

Could you also modify copilot--start-agent to load the agent.js from the new installing method?
It would be better if you check the currently installed version of the NPM package and send a warning if it is mismatched with copilot-version.

👍 Done.

zerolfx · 2024-01-31T10:12:09Z

@jcs090218 Very cool! You solved the long-term issue!

One last request:
Could you test the new installation method on Windows? (In case you have a Windows machine available.)

jcs090218 · 2024-01-31T10:15:12Z

Could you test the new installation method on Windows? (In case you have a Windows machine available.)

Actually, I need someone to test on Unix-like system (linux or macOS), because I'm using Windows now. 😂

I assumed it will work since the code are dragged directly from lsp-mode.

emil-vdw

I have a few concerns:

The reliability of the copilot-node-server npm package. Can we trust it to be available, up-to-date and not to be hijacked? Should we investigate whether to pull the agent distributables straight from the corresponding github tag of the official copilot.vim repo?
The fact that at some point a mismatch between the minimum required server version and copilot.el version will break the plugin (see comment below).

copilot.el

Co-authored-by: Emil van der Westhuizen <[email protected]>

jcs090218 · 2024-02-07T09:29:32Z

The reliability of the copilot-node-server npm package. Can we trust it to be available, up-to-date and not to be hijacked? Should we investigate whether to pull the agent distributables straight from the corresponding github tag of the official copilot.vim repo?

I don't know much about the upstream copilot.vim, so I cannot suggest this. However, downloading npm packages seems very common in the Emacs world; use lsp-mode and lsp-grammarly as examples.

emil-vdw · 2024-02-07T09:40:44Z

I'm not sure but those examples may be using official distributions (albeit through npm). The copilot vim is an official github owned (official copilot) repository so it can be relied upon. The npm package being installed by us through npm is not official and we should at least strongly consider pulling from the official distribution instead.

In my opinion there should be very good reasons for using the unofficial distribution instead.

jcs090218 · 2024-02-07T09:48:36Z

Ah, okay. I thought copilot-node-server is the official... How about publishing our own npm package? Like lsp-grammarly maintains its own grammarly-language-server. 🤔

zerolfx · 2024-02-07T09:50:11Z

I found the npm package copilot-node-server from https://github.com/TerminalFi/LSP-copilot a long while ago, which is another unofficial Copilot plugin for Sublime. The NPM package is maintained at https://github.com/jfcherng/copilot-node-server.

Maybe we can use it for now and later switch to our own NPM package.

jcs090218 · 2024-02-07T09:52:14Z

Maybe we can use it for now and later switch to our own NPM package.

Yeah, I second this. It would take me some time to do this. Plus, it would make this PR too big if it was implemented in the same PR. 😅

…install-server

emil-vdw · 2024-02-11T12:59:52Z

@jcs090218 I have added some commits to fix some bugs and support Linux. I don't have access to push to your branch though.

jcs-PR#2

Fixes for Linux

jcs090218 · 2024-02-11T13:12:09Z

Merged! :)

emil-vdw · 2024-02-11T13:16:31Z

Would you mind testing those changes on Windows again as well?

jcs090218 · 2024-02-12T00:27:02Z

Would you mind testing those changes on Windows again as well?

Yeah, it works now! :)

copilot.el

jcs090218 · 2024-02-12T00:27:58Z

copilot.el

@@ -245,27 +246,27 @@ Please upgrade the server via `M-x copilot-reinstall-server`"))
  "Login to Copilot."
  (interactive)
  (copilot--dbind


The rest are indentation errors.

jcs090218 · 2024-02-16T00:39:43Z

I've made the changes to this PR. Can we merge this? 🤔

emil-vdw

Remember to pull main (jsonrpc error handling changes).

Thank you for all of your work on this project! You should have write access now and I will make sure you can merge without review (although try to get a review when practicable).

emil-vdw · 2024-02-17T09:31:18Z

copilot.el

+  (cond
+   ((null copilot-node-executable)
+    (user-error "Could not find node executable"))
+   ((not (file-exists-p copilot-install-dir))


This did not catch the condition correctly when I uninstalled the copilot server (this directory is still present). Maybe make it more reliable?

How did you uninstall the server? 🤔 The copilot-install-dir will be gone if you uninstall it through M-x copilot-uninstall-server.

Ah that makes sense. I did:

npm -g --prefix ~/.emacs.d/.cache/copilot uninstall copilot-node-server

readme.md

Co-authored-by: Emil van der Westhuizen <[email protected]>

emil-vdw · 2024-02-17T11:47:34Z

Shall we do the cleanup in a subsequent PR @jcs090218 @zerolfx?

emil-vdw · 2024-02-17T13:08:02Z

I think it's fine to clean up in a subsequent PR. Feel free to merge 👍.

sho-87 · 2024-02-18T22:53:43Z

this seems to install (or try to start) copilot server at the wrong location:

[stderr] node:internal/modules/cjs/loader:1152
[stderr]   throw err;
[stderr]   ^
[stderr] 
[stderr] Error: Cannot find module 'c:\Users\simon\.emacs.d\.cache\copilot\node_modules\copilot-node-server\copilot\bin\copilot-node-server'
[stderr]     at Module._resolveFilename (node:internal/modules/cjs/loader:1149:15)
[stderr]     at Module._load (node:internal/modules/cjs/loader:990:27)
[stderr]     at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:142:12)
[stderr]     at node:internal/main/run_main_module:28:49 {
[stderr]   code: 'MODULE_NOT_FOUND',
[stderr]   requireStack: []
[stderr] }
[stderr] 
[stderr] Node.js v21.6.1

the extra copilot beforebin leads to the wrong directory. for ref, the server was installed to C:\Users\simon\.emacs.d\.cache\copilot\node_modules\copilot-node-server\bin\copilot-node-server

jcs090218 · 2024-02-18T23:03:13Z

You are right. It's weird since I did applied the change but it some how got reverted. 🤔

I'm on it.

jcs090218 · 2024-02-18T23:09:33Z

FYI, I've opened the fix in #267. However, I cannot merge PRs without other maintainer's approval. Therefore, the current workaround is to update copilot--server-executable manually.

(setq copilot--server-executable (f-join copilot-install-dir "node_modules" "copilot-node-server" "bin" "copilot-node-server"))

jmcarcell · 2024-02-19T07:01:37Z

When opening emacs with this PR I get the following it complains that I should use copilot-install-server but when I try I get
Symbol's value as variable is void: package

jcs090218 · 2024-02-19T07:17:51Z

Can you try toggle-debug-on-error? Paste the backtrack here. Thanks!

Edit: make sure you've updated to the latest version.

jmcarcell · 2024-02-19T07:24:06Z

Debugger entered--Lisp error: (void-variable package)
  copilot-install-server()
  funcall-interactively(copilot-install-server)
  call-interactively(copilot-install-server record nil)
  command-execute(copilot-install-server record)
  execute-extended-command(nil "copilot-install-server" "install copilot")
  funcall-interactively(execute-extended-command nil "copilot-install-server" "install copilot")
  call-interactively(execute-extended-command nil nil)
  command-execute(execute-extended-command)

Indeed, I get this with main but don't get it if I checkout right before this PR

jcs090218 · 2024-02-19T07:58:01Z

I've made the fix in #268. Thanks for the bug report!

jcs090218 added 2 commits January 29, 2024 00:55

feat: Install server

8496da5

resolve conflict

0c35139

jcs090218 mentioned this pull request Jan 29, 2024

Consider more maintainers? #246

Closed

jcs090218 commented Jan 29, 2024

View reviewed changes

jcs090218 added 2 commits January 31, 2024 01:45

Prompt to install server

388130b

fix metadata

ac30026

check mismatch

90a9051

improve documentation a bit

683e27e

emil-vdw requested changes Feb 4, 2024

View reviewed changes

copilot.el Outdated Show resolved Hide resolved

copilot.el Outdated Show resolved Hide resolved

copilot.el Outdated Show resolved Hide resolved

jcs090218 and others added 3 commits February 7, 2024 01:04

Use warn instaed

56e8df5

Co-authored-by: Emil van der Westhuizen <[email protected]>

Use straight up name

82ef49d

Co-authored-by: Emil van der Westhuizen <[email protected]>

fix: Work on Unix systems

9a5791d

jcs090218 and others added 6 commits February 8, 2024 02:20

docs: Document copilot-version

6c8a3fb

Fix agent executable path for Linux

228be50

Improve formatting

32a7861

Only call callbacks when non-nil

3fb251c

Merge branch 'main' of github.com:copilot-emacs/copilot.el into feat/…

055f8f1

…install-server

Fix wrong copilot-installed-version Linux path

8379e61

Merge pull request #2 from emil-vdw/install-server

25e683f

Fixes for Linux

Use bin instead

57ab87b

jcs090218 commented Feb 12, 2024

View reviewed changes

emil-vdw mentioned this pull request Feb 12, 2024

Improve upstream binary sync workflow #242

Closed

jcs090218 added 2 commits February 15, 2024 03:06

Merge branch 'main' into feat/install-server

a4e4f43

fix: merge conflict

37259ea

jcs090218 mentioned this pull request Feb 15, 2024

fix: Find jsonrpc by header file #260

Closed

emil-vdw approved these changes Feb 17, 2024

View reviewed changes

jcs090218 and others added 3 commits February 17, 2024 01:55

Update readme.md

27dd6cc

Co-authored-by: Emil van der Westhuizen <[email protected]>

Update readme.md

62f0d7a

Co-authored-by: Emil van der Westhuizen <[email protected]>

Merge branch 'main' into feat/install-server

d7ab8df

jcs090218 merged commit 26fa165 into copilot-emacs:main Feb 17, 2024
11 checks passed

jcs090218 mentioned this pull request Feb 18, 2024

fix(copilot.el): Windows' executable path #267

Merged

jcs090218 mentioned this pull request Feb 19, 2024

fix: void variable package #268

Merged

jcs090218 mentioned this pull request Feb 20, 2024

Add package to MELPA #120

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Install server #247

feat: Install server #247

jcs090218 commented Jan 29, 2024

jcs090218 Jan 29, 2024

emil-vdw commented Jan 31, 2024

zerolfx commented Jan 31, 2024

zerolfx commented Jan 31, 2024

zerolfx commented Jan 31, 2024

jcs090218 commented Jan 31, 2024

zerolfx commented Jan 31, 2024

jcs090218 commented Jan 31, 2024 •

edited

Loading

emil-vdw left a comment

jcs090218 commented Feb 7, 2024

emil-vdw commented Feb 7, 2024

jcs090218 commented Feb 7, 2024

zerolfx commented Feb 7, 2024

jcs090218 commented Feb 7, 2024 •

edited

Loading

emil-vdw commented Feb 11, 2024 •

edited

Loading

jcs090218 commented Feb 11, 2024

emil-vdw commented Feb 11, 2024

jcs090218 commented Feb 12, 2024

jcs090218 Feb 12, 2024

jcs090218 commented Feb 16, 2024

emil-vdw left a comment

emil-vdw Feb 17, 2024

jcs090218 Feb 17, 2024

emil-vdw Feb 17, 2024

emil-vdw commented Feb 17, 2024

emil-vdw commented Feb 17, 2024

sho-87 commented Feb 18, 2024 •

edited

Loading

jcs090218 commented Feb 18, 2024

jcs090218 commented Feb 18, 2024

jmcarcell commented Feb 19, 2024

jcs090218 commented Feb 19, 2024 •

edited

Loading

jmcarcell commented Feb 19, 2024 •

edited

Loading

jcs090218 commented Feb 19, 2024 •

edited

Loading

feat: Install server #247

feat: Install server #247

Conversation

jcs090218 commented Jan 29, 2024

jcs090218 Jan 29, 2024

Choose a reason for hiding this comment

emil-vdw commented Jan 31, 2024

zerolfx commented Jan 31, 2024

zerolfx commented Jan 31, 2024

zerolfx commented Jan 31, 2024

jcs090218 commented Jan 31, 2024

zerolfx commented Jan 31, 2024

jcs090218 commented Jan 31, 2024 • edited Loading

emil-vdw left a comment

Choose a reason for hiding this comment

jcs090218 commented Feb 7, 2024

emil-vdw commented Feb 7, 2024

jcs090218 commented Feb 7, 2024

zerolfx commented Feb 7, 2024

jcs090218 commented Feb 7, 2024 • edited Loading

emil-vdw commented Feb 11, 2024 • edited Loading

jcs090218 commented Feb 11, 2024

emil-vdw commented Feb 11, 2024

jcs090218 commented Feb 12, 2024

jcs090218 Feb 12, 2024

Choose a reason for hiding this comment

jcs090218 commented Feb 16, 2024

emil-vdw left a comment

Choose a reason for hiding this comment

emil-vdw Feb 17, 2024

Choose a reason for hiding this comment

jcs090218 Feb 17, 2024

Choose a reason for hiding this comment

emil-vdw Feb 17, 2024

Choose a reason for hiding this comment

emil-vdw commented Feb 17, 2024

emil-vdw commented Feb 17, 2024

sho-87 commented Feb 18, 2024 • edited Loading

jcs090218 commented Feb 18, 2024

jcs090218 commented Feb 18, 2024

jmcarcell commented Feb 19, 2024

jcs090218 commented Feb 19, 2024 • edited Loading

jmcarcell commented Feb 19, 2024 • edited Loading

jcs090218 commented Feb 19, 2024 • edited Loading

jcs090218 commented Jan 31, 2024 •

edited

Loading

jcs090218 commented Feb 7, 2024 •

edited

Loading

emil-vdw commented Feb 11, 2024 •

edited

Loading

sho-87 commented Feb 18, 2024 •

edited

Loading

jcs090218 commented Feb 19, 2024 •

edited

Loading

jmcarcell commented Feb 19, 2024 •

edited

Loading

jcs090218 commented Feb 19, 2024 •

edited

Loading