Auto merge of #2988 - micbou:extra-conf-resend-request, r=micbou

[READY] Resend request when extra conf is loaded or ignored

When the client sends a request to the server, if an extra conf file is found that is not already white/blacklisted, the server stops processing the request and tells the client that an unknown extra conf file has been found. The client then asks the user if that file should be loaded or not. Depending on the
user's answer, the client sends a request to the server to load or ignore the extra conf file. Finally, the server loads the file or adds it to the blacklist. However, the initial request was not processed by the server and should be sent again.

Here's a demo illustrating the current situation:

![extra-conf-request-not-resent](https://user-images.githubusercontent.com/10026824/38840090-2d700936-41de-11e8-8b76-87c3eab960cc.gif)

As you can see, the file is not parsed after loading the extra conf file. With the proposed changes:

![extra-conf-request-resent](https://user-images.githubusercontent.com/10026824/38840137-67126670-41de-11e8-8707-88441ab35ac3.gif)

Fixes #2962.

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

autoload/youcompleteme.vim

@@ -565,6 +565,9 @@ function! s:PollFileParseResponse( ... )
   endif
 
   exec s:python_command "ycm_state.HandleFileParseRequest()"
+  if s:Pyeval( "ycm_state.ShouldResendFileParseRequest()" )
+    call s:OnFileReadyToParse( 1 )
+  endif
 endfunction
 
 

python/ycm/buffer.py

@@ -66,6 +66,10 @@ class Buffer( object ):
     return self._parse_tick != self._ChangedTick()
 
 
+  def ShouldResendParseRequest( self ):
+    return self._parse_request.ShouldResend()
+
+
   def UpdateDiagnostics( self, force=False ):
     if force or not self._async_diags:
       self.UpdateWithNewDiagnostics( self._parse_request.Response() )

python/ycm/client/base_request.py

@@ -43,7 +43,7 @@ _logger = logging.getLogger( __name__ )
 class BaseRequest( object ):
 
   def __init__( self ):
-    pass
+    self._should_resend = False
 
 
   def Start( self ):
@@ -58,15 +58,22 @@ class BaseRequest( object ):
     return {}
 
 
-  @staticmethod
+  def ShouldResend( self ):
-  def HandleFuture( future, display_message = True, truncate_message = False ):
+    return self._should_resend
+
+
+  def HandleFuture( self,
+                    future,
+                    display_message = True,
+                    truncate_message = False ):
     """Get the server response from a |future| object and catch any exception
     while doing so. If an exception is raised because of a unknown
     .ycm_extra_conf.py file, load the file or ignore it after asking the user.
-    For other exceptions, log the exception and display its message to the user
+    An identical request should be sent again to the server. For other
-    on the Vim status line. Unset the |display_message| parameter to hide the
+    exceptions, log the exception and display its message to the user on the Vim
-    message from the user. Set the |truncate_message| parameter to avoid
+    status line. Unset the |display_message| parameter to hide the message from
-    hit-enter prompts from this message."""
+    the user. Set the |truncate_message| parameter to avoid hit-enter prompts
+    from this message."""
     try:
       try:
         return _JsonFromFuture( future )
@@ -75,6 +82,7 @@ class BaseRequest( object ):
           _LoadExtraConfFile( e.extra_conf_file )
         else:
           _IgnoreExtraConfFile( e.extra_conf_file )
+        self._should_resend = True
     except BaseRequest.Requests().exceptions.ConnectionError:
       # We don't display this exception to the user since it is likely to happen
       # for each subsequent request (typically if the server crashed) and we
@@ -93,12 +101,12 @@ class BaseRequest( object ):
   # up; see Requests docs for details (we just pass the param along).
   # See the HandleFuture method for the |display_message| and |truncate_message|
   # parameters.
-  @staticmethod
+  def GetDataFromHandler( self,
-  def GetDataFromHandler( handler,
+                          handler,
                           timeout = _READ_TIMEOUT_SEC,
                           display_message = True,
                           truncate_message = False ):
-    return BaseRequest.HandleFuture(
+    return self.HandleFuture(
         BaseRequest._TalkToHandlerAsync( '', handler, 'GET', timeout ),
         display_message,
         truncate_message )
@@ -109,13 +117,13 @@ class BaseRequest( object ):
   # up; see Requests docs for details (we just pass the param along).
   # See the HandleFuture method for the |display_message| and |truncate_message|
   # parameters.
-  @staticmethod
+  def PostDataToHandler( self,
-  def PostDataToHandler( data,
+                         data,
                          handler,
                          timeout = _READ_TIMEOUT_SEC,
                          display_message = True,
                          truncate_message = False ):
-    return BaseRequest.HandleFuture(
+    return self.HandleFuture(
         BaseRequest.PostDataToHandlerAsync( data, handler, timeout ),
         display_message,
         truncate_message )
@@ -243,12 +251,12 @@ def _JsonFromFuture( future ):
 
 
 def _LoadExtraConfFile( filepath ):
-  BaseRequest.PostDataToHandler( { 'filepath': filepath },
+  BaseRequest().PostDataToHandler( { 'filepath': filepath },
                                    'load_extra_conf_file' )
 
 
 def _IgnoreExtraConfFile( filepath ):
-  BaseRequest.PostDataToHandler( { 'filepath': filepath },
+  BaseRequest().PostDataToHandler( { 'filepath': filepath },
                                    'ignore_extra_conf_file' )
 
 

python/ycm/client/ycmd_keepalive.py

@@ -45,4 +45,4 @@ class YcmdKeepalive( object ):
     while True:
       time.sleep( self._ping_interval_seconds )
 
-      BaseRequest.GetDataFromHandler( 'healthy', display_message = False )
+      BaseRequest().GetDataFromHandler( 'healthy', display_message = False )

python/ycm/omni_completer.py

@@ -128,6 +128,6 @@ class OmniCompleter( Completer ):
       'query': query
     }
 
-    response = BaseRequest.PostDataToHandler( request_data,
+    response = BaseRequest().PostDataToHandler( request_data,
                                                 'filter_and_sort_candidates' )
     return response if response is not None else []

python/ycm/tests/__init__.py

@@ -64,7 +64,7 @@ def MakeUserOptions( custom_options = {} ):
 
 
 def _IsReady():
-  return BaseRequest.GetDataFromHandler( 'ready' )
+  return BaseRequest().GetDataFromHandler( 'ready' )
 
 
 def WaitUntilReady( timeout = 5 ):

python/ycm/tests/event_notification_test.py

@@ -125,6 +125,8 @@ def EventNotification_FileReadyToParse_NonDiagnostic_Error_test(
         call( ERROR_TEXT, truncate = True )
       ] )
 
+      ok_( not ycm.ShouldResendFileParseRequest() )
+
       # But it does if a subsequent event raises again
       ycm.OnFileReadyToParse()
       ok_( ycm.FileParseRequestReady() )
@@ -134,6 +136,8 @@ def EventNotification_FileReadyToParse_NonDiagnostic_Error_test(
         call( ERROR_TEXT, truncate = True )
       ] )
 
+      ok_( not ycm.ShouldResendFileParseRequest() )
+
 
 @YouCompleteMeInstance()
 def EventNotification_FileReadyToParse_NonDiagnostic_Error_NonNative_test(
@@ -154,20 +158,16 @@ def EventNotification_FileReadyToParse_NonDiagnostic_Error_NonNative_test(
         test_utils.VIM_SIGNS,
         contains()
       )
+      ok_( not ycm.ShouldResendFileParseRequest() )
 
 
-@patch( 'ycm.client.base_request._LoadExtraConfFile',
-        new_callable = ExtendedMock )
-@patch( 'ycm.client.base_request._IgnoreExtraConfFile',
-        new_callable = ExtendedMock )
 @YouCompleteMeInstance()
 def EventNotification_FileReadyToParse_NonDiagnostic_ConfirmExtraConf_test(
-    ycm, ignore_extra_conf, load_extra_conf ):
+    ycm ):
 
   # This test validates the behaviour of YouCompleteMe.HandleFileParseRequest
   # in combination with YouCompleteMe.OnFileReadyToParse when the completer
   # raises the (special) UnknownExtraConf exception
-
   FILE_NAME = 'a_file'
   MESSAGE = ( 'Found ' + FILE_NAME + '. Load? \n\n(Question can be '
               'turned off with options, see YCM docs)' )
@@ -175,6 +175,8 @@ def EventNotification_FileReadyToParse_NonDiagnostic_ConfirmExtraConf_test(
   def UnknownExtraConfResponse( *args ):
     raise UnknownExtraConf( FILE_NAME )
 
+  with patch( 'ycm.client.base_request.BaseRequest.PostDataToHandler',
+              new_callable = ExtendedMock ) as post_data_to_handler:
     with MockArbitraryBuffer( 'javascript' ):
       with MockEventNotification( UnknownExtraConfResponse ):
 
@@ -189,8 +191,8 @@ def EventNotification_FileReadyToParse_NonDiagnostic_ConfirmExtraConf_test(
           present_dialog.assert_has_exact_calls( [
             PresentDialog_Confirm_Call( MESSAGE ),
           ] )
-        load_extra_conf.assert_has_exact_calls( [
+          post_data_to_handler.assert_has_exact_calls( [
-          call( FILE_NAME ),
+            call( { 'filepath': FILE_NAME }, 'load_extra_conf_file' )
           ] )
 
           # Subsequent calls don't re-raise the warning
@@ -199,10 +201,12 @@ def EventNotification_FileReadyToParse_NonDiagnostic_ConfirmExtraConf_test(
           present_dialog.assert_has_exact_calls( [
             PresentDialog_Confirm_Call( MESSAGE )
           ] )
-        load_extra_conf.assert_has_exact_calls( [
+          post_data_to_handler.assert_has_exact_calls( [
-          call( FILE_NAME ),
+            call( { 'filepath': FILE_NAME }, 'load_extra_conf_file' )
           ] )
 
+          ok_( ycm.ShouldResendFileParseRequest() )
+
           # But it does if a subsequent event raises again
           ycm.OnFileReadyToParse()
           ok_( ycm.FileParseRequestReady() )
@@ -212,11 +216,15 @@ def EventNotification_FileReadyToParse_NonDiagnostic_ConfirmExtraConf_test(
             PresentDialog_Confirm_Call( MESSAGE ),
             PresentDialog_Confirm_Call( MESSAGE ),
           ] )
-        load_extra_conf.assert_has_exact_calls( [
+          post_data_to_handler.assert_has_exact_calls( [
-          call( FILE_NAME ),
+            call( { 'filepath': FILE_NAME }, 'load_extra_conf_file' ),
-          call( FILE_NAME ),
+            call( { 'filepath': FILE_NAME }, 'load_extra_conf_file' )
           ] )
 
+          ok_( ycm.ShouldResendFileParseRequest() )
+
+        post_data_to_handler.reset_mock()
+
         # When the user rejects the extra conf, we reject it
         with patch( 'ycm.vimsupport.PresentDialog',
                     return_value = 1,
@@ -228,8 +236,8 @@ def EventNotification_FileReadyToParse_NonDiagnostic_ConfirmExtraConf_test(
           present_dialog.assert_has_exact_calls( [
             PresentDialog_Confirm_Call( MESSAGE ),
           ] )
-        ignore_extra_conf.assert_has_exact_calls( [
+          post_data_to_handler.assert_has_exact_calls( [
-          call( FILE_NAME ),
+            call( { 'filepath': FILE_NAME }, 'ignore_extra_conf_file' )
           ] )
 
           # Subsequent calls don't re-raise the warning
@@ -238,10 +246,12 @@ def EventNotification_FileReadyToParse_NonDiagnostic_ConfirmExtraConf_test(
           present_dialog.assert_has_exact_calls( [
             PresentDialog_Confirm_Call( MESSAGE )
           ] )
-        ignore_extra_conf.assert_has_exact_calls( [
+          post_data_to_handler.assert_has_exact_calls( [
-          call( FILE_NAME ),
+            call( { 'filepath': FILE_NAME }, 'ignore_extra_conf_file' )
           ] )
 
+          ok_( ycm.ShouldResendFileParseRequest() )
+
           # But it does if a subsequent event raises again
           ycm.OnFileReadyToParse()
           ok_( ycm.FileParseRequestReady() )
@@ -251,11 +261,13 @@ def EventNotification_FileReadyToParse_NonDiagnostic_ConfirmExtraConf_test(
             PresentDialog_Confirm_Call( MESSAGE ),
             PresentDialog_Confirm_Call( MESSAGE ),
           ] )
-        ignore_extra_conf.assert_has_exact_calls( [
+          post_data_to_handler.assert_has_exact_calls( [
-          call( FILE_NAME ),
+            call( { 'filepath': FILE_NAME }, 'ignore_extra_conf_file' ),
-          call( FILE_NAME ),
+            call( { 'filepath': FILE_NAME }, 'ignore_extra_conf_file' )
           ] )
 
+          ok_( ycm.ShouldResendFileParseRequest() )
+
 
 @YouCompleteMeInstance()
 def EventNotification_FileReadyToParse_Diagnostic_Error_Native_test( ycm ):
@@ -302,6 +314,8 @@ def _Check_FileReadyToParse_Diagnostic_Error( ycm ):
       eq_( ycm.GetErrorCount(), 1 )
       eq_( ycm.GetWarningCount(), 0 )
 
+      ok_( not ycm.ShouldResendFileParseRequest() )
+
       # New identical requests should result in the same diagnostics.
       ycm.OnFileReadyToParse()
       ok_( ycm.FileParseRequestReady() )
@@ -315,6 +329,8 @@ def _Check_FileReadyToParse_Diagnostic_Error( ycm ):
       eq_( ycm.GetErrorCount(), 1 )
       eq_( ycm.GetWarningCount(), 0 )
 
+      ok_( not ycm.ShouldResendFileParseRequest() )
+
 
 def _Check_FileReadyToParse_Diagnostic_Warning( ycm ):
   # Tests Vim sign placement/unplacement and error/warning count python API
@@ -353,6 +369,8 @@ def _Check_FileReadyToParse_Diagnostic_Warning( ycm ):
       eq_( ycm.GetErrorCount(), 0 )
       eq_( ycm.GetWarningCount(), 1 )
 
+      ok_( not ycm.ShouldResendFileParseRequest() )
+
 
 def _Check_FileReadyToParse_Diagnostic_Clean( ycm ):
   # Tests Vim sign unplacement and error/warning count python API
@@ -368,6 +386,7 @@ def _Check_FileReadyToParse_Diagnostic_Clean( ycm ):
       )
       eq_( ycm.GetErrorCount(), 0 )
       eq_( ycm.GetWarningCount(), 0 )
+      ok_( not ycm.ShouldResendFileParseRequest() )
 
 
 @patch( 'ycm.youcompleteme.YouCompleteMe._AddUltiSnipsDataIfNeeded' )

python/ycm/youcompleteme.py

@@ -228,7 +228,7 @@ class YouCompleteMe( object ):
 
   def CheckIfServerIsReady( self ):
     if not self._server_is_ready_with_cache and self.IsServerAlive():
-      self._server_is_ready_with_cache = BaseRequest.GetDataFromHandler(
+      self._server_is_ready_with_cache = BaseRequest().GetDataFromHandler(
           'ready', display_message = False )
     return self._server_is_ready_with_cache
 
@@ -329,7 +329,7 @@ class YouCompleteMe( object ):
 
 
   def GetDefinedSubcommands( self ):
-    subcommands = BaseRequest.PostDataToHandler( BuildRequestData(),
+    subcommands = BaseRequest().PostDataToHandler( BuildRequestData(),
                                                    'defined_subcommands' )
     return subcommands if subcommands else []
 
@@ -555,6 +555,10 @@ class YouCompleteMe( object ):
       current_buffer.MarkResponseHandled()
 
 
+  def ShouldResendFileParseRequest( self ):
+    return self.CurrentBuffer().ShouldResendParseRequest()
+
+
   def DebugInfo( self ):
     debug_info = ''
     if self._client_logfile:
@@ -644,7 +648,7 @@ class YouCompleteMe( object ):
 
 
   def ShowDetailedDiagnostic( self ):
-    detailed_diagnostic = BaseRequest.PostDataToHandler(
+    detailed_diagnostic = BaseRequest().PostDataToHandler(
         BuildRequestData(), 'detailed_diagnostic' )
 
     if 'message' in detailed_diagnostic: