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

[READY] Restore cursor position after omnifunc call #2707

Merged
merged 2 commits into from
Jul 7, 2017

Conversation

micbou
Copy link
Collaborator

@micbou micbou commented Jul 7, 2017

When compiled without C-family support, YCM will use the default omnifunc from Vim (ccomplete#Complete) to provide semantic completion. This omnifunc calls searchdecl to find a declaration, which is supposed to move the cursor to that declaration. However, the cursor is not moved when called through the omni completion mapping (CTRL-X CTRL-O). Since PR #2657, YCM calls the omnifunc outside completion mode and thus the cursor is moved to the found declaration after typing . or ->.

Considering this searchdecl trick may be used by other omnifuncs, we fix the issue by always restoring the cursor position after calling the omnifunc.

Fixes #2698.


This change is Reviewable

Calling directly the omnifunc may move the cursor position. This is the case
with the default Vim omnifunc for C-family languages (ccomplete#Complete) which
calls searchdecl to find a declaration. This function is supposed to move the
cursor to the found declaration but it doesn't when called through the omni
completion mapping (CTRL-X CTRL-O). So, we restore the cursor position after
calling the omnifunc.
@codecov-io
Copy link

codecov-io commented Jul 7, 2017

Codecov Report

Merging #2707 into master will increase coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master   #2707      +/-   ##
=========================================
+ Coverage   90.98%     91%   +0.01%     
=========================================
  Files          20      20              
  Lines        1941    1945       +4     
=========================================
+ Hits         1766    1770       +4     
  Misses        175     175

@vheon
Copy link
Contributor

vheon commented Jul 7, 2017

Do you think that adding some tests would be of value here? It looks like the only way to test this would be to mock the cursor to move since we don't run an actual vim here so I'm a little conflicted here 😕


Reviewed 2 of 2 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@micbou
Copy link
Collaborator Author

micbou commented Jul 7, 2017

We are already mocking the cursor position in the omnifunc tests. Added a test.


Reviewed 2 of 2 files at r1, 1 of 1 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@bstaletic
Copy link
Collaborator

:lgtm:

Nice catch!

Reviewed 2 of 2 files at r1, 1 of 1 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@vheon
Copy link
Contributor

vheon commented Jul 7, 2017

I wasn't thinking that with the new Omnifunc tests this would be so easy to write and especially to read!

:lgtm_strong: @zzbot r=bstaletic


Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@zzbot
Copy link
Contributor

zzbot commented Jul 7, 2017

📌 Commit dec9c0f has been approved by bstaletic

@zzbot
Copy link
Contributor

zzbot commented Jul 7, 2017

⌛ Testing commit dec9c0f with merge d299f9e...

zzbot added a commit that referenced this pull request Jul 7, 2017
[READY] Restore cursor position after omnifunc call

When compiled without C-family support, YCM will use the default omnifunc from Vim (`ccomplete#Complete`) to provide semantic completion. This omnifunc calls [`searchdecl`](http://vimdoc.sourceforge.net/htmldoc/eval.html#searchdecl()) to find a declaration, which is supposed to move the cursor to that declaration. However, the cursor is not moved when called through the omni completion mapping (`CTRL-X CTRL-O`). Since PR #2657, YCM calls the omnifunc outside completion mode and thus the cursor is moved to the found declaration after typing `.` or `->`.

Considering this `searchdecl` trick may be used by other omnifuncs, we fix the issue by always restoring the cursor position after calling the omnifunc.

Fixes #2698.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/valloric/youcompleteme/2707)
<!-- Reviewable:end -->
@zzbot
Copy link
Contributor

zzbot commented Jul 7, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: bstaletic
Pushing d299f9e to master...

@zzbot zzbot merged commit dec9c0f into ycm-core:master Jul 7, 2017
@micbou micbou deleted the fix-omnifunc-cursor-move branch July 8, 2017 12:57
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.

insert mode . is sometimes mapped to ESC*i by YCM
5 participants