Browse Source

Improve diagnostic sign placement

micbou 7 years ago
parent
commit
5e98da5b09

+ 16 - 40
python/ycm/diagnostic_interface.py

@@ -23,7 +23,7 @@ from __future__ import absolute_import
 from builtins import *  # noqa
 
 from future.utils import itervalues, iteritems
-from collections import defaultdict, namedtuple
+from collections import defaultdict
 from ycm import vimsupport
 from ycm.diagnostic_filter import DiagnosticFilter, CompileLevel
 
@@ -36,8 +36,7 @@ class DiagnosticInterface( object ):
     self._diag_filter = DiagnosticFilter.CreateFromOptions( user_options )
     # Line and column numbers are 1-based
     self._line_to_diags = defaultdict( list )
-    self._placed_signs = []
-    self._next_sign_id = 1
+    self._next_sign_id = vimsupport.SIGN_BUFFER_ID_INITIAL_VALUE
     self._previous_diag_line_number = -1
     self._diag_message_needs_clearing = False
 
@@ -162,43 +161,29 @@ class DiagnosticInterface( object ):
 
 
   def _UpdateSigns( self ):
-    new_signs, obsolete_signs = self._GetNewAndObsoleteSigns()
+    signs_to_unplace = vimsupport.GetSignsInBuffer( self._bufnr )
 
-    self._PlaceNewSigns( new_signs )
-
-    self._UnplaceObsoleteSigns( obsolete_signs )
-
-
-  def _GetNewAndObsoleteSigns( self ):
-    new_signs = []
-    obsolete_signs = list( self._placed_signs )
     for line, diags in iteritems( self._line_to_diags ):
       if not diags:
         continue
 
-      # We always go for the first diagnostic on line,
-      # because it is sorted giving priority to the Errors.
-      diag = diags[ 0 ]
-      sign = _DiagSignPlacement( self._next_sign_id,
-                                 line, _DiagnosticIsError( diag ) )
+      # We always go for the first diagnostic on the line because diagnostics
+      # are sorted by errors in priority and Vim can only display one sign by
+      # line.
+      name = 'YcmError' if _DiagnosticIsError( diags[ 0 ] ) else 'YcmWarning'
+      sign = vimsupport.DiagnosticSign( self._next_sign_id,
+                                        line,
+                                        name,
+                                        self._bufnr )
+
       try:
-        obsolete_signs.remove( sign )
+        signs_to_unplace.remove( sign )
       except ValueError:
-        new_signs.append( sign )
+        vimsupport.PlaceSign( sign )
         self._next_sign_id += 1
-    return new_signs, obsolete_signs
-
 
-  def _PlaceNewSigns( self, new_signs ):
-    for sign in new_signs:
-      vimsupport.PlaceSign( sign.id, sign.line, self._bufnr, sign.is_error )
-      self._placed_signs.append( sign )
-
-
-  def _UnplaceObsoleteSigns( self, obsolete_signs ):
-    for sign in obsolete_signs:
-      self._placed_signs.remove( sign )
-      vimsupport.UnplaceSignInBuffer( self._bufnr, sign.id )
+    for sign in signs_to_unplace:
+      vimsupport.UnplaceSign( sign )
 
 
   def _ConvertDiagListToDict( self ):
@@ -229,12 +214,3 @@ def _NormalizeDiagnostic( diag ):
   location[ 'column_num' ] = ClampToOne( location[ 'column_num' ] )
   location[ 'line_num' ] = ClampToOne( location[ 'line_num' ] )
   return diag
-
-
-class _DiagSignPlacement( namedtuple( "_DiagSignPlacement",
-                                      [ 'id', 'line', '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.is_error == other.is_error )

+ 65 - 38
python/ycm/tests/event_notification_test.py

@@ -25,18 +25,21 @@ from __future__ import absolute_import
 from builtins import *  # noqa
 
 from ycm.tests.test_utils import ( CurrentWorkingDirectory, ExtendedMock,
-                                   MockVimBuffers, MockVimModule, VimBuffer )
+                                   MockVimBuffers, MockVimModule, VimBuffer,
+                                   VimSign )
 MockVimModule()
 
 import contextlib
 import os
 
-from ycm.tests import PathToTestFile, YouCompleteMeInstance, WaitUntilReady
+from ycm.tests import ( PathToTestFile, test_utils, YouCompleteMeInstance,
+                        WaitUntilReady )
+from ycm.vimsupport import SIGN_BUFFER_ID_INITIAL_VALUE
 from ycmd.responses import ( BuildDiagnosticData, Diagnostic, Location, Range,
                              UnknownExtraConf, ServerError )
 
-from hamcrest import ( assert_that, contains, has_entries, has_entry, has_item,
-                       has_items, has_key, is_not )
+from hamcrest import ( assert_that, contains, empty, has_entries, has_entry,
+                       has_item, has_items, has_key, is_not )
 from mock import call, MagicMock, patch
 from nose.tools import eq_, ok_
 
@@ -47,17 +50,6 @@ def PresentDialog_Confirm_Call( message ):
   return call( message, [ 'Ok', 'Cancel' ] )
 
 
-def PlaceSign_Call( sign_id, line_num, buffer_num, is_error ):
-  sign_name = 'YcmError' if is_error else 'YcmWarning'
-  return call( 'sign place {0} name={1} line={2} buffer={3}'
-                  .format( sign_id, sign_name, line_num, buffer_num ) )
-
-
-def UnplaceSign_Call( sign_id, buffer_num ):
-  return call( 'try | exec "sign unplace {0} buffer={1}" |'
-               ' catch /E158/ | endtry'.format( sign_id, buffer_num ) )
-
-
 @contextlib.contextmanager
 def MockArbitraryBuffer( filetype ):
   """Used via the with statement, set up a single buffer with an arbitrary name
@@ -148,16 +140,25 @@ def EventNotification_FileReadyToParse_NonDiagnostic_Error_test(
       ] )
 
 
-@patch( 'vim.command' )
 @YouCompleteMeInstance()
 def EventNotification_FileReadyToParse_NonDiagnostic_Error_NonNative_test(
-    ycm, vim_command ):
+  ycm ):
+
+  test_utils.VIM_MATCHES = []
+  test_utils.VIM_SIGNS = []
 
   with MockArbitraryBuffer( 'javascript' ):
     with MockEventNotification( None, False ):
       ycm.OnFileReadyToParse()
       ycm.HandleFileParseRequest()
-      vim_command.assert_not_called()
+      assert_that(
+        test_utils.VIM_MATCHES,
+        contains()
+      )
+      assert_that(
+        test_utils.VIM_SIGNS,
+        contains()
+      )
 
 
 @patch( 'ycm.client.base_request._LoadExtraConfFile',
@@ -263,13 +264,14 @@ def EventNotification_FileReadyToParse_NonDiagnostic_ConfirmExtraConf_test(
 
 @YouCompleteMeInstance()
 def EventNotification_FileReadyToParse_Diagnostic_Error_Native_test( ycm ):
+  test_utils.VIM_SIGNS = []
+
   _Check_FileReadyToParse_Diagnostic_Error( ycm )
   _Check_FileReadyToParse_Diagnostic_Warning( ycm )
   _Check_FileReadyToParse_Diagnostic_Clean( ycm )
 
 
-@patch( 'vim.command' )
-def _Check_FileReadyToParse_Diagnostic_Error( ycm, vim_command ):
+def _Check_FileReadyToParse_Diagnostic_Error( ycm ):
   # Tests Vim sign placement and error/warning count python API
   # when one error is returned.
   def DiagnosticResponse( *args ):
@@ -284,23 +286,42 @@ def _Check_FileReadyToParse_Diagnostic_Error( ycm, vim_command ):
       ycm.OnFileReadyToParse()
       ok_( ycm.FileParseRequestReady() )
       ycm.HandleFileParseRequest()
-      vim_command.assert_has_calls( [
-        PlaceSign_Call( 1, 1, 1, True )
-      ] )
+      assert_that(
+        test_utils.VIM_SIGNS,
+        contains(
+          VimSign( SIGN_BUFFER_ID_INITIAL_VALUE, 1, 'YcmError', 1 )
+        )
+      )
       eq_( ycm.GetErrorCount(), 1 )
       eq_( ycm.GetWarningCount(), 0 )
 
       # Consequent calls to HandleFileParseRequest shouldn't mess with
       # existing diagnostics, when there is no new parse request.
-      vim_command.reset_mock()
       ycm.HandleFileParseRequest()
-      vim_command.assert_not_called()
+      assert_that(
+        test_utils.VIM_SIGNS,
+        contains(
+          VimSign( SIGN_BUFFER_ID_INITIAL_VALUE, 1, 'YcmError', 1 )
+        )
+      )
+      eq_( ycm.GetErrorCount(), 1 )
+      eq_( ycm.GetWarningCount(), 0 )
+
+      # New identical requests should result in the same diagnostics.
+      ycm.OnFileReadyToParse()
+      ok_( ycm.FileParseRequestReady() )
+      ycm.HandleFileParseRequest()
+      assert_that(
+        test_utils.VIM_SIGNS,
+        contains(
+          VimSign( SIGN_BUFFER_ID_INITIAL_VALUE, 1, 'YcmError', 1 )
+        )
+      )
       eq_( ycm.GetErrorCount(), 1 )
       eq_( ycm.GetWarningCount(), 0 )
 
 
-@patch( 'vim.command' )
-def _Check_FileReadyToParse_Diagnostic_Warning( ycm, vim_command ):
+def _Check_FileReadyToParse_Diagnostic_Warning( ycm ):
   # Tests Vim sign placement/unplacement and error/warning count python API
   # when one warning is returned.
   # Should be called after _Check_FileReadyToParse_Diagnostic_Error
@@ -316,24 +337,29 @@ def _Check_FileReadyToParse_Diagnostic_Warning( ycm, vim_command ):
       ycm.OnFileReadyToParse()
       ok_( ycm.FileParseRequestReady() )
       ycm.HandleFileParseRequest()
-      vim_command.assert_has_calls( [
-        PlaceSign_Call( 2, 2, 1, False ),
-        UnplaceSign_Call( 1, 1 )
-      ] )
+      assert_that(
+        test_utils.VIM_SIGNS,
+        contains(
+          VimSign( SIGN_BUFFER_ID_INITIAL_VALUE + 1, 2, 'YcmWarning', 1 )
+        )
+      )
       eq_( ycm.GetErrorCount(), 0 )
       eq_( ycm.GetWarningCount(), 1 )
 
       # Consequent calls to HandleFileParseRequest shouldn't mess with
       # existing diagnostics, when there is no new parse request.
-      vim_command.reset_mock()
       ycm.HandleFileParseRequest()
-      vim_command.assert_not_called()
+      assert_that(
+        test_utils.VIM_SIGNS,
+        contains(
+          VimSign( SIGN_BUFFER_ID_INITIAL_VALUE + 1, 2, 'YcmWarning', 1 )
+        )
+      )
       eq_( ycm.GetErrorCount(), 0 )
       eq_( ycm.GetWarningCount(), 1 )
 
 
-@patch( 'vim.command' )
-def _Check_FileReadyToParse_Diagnostic_Clean( ycm, vim_command ):
+def _Check_FileReadyToParse_Diagnostic_Clean( ycm ):
   # Tests Vim sign unplacement and error/warning count python API
   # when there are no errors/warnings left.
   # Should be called after _Check_FileReadyToParse_Diagnostic_Warning
@@ -341,9 +367,10 @@ def _Check_FileReadyToParse_Diagnostic_Clean( ycm, vim_command ):
     with MockEventNotification( MagicMock( return_value = [] ) ):
       ycm.OnFileReadyToParse()
       ycm.HandleFileParseRequest()
-      vim_command.assert_has_calls( [
-        UnplaceSign_Call( 2, 1 )
-      ] )
+      assert_that(
+        test_utils.VIM_SIGNS,
+        empty()
+      )
       eq_( ycm.GetErrorCount(), 0 )
       eq_( ycm.GetWarningCount(), 0 )
 

+ 128 - 12
python/ycm/tests/test_utils.py

@@ -23,7 +23,7 @@ from __future__ import absolute_import
 from builtins import *  # noqa
 
 from future.utils import PY2
-from mock import MagicMock, patch
+from mock import DEFAULT, MagicMock, patch
 from hamcrest import assert_that, equal_to
 import contextlib
 import functools
@@ -47,6 +47,15 @@ MATCHDELETE_REGEX = re.compile( '^matchdelete\((?P<id>\d+)\)$' )
 OMNIFUNC_REGEX_FORMAT = (
   '^{omnifunc_name}\((?P<findstart>[01]),[\'"](?P<base>.*)[\'"]\)$' )
 FNAMEESCAPE_REGEX = re.compile( '^fnameescape\(\'(?P<filepath>.+)\'\)$' )
+SIGN_LIST_REGEX = re.compile(
+  "^silent sign place buffer=(?P<bufnr>\d+)$" )
+SIGN_PLACE_REGEX = re.compile(
+  '^sign place (?P<id>\d+) name=(?P<name>\w+) line=(?P<line>\d+) '
+  'buffer=(?P<bufnr>\d+)$' )
+SIGN_UNPLACE_REGEX = re.compile(
+  '^sign unplace (?P<id>\d+) buffer=(?P<bufnr>\d+)$' )
+REDIR_START_REGEX = re.compile( '^redir => (?P<variable>[\w:]+)$' )
+REDIR_END_REGEX = re.compile( '^redir END$' )
 
 # One-and only instance of mocked Vim object. The first 'import vim' that is
 # executed binds the vim module to the instance of MagicMock that is created,
@@ -59,6 +68,13 @@ FNAMEESCAPE_REGEX = re.compile( '^fnameescape\(\'(?P<filepath>.+)\'\)$' )
 VIM_MOCK = MagicMock()
 
 VIM_MATCHES = []
+VIM_SIGNS = []
+
+REDIR = {
+  'status': False,
+  'variable': '',
+  'output': ''
+}
 
 
 @contextlib.contextmanager
@@ -158,6 +174,19 @@ def _MockVimOptionsEval( value ):
   return None
 
 
+def _MockVimFunctionsEval( value ):
+  if value == 'tempname()':
+    return '_TEMP_FILE_'
+
+  if value == 'tagfiles()':
+    return [ 'tags' ]
+
+  if value == 'shiftwidth()':
+    return 2
+
+  return None
+
+
 def _MockVimMatchEval( value ):
   if value == 'getmatches()':
     # Returning a copy, because ClearYcmSyntaxMatches() gets the result of
@@ -196,14 +225,9 @@ def _MockVimEval( value ):
   if value == 'g:ycm_server_python_interpreter':
     return server_python_interpreter
 
-  if value == 'tempname()':
-    return '_TEMP_FILE_'
-
-  if value == 'tagfiles()':
-    return [ 'tags' ]
-
-  if value == 'shiftwidth()':
-    return 2
+  result = _MockVimFunctionsEval( value )
+  if result is not None:
+    return result
 
   result = _MockVimOptionsEval( value )
   if result is not None:
@@ -221,6 +245,9 @@ def _MockVimEval( value ):
   if match:
     return match.group( 'filepath' )
 
+  if value == REDIR[ 'variable' ]:
+    return REDIR[ 'output' ]
+
   raise VimError( 'Unexpected evaluation: {0}'.format( value ) )
 
 
@@ -232,12 +259,65 @@ def _MockWipeoutBuffer( buffer_number ):
       return buffers.pop( index )
 
 
-def MockVimCommand( command ):
+def _MockSignCommand( command ):
+  match = SIGN_LIST_REGEX.search( command )
+  if match and REDIR[ 'status' ]:
+    bufnr = int( match.group( 'bufnr' ) )
+    REDIR[ 'output' ] = ( '--- Signs ---\n'
+                          'Signs for foo:\n' )
+    for sign in VIM_SIGNS:
+      if sign.bufnr == bufnr:
+        REDIR[ 'output' ] += (
+          '    line={0}  id={1}  name={2}'.format( sign.line,
+                                                   sign.id,
+                                                   sign.name ) )
+    return True
+
+  match = SIGN_PLACE_REGEX.search( command )
+  if match:
+    VIM_SIGNS.append( VimSign( int( match.group( 'id' ) ),
+                               int( match.group( 'line' ) ),
+                               match.group( 'name' ),
+                               int( match.group( 'bufnr' ) ) ) )
+    return True
+
+  match = SIGN_UNPLACE_REGEX.search( command )
+  if match:
+    sign_id = int( match.group( 'id' ) )
+    bufnr = int( match.group( 'bufnr' ) )
+    for sign in VIM_SIGNS:
+      if sign.id == sign_id and sign.bufnr == bufnr:
+        VIM_SIGNS.remove( sign )
+        return True
+
+  return False
+
+
+def _MockVimCommand( command ):
   match = BWIPEOUT_REGEX.search( command )
   if match:
     return _MockWipeoutBuffer( int( match.group( 1 ) ) )
 
-  raise RuntimeError( 'Unexpected command: ' + command )
+  match = REDIR_START_REGEX.search( command )
+  if match:
+    REDIR[ 'status' ] = True
+    REDIR[ 'variable' ] = match.group( 'variable' )
+    return
+
+  match = REDIR_END_REGEX.search( command )
+  if match:
+    REDIR[ 'status' ] = False
+    return
+
+  if command == 'unlet ' + REDIR[ 'variable' ]:
+    REDIR[ 'variable' ] = ''
+    return
+
+  result = _MockSignCommand( command )
+  if result:
+    return
+
+  return DEFAULT
 
 
 class VimBuffer( object ):
@@ -318,7 +398,7 @@ class VimBuffers( object ):
 
   def __init__( self, *buffers ):
     """Arguments are VimBuffer objects."""
-    self._buffers = buffers
+    self._buffers = list( buffers )
 
 
   def __getitem__( self, number ):
@@ -334,6 +414,10 @@ class VimBuffers( object ):
     return iter( self._buffers )
 
 
+  def pop( self, index ):
+    return self._buffers.pop( index )
+
+
 class VimMatch( object ):
 
   def __init__( self, group, pattern ):
@@ -358,6 +442,37 @@ class VimMatch( object ):
       return self.id
 
 
+class VimSign( object ):
+
+  def __init__( self, sign_id, line, name, bufnr ):
+    self.id = sign_id
+    self.line = line
+    self.name = name
+    self.bufnr = bufnr
+
+
+  def __eq__( self, other ):
+    return ( self.id == other.id and
+             self.line == other.line and
+             self.name == other.name and
+             self.bufnr == other.bufnr )
+
+
+  def __repr__( self ):
+    return ( "VimSign( id = {0}, line = {1}, "
+                      "name = '{2}', bufnr = {3} )".format( self.id,
+                                                            self.line,
+                                                            self.name,
+                                                            self.bufnr ) )
+
+
+  def __getitem__( self, key ):
+    if key == 'group':
+      return self.group
+    elif key == 'id':
+      return self.id
+
+
 @contextlib.contextmanager
 def MockVimBuffers( buffers, current_buffer, cursor_position = ( 1, 1 ) ):
   """Simulates the Vim buffers list |buffers| where |current_buffer| is the
@@ -398,6 +513,7 @@ def MockVimModule():
   tests."""
 
   VIM_MOCK.buffers = {}
+  VIM_MOCK.command = MagicMock( side_effect = _MockVimCommand )
   VIM_MOCK.eval = MagicMock( side_effect = _MockVimEval )
   VIM_MOCK.error = VimError
   sys.modules[ 'vim' ] = VIM_MOCK

+ 7 - 12
python/ycm/tests/vimsupport_test.py

@@ -26,13 +26,14 @@ from builtins import *  # noqa
 
 from ycm.tests import PathToTestFile
 from ycm.tests.test_utils import ( CurrentWorkingDirectory, ExtendedMock,
-                                   MockVimBuffers, MockVimCommand,
-                                   MockVimModule, VimBuffer, VimError )
+                                   MockVimBuffers, MockVimModule, VimBuffer,
+                                   VimError )
 MockVimModule()
 
 from ycm import vimsupport
 from nose.tools import eq_
-from hamcrest import assert_that, calling, contains, equal_to, has_entry, raises
+from hamcrest import ( assert_that, calling, contains, empty, equal_to,
+                       has_entry, raises )
 from mock import MagicMock, call, patch
 from ycmd.utils import ToBytes
 import os
@@ -1373,22 +1374,16 @@ def BufferIsVisibleForFilename_test():
     eq_( vimsupport.BufferIsVisibleForFilename( 'another_filename' ), False )
 
 
-@patch( 'vim.command',
-        side_effect = MockVimCommand,
-        new_callable = ExtendedMock )
-def CloseBuffersForFilename_test( vim_command, *args ):
+def CloseBuffersForFilename_test():
   vim_buffers = [
     VimBuffer( 'some_filename', number = 2 ),
     VimBuffer( 'some_filename', number = 5 )
   ]
 
-  with patch( 'vim.buffers', vim_buffers ):
+  with MockVimBuffers( vim_buffers, vim_buffers[ 0 ] ) as vim:
     vimsupport.CloseBuffersForFilename( 'some_filename' )
 
-  vim_command.assert_has_exact_calls( [
-    call( 'silent! bwipeout! 2' ),
-    call( 'silent! bwipeout! 5' )
-  ], any_order = True )
+  assert_that( vim.buffers, empty() )
 
 
 @patch( 'vim.command', new_callable = ExtendedMock )

+ 16 - 11
python/ycm/tests/youcompleteme_test.py

@@ -23,7 +23,7 @@ from __future__ import absolute_import
 from builtins import *  # noqa
 
 from ycm.tests.test_utils import ( ExtendedMock, MockVimBuffers, MockVimModule,
-                                   VimBuffer, VimMatch )
+                                   VimBuffer, VimMatch, VimSign )
 MockVimModule()
 
 import os
@@ -33,6 +33,7 @@ from hamcrest import ( assert_that, contains, empty, equal_to, is_in, is_not,
 from mock import call, MagicMock, patch
 
 from ycm.paths import _PathToPythonUsedDuringBuild
+from ycm.vimsupport import SIGN_BUFFER_ID_INITIAL_VALUE
 from ycm.youcompleteme import YouCompleteMe
 from ycm.tests import ( MakeUserOptions, StopServer, test_utils,
                         WaitUntilReady, YouCompleteMeInstance )
@@ -517,9 +518,8 @@ def YouCompleteMe_ShowDiagnostics_DiagnosticsFound_OpenLocationList_test(
 @patch( 'ycm.youcompleteme.YouCompleteMe.FiletypeCompleterExistsForFiletype',
         return_value = True )
 @patch( 'ycm.vimsupport.PostVimMessage', new_callable = ExtendedMock )
-@patch( 'vim.command', new_callable = ExtendedMock )
 def YouCompleteMe_UpdateDiagnosticInterface_PrioritizeErrorsOverWarnings_test(
-  ycm, vim_command, post_vim_message, *args ):
+  ycm, post_vim_message, *args ):
 
   contents = """int main() {
   int x, y;
@@ -592,6 +592,7 @@ def YouCompleteMe_UpdateDiagnosticInterface_PrioritizeErrorsOverWarnings_test(
                               window = 2 )
 
   test_utils.VIM_MATCHES = []
+  test_utils.VIM_SIGNS = []
 
   with MockVimBuffers( [ current_buffer ], current_buffer, ( 3, 1 ) ):
     with patch( 'ycm.client.event_notification.EventNotification.Response',
@@ -615,9 +616,12 @@ def YouCompleteMe_UpdateDiagnosticInterface_PrioritizeErrorsOverWarnings_test(
     )
 
     # Only the error sign is placed.
-    vim_command.assert_has_exact_calls( [
-      call( 'sign place 1 name=YcmError line=3 buffer=5' ),
-    ] )
+    assert_that(
+      test_utils.VIM_SIGNS,
+      contains(
+        VimSign( SIGN_BUFFER_ID_INITIAL_VALUE, 3, 'YcmError', 5 )
+      )
+    )
 
   # The error is not echoed again when moving the cursor along the line.
   with MockVimBuffers( [ current_buffer ], current_buffer, ( 3, 2 ) ):
@@ -639,7 +643,6 @@ def YouCompleteMe_UpdateDiagnosticInterface_PrioritizeErrorsOverWarnings_test(
       "expected ';' after expression (FixIt)",
       truncate = True, warning = False )
 
-    vim_command.reset_mock()
     with patch( 'ycm.client.event_notification.EventNotification.Response',
                 return_value = diagnostics[ 1 : ] ):
       ycm.OnFileReadyToParse()
@@ -653,10 +656,12 @@ def YouCompleteMe_UpdateDiagnosticInterface_PrioritizeErrorsOverWarnings_test(
       )
     )
 
-    vim_command.assert_has_exact_calls( [
-      call( 'sign place 2 name=YcmWarning line=3 buffer=5' ),
-      call( 'try | exec "sign unplace 1 buffer=5" | catch /E158/ | endtry' )
-    ] )
+    assert_that(
+      test_utils.VIM_SIGNS,
+      contains(
+        VimSign( SIGN_BUFFER_ID_INITIAL_VALUE + 1, 3, 'YcmWarning', 5 )
+      )
+    )
 
 
 @YouCompleteMeInstance( { 'echo_current_diagnostic': 1,

+ 40 - 14
python/ycm/vimsupport.py

@@ -27,7 +27,7 @@ import vim
 import os
 import json
 import re
-from collections import defaultdict
+from collections import defaultdict, namedtuple
 from ycmd.utils import ( ByteOffsetToCodepointOffset, GetCurrentDirectory,
                          JoinLinesAsUnicode, ToBytes, ToUnicode )
 from ycmd import user_options_store
@@ -45,6 +45,14 @@ FIXIT_OPENING_BUFFERS_MESSAGE_FORMAT = (
 
 NO_SELECTION_MADE_MSG = "No valid selection was made; aborting."
 
+# This is the starting value assigned to the sign's id of each buffer. This
+# value is then incremented for each new sign. This should prevent conflicts
+# with other plugins using signs.
+SIGN_BUFFER_ID_INITIAL_VALUE = 100000000
+
+SIGN_PLACE_REGEX = re.compile(
+  r"^.*=(?P<line>\d+).*=(?P<id>\d+).*=(?P<name>Ycm\w+)$" )
+
 
 def CurrentLineAndColumn():
   """Returns the 0-based current line and 0-based current column."""
@@ -163,23 +171,41 @@ def GetBufferChangedTick( bufnr ):
   return GetIntValue( 'getbufvar({0}, "changedtick")'.format( bufnr ) )
 
 
-def UnplaceSignInBuffer( buffer_number, sign_id ):
-  if buffer_number < 0:
-    return
-  vim.command(
-    'try | exec "sign unplace {0} buffer={1}" | catch /E158/ | endtry'.format(
-        sign_id, buffer_number ) )
+class DiagnosticSign( namedtuple( 'DiagnosticSign',
+                                  [ 'id', 'line', 'name', 'buffer_number' ] ) ):
+  # 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.name == other.name and
+             self.buffer_number == other.buffer_number )
+
+
+def GetSignsInBuffer( buffer_number ):
+  vim.command( 'redir => b:ycm_sign' )
+  vim.command( 'silent sign place buffer={}'.format( buffer_number ) )
+  vim.command( 'redir END' )
+  sign_output = vim.eval( 'b:ycm_sign' )
+  vim.command( 'unlet b:ycm_sign' )
+  signs = []
+  for line in sign_output.split( '\n' ):
+    match = SIGN_PLACE_REGEX.search( line )
+    if match:
+      signs.append( DiagnosticSign( int( match.group( 'id' ) ),
+                                    int( match.group( 'line' ) ),
+                                    match.group( 'name' ),
+                                    buffer_number ) )
+  return signs
+
 
+def UnplaceSign( sign ):
+  vim.command( 'sign unplace {0} buffer={1}'.format( sign.id,
+                                                     sign.buffer_number ) )
 
-def PlaceSign( sign_id, line_num, buffer_num, is_error = True ):
-  # libclang can give us diagnostics that point "outside" the file; Vim borks
-  # on these.
-  if line_num < 1:
-    line_num = 1
 
-  sign_name = 'YcmError' if is_error else 'YcmWarning'
+def PlaceSign( sign ):
   vim.command( 'sign place {0} name={1} line={2} buffer={3}'.format(
-    sign_id, sign_name, line_num, buffer_num ) )
+    sign.id, sign.name, sign.line, sign.buffer_number ) )
 
 
 def ClearYcmSyntaxMatches():