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

Improve struct and pointer autocompletion in C #4231

Merged
merged 2 commits into from
Jul 4, 2022
Merged

Improve struct and pointer autocompletion in C #4231

merged 2 commits into from
Jul 4, 2022

Conversation

s-marios
Copy link
Contributor

This pull request addresses #4226 and comes in two patches:

  1. add explicit trigger characters for triggering autocompletion in C,
  2. stop the previous autocompletion operation when a new autocompletion request is sent to the LSP.

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!

@hsanson
Copy link
Contributor

hsanson commented Jun 23, 2022

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.

@s-marios
Copy link
Contributor Author

@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?

hsanson
hsanson previously approved these changes Jul 3, 2022
Copy link
Contributor

@hsanson hsanson left a 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.

@hsanson
Copy link
Contributor

hsanson commented Jul 3, 2022

@s-marios seems that the new patterns need to be also added to this test: test/python/test_deoplete_source.py.

@s-marios
Copy link
Contributor Author

s-marios commented Jul 4, 2022

Ah, sorry about that. Amended the test, run locally all tests passed.

History rewritten, rebased, it should be ready to go!

@hsanson hsanson merged commit a918f8c into dense-analysis:master Jul 4, 2022
cyyever pushed a commit to cyyever/ale that referenced this pull request Jul 11, 2022
* Add explicit trigger characters for C (dense-analysis#4226)

* Stop completion before issuing subsequent requests (dense-analysis#4226)

Co-authored-by: Marios Sioutis <26476573+s-marios@users.noreply.github.com>
cyyever pushed a commit to cyyever/ale that referenced this pull request Jul 11, 2022
* Add explicit trigger characters for C (dense-analysis#4226)

* Stop completion before issuing subsequent requests (dense-analysis#4226)

Co-authored-by: Marios Sioutis <26476573+s-marios@users.noreply.github.com>
hsanson added a commit to hsanson/ale that referenced this pull request Jul 13, 2022
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.
hsanson added a commit that referenced this pull request Jul 13, 2022
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.
mnikulin pushed a commit to mnikulin/ale that referenced this pull request Nov 12, 2023
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.
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.

2 participants