Fix traceback in neovim when changin tabs; use WindowID rather than window number for getting the location lists

python/ycm/tests/test_utils.py

@@ -489,6 +489,26 @@ class VimBuffers:
     return self._buffers.pop( index )
 
 
+class VimTabpages:
+  def __init__( self, *args ):
+    """|buffers| is a list of VimBuffer objects."""
+    self._tabpages = []
+    self._tabpages.extend( args )
+
+
+  def __getitem__( self, number ):
+    """Emulates vim.buffers[ number ]"""
+    for tabpage in self._tabpages:
+      if number == tabpage.number:
+        return tabpage
+    raise KeyError( number )
+
+
+  def __iter__( self ):
+    """Emulates for loop on vim.buffers"""
+    return iter( self._tabpages )
+
+
 class VimWindow:
   """An object that looks like a vim.window object:
     - |number|: number of the window;
@@ -496,7 +516,8 @@ class VimWindow:
       window;
     - |cursor|: a tuple corresponding to the cursor position."""
 
-  def __init__( self, number, buffer_object, cursor = None ):
+  def __init__( self, tabpage, number, buffer_object, cursor = None ):
+    self.tabpage = tabpage
     self.number = number
     self.buffer = buffer_object
     self.cursor = cursor
@@ -510,31 +531,33 @@ class VimWindow:
                         f'cursor = { self.cursor } )' )
 
 
-class VimWindows:
+class VimTabpage:
   """An object that looks like a vim.windows object."""
 
-  def __init__( self, buffers, cursor ):
+  def __init__( self, number, buffers, cursor ):
     """|buffers| is a list of VimBuffer objects corresponding to the window
     layout. The first element of that list is assumed to be the current window.
     |cursor| is the cursor position of that window."""
-    windows = []
-    windows.append( VimWindow( 1, buffers[ 0 ], cursor ) )
+    self.number = number
+    self.windows = []
+    self.windows.append( VimWindow( self, 1, buffers[ 0 ], cursor ) )
     for window_number in range( 1, len( buffers ) ):
-      windows.append( VimWindow( window_number + 1, buffers[ window_number ] ) )
-    self._windows = windows
+      self.windows.append( VimWindow( self,
+                                      window_number + 1,
+                                      buffers[ window_number ] ) )
 
 
   def __getitem__( self, number ):
     """Emulates vim.windows[ number ]"""
     try:
-      return self._windows[ number ]
+      return self.windows[ number ]
     except IndexError:
       raise IndexError( 'no such window' )
 
 
   def __iter__( self ):
     """Emulates for loop on vim.windows"""
-    return iter( self._windows )
+    return iter( self.windows )
 
 
 class VimCurrent:
@@ -544,6 +567,7 @@ class VimCurrent:
   def __init__( self, current_window ):
     self.buffer = current_window.buffer
     self.window = current_window
+    self.tabpage = current_window.tabpage
     self.line = self.buffer.contents[ current_window.cursor[ 0 ] - 1 ]
 
 
@@ -639,10 +663,11 @@ def MockVimBuffers( buffers, window_buffers, cursor_position = ( 1, 1 ) ):
                         'which corresponds to the current window.' )
 
   with patch( 'vim.buffers', VimBuffers( buffers ) ):
-    with patch( 'vim.windows', VimWindows( window_buffers,
-                                           cursor_position ) ) as windows:
-      with patch( 'vim.current', VimCurrent( windows[ 0 ] ) ):
-        yield VIM_MOCK
+    with patch( 'vim.tabpages', VimTabpages(
+      VimTabpage( 1, window_buffers, cursor_position ) ) ) as tabpages:
+      with patch( 'vim.windows', tabpages[ 1 ] ) as windows:
+        with patch( 'vim.current', VimCurrent( windows[ 0 ] ) ):
+          yield VIM_MOCK
 
 
 def MockVimModule():

python/ycm/tests/vimsupport_test.py

@@ -69,8 +69,9 @@ def _BuildLocations( start_line, start_column, end_line, end_column ):
 
 
 class VimsupportTest( TestCase ):
+  @patch( 'ycm.vimsupport.WinIDForWindow', side_effect = range( 1001, 1010 ) )
   @patch( 'vim.eval', new_callable = ExtendedMock )
-  def test_SetLocationListsForBuffer_Current( self, vim_eval ):
+  def test_SetLocationListsForBuffer_Current( self, vim_eval, *args ):
     diagnostics = [ {
       'bufnr': 3,
       'filename': 'some_filename',
@@ -84,10 +85,10 @@ class VimsupportTest( TestCase ):
       vimsupport.SetLocationListsForBuffer( 3, diagnostics )
 
     vim_eval.assert_has_exact_calls( [
-      call( 'setloclist( 1, [], " ", { "title": "ycm_loc", '
+      call( 'setloclist( 1001, [], " ", { "title": "ycm_loc", '
             '"items": [{"bufnr": 3, "filename": "some_filename", "lnum": 5, '
             '"col": 22, "type": "E", "valid": 1}] } )' ),
-      call( 'getloclist( 1, { "nr": "$", "id": 0 } ).id' ),
+      call( 'getloclist( 1001, { "nr": "$", "id": 0 } ).id' ),
     ] )
 
 
@@ -127,8 +128,9 @@ class VimsupportTest( TestCase ):
     vim_eval.assert_not_called()
 
 
+  @patch( 'ycm.vimsupport.WinIDForWindow', side_effect = range( 1001, 1010 ) )
   @patch( 'vim.eval', new_callable = ExtendedMock, side_effect = [ -1, 1 ] )
-  def test_SetLocationListsForBuffer_MultipleWindows( self, vim_eval ):
+  def test_SetLocationListsForBuffer_MultipleWindows( self, vim_eval, *args ):
     diagnostics = [ {
       'bufnr': 3,
       'filename': 'some_filename',
@@ -144,14 +146,15 @@ class VimsupportTest( TestCase ):
       vimsupport.SetLocationListsForBuffer( 1, diagnostics )
 
     vim_eval.assert_has_exact_calls( [
-      call( 'setloclist( 2, [], " ", { "title": "ycm_loc", "items": '
+      call( 'setloclist( 1001, [], " ", { "title": "ycm_loc", "items": '
            f'{ json.dumps( diagnostics ) } }} )' ),
-      call( 'getloclist( 2, { "nr": "$", "id": 0 } ).id' ),
+      call( 'getloclist( 1001, { "nr": "$", "id": 0 } ).id' ),
     ] )
 
 
+  @patch( 'ycm.vimsupport.WinIDForWindow', side_effect = range( 1001, 1010 ) )
   @patch( 'vim.eval', new_callable = ExtendedMock )
-  def test_SetLocationList( self, vim_eval ):
+  def test_SetLocationList( self, vim_eval, *args ):
     diagnostics = [ {
       'bufnr': 3,
       'filename': 'some_filename',
@@ -165,15 +168,16 @@ class VimsupportTest( TestCase ):
       vimsupport.SetLocationList( diagnostics )
 
     vim_eval.assert_has_exact_calls( [
-      call( 'setloclist( 0, [], " ", { "title": "ycm_loc", "items": [{"bufnr": '
-            '3, "filename": "some_filename", "lnum": '
+      call( 'setloclist( 1001, [], " ", { "title": "ycm_loc", "items": '
+            '[{"bufnr": 3, "filename": "some_filename", "lnum": '
             '5, "col": 22, "type": "E", "valid": 1}] } )' ),
-      call( 'getloclist( 0, { "nr": "$", "id": 0 } ).id' ),
+      call( 'getloclist( 1001, { "nr": "$", "id": 0 } ).id' ),
     ] )
 
 
+  @patch( 'ycm.vimsupport.WinIDForWindow', side_effect = range( 1001, 1010 ) )
   @patch( 'vim.eval', new_callable = ExtendedMock )
-  def test_SetLocationList_NotCurrent( self, vim_eval ):
+  def test_SetLocationList_NotCurrent( self, vim_eval, *args ):
     diagnostics = [ {
       'bufnr': 3,
       'filename': 'some_filename',
@@ -192,10 +196,10 @@ class VimsupportTest( TestCase ):
     # This version does not check the current
     # buffer and just sets the current win
     vim_eval.assert_has_exact_calls( [
-      call( 'setloclist( 0, [], " ", { "title": "ycm_loc", "items": [{"bufnr": '
-            '3, "filename": "some_filename", "lnum": 5, "col": 22, '
+      call( 'setloclist( 1001, [], " ", { "title": "ycm_loc", "items": '
+            '[{"bufnr": 3, "filename": "some_filename", "lnum": 5, "col": 22, '
             '"type": "E", "valid": 1}] } )' ),
-      call( 'getloclist( 0, { "nr": "$", "id": 0 } ).id' ),
+      call( 'getloclist( 1001, { "nr": "$", "id": 0 } ).id' ),
     ] )
 
 
@@ -2052,9 +2056,9 @@ class VimsupportTest( TestCase ):
     current_window = MagicMock( buffer = current_buffer )
     different_window = MagicMock( buffer = different_buffer )
     current_tab = MagicMock( windows = [ current_window, different_window ] )
-    with patch( 'vim.tabpages', [ current_tab ] ):
-      with MockVimBuffers( [ current_buffer, different_buffer ],
-                           [ current_buffer ] ) as vim:
+    with MockVimBuffers( [ current_buffer, different_buffer ],
+                         [ current_buffer ] ) as vim:
+      with patch( 'vim.tabpages', [ current_tab ] ):
         vimsupport.JumpToLocation( os.path.realpath( 'different_uni¢𐍈d€' ),
                                    2,
                                    5,
@@ -2102,9 +2106,9 @@ class VimsupportTest( TestCase ):
     current_window = MagicMock( buffer = current_buffer )
     different_window = MagicMock( buffer = different_buffer )
     current_tab = MagicMock( windows = [ current_window, different_window ] )
-    with patch( 'vim.tabpages', [ current_tab ] ):
-      with MockVimBuffers( [ current_buffer, different_buffer ],
-                           [ current_buffer ] ) as vim:
+    with MockVimBuffers( [ current_buffer, different_buffer ],
+                         [ current_buffer ] ) as vim:
+      with patch( 'vim.tabpages', [ current_tab ] ):
         vimsupport.JumpToLocation( os.path.realpath( 'different_uni¢𐍈d€' ),
                                    2,
                                    5,

python/ycm/tests/youcompleteme_test.py

@@ -25,6 +25,8 @@ from ycm.tests.test_utils import ( ExtendedMock,
                                    VimSign )
 MockVimModule()
 
+import vim
+
 import os
 import sys
 from hamcrest import ( assert_that, contains_exactly, empty, equal_to,
@@ -588,6 +590,10 @@ class YouCompleteMeTest( TestCase ):
       with patch( 'ycm.client.event_notification.EventNotification.Response',
                   return_value = {} ):
         ycm.ShowDiagnostics()
+        set_location_list_for_window.assert_called_once_with(
+          vim.current.window,
+          [],
+          1 )
 
     post_vim_message.assert_has_exact_calls( [
       call( 'Forcing compilation, this will block Vim until done.',
@@ -595,7 +601,6 @@ class YouCompleteMeTest( TestCase ):
       call( 'Diagnostics refreshed', warning = False ),
       call( 'No warnings or errors detected.', warning = False )
     ] )
-    set_location_list_for_window.assert_called_once_with( 1, [], 1 )
 
 
   @YouCompleteMeInstance( { 'g:ycm_log_level': 'debug',
@@ -629,20 +634,21 @@ class YouCompleteMeTest( TestCase ):
       with patch( 'ycm.client.event_notification.EventNotification.Response',
                   return_value = [ diagnostic ] ):
         ycm.ShowDiagnostics()
+        set_location_list_for_window.assert_called_once_with(
+          vim.current.window, [ {
+            'bufnr': 3,
+            'lnum': 19,
+            'col': 2,
+            'text': 'error text',
+            'type': 'E',
+            'valid': 1
+          } ], 0 )
 
     post_vim_message.assert_has_exact_calls( [
       call( 'Forcing compilation, this will block Vim until done.',
             warning = False ),
       call( 'Diagnostics refreshed', warning = False )
     ] )
-    set_location_list_for_window.assert_called_once_with( 1, [ {
-        'bufnr': 3,
-        'lnum': 19,
-        'col': 2,
-        'text': 'error text',
-        'type': 'E',
-        'valid': 1
-    } ], 0 )
 
 
   @YouCompleteMeInstance( { 'g:ycm_open_loclist_on_ycm_diags': 1 } )
@@ -677,20 +683,21 @@ class YouCompleteMeTest( TestCase ):
       with patch( 'ycm.client.event_notification.EventNotification.Response',
                   return_value = [ diagnostic ] ):
         ycm.ShowDiagnostics()
+        set_location_list_for_window.assert_called_once_with(
+          vim.current.window, [ {
+            'bufnr': 3,
+            'lnum': 19,
+            'col': 2,
+            'text': 'error text',
+            'type': 'E',
+            'valid': 1
+          } ], 1 )
 
     post_vim_message.assert_has_exact_calls( [
       call( 'Forcing compilation, this will block Vim until done.',
             warning = False ),
       call( 'Diagnostics refreshed', warning = False )
     ] )
-    set_location_list_for_window.assert_called_once_with( 1, [ {
-        'bufnr': 3,
-        'lnum': 19,
-        'col': 2,
-        'text': 'error text',
-        'type': 'E',
-        'valid': 1
-    } ], 1 )
     open_location_list.assert_called_once_with( focus = True )
 
 
@@ -917,45 +924,45 @@ class YouCompleteMeTest( TestCase ):
                 new_callable = ExtendedMock ) as set_location_list_for_window:
       with MockVimBuffers( buffers, windows ):
         ycm.UpdateWithNewDiagnosticsForFile( '/current', diagnostics )
+        # Ensure we included all the diags though
+        set_location_list_for_window.assert_has_exact_calls( [
+          call( vim.current.window, [
+            {
+              'lnum': 1,
+              'col': 1,
+              'bufnr': 1,
+              'valid': 1,
+              'type': 'E',
+              'text': 'error text in current buffer',
+            },
+            {
+              'lnum': 4,
+              'col': 2,
+              'bufnr': 3,
+              'valid': 1,
+              'type': 'E',
+              'text': 'error text in hidden buffer',
+            },
+            {
+              'lnum': 8,
+              'col': 4,
+              'bufnr': -1, # sic: Our mocked bufnr function actually returns -1,
+                           # even though YCM is passing "create if needed".
+                           # FIXME? we shouldn't do that, and we should pass
+                           # filename instead
+              'valid': 1,
+              'type': 'E',
+              'text': 'error text in buffer not open in Vim'
+            }
+          ], False )
+        ] )
+
 
     # We update the diagnostic on the current cursor position
     post_vim_message.assert_has_exact_calls( [
       call( "error text in current buffer", truncate = True, warning = False ),
     ] )
 
-    # Ensure we included all the diags though
-    set_location_list_for_window.assert_has_exact_calls( [
-      call( 1, [
-        {
-          'lnum': 1,
-          'col': 1,
-          'bufnr': 1,
-          'valid': 1,
-          'type': 'E',
-          'text': 'error text in current buffer',
-        },
-        {
-          'lnum': 4,
-          'col': 2,
-          'bufnr': 3,
-          'valid': 1,
-          'type': 'E',
-          'text': 'error text in hidden buffer',
-        },
-        {
-          'lnum': 8,
-          'col': 4,
-          'bufnr': -1, # sic: Our mocked bufnr function actually returns -1,
-                       # even though YCM is passing "create if needed".
-                       # FIXME? we shouldn't do that, and we should pass
-                       # filename instead
-          'valid': 1,
-          'type': 'E',
-          'text': 'error text in buffer not open in Vim'
-        }
-      ], False )
-    ] )
-
     assert_that(
       test_utils.VIM_PROPS_FOR_BUFFER,
       has_entries( {
@@ -1112,36 +1119,37 @@ class YouCompleteMeTest( TestCase ):
         for filename, diagnostics in diagnostics_per_file:
           ycm.UpdateWithNewDiagnosticsForFile( filename, diagnostics )
 
+        # Ensure we included all the diags though
+        set_location_list_for_window.assert_has_exact_calls( [
+          call( vim.windows[ 0 ], [
+            {
+              'lnum': 1,
+              'col': 1,
+              'bufnr': 1,
+              'valid': 1,
+              'type': 'E',
+              'text': 'error text in current buffer',
+            },
+          ], False ),
+
+          call( vim.windows[ 2 ], [
+            {
+              'lnum': 3,
+              'col': 3,
+              'bufnr': 3,
+              'valid': 1,
+              'type': 'E',
+              'text': 'error text in a buffer open in a separate window',
+            },
+          ], False )
+        ] )
+
+
     # We update the diagnostic on the current cursor position
     post_vim_message.assert_has_exact_calls( [
       call( "error text in current buffer", truncate = True, warning = False ),
     ] )
 
-    # Ensure we included all the diags though
-    set_location_list_for_window.assert_has_exact_calls( [
-      call( 1, [
-        {
-          'lnum': 1,
-          'col': 1,
-          'bufnr': 1,
-          'valid': 1,
-          'type': 'E',
-          'text': 'error text in current buffer',
-        },
-      ], False ),
-
-      call( 3, [
-        {
-          'lnum': 3,
-          'col': 3,
-          'bufnr': 3,
-          'valid': 1,
-          'type': 'E',
-          'text': 'error text in a buffer open in a separate window',
-        },
-      ], False )
-    ] )
-
     assert_that(
       test_utils.VIM_PROPS_FOR_BUFFER,
       has_entries( {

python/ycm/vimsupport.py

@@ -421,7 +421,7 @@ def LineAndColumnNumbersClamped( bufnr, line_num, column_num ):
 
 def SetLocationList( diagnostics ):
   """Set the location list for the current window to the supplied diagnostics"""
-  SetLocationListForWindow( 0, diagnostics )
+  SetLocationListForWindow( vim.current.window, diagnostics )
 
 
 def GetWindowsForBufferNumber( buffer_number ):
@@ -437,49 +437,50 @@ def SetLocationListsForBuffer( buffer_number,
   """Populate location lists for all windows containing the buffer with number
   |buffer_number|. See SetLocationListForWindow for format of diagnostics."""
   for window in GetWindowsForBufferNumber( buffer_number ):
-    SetLocationListForWindow( window.number, diagnostics, open_on_edit )
+    SetLocationListForWindow( window, diagnostics, open_on_edit )
 
 
-def SetLocationListForWindow( window_number,
+def SetLocationListForWindow( window,
                               diagnostics,
                               open_on_edit = False ):
+  window_id = WinIDForWindow( window )
   """Populate the location list with diagnostics. Diagnostics should be in
   qflist format; see ":h setqflist" for details."""
-  ycm_loc_id = vim.windows[ window_number - 1 ].vars.get( 'ycm_loc_id' )
+  ycm_loc_id = window.vars.get( 'ycm_loc_id' )
   # User may have made a bunch of `:lgrep` calls and we do not own the
   # location list with the ID we remember any more.
   if ( ycm_loc_id is not None and
-       vim.eval( f'getloclist( { window_number }, '
+       vim.eval( f'getloclist( { window_id }, '
                                f'{{ "id": { ycm_loc_id }, '
                                 '"title": 0 } ).title' ) == 'ycm_loc' ):
     ycm_loc_id = None
 
   if ycm_loc_id is None:
     # Create new and populate
-    vim.eval( f'setloclist( { window_number }, '
+    vim.eval( f'setloclist( { window_id }, '
                            '[], '
                            '" ", '
                            '{ "title": "ycm_loc", '
                             f'"items": { json.dumps( diagnostics ) } }} )' )
-    vim.windows[ window_number - 1 ].vars[ 'ycm_loc_id' ] = GetIntValue(
-        f'getloclist( { window_number }, {{ "nr": "$", "id": 0 }} ).id' )
+    window.vars[ 'ycm_loc_id' ] = GetIntValue(
+        f'getloclist( { window_id }, {{ "nr": "$", "id": 0 }} ).id' )
   elif open_on_edit:
     # Remove old and create new list
-    vim.eval( f'setloclist( { window_number }, '
+    vim.eval( f'setloclist( { window_id }, '
                            '[], '
                            '"r", '
                           f'{{ "id": { ycm_loc_id }, '
                               '"items": [], "title": "" } )' )
-    vim.eval( f'setloclist( { window_number }, '
+    vim.eval( f'setloclist( { window_id }, '
                            '[], '
                            '" ", '
                            '{ "title": "ycm_loc", '
                             f'"items": { json.dumps( diagnostics ) } }} )' )
-    vim.windows[ window_number - 1 ].vars[ 'ycm_loc_id' ] = GetIntValue(
-        f'getloclist( { window_number }, {{ "nr": "$", "id": 0 }} ).id' )
+    window.vars[ 'ycm_loc_id' ] = GetIntValue(
+        f'getloclist( { window_id }, {{ "nr": "$", "id": 0 }} ).id' )
   else:
     # Just populate the old one
-    vim.eval( f'setloclist( { window_number }, '
+    vim.eval( f'setloclist( { window_id }, '
                            '[], '
                            '"r", '
                           f'{{ "id": { ycm_loc_id }, '