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] Do not disable omnifunc when filetype completion is disabled #2978

Merged

Conversation

micbou
Copy link
Collaborator

@micbou micbou commented Apr 6, 2018

Prior to PR #2657, it was possible to trigger Vim's omnifunc with <C-Space> even if semantic completion was disabled for the current filetype through the g:ycm_filetype_specific_completion_to_disable option. It worked because <C-Space> was mapped to <C-X><C-O><C-P>, which are the keys to trigger the omnifunc. PR #2657 changed that by making <C-Space> directly call the SendCompletionRequest function with force_semantic sets to True. This change was necessary to get fuzzy matching with the omnifunc (see issue #961) but broke the <C-Space> behavior when filetype completion is disabled. This PR restores that behavior.

Fixes #2950.


This change is Reviewable

@micbou micbou force-pushed the fix-filetype-specific-completion-to-disable branch from 2b0c34f to ad13b62 Compare April 6, 2018 20:30
@codecov-io
Copy link

codecov-io commented Apr 6, 2018

Codecov Report

Merging #2978 into master will increase coverage by 0.73%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2978      +/-   ##
==========================================
+ Coverage    92.6%   93.34%   +0.73%     
==========================================
  Files          21       21              
  Lines        2070     2359     +289     
==========================================
+ Hits         1917     2202     +285     
- Misses        153      157       +4

@micbou micbou force-pushed the fix-filetype-specific-completion-to-disable branch from ad13b62 to cbf5430 Compare April 7, 2018 00:15
@bstaletic
Copy link
Collaborator

:lgtm:


Reviewed 3 of 3 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.


Comments from Reviewable

@puremourning
Copy link
Member

Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.


python/ycm/omni_completer.py, line 67 at r1 (raw file):

  def _CurrentFiletypeCompletionDisabled( self ):

This looks like the same logic from youcompleteme.py ? Can we avoid duplicating it ?


Comments from Reviewable

Allow users to still trigger Vim's omnifunc through C-Space when the
g:ycm_filetype_specific_completion_to_disable option is set for the current
filetype.
@micbou micbou force-pushed the fix-filetype-specific-completion-to-disable branch from cbf5430 to e96ef7c Compare April 13, 2018 00:41
@micbou
Copy link
Collaborator Author

micbou commented Apr 13, 2018

Reviewed 1 of 3 files at r1, 3 of 3 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion.


python/ycm/omni_completer.py, line 67 at r1 (raw file):

Previously, puremourning (Ben Jackson) wrote…

This looks like the same logic from youcompleteme.py ? Can we avoid duplicating it ?

Added a new function CurrentFiletypesEnabled in vimsupport.


Comments from Reviewable

@puremourning
Copy link
Member

:lgtm: @zzbot r+


Reviewed 1 of 3 files at r1, 3 of 3 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.


Comments from Reviewable

@zzbot
Copy link
Contributor

zzbot commented Apr 14, 2018

📌 Commit e96ef7c has been approved by puremourning

@zzbot
Copy link
Contributor

zzbot commented Apr 14, 2018

⌛ Testing commit e96ef7c with merge 7d72519...

zzbot added a commit that referenced this pull request Apr 14, 2018
…ble, r=puremourning

[READY] Do not disable omnifunc when filetype completion is disabled

Prior to PR #2657, it was possible to trigger Vim's omnifunc with `<C-Space>` even if semantic completion was disabled for the current filetype through the `g:ycm_filetype_specific_completion_to_disable` option. It worked because `<C-Space>` was mapped to `<C-X><C-O><C-P>`, which are the keys to trigger the omnifunc. PR #2657 changed that by making `<C-Space>` directly call the `SendCompletionRequest` function with `force_semantic` sets to `True`. This change was necessary to get fuzzy matching with the omnifunc (see issue #961) but broke the `<C-Space>` behavior when filetype completion is disabled. This PR restores that behavior.

Fixes #2950.

<!-- 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/2978)
<!-- Reviewable:end -->
@zzbot
Copy link
Contributor

zzbot commented Apr 14, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: puremourning
Pushing 7d72519 to master...

@zzbot zzbot merged commit e96ef7c into ycm-core:master Apr 14, 2018
@micbou micbou deleted the fix-filetype-specific-completion-to-disable branch April 15, 2018 01:10
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.

None yet

5 participants