-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Improve struct and pointer autocompletion in C #4231
Conversation
I tested this with the sample code in the issue and auto-completion works fine with the chages to fix "1.". The changes done for "2." break auto-completion and from my testing are not required. Note that I use asynccomplete for auto-completion instead of ALE's built in auto-completion. |
@hsanson unfortunate to hear that patch 2 breaks autocompletion with asynccomplete. I do not know enough about how it interacts with ALE. Can you try with ale's built-in autocompletion function? One more question, I tried to look into the appveyor results, but for the life of me I couldn't find which tests broke. What should I be looking for? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the work. Looks good and tested works on my setup.
@s-marios seems that the new patterns need to be also added to this test: |
Ah, sorry about that. Amended the test, run locally all tests passed. History rewritten, rebased, it should be ready to go! |
* Add explicit trigger characters for C (dense-analysis#4226) * Stop completion before issuing subsequent requests (dense-analysis#4226) Co-authored-by: Marios Sioutis <[email protected]>
* Add explicit trigger characters for C (dense-analysis#4226) * Stop completion before issuing subsequent requests (dense-analysis#4226) Co-authored-by: Marios Sioutis <[email protected]>
In dense-analysis#4231 some code was added to stop the completion menu if any when opening a new one. This resulted in an issue in Vim that fills the buffer with Ctrl-Z characters when deleting to the end of a line in a position that triggers auto-completion. Since auto-completion seems to work fine on all my tests I am reverting this specific change.
In #4231 some code was added to stop the completion menu if any when opening a new one. This resulted in an issue in Vim that fills the buffer with Ctrl-Z characters when deleting to the end of a line in a position that triggers auto-completion. Since auto-completion seems to work fine on all my tests I am reverting this specific change.
In dense-analysis#4231 some code was added to stop the completion menu if any when opening a new one. This resulted in an issue in Vim that fills the buffer with Ctrl-Z characters when deleting to the end of a line in a position that triggers auto-completion. Since auto-completion seems to work fine on all my tests I am reverting this specific change.
This pull request addresses #4226 and comes in two patches:
For the first patch, ALE currently does not use the trigger characters advertised by the LSP. Thus it is necessary to hard-code trigger characters. This patch introduces
->
in order to make autocompletion with pointers possible.The second patch is a proof-of-concept on how ALE should behave when a newer autocompletion request has been sent to the server. Currently, if there was a previous autocomplete request, the autocomplete operation will not stop if the user enters the full name of a variable and then a trigger character, which results in no completion candidates even though ALE did talk to the LSP and retrieved completion candidates successfully (see #4226 for this description). This patch stops the previous completion operation, and ALE is able to show a new autocompletion menu with the updated, more recent completion candidates.
Review and further comments on getting this merged are greatly appreciated!