From 3fcc63127f58840bcc362a0a7b7b2f359c25db4e Mon Sep 17 00:00:00 2001 From: Stanislav Seletskiy Date: Sun, 14 Sep 2014 01:15:40 +0700 Subject: [PATCH 1/2] Fix handling of signs placed by other plugins It appears to address numerous amount of issues, including: #812, #801, #887. Proposed solution uses dummy sign which is placed before updating diagnostic signs and unplaced afterwards, which eliminates any flickering. Also, it not just unplace all, it unplaces only that marks that are changed, so performance should not be an issue in case of many diagnostic messages. It's common solution that can be found in some vim plugins that manage signatures. Signed-off-by: Stanislav Seletskiy --- python/ycm/diagnostic_interface.py | 97 ++++++++++++++++++++++++++---- python/ycm/vimsupport.py | 32 +++++++--- 2 files changed, 109 insertions(+), 20 deletions(-) diff --git a/python/ycm/diagnostic_interface.py b/python/ycm/diagnostic_interface.py index f58baf9d..d799ec65 100644 --- a/python/ycm/diagnostic_interface.py +++ b/python/ycm/diagnostic_interface.py @@ -17,7 +17,7 @@ # You should have received a copy of the GNU General Public License # along with YouCompleteMe. If not, see . -from collections import defaultdict +from collections import defaultdict, namedtuple from ycm import vimsupport import vim @@ -31,6 +31,7 @@ class DiagnosticInterface( object ): self._next_sign_id = 1 self._previous_line_number = -1 self._diag_message_needs_clearing = False + self._placed_signs = [] def OnCursorMoved( self ): @@ -47,8 +48,10 @@ class DiagnosticInterface( object ): self._buffer_number_to_line_to_diags = _ConvertDiagListToDict( diags ) if self._user_options[ 'enable_diagnostic_signs' ]: - self._next_sign_id = _UpdateSigns( self._buffer_number_to_line_to_diags, - self._next_sign_id ) + self._placed_signs, self._next_sign_id = _UpdateSigns( + self._placed_signs, + self._buffer_number_to_line_to_diags, + self._next_sign_id ) if self._user_options[ 'enable_diagnostic_highlighting' ]: _UpdateSquiggles( self._buffer_number_to_line_to_diags ) @@ -102,21 +105,80 @@ def _UpdateSquiggles( buffer_number_to_line_to_diags ): is_error = is_error ) -def _UpdateSigns( buffer_number_to_line_to_diags, next_sign_id ): - vimsupport.UnplaceAllSignsInBuffer( vim.current.buffer.number ) +def _UpdateSigns( placed_signs, buffer_number_to_line_to_diags, next_sign_id ): + new_signs, kept_signs, next_sign_id = _GetKeptAndNewSigns( + placed_signs, buffer_number_to_line_to_diags, next_sign_id + ) + # Dummy sign used to prevent "flickering" in vim when last mark gets + # deleted from buffer. Dummy sign prevent vim to collapse sign column in + # that case. + # There also a vim "bug", which cause whole window to redraw in some + # conditions (vim redraw logic is very complex). But, somehow, if we place + # dummy sign before placing other real visible sign, it will not redraw the + # buffer (patch to vim pending). + dummy_sign_needed = False + if kept_signs == [] and new_signs != []: + dummy_sign_needed = True + + if dummy_sign_needed: + vimsupport.PlaceDummySign( next_sign_id + 1, vim.current.buffer.number, + new_signs[0].line ) + + # Now we can place only that signs that are not placed yet. + new_placed_signs = _PlaceNewSigns( kept_signs, new_signs ) + + # We use incremental placement, so signs that already placed on right lines + # will not be deleted and placed again, which should improve performance in + # case of many diags. + # Signs which are not exist in current diag should be deleted. + _UnplaceRottenSigns( kept_signs, placed_signs ) + + if dummy_sign_needed: + vimsupport.UnPlaceDummySign( next_sign_id + 1, vim.current.buffer.number ) + + return new_placed_signs, next_sign_id + +def _GetKeptAndNewSigns( placed_signs, buffer_number_to_line_to_diags, + next_sign_id ): + new_signs = [] + kept_signs = [] for buffer_number, line_to_diags in buffer_number_to_line_to_diags.iteritems(): if not vimsupport.BufferIsVisible( buffer_number ): continue - vimsupport.UnplaceAllSignsInBuffer( buffer_number ) for line, diags in line_to_diags.iteritems(): for diag in diags: - vimsupport.PlaceSign( next_sign_id, - line, - buffer_number, - _DiagnosticIsError( diag ) ) - next_sign_id += 1 - return next_sign_id + sign = _DiagSignPlacement( next_sign_id, + line, + buffer_number, + _DiagnosticIsError( diag ) ) + if sign not in placed_signs: + new_signs += [ sign ] + next_sign_id += 1 + else: + # We should use .index here because of `sign` contains new id, but + # we need old id to unplace sign in future. + kept_signs += [ placed_signs[ placed_signs.index( sign ) ] ] + return new_signs, kept_signs, next_sign_id + + + +def _PlaceNewSigns( kept_signs, new_signs ): + placed_signs = kept_signs + for sign in new_signs: + # Do not add two signs at the same line, it will screw signs remembering. + if sign in placed_signs: + continue + vimsupport.PlaceSign( sign.id, sign.line, sign.buffer, sign.is_err ) + placed_signs += [ sign ] + return placed_signs + + +def _UnplaceRottenSigns( kept_signs, placed_signs ): + for sign in placed_signs: + if sign not in kept_signs: + print("rotten", sign) + vimsupport.UnplaceSignInBuffer( sign.buffer, sign.id ) def _ConvertDiagListToDict( diag_list ): @@ -140,3 +202,14 @@ def _ConvertDiagListToDict( diag_list ): def _DiagnosticIsError( diag ): return diag[ 'kind' ] == 'ERROR' + +class _DiagSignPlacement(namedtuple( "_DiagSignPlacement", + [ 'id', 'line', 'buffer', 'is_err' ])): + def __eq__(self, another_sign): + if self.line != another_sign.line: + return False + if self.buffer != another_sign.buffer: + return False + if self.is_err != another_sign.is_err: + return False + return True diff --git a/python/ycm/vimsupport.py b/python/ycm/vimsupport.py index dc150309..fff54f4f 100644 --- a/python/ycm/vimsupport.py +++ b/python/ycm/vimsupport.py @@ -137,16 +137,12 @@ def GetBufferFilepath( buffer_object ): return os.path.join( folder_path, str( buffer_object.number ) ) -# NOTE: This unplaces *all* signs in a buffer, not just the ones we placed. We -# used to track which signs we ended up placing and would then only unplace -# ours, but that causes flickering Vim since we have to call -# sign unplace buffer= -# in a loop. So we're forced to unplace all signs, which might conflict with -# other Vim plugins. -def UnplaceAllSignsInBuffer( buffer_number ): +def UnplaceSignInBuffer( buffer_number, sign_id ): if buffer_number < 0: return - vim.command( 'sign unplace * buffer={0}'.format( buffer_number ) ) + vim.command( + 'try | exec "sign unplace {0} buffer={1}" | catch /E158/ | endtry'.format( + sign_id, buffer_number ) ) def PlaceSign( sign_id, line_num, buffer_num, is_error = True ): @@ -160,6 +156,26 @@ def PlaceSign( sign_id, line_num, buffer_num, is_error = True ): sign_id, line_num, sign_name, buffer_num ) ) +def PlaceDummySign( sign_id, buffer_num, line_num ): + if buffer_num < 0 or line_num < 0: + return + vim.command( 'sign define ycm_dummy_sign' ) + vim.command( + 'sign place {0} name=ycm_dummy_sign line={1} buffer={2}'.format( + sign_id, + line_num, + buffer_num, + ) + ) + + +def UnPlaceDummySign( sign_id, buffer_num ): + if buffer_num < 0: + return + vim.command( 'sign undefine ycm_dummy_sign' ) + vim.command( 'sign unplace {0} buffer={1}'.format( sign_id, buffer_num ) ) + + def ClearYcmSyntaxMatches(): matches = VimExpressionToPythonType( 'getmatches()' ) for match in matches: From 33b5b1eb028e95624f739fbe1af14076270aac4b Mon Sep 17 00:00:00 2001 From: Strahinja Val Markovic Date: Fri, 19 Sep 2014 12:30:15 -0700 Subject: [PATCH 2/2] Minor refactoring & stylistic changes. --- python/ycm/diagnostic_interface.py | 65 +++++++++++++++--------------- 1 file changed, 32 insertions(+), 33 deletions(-) diff --git a/python/ycm/diagnostic_interface.py b/python/ycm/diagnostic_interface.py index d799ec65..29092647 100644 --- a/python/ycm/diagnostic_interface.py +++ b/python/ycm/diagnostic_interface.py @@ -109,35 +109,35 @@ def _UpdateSigns( placed_signs, buffer_number_to_line_to_diags, next_sign_id ): new_signs, kept_signs, next_sign_id = _GetKeptAndNewSigns( placed_signs, buffer_number_to_line_to_diags, next_sign_id ) - # Dummy sign used to prevent "flickering" in vim when last mark gets - # deleted from buffer. Dummy sign prevent vim to collapse sign column in - # that case. - # There also a vim "bug", which cause whole window to redraw in some - # conditions (vim redraw logic is very complex). But, somehow, if we place - # dummy sign before placing other real visible sign, it will not redraw the + # Dummy sign used to prevent "flickering" in Vim when last mark gets + # deleted from buffer. Dummy sign prevents Vim to collapsing the sign column + # in that case. + # There's also a vim bug which causes the whole window to redraw in some + # conditions (vim redraw logic is very complex). But, somehow, if we place a + # dummy sign before placing other "real" signs, it will not redraw the # buffer (patch to vim pending). - dummy_sign_needed = False - if kept_signs == [] and new_signs != []: - dummy_sign_needed = True + dummy_sign_needed = not kept_signs and new_signs if dummy_sign_needed: - vimsupport.PlaceDummySign( next_sign_id + 1, vim.current.buffer.number, - new_signs[0].line ) + vimsupport.PlaceDummySign( next_sign_id + 1, + vim.current.buffer.number, + new_signs[ 0 ].line ) - # Now we can place only that signs that are not placed yet. + # We place only those signs that haven't been placed yet. new_placed_signs = _PlaceNewSigns( kept_signs, new_signs ) - # We use incremental placement, so signs that already placed on right lines - # will not be deleted and placed again, which should improve performance in - # case of many diags. - # Signs which are not exist in current diag should be deleted. - _UnplaceRottenSigns( kept_signs, placed_signs ) + # We use incremental placement, so signs that already placed on the correct + # lines will not be deleted and placed again, which should improve performance + # in case of many diags. Signs which don't exist in the current diag should be + # deleted. + _UnplaceObsoleteSigns( kept_signs, placed_signs ) if dummy_sign_needed: vimsupport.UnPlaceDummySign( next_sign_id + 1, vim.current.buffer.number ) return new_placed_signs, next_sign_id + def _GetKeptAndNewSigns( placed_signs, buffer_number_to_line_to_diags, next_sign_id ): new_signs = [] @@ -156,8 +156,9 @@ def _GetKeptAndNewSigns( placed_signs, buffer_number_to_line_to_diags, new_signs += [ sign ] next_sign_id += 1 else: - # We should use .index here because of `sign` contains new id, but - # we need old id to unplace sign in future. + # We use .index here because `sign` contains a new id, but + # we need the sign with the old id to unplace it later on. + # We won't be placing the new sign. kept_signs += [ placed_signs[ placed_signs.index( sign ) ] ] return new_signs, kept_signs, next_sign_id @@ -166,18 +167,18 @@ def _GetKeptAndNewSigns( placed_signs, buffer_number_to_line_to_diags, def _PlaceNewSigns( kept_signs, new_signs ): placed_signs = kept_signs for sign in new_signs: - # Do not add two signs at the same line, it will screw signs remembering. + # Do not set two signs on the same line, it will screw up storing sign + # locations. if sign in placed_signs: continue - vimsupport.PlaceSign( sign.id, sign.line, sign.buffer, sign.is_err ) + vimsupport.PlaceSign( sign.id, sign.line, sign.buffer, sign.is_error ) placed_signs += [ sign ] return placed_signs -def _UnplaceRottenSigns( kept_signs, placed_signs ): +def _UnplaceObsoleteSigns( kept_signs, placed_signs ): for sign in placed_signs: if sign not in kept_signs: - print("rotten", sign) vimsupport.UnplaceSignInBuffer( sign.buffer, sign.id ) @@ -203,13 +204,11 @@ def _DiagnosticIsError( diag ): return diag[ 'kind' ] == 'ERROR' -class _DiagSignPlacement(namedtuple( "_DiagSignPlacement", - [ 'id', 'line', 'buffer', 'is_err' ])): - def __eq__(self, another_sign): - if self.line != another_sign.line: - return False - if self.buffer != another_sign.buffer: - return False - if self.is_err != another_sign.is_err: - return False - return True +class _DiagSignPlacement( namedtuple( "_DiagSignPlacement", + [ 'id', 'line', 'buffer', 'is_error' ] ) ): + # We want two signs that have different ids but the same location to compare + # equal. ID doesn't matter. + def __eq__( self, other ): + return ( self.line == other.line and + self.buffer == other.buffer and + self.is_error == other.is_error )