Auto merge of #3120 - micbou:goto-modifiers-version, r=micbou

[READY] Only use command modifiers if available

Command modifiers (see `:h mods`) were added in [Vim 7.4.1898](63a60ded3f (diff-28587d36c24b61c33d4d01601f5974ee)) while we support 7.4.1578 and later.

Fixes https://github.com/Valloric/YouCompleteMe/issues/3105.

<!-- 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/3120)
<!-- Reviewable:end -->

README.md

@@ -2879,15 +2879,14 @@ let g:ycm_use_ultisnips_completer = 1
 ### The `g:ycm_goto_buffer_command` option
 
 Defines where `GoTo*` commands result should be opened. Can take one of the
-following values:
+following values: `'same-buffer'`, `'split'`, or `'split-or-existing-window'`.
-`[ 'same-buffer', 'split', 'split-or-existing-window' ]`
+If this option is set to the `'same-buffer'` but current buffer can not be
-If this option is set to the `'same-buffer'` but current buffer can not
+switched (when buffer is modified and `nohidden` option is set), then result
-be switched (when buffer is modified and `nohidden` option is set),
+will be opened in a split. When the option is set to
-then result will be opened in a split. When the option is set to
 `'split-or-existing-window'`, if the result is already open in a window of the
-current tab page (or any tab pages with the `:tab` modifier; see below), it
+current tab page (or any tab pages with the `:tab` modifier; see below), it will
-will jump to that window. Otherwise, the result will be opened in a split as if
+jump to that window. Otherwise, the result will be opened in a split as if the
-the option was set to `'split'`.
+option was set to `'split'`.
 
 To customize the way a new window is split, prefix the `GoTo*` command with one
 of the following modifiers: `:aboveleft`, `:belowright`, `:botright`,
@@ -2903,6 +2902,10 @@ To open in a new tab page, use the `:tab` modifier with the `'split'` or
 :tab YcmCompleter GoTo
 ```
 
+**NOTE:** command modifiers were added in Vim 7.4.1898. If you are using an
+older version, you can still configure this by setting the option to one of the
+deprecated values: `'vertical-split'`, `'new-tab'`, or `'new-or-existing-tab'`.
+
 Default: `'same-buffer'`
 
 ```viml

autoload/youcompleteme.vim

@@ -844,12 +844,21 @@ function! s:SetUpCommands()
   command! YcmDebugInfo call s:DebugInfo()
   command! -nargs=* -complete=custom,youcompleteme#LogsComplete
         \ YcmToggleLogs call s:ToggleLogs(<f-args>)
+  if s:Pyeval( 'vimsupport.VimVersionAtLeast( "7.4.1898" )' )
     command! -nargs=* -complete=custom,youcompleteme#SubCommandsComplete -range
           \ YcmCompleter call s:CompleterCommand(<q-mods>,
           \                                      <count>,
           \                                      <line1>,
           \                                      <line2>,
           \                                      <f-args>)
+  else
+    command! -nargs=* -complete=custom,youcompleteme#SubCommandsComplete -range
+          \ YcmCompleter call s:CompleterCommand('',
+          \                                      <count>,
+          \                                      <line1>,
+          \                                      <line2>,
+          \                                      <f-args>)
+  endif
   command! YcmDiags call s:ShowDiagnostics()
   command! YcmShowDetailedDiagnostic call s:ShowDetailedDiagnostic()
   command! YcmForceCompileAndDiagnostics call s:ForceCompileAndDiagnostics()

doc/youcompleteme.txt

@@ -3098,8 +3098,8 @@ Default: '1'
 The *g:ycm_goto_buffer_command* option
 
 Defines where 'GoTo*' commands result should be opened. Can take one of the
-following values: "[ 'same-buffer', 'split', 'split-or-existing-window' ]" If
+following values: "'same-buffer'", "'split'", or "'split-or-existing-window'".
-this option is set to the "'same-buffer'" but current buffer can not be
+If this option is set to the "'same-buffer'" but current buffer can not be
 switched (when buffer is modified and 'nohidden' option is set), then result
 will be opened in a split. When the option is set to "'split-or-existing-
 window'", if the result is already open in a window of the current tab page (or
@@ -3119,6 +3119,10 @@ To open in a new tab page, use the ':tab' modifier with the "'split'" or
 >
   :tab YcmCompleter GoTo
 <
+**NOTE:** command modifiers were added in Vim 7.4.1898. If you are using an
+older version, you can still configure this by setting the option to one of the
+deprecated values: "'vertical-split'", "'new-tab'", or "'new-or-existing-tab'".
+
 Default: "'same-buffer'"
 >
   let g:ycm_goto_buffer_command = 'same-buffer'

python/ycm/tests/test_utils.py

@@ -22,7 +22,7 @@ from __future__ import absolute_import
 # Not installing aliases from python-future; it's unreliable and slow.
 from builtins import *  # noqa
 
-from collections import defaultdict
+from collections import defaultdict, namedtuple
 from future.utils import iteritems, PY2
 from mock import DEFAULT, MagicMock, patch
 from hamcrest import assert_that, equal_to
@@ -60,6 +60,7 @@ REDIR_START_REGEX = re.compile( '^redir => (?P<variable>[\w:]+)$' )
 REDIR_END_REGEX = re.compile( '^redir END$' )
 EXISTS_REGEX = re.compile( '^exists\( \'(?P<option>[\w:]+)\' \)$' )
 LET_REGEX = re.compile( '^let (?P<option>[\w:]+) = (?P<value>.*)$' )
+HAS_PATCH_REGEX = re.compile( '^has\( \'patch(?P<patch>\d+)\' \)$' )
 
 # One-and only instance of mocked Vim object. The first 'import vim' that is
 # executed binds the vim module to the instance of MagicMock that is created,
@@ -83,6 +84,15 @@ VIM_OPTIONS = {
   '&expandtab': 1
 }
 
+# This variable must be patched with a Version object for tests depending on the
+# Vim version. Example:
+#
+#   @patch( 'ycm.tests.test_utils.VIM_VERSION', Version( 7, 4, 1578 ) )
+#   def ThisTestDependsOnTheVimVersion_test():
+#     ...
+#
+VIM_VERSION = None
+
 REDIR = {
   'status': False,
   'variable': '',
@@ -90,6 +100,9 @@ REDIR = {
 }
 
 
+Version = namedtuple( 'Version', [ 'major', 'minor', 'patch' ] )
+
+
 @contextlib.contextmanager
 def CurrentWorkingDirectory( path ):
   old_cwd = GetCurrentDirectory()
@@ -225,6 +238,21 @@ def _MockVimMatchEval( value ):
   return None
 
 
+def _MockVimVersionEval( value ):
+  match = HAS_PATCH_REGEX.search( value )
+  if match:
+    if not isinstance( VIM_VERSION, Version ):
+      raise RuntimeError( 'Vim version is not set.' )
+    return VIM_VERSION.patch >= int( match.group( 'patch' ) )
+
+  if value == 'v:version':
+    if not isinstance( VIM_VERSION, Version ):
+      raise RuntimeError( 'Vim version is not set.' )
+    return VIM_VERSION.major * 100 + VIM_VERSION.minor
+
+  return None
+
+
 def _MockVimEval( value ):
   result = _MockVimOptionsEval( value )
   if result is not None:
@@ -242,6 +270,10 @@ def _MockVimEval( value ):
   if result is not None:
     return result
 
+  result = _MockVimVersionEval( value )
+  if result is not None:
+    return result
+
   match = FNAMEESCAPE_REGEX.search( value )
   if match:
     return match.group( 'filepath' )

python/ycm/tests/vimsupport_test.py

@@ -26,8 +26,8 @@ from builtins import *  # noqa
 
 from ycm.tests import PathToTestFile
 from ycm.tests.test_utils import ( CurrentWorkingDirectory, ExtendedMock,
-                                   MockVimBuffers, MockVimModule, VimBuffer,
+                                   MockVimBuffers, MockVimModule, Version,
-                                   VimError )
+                                   VimBuffer, VimError )
 MockVimModule()
 
 from ycm import vimsupport
@@ -1879,3 +1879,12 @@ def JumpToLocation_DifferentFile_NewOrExistingTab_AlreadyOpened_test(
         call( 'normal! m\'' ),
         call( 'normal! zz' )
       ] )
+
+
+@patch( 'ycm.tests.test_utils.VIM_VERSION', Version( 7, 4, 1578 ) )
+def VimVersionAtLeast_test():
+  assert_that( vimsupport.VimVersionAtLeast( '7.3.414' ) )
+  assert_that( vimsupport.VimVersionAtLeast( '7.4.1578' ) )
+  assert_that( not vimsupport.VimVersionAtLeast( '7.4.1579' ) )
+  assert_that( not vimsupport.VimVersionAtLeast( '7.4.1898' ) )
+  assert_that( not vimsupport.VimVersionAtLeast( '8.1.278' ) )

python/ycm/vimsupport.py

@@ -1228,3 +1228,16 @@ def SwitchWindow( window ):
   the CurrentWindow context if you are going to switch back to the original
   window."""
   vim.current.window = window
+
+
+# Expects version_string in 'MAJOR.MINOR.PATCH' format, e.g. '8.1.278'
+def VimVersionAtLeast( version_string ):
+  major, minor, patch = ( int( x ) for x in version_string.split( '.' ) )
+
+  # For Vim 8.1.278, v:version is '801'
+  actual_major_and_minor = GetIntValue( 'v:version' )
+  matching_major_and_minor = major * 100 + minor
+  if actual_major_and_minor != matching_major_and_minor:
+    return actual_major_and_minor > matching_major_and_minor
+
+  return GetBoolValue( "has( 'patch{0}' )".format( patch ) )