Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add async mode #58

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Add async mode #58

wants to merge 6 commits into from

Conversation

karta0807913
Copy link

@karta0807913 karta0807913 commented Aug 11, 2021

I want to implement the current mistakes:

  1. If company-tabnine-wait is too small, the result from tabnine will be dropped.
  2. If company-tabnine-wait is too big, some old computers (like mine) are slow when typing.
  3. When the input is too fast, the company-tabnine--process-filter cannot parse output from the process correctly.
  4. sometimes the option disappears when i type too fast
    magic
  • remove company-tabnine--response, add company-tabnine--response-cache
    and company-tabnine--response-cache.
  • add show-user-message option.
  • add company-tabnine-max-request-buffer option.
  • add buffer name in process.
  • move company-tabnine-kill-process to interactive part.
  • rewrite company-tabnine--decode to accept multiline inputs.
  • remove 'stop symbol in company-tabnine--prefix

* remove company-tabnine--response, add company-tabnine--response-cache
  and company-tabnine--response-cache.
* add show-user-message option.
* add company-tabnine-max-request-buffer option.
* add buffer name in process.
* move company-tabnine-kill-process to interactive part.
* rewrite company-tabnine--decode to accept multiline inputs.
* remove 'stop symbol in company-tabnine--prefix
@dmorady1
Copy link

I guess that you @karta0807913 forgot to add @TommyX12 as a reviewer in this pr and therefore he might not get notified for this pr.
Since, I had similar issues it would be nice to see if this can be merged.

@TommyX12
Copy link
Owner

Thanks for the change. When I tested it out, it seems like sometimes the completion candidates will glitch out, sometimes having “??” appearing inside the text. Not sure if it’s a problem only on my machine; could you also reproduce the issue? I’m using Emacs 27.2 on Apple M1.

@karta0807913
Copy link
Author

karta0807913 commented Aug 30, 2021

@TommyX12
It's because the escape character (like \n or \r) cannot be shown in emacs.
I tried to change the display property to fix this problem before.
Unfortunately, it doesn't work.

You can use the following code to reproduce this problem.

(defun company-test (command &rest args)
  (interactive (list 'interactive))
  (cl-case command
    (interactive (company-begin-backend 'company-test))
    (prefix '("T" . t))
    (candidates (list (concat "AB" (propertize "E" 'display "¬") "CD") ;; display property works.
                      (concat "AB" (propertize "\\n" 'display "¬") "CD") ;; display property works.
                      (concat "AB" (propertize "\n" 'display "¬") "CD") ;; display property not working :(
                      (concat (propertize "A\n" 'display "A¬B¬C") ))))) ;; display property not working :(

I think I will create a new issue in company-mode later.

@karta0807913
Copy link
Author

@dmorady1 thank you for replay.
But I cannot add new reviewer in this pull request.
I don't know why :'(.

@TommyX12
Copy link
Owner

@TommyX12
It's because the escape character (like \n or \r) cannot show in emacs.
I tried to change the display property to fix this problem before.
Unfortunately, it doesn't work.

You can use the following code to reproduce this problem.

(defun company-test (command &rest args)
  (interactive (list 'interactive))
  (cl-case command
    (interactive (company-begin-backend 'company-test))
    (prefix '("T" . t))
    (candidates (list (concat "AB" (propertize "E" 'display "¬") "CD") ;; display property works.
                      (concat "AB" (propertize "\\n" 'display "¬") "CD") ;; display property works.
                      (concat "AB" (propertize "\n" 'display "¬") "CD") ;; display property not working :(
                      (concat (propertize "A\n" 'display "A¬B¬C") ))))) ;; display property not working :(

I think I will create a new issue in compnay-mode later.

That should explain the “??” Character showing up; though sometimes even without “??”, the completion candidates become glitched, particularly when I type slowly. In my experience this happened while editing a file that is saved to disk. Let me know if you can reproduce this issue as well.

@karta0807913
Copy link
Author

karta0807913 commented Aug 31, 2021

@TommyX12
Hello, I fix the "??" characters problem.
Thanks for the help from @dgutov.

Unfortunately, I cannot reproduce the error -- it looks fine on my computer (emacs 27.1).
Can you try this version again? Perhaps it is fixed in this commit.

@TommyX12
Copy link
Owner

Just tried it on Emacs 26.2 on an older Mac:
image

Same problem persists 🤔

@karta0807913
Copy link
Author

@TommyX12 I am little bit confuse.
In the picture,I cannot get anything wrong.
Can you please tell me where is the error?

@TommyX12
Copy link
Owner

TommyX12 commented Oct 4, 2021

@TommyX12 I am little bit confuse. In the picture,I cannot get anything wrong. Can you please tell me where is the error?

My tabnine settings is to show 5 completion candidates, but in the image, there's clearly more candidates than it should be. All the candidates after the fifth one are not supposed to be there. Also, their text are wrong: there shouldn't be 4 newlines in the completed text.

@karta0807913
Copy link
Author

My tabnine settings is to show 5 completion candidates, but in the image, there's clearly more candidates than it should be.

@TommyX12 that because the result of the last prediction has been cached. For example, when you are inserting "This is a[n]" ([] is the cursor). Since the result from the Tabnine server or your local network is delayed, the programmer cannot get the result immediately.

However, the programmer can keep inserting the result. To solve this problem, when the predicted data is back, my implementation compares results from Tabnine and the current text to see if the prediction is correct. For example (if the candidate is 3):

Prediction table:
Current text This is an[o]
Prefix sent to Tabnine: This is an

index predict result
1-1 This is anonymous keep
1-2 This is an apple delete
1-3 This is another keep

Therefore, the prediction 1-1 and 1-3 will be shown, and keep sending requests to Tabnine in the background.
So in next time, the prediction returns from the Tabnine keep comparing to the current text.

Prediction table:
Current text: This is anoth[e]
Prefix sent to Tabnine: This is anot

index predict result
2-1 This is another text keep
2-2 This is another example keep
2-3 This is another words keep
1-1 (old result) This is anonymous delete
1-3 (old result) This is another keep

That's why you see the candidates are more than 3.
I implemented this function because of the following reasons:

  1. The delay of Tabnine. (As I said before)
  2. Sometimes current result from Tabnine is not actually what I need. But the result was showing in the previous result. So I want to keep the current prediction and the previous prediction (matching the current text).

Of course, I can delete or change this feature if you don't like it. We can use some algorithm that changes the mechanism to make the numbers of candidates always is fixed. For example, we can use the length, confidence, and age of the candidate to filter and make sure the result is sensible.

Also, their text are wrong: there shouldn't be 4 newlines in the completed text.

Unfortunately, this is the actual result of Tabnine. :(
The program will not change results.

@CyberShadow
Copy link

CyberShadow commented Oct 5, 2022

Hi @karta0807913, thank you for your work!

I tried your branch, and unfortunately I'm finding that asynchronous mode works worse than the current version in this repository.

As a test, I'm trying the following Python file:

import numpy as np

ML_LABELS = np.array([
	[1, 1, 0, 0, 0],
	[1, 0, 0, 0, 0],
	[1, 0, 1, 0, 0],
	[1, 0, 1, 1, 0],
	[1, 0, 1, 1, 1],
])

# Transpose ML_LABELS
b =

(point is at the end, after =)

With the old non-asynchronous version, I can invoke company-complete three times, and each invocation consecutively inserts ML_LABELS, then .transpose, then (). All good.

With your branch, I have to invoke company-complete three times every time to get the next completion (nine times in total). This seems like a regression.

Hope you find this feedback helpful!

@karta0807913
Copy link
Author

Hello @CyberShadow,
Thank you for testing my code 😄

In the old non-asynchronous version, the user gets a huge delay when calling company-complete (unless you are using a faster computer), and this will annoy the user who want to predict when they are typing. Therefore, I make the company-complete function returns immediately and process the result in the following call. So that is the reason you have to call company-complete more times than before.

In this version, I want to improve users experience when they want to predict when they are typing. So I did the change I mentioned before. In my changes, I send a request to the company server and return the function call right away. Then use the user input between this and previous call to filter out these outdated result which predict incorrectly.

If you have a better idea, please tell me!
Thank you!

@CyberShadow
Copy link

CyberShadow commented Oct 8, 2022

Have you considered using company's built-in support for asynchronous backends? https://www.badykov.com/emacs/async-company-mode-backend/

I see there is some commented-out code in this direction:

;; TODO: should we use async or not?
;; '(:async . (lambda (callback)
;; (funcall callback (company-tabnine--candidates) arg))))

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.

4 participants