copilot-emacs / copilot.el

An unofficial Copilot plugin for Emacs.
MIT License
1.79k stars 126 forks source link

Differences from JetBrains version #150

Closed haukot closed 1 year ago

haukot commented 1 year ago

HI! I've done research and reverse-engineered the Github Copilot protocol between IDE plugin and agent.js, and have tried to document it. This is based on Github Copilot plugin version for JetBrains and some parts of agent.js (repo https://github.com/haukot/copilot_specifications)

I'd like to make Emacs plugin a bit more complete. Specifically I want to add commands "textDocument/didFocus"(to have better context based on last access time), "textDocument/didChange" with range(so it'll not send whole document to the agent.js on each request), and "textDocument/didClose"(to remove unused files from current context). And probably update agent.js binary.

Will you consider PR with these changes if I made them, or do you want to stick with the Vim implementation?

zerolfx commented 1 year ago

One crucial missing feature is the ability to provide completions based on other files.

Thank you for creating these wonderful specifications, which are much easier to read than Copilot.vim's source code. I truly appreciate your efforts in bridging the gap between Copilot in Emacs and Jetbrains.

There are other projects that will benefit from your research, such as:

haukot commented 1 year ago

Thanks!

There are other projects that will benefit from your research, such as:

Thanks for the note, I've sent them too!

One crucial missing feature is the ability to provide completions based on other files.

It actually already exists and works, it just could work better.

It works on the mechanic based on 'open tabs' (or, in the emacs case, open buffers). When you open a file(with didOpen), the file will be added to the context with files of the same language. But there are limitations - only 20 files max could be in the context, and not all content from these files is used(because there are limitations to the prompt length). So all files that you open are firstly sorted by the time of the last access, and after that from the file parts the prompt is built(the prompt creating algorithm is actually pretty complex and I don't know much about it. There are other analyses about that)

And two things, that Vim implementation doesn't have, are an event on a file focus("textDocument/didFocus", which sets the last access time), and an event on a file close("textDocument/didClose", which removes the file from context)

philipsd6 commented 1 year ago

Another item we would like is the ability to report telemetry, so we can gather insights on number of code completions accepted, and number accepted by language. VSCode does that but I want to keep using Emacs. Obviously this would have to be an opt in feature.

haukot commented 1 year ago

VS Code version could actually use another mechanics e.g. based on git commits, on workspace or cursor click count, but these are behind A/B test even in the VS Code version.

haukot commented 1 year ago

Another item we would like is the ability to report telemetry, so we can gather insights on number of code completions accepted, and number accepted by language. VSCode does that but I want to keep using Emacs. Obviously this would have to be an opt in feature.

As I understand this telemery is already sent with notifyShown, notifyAccepted, and notifyRejected

https://github.com/zerolfx/copilot.el/blob/main/copilot.el#L534 https://github.com/zerolfx/copilot.el/blob/main/copilot.el#L556 https://github.com/zerolfx/copilot.el/blob/main/copilot.el#L541

haukot commented 1 year ago

There is telemetry that is not sent - the telemetry about exceptions that occur. But I'm not sure we need that

github-actions[bot] commented 1 year ago

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

CeleritasCelery commented 1 year ago

I would be interested in implementing this. My understanding is that we need two sets of changes, one for completions from other buffers, and one for focus/close.

When you open a file(with didOpen), the file will be added to the context with files of the same language. But there are limitations - only 20 files max could be in the context, and not all content from these files is used(because there are limitations to the prompt length).

Does this actually trigger on opening a file? From looking at the code it looks like this is only sent when completions are actually requested in a buffer. Actually opening the buffer does not trigger this. But it sounds like it should.

both textDocument/didFocus and textDocument/didClose seem like they should be easy to add to window-buffer-change-function and kill-buffer-hook respectively.

emil-vdw commented 1 year ago

@CeleritasCelery are you still planning on implementing this? I'm currently playing around with this myself and wouldn't mind submitting a PR.

Also why add to window-buffer-change-function instead of window-selection-change-functions? Surely switching to an already open buffer should trigger textDocument/didFocus?

emil-vdw commented 1 year ago

@haukot I'm closing this issue as complete due to #164.

If there's some functionality that's still missing please re-open or log a new issue for that functionality?

haukot commented 1 year ago

@emil-vdw Sure, thank you for your work!