From 664d2c99d93bfbca44e44fecdf475dcd3c57dfb9 Mon Sep 17 00:00:00 2001 From: micbou Date: Fri, 17 Nov 2017 00:52:40 +0100 Subject: [PATCH] Change GetBufferNumberForFilename default behavior If no buffer exists for a given filename, the GetBufferNumberForFilename function will create a buffer for that file by default. This behavior is unexpected given the name of that function and may lead to performance issues when ycmd returns diagnostics for a lot of files with no corresponding buffers. The default behavior for that function should be to not create a buffer. --- python/ycm/tests/vimsupport_test.py | 22 +++++++++++----------- python/ycm/vimsupport.py | 19 ++++++++++--------- 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/python/ycm/tests/vimsupport_test.py b/python/ycm/tests/vimsupport_test.py index 7394a0ca..d6975de5 100644 --- a/python/ycm/tests/vimsupport_test.py +++ b/python/ycm/tests/vimsupport_test.py @@ -797,8 +797,8 @@ def ReplaceChunks_SingleFile_Open_test( vim_command, # raise a warning) # - once whilst applying the changes get_buffer_number_for_filename.assert_has_exact_calls( [ - call( single_buffer_name, False ), - call( single_buffer_name, False ), + call( single_buffer_name ), + call( single_buffer_name ), ] ) # BufferIsVisible is called twice for the same reasons as above @@ -895,9 +895,9 @@ def ReplaceChunks_SingleFile_NotOpen_test( vim_command, # - once whilst applying the changes (-1 return) # - finally after calling OpenFilename (1 return) get_buffer_number_for_filename.assert_has_exact_calls( [ - call( single_buffer_name, False ), - call( single_buffer_name, False ), - call( single_buffer_name, False ), + call( single_buffer_name ), + call( single_buffer_name ), + call( single_buffer_name ), ] ) # BufferIsVisible is called 3 times for the same reasons as above, with the @@ -1007,7 +1007,7 @@ def ReplaceChunks_User_Declines_To_Open_File_test( # - once to the check if we would require opening the file (so that we can # raise a warning) (-1 return) get_buffer_number_for_filename.assert_has_exact_calls( [ - call( single_buffer_name, False ), + call( single_buffer_name ), ] ) # BufferIsVisible is called once for the above file, which wasn't visible. @@ -1174,11 +1174,11 @@ def ReplaceChunks_MultiFile_Open_test( vim_command, # We checked for the right file names get_buffer_number_for_filename.assert_has_exact_calls( [ - call( first_buffer_name, False ), - call( second_buffer_name, False ), - call( first_buffer_name, False ), - call( second_buffer_name, False ), - call( second_buffer_name, False ), + call( first_buffer_name ), + call( second_buffer_name ), + call( first_buffer_name ), + call( second_buffer_name ), + call( second_buffer_name ), ] ) # We checked if it was OK to open the file diff --git a/python/ycm/vimsupport.py b/python/ycm/vimsupport.py index 82881083..bc5dbbd7 100644 --- a/python/ycm/vimsupport.py +++ b/python/ycm/vimsupport.py @@ -139,10 +139,10 @@ def GetUnsavedAndSpecifiedBufferData( including_filepath ): return buffers_data -def GetBufferNumberForFilename( filename, open_file_if_needed = True ): +def GetBufferNumberForFilename( filename, create_buffer_if_needed = False ): return GetIntValue( u"bufnr('{0}', {1})".format( EscapeForVim( os.path.realpath( filename ) ), - int( open_file_if_needed ) ) ) + int( create_buffer_if_needed ) ) ) def GetCurrentBufferFilepath(): @@ -328,7 +328,8 @@ def ConvertDiagnosticsToQfList( diagnostics ): text += ' (FixIt available)' return { - 'bufnr' : GetBufferNumberForFilename( location[ 'filepath' ] ), + 'bufnr' : GetBufferNumberForFilename( location[ 'filepath' ], + create_buffer_if_needed = True ), 'lnum' : line_num, 'col' : location[ 'column_num' ], 'text' : text, @@ -641,7 +642,7 @@ def _GetNumNonVisibleFiles( file_list ): are not curerntly open in visible windows""" return len( [ f for f in file_list - if not BufferIsVisible( GetBufferNumberForFilename( f, False ) ) ] ) + if not BufferIsVisible( GetBufferNumberForFilename( f ) ) ] ) def _OpenFileInSplitIfNeeded( filepath ): @@ -658,7 +659,7 @@ def _OpenFileInSplitIfNeeded( filepath ): not to open a file, or if opening fails, this method raises RuntimeError, otherwise, guarantees to return a visible buffer number in buffer_num.""" - buffer_num = GetBufferNumberForFilename( filepath, False ) + buffer_num = GetBufferNumberForFilename( filepath ) # We only apply changes in the current tab page (i.e. "visible" windows). # Applying changes in tabs does not lead to a better user experience, as the @@ -681,7 +682,7 @@ def _OpenFileInSplitIfNeeded( filepath ): # OpenFilename returns us to the original cursor location. This is what we # want, because we don't want to disorientate the user, but we do need to # know the (now open) buffer number for the filename - buffer_num = GetBufferNumberForFilename( filepath, False ) + buffer_num = GetBufferNumberForFilename( filepath ) if not BufferIsVisible( buffer_num ): # This happens, for example, if there is a swap file and the user # selects the "Quit" or "Abort" options. We just raise an exception to @@ -959,16 +960,16 @@ def WriteToPreviewWindow( message ): def BufferIsVisibleForFilename( filename ): """Check if a buffer exists for a specific file.""" - buffer_number = GetBufferNumberForFilename( filename, False ) + buffer_number = GetBufferNumberForFilename( filename ) return BufferIsVisible( buffer_number ) def CloseBuffersForFilename( filename ): """Close all buffers for a specific file.""" - buffer_number = GetBufferNumberForFilename( filename, False ) + buffer_number = GetBufferNumberForFilename( filename ) while buffer_number != -1: vim.command( 'silent! bwipeout! {0}'.format( buffer_number ) ) - new_buffer_number = GetBufferNumberForFilename( filename, False ) + new_buffer_number = GetBufferNumberForFilename( filename ) if buffer_number == new_buffer_number: raise RuntimeError( "Buffer {0} for filename '{1}' should already be " "wiped out.".format( buffer_number, filename ) )