Alexey-T / CudaText

Cross-platform text editor, written in Free Pascal
Mozilla Public License 2.0
2.54k stars 174 forks source link

Language server can't keep up with typing speed #4301

Closed jpgpng closed 2 years ago

jpgpng commented 2 years ago

Is it just normal overheat of rpc-json connection? Or is it the limitation of cudatext's implementation of lsp? The language server is clangd (I got it as part of the distribution from winlibs.com) and the lexer is C++. Is there anything I could do to tweak it? This is my lsp_c.json:

{
  "lexers": {
    "C++": "cpp",
    "C": "c"
  },
  "cmd_windows": [
      "clangd",
  ],
}

(I added the bin directory of mingw64 to PATH so clangd is already in PATH).

I got the sample from here: https://wiki.freepascal.org/CudaText_plugins#LSP_servers_for_C.2FC.2B.2B

It seems you didn't expect clangd to be used on Windows? The sample is for unix (cmd_unix), I changed it to use on Windows.

Alexey-T commented 2 years ago

with option "autocomplete_autoshow_chars" >0 ? I don't know but I guess LSP server cannot work as fast as you type new chars... I don't know how VSCode solves this.. @veksha do you have an idea?

veksha commented 2 years ago

@jpgpng @Alexey-T I have experimental version of cuda_lsp in this branch. https://github.com/veksha/cuda_lsp/tree/autocomplete.cache.2 you can download it and try. it will not speed up the first request (not worked on this yet), but it will show cached list while you typing your keyword.

https://user-images.githubusercontent.com/275333/186895875-fcf7d2c6-1a7c-4b75-84cc-b44bc7e887a0.mp4

veksha commented 2 years ago

wanted to install OmniPascal

you posted this in a wrong thread? @Alexey-T

jpgpng commented 2 years ago

with option "autocomplete_autoshow_chars" >0 ? I don't know but I guess LSP server cannot work as fast as you type new chars... I don't know how VSCode solves this.. @veksha do you have an idea?

I didn't set this option in my user.json. But on default.json it's "autocomplete_autoshow_chars": 0, my user.json is simply:

{
  "ui_theme" : "ebony",
  "ui_theme_syntax" : "ebony",
  "ui_toolbar_show" : true,
  "font_name" : "Consolas",
  "font_size" : 11
}
Alexey-T commented 2 years ago

@jpgpng please try version linked in @veksha ‘s post.

is it faster?

jpgpng commented 2 years ago

@Alexey-T This update is long. First, I realized I didn't have the LSP plugin installed. The wiki is lengthy and I jumped directly into LSP for C++ section. I have no idea why with only the lsp_c.json config and clangd I still can get code completion (has to press Ctrl + Space). After that, I got the official LSP plugin installed. There is no different between having it or not. The completion speed didn't improve. It seems the support for LSP is there, built-in in CudaText, without the need to install that plugin and the plugin only add more options. Second, I tried to set "autocomplete_autoshow_chars": 3 in lexer C++.json. Well, now I realized what is the different between having the LSP plugin installed or not. Without the LSP plugin installed, it will suggest nonsense and automatically append nonsense like __asm (e.g: <iostream>__asm). With the LSP plugin installed, it will work as expected. Finally, I removed the official LSP plugin, restarted CudaText and follow your instruction to install from veksha's git. Please note that you must not have the forward slash at the end of the link, otherwise CudaText will error with "Got empty list of Git branches for that repo". After removed this slash, I could select autocomplete.cache.2 and install. I restarted CudaText and realized this version is broken. If you type the word "foo" and press Ctrl + Space, you expect it to put "foobar"? No, it will put "foobarfoo" instead. It's so broken. I uninstalled it so I didn't test it with "autocomplete_autoshow_chars": 3.

jpgpng commented 2 years ago

Well, I think the best I could get is the official LSP plugin and "autocomplete_autoshow_chars": 3. I have to type slower though.

Alexey-T commented 2 years ago

After that, I got the official LSP plugin installed.

Again, you need the non-official fork of LSP plugin. use "Plugins / Addon manager / Install from Git" and URL https://github.com/veksha/cuda_lsp , then choose the branch autocomplete.cache.2 when asked.

There is no different between having it or not. The completion speed didn't improve. It seems the support for LSP is there, built-in in CudaText, without the need to install that plugin and the plugin only add more options.

No, LSP is not built-in, and only with LSP Client plugin you have the autocompletion from clangd. i asked, is it faster in 2nd case:

1st case: official LSP Client pluigin. 2nd case: from URL https://github.com/veksha/cuda_lsp

Alexey-T commented 2 years ago

Please note that you must not have the forward slash at the end of the link, otherwise CudaText will error with "Got empty list of Git branches for that repo"

Will be fixed in the next update, tks.

If you type the word "foo" and press Ctrl + Space, you expect it to put "foobar"? No, it will put "foobarfoo" instead. It's so broken.

@veksha Bug in the fork. Pls see?

veksha commented 2 years ago

@Alexey-T @jpgpng made a fix in autocomplete.cache.2 branch. please, try again.

Alexey-T commented 2 years ago

@veksha Made the tweak in atsynedit_cmp repo

pls test from source, i like this tweak.

jpgpng commented 2 years ago

@Alexey-T @jpgpng made a fix in autocomplete.cache.2 branch. please, try again.

I think I will stick with the official LSP plugin.

veksha commented 2 years ago

I think I will stick with the official LSP plugin.

@jpgpng , no problem. Experimental branch could be merged in the future, if changes will be worth of it.

@Alexey-T , ok, I will try, interesting.

Alexey-T commented 2 years ago

veksha, did you see the real speedup in your fork? in which use-case? I am lazy to install old version and test...

veksha commented 2 years ago

veksha, did you see the real speedup in your fork? in which use-case? I am lazy to install old version and test...

@Alexey-T why it must be faster? I noticed only that list is not hiding while you type or move with Left/Right key. in my fork I see almost no difference. because result for current word is populated from cache, there is no request to the server. instant action.

But in main branch:

Alexey-T commented 2 years ago

so you cache the result for the 1 word. okay. if word is being editing, (last chars are appended), cache won't help?

now it is like: type something, list stays, waits, then quickly hides and quickly shows.

in new Cud, I use timer delay 300 ms for this. you can play with bigger delay: atsynedit_cmp_form.pas

    ClosingTimerInverval:= 300;

can we avoid 'list hides/shows' with different delay?

Alexey-T commented 2 years ago

'but in main branch'. maybe I did not understand you. the 'list hides/shows' is fixed in your branch?

veksha commented 2 years ago

how my cache works: I am sending to lsp-server not the real caret.X position, but caret.X position minus length of word to the left of caret. (basically before first letter of the word). so if you are typing something, the caret.X position that will be sent to lsp-server stays the same (beginning of the word).

because while we are typing some word, we don't need new autocomplete data yet. we can use old. (if it is correct data of course) after word delimiter we are starting new word, so new autocompletion data is required.

veksha commented 2 years ago

the 'list hides/shows' is fixed in your branch?

yes. does it work for you? please, send me a video of how it is working on your side.

Alexey-T commented 2 years ago

now I got 'main' and got new branch. yes, new is faster and don't flicker..

Screencast 2022-08-28 22:45:21.zip

Alexey-T commented 2 years ago

IMO it's needed to embed this msg https://github.com/Alexey-T/CudaText/issues/4301#issuecomment-1229536528 as comment near the cache code

veksha commented 2 years ago

got new branch. yes, new is faster and don't flicker..

i see it works OK for you too. 👍

Alexey-T commented 2 years ago

yes, so let's test it for some days, and then merge the PR

jpgpng commented 2 years ago

yes, so let's test it for some days, and then merge the PR

No, pls. Don't merge something that is non-standard and not working for everyone (except it author!)! I told him I will stick with the official LSP plugin but indeed I tested his updated branch. I met with other nonsense bugs but since my English is too bad I can't express this fluently so I decided to politely refuse him don't want to insult him. But with the current state of his pls don't do this. You will break something that is already work for the sake of nothing (as he said, it's not even any faster!). Why I said his is non-standard? Yes, caching is good but caching like his that suggested completely wrong and nonsense suggestions is not good at all. He should spend more time polishing it before even attempt to get it merged. You could, create two LSP plugin and let the user to install which they want. One is the official LSP plugin, one is his (with comment that it's using caching and different from the other that is not using caching), should be called LSP Client Autocomplete.cache or you could choose a better name.

ghost commented 2 years ago

for the sake of nothing...

for the sake of [it seems, the illusion of as] there is progress and it's actively developed... software development nowadays, some weeks and a new browser version... this text editor's release speed is even more furious. do you expect it stable?

veksha commented 2 years ago

Yes, caching is good but caching like his that suggested completely wrong and nonsense suggestions is not good at all.

what do you mean? how to cache correctly? (and you are not polite at all talking about me in third person while i'm here 😆 )

Alexey-T commented 2 years ago

for me, cache is not a problem - but the changed 'filtering of listbox' is! we must

jpgpng commented 2 years ago

and you are not polite at all talking about me in third person while i'm here laughing )

Sorry :smile:

veksha commented 2 years ago

@Alexey-T @jpgpng

try to change 250 to 10 for MAX_TIMER_TIME in file cudatext\py\cuda_lsp\language.py (path on Windows OS)

# change this to 10
MAX_TIMER_TIME = 250    # ms

https://user-images.githubusercontent.com/275333/187281129-58e14434-cea2-41ac-884c-3abc058da59d.mp4

Alexey-T commented 2 years ago

so, maybe let's publish this interval as optioon?

veksha commented 2 years ago

we can also do testing for a while. I don't see any problems with 10 ms. It can be a new default. why not? new option could be confusing to users, it only makes sense to devs.

Alexey-T commented 2 years ago

tested on the file cudatext/py/cuda_hilite_occurrences/__init__.py, line 560 res = _g|et_occurrences(ed_self)

moving left/right -> anyway, still slow calculations of lsp, listbox hides/shows.

Alexey-T commented 2 years ago

(tested on the 'main' branch!)

veksha commented 2 years ago

(tested on the 'main' branch!)

yes. me too. ditched cache branch already.

moving left/right -> anyway, still slow calculations of lsp, listbox hides/shows.

I see clear improvement on clangd, but yes, python (pylsp) is still slow for some reason (it is fast in Sublime)

veksha commented 2 years ago

moving left/right

Sublime has not such functionality. I need to type and erase with backspace and type again to test speed.

Alexey-T commented 2 years ago

veksha, i 've read about new Py server:

Running alongside LSP-pyright

LSP-pyright is a more modern, faster and actively supported alternative to LSP-pylsp. While it's arguably much better solution for validating the code and type-checking, it doesn't support linters or code formatters like flake8, pyflakes, pydocstyle, yapf or black. The solution to that could be to run them alongside each other with code checking features disabled in LSP-pylsp. To achieve that,...

https://packagecontrol.io/packages/LSP-pylsp I didn't try it

veksha commented 2 years ago

pylsp is very fast in Sublime. 🤔

jpgpng commented 2 years ago

Full list of language servers: https://langserver.org/ Python has 5 language servers as it shown.

veksha commented 2 years ago

i'm using this one: https://github.com/python-lsp/python-lsp-server

veksha commented 2 years ago

And I think I have to return to my cache branch experiments, because I see in specs:

clangd returns isIncomplete=True, so I will need to disable cache for such servers.

Alexey-T commented 2 years ago

veksha, any progress with this topic?

veksha commented 2 years ago

It was on hold. I learned a lot more since then. I will try to mimic Sublime Text.

veksha commented 2 years ago

rebased and improved: https://github.com/veksha/cuda_lsp/tree/autocomplete.cache.2 now cache works with latest cuda_lsp + made the following change:

let's test it for some long time.

veksha commented 2 years ago

https://user-images.githubusercontent.com/275333/192165514-52121654-709a-48be-bb44-4a01322cc00d.mp4

Alexey-T commented 2 years ago

@jpgpng Pls install from veksha's Git and test. git branch 'autocomplete.cache.2' !

veksha commented 2 years ago

Yes, caching is good but caching like his that suggested completely wrong and nonsense suggestions is not good at all. He should spend more time polishing it before even attempt to get it merged.

@jpgpng cache branch is merged to main branch. you can test and tell if it suggests nonsense suggestions to you now.

Note: there is a new option "hard_filter" (default 0) also speed must be improved with use_cache=1 option (default 1)

veksha commented 2 years ago

can be closed?

Alexey-T commented 2 years ago

yes, thanks.