vim/YouCompleteMe
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Auto merge of #2828 - micbou:get-buffer-number-for-filename, r=bstaletic

Browse Source 
[READY] Change GetBufferNumberForFilename default behavior

While I was looking at PR https://github.com/Valloric/YouCompleteMe/pull/2827, I noticed that the default behavior of `GetBufferNumberForFilename` is to create a buffer for the given filename if no buffer already exists for that filename. This behavior is rather unexpected given the name of that function. In fact, `GetBufferNumberForFilename` is almost always called with the `open_file_if_needed` parameter (renamed `create_buffer_if_needed` for clarity) sets to `False`. This really suggests that the default value should be `False` instead of `True`.

In addition, that default behavior may lead to performance issues when the server returns diagnostics for a lot of files with no corresponding buffers since this will create a buffer for each one of these files.

<!-- 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/2828)
<!-- Reviewable:end -->
This commit is contained in:
zzbot 2017-11-17 01:27:25 -08:00 committed by GitHub
commit 31abb9cee5
2 changed files with 21 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
python/ycm/tests/vimsupport_test.py
View File

@ -797,8 +797,8 @@ def ReplaceChunks_SingleFile_Open_test( vim_command,
# raise a warning) # raise a warning)
# - once whilst applying the changes # - once whilst applying the changes
get_buffer_number_for_filename.assert_has_exact_calls( [ get_buffer_number_for_filename.assert_has_exact_calls( [
call( single_buffer_name, False ), call( single_buffer_name ),
call( single_buffer_name, False ), call( single_buffer_name ),
] ) ] )
# BufferIsVisible is called twice for the same reasons as above # 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) # - once whilst applying the changes (-1 return)
# - finally after calling OpenFilename (1 return) # - finally after calling OpenFilename (1 return)
get_buffer_number_for_filename.assert_has_exact_calls( [ get_buffer_number_for_filename.assert_has_exact_calls( [
call( single_buffer_name, False ), call( single_buffer_name ),
call( single_buffer_name, False ), call( single_buffer_name ),
call( single_buffer_name, False ), call( single_buffer_name ),
] ) ] )
# BufferIsVisible is called 3 times for the same reasons as above, with the # 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 # - once to the check if we would require opening the file (so that we can
# raise a warning) (-1 return) # raise a warning) (-1 return)
get_buffer_number_for_filename.assert_has_exact_calls( [ 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. # 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 # We checked for the right file names
get_buffer_number_for_filename.assert_has_exact_calls( [ get_buffer_number_for_filename.assert_has_exact_calls( [
call( first_buffer_name, False ), call( first_buffer_name ),
call( second_buffer_name, False ), call( second_buffer_name ),
call( first_buffer_name, False ), call( first_buffer_name ),
call( second_buffer_name, False ), call( second_buffer_name ),
call( second_buffer_name, False ), call( second_buffer_name ),
] ) ] )
# We checked if it was OK to open the file # We checked if it was OK to open the file

19
python/ycm/vimsupport.py
View File

@ -139,10 +139,10 @@ def GetUnsavedAndSpecifiedBufferData( including_filepath ):
return buffers_data 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( return GetIntValue( u"bufnr('{0}', {1})".format(
EscapeForVim( os.path.realpath( filename ) ), EscapeForVim( os.path.realpath( filename ) ),
int( open_file_if_needed ) ) ) int( create_buffer_if_needed ) ) )
def GetCurrentBufferFilepath(): def GetCurrentBufferFilepath():
@ -328,7 +328,8 @@ def ConvertDiagnosticsToQfList( diagnostics ):
text += ' (FixIt available)' text += ' (FixIt available)'
return { return {
'bufnr' : GetBufferNumberForFilename( location[ 'filepath' ] ), 'bufnr' : GetBufferNumberForFilename( location[ 'filepath' ],
create_buffer_if_needed = True ),
'lnum' : line_num, 'lnum' : line_num,
'col' : location[ 'column_num' ], 'col' : location[ 'column_num' ],
'text' : text, 'text' : text,
@ -641,7 +642,7 @@ def _GetNumNonVisibleFiles( file_list ):
are not curerntly open in visible windows""" are not curerntly open in visible windows"""
return len( return len(
[ f for f in file_list [ f for f in file_list
if not BufferIsVisible( GetBufferNumberForFilename( f, False ) ) ] ) if not BufferIsVisible( GetBufferNumberForFilename( f ) ) ] )
def _OpenFileInSplitIfNeeded( filepath ): def _OpenFileInSplitIfNeeded( filepath ):
@ -658,7 +659,7 @@ def _OpenFileInSplitIfNeeded( filepath ):
not to open a file, or if opening fails, this method raises RuntimeError, not to open a file, or if opening fails, this method raises RuntimeError,
otherwise, guarantees to return a visible buffer number in buffer_num.""" 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). # 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 # 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 # 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 # want, because we don't want to disorientate the user, but we do need to
# know the (now open) buffer number for the filename # know the (now open) buffer number for the filename
buffer_num = GetBufferNumberForFilename( filepath, False ) buffer_num = GetBufferNumberForFilename( filepath )
if not BufferIsVisible( buffer_num ): if not BufferIsVisible( buffer_num ):
# This happens, for example, if there is a swap file and the user # 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 # selects the "Quit" or "Abort" options. We just raise an exception to
@ -959,16 +960,16 @@ def WriteToPreviewWindow( message ):
def BufferIsVisibleForFilename( filename ): def BufferIsVisibleForFilename( filename ):
"""Check if a buffer exists for a specific file.""" """Check if a buffer exists for a specific file."""
buffer_number = GetBufferNumberForFilename( filename, False ) buffer_number = GetBufferNumberForFilename( filename )
return BufferIsVisible( buffer_number ) return BufferIsVisible( buffer_number )
def CloseBuffersForFilename( filename ): def CloseBuffersForFilename( filename ):
"""Close all buffers for a specific file.""" """Close all buffers for a specific file."""
buffer_number = GetBufferNumberForFilename( filename, False ) buffer_number = GetBufferNumberForFilename( filename )
while buffer_number != -1: while buffer_number != -1:
vim.command( 'silent! bwipeout! {0}'.format( buffer_number ) ) 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: if buffer_number == new_buffer_number:
raise RuntimeError( "Buffer {0} for filename '{1}' should already be " raise RuntimeError( "Buffer {0} for filename '{1}' should already be "
"wiped out.".format( buffer_number, filename ) ) "wiped out.".format( buffer_number, filename ) )