Auto merge of #2922 - micbou:improve-diagnostic-match, r=bstaletic

[READY] Improve diagnostic matches display

There are two issues with how we display diagnostic matches. The first issue is that if the current buffer contains diagnostic matches and is split in a new window, all the matches are cleared and only matches in the new window are shown. The second issue is that if a new buffer with no diagnostic support is open in the current window and that window already contained matches then the matches are still displayed. Here's an illustration of both issues (signs are disabled):

![diagnostic-matches-issue](https://user-images.githubusercontent.com/10026824/36352338-bfeb2092-14b7-11e8-88f4-ae8cf6903304.gif)

The solution is to add an autocommand that updates matches on the `BufEnter` and `WinEnter` events. Here's the result:

![diagnostic-matches-fix](https://user-images.githubusercontent.com/10026824/36352340-c2a64a8c-14b7-11e8-8db2-4f1f54448c65.gif)

Note that it's not perfect as multiple windows of the same buffer won't be updated simultaneously. Supporting that scenario is rather tricky because we would need to go through all the windows to update the matches and switching windows can lead to a lot of issues (like interrupting visual mode) so we don't.

This PR also improves how we update matches by only displaying matches that are not already present and then clearing the remaining ones (similarly to what we do with signs; see PR https://github.com/Valloric/YouCompleteMe/pull/2915) instead of always clearing all the matches then displaying the new ones.

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

autoload/youcompleteme.vim

@@ -136,6 +136,7 @@ function! youcompleteme#Enable()
     autocmd InsertLeave * call s:OnInsertLeave()
     autocmd VimLeave * call s:OnVimLeave()
     autocmd CompleteDone * call s:OnCompleteDone()
+    autocmd BufEnter,WinEnter * call s:UpdateMatches()
   augroup END
 
   " The FileType event is not triggered for the first loaded file. We wait until
@@ -513,6 +514,11 @@ function! s:OnBufferUnload()
 endfunction
 
 
+function! s:UpdateMatches()
+  exec s:python_command "ycm_state.UpdateMatches()"
+endfunction
+
+
 function! s:PollServerReady( timer_id )
   if !s:Pyeval( 'ycm_state.IsServerAlive()' )
     exec s:python_command "ycm_state.NotifyUserIfServerCrashed()"

python/ycm/buffer.py

@@ -80,6 +80,10 @@ class Buffer( object ):
     self._diag_interface.UpdateWithNewDiagnostics( diagnostics )
 
 
+  def UpdateMatches( self ):
+    self._diag_interface.UpdateMatches()
+
+
   def PopulateLocationList( self ):
     return self._diag_interface.PopulateLocationList()
 

python/ycm/diagnostic_interface.py

@@ -75,8 +75,7 @@ class DiagnosticInterface( object ):
     if self._user_options[ 'enable_diagnostic_signs' ]:
       self._UpdateSigns()
 
-    if self._user_options[ 'enable_diagnostic_highlighting' ]:
-      self._UpdateSquiggles()
+    self.UpdateMatches()
 
     if self._user_options[ 'always_populate_location_list' ]:
       self._UpdateLocationList()
@@ -127,37 +126,50 @@ class DiagnosticInterface( object ):
       vimsupport.ConvertDiagnosticsToQfList( self._diagnostics ) )
 
 
-  def _UpdateSquiggles( self ):
+  def UpdateMatches( self ):
+    if not self._user_options[ 'enable_diagnostic_highlighting' ]:
+      return
 
-    vimsupport.ClearYcmSyntaxMatches()
+    matches_to_remove = vimsupport.GetDiagnosticMatchesInCurrentWindow()
 
     for diags in itervalues( self._line_to_diags ):
       # Insert squiggles in reverse order so that errors overlap warnings.
       for diag in reversed( diags ):
-        location_extent = diag[ 'location_extent' ]
-        is_error = _DiagnosticIsError( diag )
+        patterns = []
 
+        group = ( 'YcmErrorSection' if _DiagnosticIsError( diag ) else
+                  'YcmWarningSection' )
+
+        location_extent = diag[ 'location_extent' ]
         if location_extent[ 'start' ][ 'line_num' ] <= 0:
           location = diag[ 'location' ]
-          vimsupport.AddDiagnosticSyntaxMatch(
-              location[ 'line_num' ],
-              location[ 'column_num' ],
-              is_error = is_error )
+          patterns.append( vimsupport.GetDiagnosticMatchPattern(
+            location[ 'line_num' ],
+            location[ 'column_num' ] ) )
         else:
-          vimsupport.AddDiagnosticSyntaxMatch(
+          patterns.append( vimsupport.GetDiagnosticMatchPattern(
             location_extent[ 'start' ][ 'line_num' ],
             location_extent[ 'start' ][ 'column_num' ],
             location_extent[ 'end' ][ 'line_num' ],
-            location_extent[ 'end' ][ 'column_num' ],
-            is_error = is_error )
+            location_extent[ 'end' ][ 'column_num' ] ) )
 
         for diag_range in diag[ 'ranges' ]:
-          vimsupport.AddDiagnosticSyntaxMatch(
+          patterns.append( vimsupport.GetDiagnosticMatchPattern(
             diag_range[ 'start' ][ 'line_num' ],
             diag_range[ 'start' ][ 'column_num' ],
             diag_range[ 'end' ][ 'line_num' ],
-            diag_range[ 'end' ][ 'column_num' ],
-            is_error = is_error )
+            diag_range[ 'end' ][ 'column_num' ] ) )
+
+        for pattern in patterns:
+          # The id doesn't matter for matches that we may add.
+          match = vimsupport.DiagnosticMatch( 0, group, pattern )
+          try:
+            matches_to_remove.remove( match )
+          except ValueError:
+            vimsupport.AddDiagnosticMatch( match )
+
+    for match in matches_to_remove:
+      vimsupport.RemoveDiagnosticMatch( match )
 
 
   def _UpdateSigns( self ):

python/ycm/tests/test_utils.py

@@ -203,9 +203,9 @@ def _MockVimMatchEval( value ):
 
   match = MATCHDELETE_REGEX.search( value )
   if match:
-    identity = int( match.group( 'id' ) )
+    match_id = int( match.group( 'id' ) )
     for index, vim_match in enumerate( VIM_MATCHES ):
-      if vim_match.id == identity:
+      if vim_match.id == match_id:
         VIM_MATCHES.pop( index )
         return -1
     return 0
@@ -421,7 +421,7 @@ class VimBuffers( object ):
 class VimMatch( object ):
 
   def __init__( self, group, pattern ):
-    self.id = len( VIM_MATCHES )
+    self.id = len( VIM_MATCHES ) + 1
     self.group = group
     self.pattern = pattern
 

python/ycm/tests/vimsupport_test.py

@@ -1256,32 +1256,30 @@ def _BuildChunk( start_line,
   }
 
 
-@patch( 'vim.eval', new_callable = ExtendedMock )
-def AddDiagnosticSyntaxMatch_ErrorInMiddleOfLine_test( vim_eval ):
+def GetDiagnosticMatchPattern_ErrorInMiddleOfLine_test():
   current_buffer = VimBuffer(
     'some_file',
     contents = [ 'Highlight this error please' ]
   )
 
   with patch( 'vim.current.buffer', current_buffer ):
-    vimsupport.AddDiagnosticSyntaxMatch( 1, 16, 1, 21 )
-
-  vim_eval.assert_called_once_with(
-    r"matchadd('YcmErrorSection', '\%1l\%16c\_.\{-}\%1l\%21c')" )
+    assert_that(
+      vimsupport.GetDiagnosticMatchPattern( 1, 16, 1, 21 ),
+      equal_to( '\%1l\%16c\_.\{-}\%1l\%21c' )
+    )
 
 
-@patch( 'vim.eval', new_callable = ExtendedMock )
-def AddDiagnosticSyntaxMatch_WarningAtEndOfLine_test( vim_eval ):
+def AddDiagnosticSyntaxMatch_WarningAtEndOfLine_test():
   current_buffer = VimBuffer(
     'some_file',
     contents = [ 'Highlight this warning' ]
   )
 
   with patch( 'vim.current.buffer', current_buffer ):
-    vimsupport.AddDiagnosticSyntaxMatch( 1, 16, 1, 23, is_error = False )
-
-  vim_eval.assert_called_once_with(
-    r"matchadd('YcmWarningSection', '\%1l\%16c\_.\{-}\%1l\%23c')" )
+    assert_that(
+      vimsupport.GetDiagnosticMatchPattern( 1, 16, 1, 23 ),
+      equal_to( '\%1l\%16c\_.\{-}\%1l\%23c' )
+    )
 
 
 @patch( 'vim.command', new_callable=ExtendedMock )

python/ycm/tests/youcompleteme_test.py

@@ -664,6 +664,25 @@ def YouCompleteMe_UpdateDiagnosticInterface_PrioritizeErrorsOverWarnings_test(
     )
 
 
+@YouCompleteMeInstance( { 'enable_diagnostic_highlighting': 1 } )
+def YouCompleteMe_UpdateMatches_ClearDiagnosticMatchesInNewBuffer_test( ycm ):
+  current_buffer = VimBuffer( 'buffer',
+                              filetype = 'c',
+                              number = 5,
+                              window = 2 )
+
+  test_utils.VIM_MATCHES = [
+    VimMatch( 'YcmWarningSection', '\%3l\%5c\_.\{-}\%3l\%7c' ),
+    VimMatch( 'YcmWarningSection', '\%3l\%3c\_.\{-}\%3l\%9c' ),
+    VimMatch( 'YcmErrorSection', '\%3l\%8c' )
+  ]
+
+  with MockVimBuffers( [ current_buffer ], current_buffer ):
+    ycm.UpdateMatches()
+
+    assert_that( test_utils.VIM_MATCHES, empty() )
+
+
 @YouCompleteMeInstance( { 'echo_current_diagnostic': 1,
                           'always_populate_location_list': 1 } )
 @patch.object( ycm_buffer_module,

python/ycm/vimsupport.py

@@ -214,39 +214,47 @@ def PlaceSign( sign ):
     sign.id, sign.name, sign.line, sign.buffer_number ) )
 
 
-def ClearYcmSyntaxMatches():
-  matches = VimExpressionToPythonType( 'getmatches()' )
-  for match in matches:
-    if match[ 'group' ].startswith( 'Ycm' ):
-      vim.eval( 'matchdelete({0})'.format( match[ 'id' ] ) )
-
-
-def AddDiagnosticSyntaxMatch( line_num,
-                              column_num,
-                              line_end_num = None,
-                              column_end_num = None,
-                              is_error = True ):
-  """Highlight a range in the current window starting from
-  (|line_num|, |column_num|) included to (|line_end_num|, |column_end_num|)
-  excluded. If |line_end_num| or |column_end_num| are not given, highlight the
-  character at (|line_num|, |column_num|). Both line and column numbers are
-  1-based. Return the ID of the newly added match."""
-  group = 'YcmErrorSection' if is_error else 'YcmWarningSection'
+class DiagnosticMatch( namedtuple( 'DiagnosticMatch',
+                                   [ 'id', 'group', 'pattern' ] ) ):
+  def __eq__( self, other ):
+    return ( self.group == other.group and
+             self.pattern == other.pattern )
+
+
+def GetDiagnosticMatchesInCurrentWindow():
+  vim_matches = vim.eval( 'getmatches()' )
+  return [ DiagnosticMatch( match[ 'id' ],
+                            match[ 'group' ],
+                            match[ 'pattern' ] )
+           for match in vim_matches if match[ 'group' ].startswith( 'Ycm' ) ]
+
+
+def AddDiagnosticMatch( match ):
+  return GetIntValue( "matchadd('{}', '{}')".format( match.group,
+                                                     match.pattern ) )
 
+
+def RemoveDiagnosticMatch( match ):
+  return GetIntValue( "matchdelete({})".format( match.id ) )
+
+
+def GetDiagnosticMatchPattern( line_num,
+                               column_num,
+                               line_end_num = None,
+                               column_end_num = None ):
   line_num, column_num = LineAndColumnNumbersClamped( line_num, column_num )
 
   if not line_end_num or not column_end_num:
-    return GetIntValue(
-      "matchadd('{0}', '\%{1}l\%{2}c')".format( group, line_num, column_num ) )
+    return '\%{}l\%{}c'.format( line_num, column_num )
 
   # -1 and then +1 to account for column end not included in the range.
   line_end_num, column_end_num = LineAndColumnNumbersClamped(
       line_end_num, column_end_num - 1 )
   column_end_num += 1
-
-  return GetIntValue(
-    "matchadd('{0}', '\%{1}l\%{2}c\_.\\{{-}}\%{3}l\%{4}c')".format(
-      group, line_num, column_num, line_end_num, column_end_num ) )
+  return '\%{}l\%{}c\_.\\{{-}}\%{}l\%{}c'.format( line_num,
+                                                  column_num,
+                                                  line_end_num,
+                                                  column_end_num )
 
 
 # Clamps the line and column numbers so that they are not past the contents of

python/ycm/youcompleteme.py

@@ -461,6 +461,10 @@ class YouCompleteMe( object ):
     SendEventNotificationAsync( 'BufferUnload', deleted_buffer_number )
 
 
+  def UpdateMatches( self ):
+    self.CurrentBuffer().UpdateMatches()
+
+
   def OnBufferVisit( self ):
     extra_data = {}
     self._AddUltiSnipsDataIfNeeded( extra_data )