Auto merge of #2302 - micbou:completions-avoid-hit-enter, r=Valloric

[READY] Avoid hit-enter prompts during completions

Being interrupted by the `Press ENTER or type command to continue` prompt while writing code is annoying. This happens when an error occurs during completion and the error message displayed on the status line is longer than the window width. We prevent that by truncating the message.

Note that the message logged to the command-line history will also be truncated.

4 functions to display messages on the status line (`PostVimMessage`, `PostMultiLineNotice`, `EchoText`, and `EchoTextVimWidth`) is a little too much. Merge them into one.

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

python/ycm/client/base_request.py

@@ -184,14 +184,14 @@ def JsonFromFuture( future ):
   return None
 
 
-def HandleServerException( exception ):
+def HandleServerException( exception, truncate = False ):
   serialized_exception = str( exception )
 
   # We ignore the exception about the file already being parsed since it comes
   # up often and isn't something that's actionable by the user.
   if 'already being parsed' in serialized_exception:
     return
-  vimsupport.PostMultiLineNotice( serialized_exception )
+  vimsupport.PostVimMessage( serialized_exception, truncate = truncate )
 
 
 def _ToUtf8Json( data ):

python/ycm/client/command_request.py

@@ -103,7 +103,8 @@ class CommandRequest( BaseRequest ):
 
   def _HandleFixitResponse( self ):
     if not len( self._response[ 'fixits' ] ):
-      vimsupport.EchoText( "No fixits found for current line" )
+      vimsupport.PostVimMessage( 'No fixits found for current line',
+                                 warning = False )
     else:
       try:
         fixit_index = 0
@@ -119,15 +120,15 @@ class CommandRequest( BaseRequest ):
         vimsupport.ReplaceChunks(
           self._response[ 'fixits' ][ fixit_index ][ 'chunks' ] )
       except RuntimeError as e:
-        vimsupport.PostMultiLineNotice( str( e ) )
+        vimsupport.PostVimMessage( str( e ) )
 
 
   def _HandleBasicResponse( self ):
-    vimsupport.EchoText( self._response )
+    vimsupport.PostVimMessage( self._response, warning = False )
 
 
   def _HandleMessageResponse( self ):
-    vimsupport.EchoText( self._response[ 'message' ] )
+    vimsupport.PostVimMessage( self._response[ 'message' ], warning = False )
 
 
   def _HandleDetailedInfoResponse( self ):

python/ycm/client/completion_request.py

@@ -62,7 +62,7 @@ class CompletionRequest( BaseRequest ):
 
       return JsonFromFuture( self._response_future )[ 'completions' ]
     except ( ServerError, ReadTimeout ) as e:
-      HandleServerException( e )
+      HandleServerException( e, truncate = True )
     return []
 
 

python/ycm/client/tests/command_request_test.py

@@ -122,7 +122,7 @@ class Response_Detection_test( object ):
         request = CommandRequest( [ command ] )
         request._response = response
         request.RunPostCommandActionsIfNeeded()
-        vim_command.assert_called_with( "echom '{0}'".format( response ) )
+        vim_command.assert_called_with( "echo '{0}'".format( response ) )
 
     tests = [
       [ 'AnythingYouLike',        True ],
@@ -140,14 +140,15 @@ class Response_Detection_test( object ):
     # are no fixits available
     def EmptyFixItTest( command ):
       with patch( 'ycm.vimsupport.ReplaceChunks' ) as replace_chunks:
-        with patch( 'ycm.vimsupport.EchoText' ) as echo_text:
+        with patch( 'ycm.vimsupport.PostVimMessage' ) as post_vim_message:
           request = CommandRequest( [ command ] )
           request._response = {
             'fixits': []
           }
           request.RunPostCommandActionsIfNeeded()
 
-          echo_text.assert_called_with( 'No fixits found for current line' )
+          post_vim_message.assert_called_with(
+            'No fixits found for current line', warning = False )
           replace_chunks.assert_not_called()
 
     for test in [ 'FixIt', 'Refactor', 'GoToHell', 'any_old_garbade!!!21' ]:
@@ -158,7 +159,7 @@ class Response_Detection_test( object ):
     # Ensures we recognise and handle fixit responses with some dummy chunk data
     def FixItTest( command, response, chunks, selection ):
       with patch( 'ycm.vimsupport.ReplaceChunks' ) as replace_chunks:
-        with patch( 'ycm.vimsupport.EchoText' ) as echo_text:
+        with patch( 'ycm.vimsupport.PostVimMessage' ) as post_vim_message:
           with patch( 'ycm.vimsupport.SelectFromList',
                       return_value = selection ):
             request = CommandRequest( [ command ] )
@@ -166,7 +167,7 @@ class Response_Detection_test( object ):
             request.RunPostCommandActionsIfNeeded()
 
             replace_chunks.assert_called_with( chunks )
-            echo_text.assert_not_called()
+            post_vim_message.assert_not_called()
 
     basic_fixit = {
       'fixits': [ {
@@ -211,11 +212,11 @@ class Response_Detection_test( object ):
     # to the user
 
     def MessageTest( command, message ):
-      with patch( 'ycm.vimsupport.EchoText' ) as echo_text:
+      with patch( 'ycm.vimsupport.PostVimMessage' ) as post_vim_message:
         request = CommandRequest( [ command ] )
         request._response = { 'message': message }
         request.RunPostCommandActionsIfNeeded()
-        echo_text.assert_called_with( message )
+        post_vim_message.assert_called_with( message, warning = False )
 
     tests = [
       [ '___________', 'This is a message' ],

python/ycm/diagnostic_interface.py

@@ -87,7 +87,7 @@ class DiagnosticInterface( object ):
     if not diags:
       if self._diag_message_needs_clearing:
         # Clear any previous diag echo
-        vimsupport.EchoText( '', False )
+        vimsupport.PostVimMessage( '', warning = False )
         self._diag_message_needs_clearing = False
       return
 
@@ -95,7 +95,7 @@ class DiagnosticInterface( object ):
     if diags[ 0 ].get( 'fixit_available', False ):
       text += ' (FixIt)'
 
-    vimsupport.EchoTextVimWidth( text )
+    vimsupport.PostVimMessage( text, warning = False, truncate = True )
     self._diag_message_needs_clearing = True
 
 

python/ycm/tests/event_notification_test.py

@@ -37,22 +37,6 @@ from mock import call, MagicMock, patch
 from nose.tools import eq_, ok_
 
 
-def PostVimMessage_Call( message ):
-  """Return a mock.call object for a call to vimsupport.PostVimMesasge with the
-  supplied message"""
-  return call( 'redraw | echohl WarningMsg | echom \''
-               + message +
-               '\' | echohl None' )
-
-
-def PostMultiLineNotice_Call( message ):
-  """Return a mock.call object for a call to vimsupport.PostMultiLineNotice with
-  the supplied message"""
-  return call( 'redraw | echohl WarningMsg | echo \''
-               + message +
-               '\' | echohl None' )
-
-
 def PresentDialog_Confirm_Call( message ):
   """Return a mock.call object for a call to vimsupport.PresentDialog, as called
   why vimsupport.Confirm with the supplied confirmation message"""
@@ -154,8 +138,8 @@ def MockEventNotification( response_method, native_filetype_completer = True ):
 
 class EventNotification_test( Server_test ):
 
-  @patch( 'vim.command', new_callable = ExtendedMock )
+  @patch( 'ycm.vimsupport.PostVimMessage', new_callable = ExtendedMock )
-  def FileReadyToParse_NonDiagnostic_Error_test( self, vim_command ):
+  def FileReadyToParse_NonDiagnostic_Error_test( self, post_vim_message ):
     # This test validates the behaviour of YouCompleteMe.HandleFileParseRequest
     # in combination with YouCompleteMe.OnFileReadyToParse when the completer
     # raises an exception handling FileReadyToParse event notification
@@ -171,23 +155,23 @@ class EventNotification_test( Server_test ):
         self._server_state.HandleFileParseRequest()
 
         # The first call raises a warning
-        vim_command.assert_has_exact_calls( [
+        post_vim_message.assert_has_exact_calls( [
-          PostMultiLineNotice_Call( ERROR_TEXT ),
+          call( ERROR_TEXT, truncate = False )
         ] )
 
         # Subsequent calls don't re-raise the warning
         self._server_state.HandleFileParseRequest()
-        vim_command.assert_has_exact_calls( [
+        post_vim_message.assert_has_exact_calls( [
-          PostMultiLineNotice_Call( ERROR_TEXT ),
+          call( ERROR_TEXT, truncate = False )
         ] )
 
         # But it does if a subsequent event raises again
         self._server_state.OnFileReadyToParse()
         assert self._server_state.FileParseRequestReady()
         self._server_state.HandleFileParseRequest()
-        vim_command.assert_has_exact_calls( [
+        post_vim_message.assert_has_exact_calls( [
-          PostMultiLineNotice_Call( ERROR_TEXT ),
+          call( ERROR_TEXT, truncate = False ),
-          PostMultiLineNotice_Call( ERROR_TEXT ),
+          call( ERROR_TEXT, truncate = False )
         ] )
 
 

python/ycm/tests/vimsupport_test.py

@@ -742,12 +742,12 @@ class MockBuffer( object ):
         return_value = True,
         new_callable = ExtendedMock )
 @patch( 'ycm.vimsupport.OpenFilename' )
-@patch( 'ycm.vimsupport.EchoTextVimWidth', new_callable=ExtendedMock )
+@patch( 'ycm.vimsupport.PostVimMessage', new_callable = ExtendedMock )
 @patch( 'vim.eval', new_callable = ExtendedMock )
 @patch( 'vim.command', new_callable = ExtendedMock )
 def ReplaceChunks_SingleFile_Open_test( vim_command,
                                         vim_eval,
-                                        echo_text_vim_width,
+                                        post_vim_message,
                                         open_filename,
                                         buffer_is_visible,
                                         get_buffer_number_for_filename,
@@ -810,8 +810,8 @@ def ReplaceChunks_SingleFile_Open_test( vim_command,
 
   # And it is ReplaceChunks that prints the message showing the number of
   # changes
-  echo_text_vim_width.assert_has_exact_calls( [
+  post_vim_message.assert_has_exact_calls( [
-      call( 'Applied 1 changes' ),
+      call( 'Applied 1 changes', warning = False ),
   ] )
 
 
@@ -825,7 +825,7 @@ def ReplaceChunks_SingleFile_Open_test( vim_command,
         new_callable = ExtendedMock )
 @patch( 'ycm.vimsupport.OpenFilename',
         new_callable = ExtendedMock )
-@patch( 'ycm.vimsupport.EchoTextVimWidth', new_callable=ExtendedMock )
+@patch( 'ycm.vimsupport.PostVimMessage', new_callable = ExtendedMock )
 @patch( 'ycm.vimsupport.Confirm',
         return_value = True,
         new_callable = ExtendedMock )
@@ -834,7 +834,7 @@ def ReplaceChunks_SingleFile_Open_test( vim_command,
 def ReplaceChunks_SingleFile_NotOpen_test( vim_command,
                                            vim_eval,
                                            confirm,
-                                           echo_text_vim_width,
+                                           post_vim_message,
                                            open_filename,
                                            buffer_is_visible,
                                            get_buffer_number_for_filename,
@@ -916,8 +916,8 @@ def ReplaceChunks_SingleFile_NotOpen_test( vim_command,
 
   # And it is ReplaceChunks that prints the message showing the number of
   # changes
-  echo_text_vim_width.assert_has_exact_calls( [
+  post_vim_message.assert_has_exact_calls( [
-    call( 'Applied 1 changes' ),
+    call( 'Applied 1 changes', warning = False ),
   ] )
 
 
@@ -929,7 +929,7 @@ def ReplaceChunks_SingleFile_NotOpen_test( vim_command,
         new_callable = ExtendedMock )
 @patch( 'ycm.vimsupport.OpenFilename',
         new_callable = ExtendedMock )
-@patch( 'ycm.vimsupport.EchoTextVimWidth',
+@patch( 'ycm.vimsupport.PostVimMessage',
         new_callable = ExtendedMock )
 @patch( 'ycm.vimsupport.Confirm',
         return_value = False,
@@ -942,7 +942,7 @@ def ReplaceChunks_User_Declines_To_Open_File_test(
                                            vim_command,
                                            vim_eval,
                                            confirm,
-                                           echo_text_vim_width,
+                                           post_vim_message,
                                            open_filename,
                                            buffer_is_visible,
                                            get_buffer_number_for_filename ):
@@ -993,7 +993,7 @@ def ReplaceChunks_User_Declines_To_Open_File_test(
   open_filename.assert_not_called()
   vim_eval.assert_not_called()
   vim_command.assert_not_called()
-  echo_text_vim_width.assert_not_called()
+  post_vim_message.assert_not_called()
 
 
 @patch( 'ycm.vimsupport.GetBufferNumberForFilename',
@@ -1005,7 +1005,7 @@ def ReplaceChunks_User_Declines_To_Open_File_test(
         new_callable = ExtendedMock )
 @patch( 'ycm.vimsupport.OpenFilename',
         new_callable = ExtendedMock )
-@patch( 'ycm.vimsupport.EchoTextVimWidth',
+@patch( 'ycm.vimsupport.PostVimMessage',
         new_callable = ExtendedMock )
 @patch( 'ycm.vimsupport.Confirm',
         return_value = True,
@@ -1019,7 +1019,7 @@ def ReplaceChunks_User_Aborts_Opening_File_test(
                                            vim_command,
                                            vim_eval,
                                            confirm,
-                                           echo_text_vim_width,
+                                           post_vim_message,
                                            open_filename,
                                            buffer_is_visible,
                                            get_buffer_number_for_filename ):
@@ -1066,7 +1066,7 @@ def ReplaceChunks_User_Aborts_Opening_File_test(
   vim_eval.assert_called_with( "&previewheight" )
 
   # But raised an exception before issuing the message at the end
-  echo_text_vim_width.assert_not_called()
+  post_vim_message.assert_not_called()
 
 
 @patch( 'ycm.vimsupport.VariableExists', return_value = False )
@@ -1089,7 +1089,7 @@ def ReplaceChunks_User_Aborts_Opening_File_test(
         new_callable = ExtendedMock)
 @patch( 'ycm.vimsupport.OpenFilename',
         new_callable = ExtendedMock)
-@patch( 'ycm.vimsupport.EchoTextVimWidth',
+@patch( 'ycm.vimsupport.PostVimMessage',
         new_callable = ExtendedMock)
 @patch( 'ycm.vimsupport.Confirm', return_value = True,
         new_callable = ExtendedMock)
@@ -1100,7 +1100,7 @@ def ReplaceChunks_User_Aborts_Opening_File_test(
 def ReplaceChunks_MultiFile_Open_test( vim_command,
                                        vim_eval,
                                        confirm,
-                                       echo_text_vim_width,
+                                       post_vim_message,
                                        open_filename,
                                        buffer_is_visible,
                                        get_buffer_number_for_filename,
@@ -1193,8 +1193,8 @@ def ReplaceChunks_MultiFile_Open_test( vim_command,
 
   # And it is ReplaceChunks that prints the message showing the number of
   # changes
-  echo_text_vim_width.assert_has_exact_calls( [
+  post_vim_message.assert_has_exact_calls( [
-    call( 'Applied 2 changes' ),
+    call( 'Applied 2 changes', warning = False ),
   ] )
 
 
@@ -1293,7 +1293,8 @@ def WriteToPreviewWindow_JumpFail_test( vim_current, vim_command ):
     call( 'silent! pclose!' ),
     call( 'silent! pedit! _TEMP_FILE_' ),
     call( 'silent! wincmd P' ),
-    call( "echom 'test'" ),
+    call( 'redraw' ),
+    call( "echo 'test'" ),
   ] )
 
   vim_current.buffer.__setitem__.assert_not_called()
@@ -1312,8 +1313,9 @@ def WriteToPreviewWindow_JumpFail_MultiLine_test( vim_current, vim_command ):
     call( 'silent! pclose!' ),
     call( 'silent! pedit! _TEMP_FILE_' ),
     call( 'silent! wincmd P' ),
-    call( "echom 'test'" ),
+    call( 'redraw' ),
-    call( "echom 'test2'" ),
+    call( "echo 'test'" ),
+    call( "echo 'test2'" ),
   ] )
 
   vim_current.buffer.__setitem__.assert_not_called()

python/ycm/vimsupport.py

@@ -436,22 +436,47 @@ def NumLinesInBuffer( buffer_object ):
 # Calling this function from the non-GUI thread will sometimes crash Vim. At
 # the time of writing, YCM only uses the GUI thread inside Vim (this used to
 # not be the case).
-# We redraw the screen before displaying the message to avoid the "Press ENTER
+def PostVimMessage( message, warning = True, truncate = False ):
-# or type command to continue" prompt when editing a new C-family file.
+  """Display a message on the Vim status line. By default, the message is
-def PostVimMessage( message ):
+  highlighted and logged to Vim command-line history (see :h history).
-  vim.command( "redraw | echohl WarningMsg | echom '{0}' | echohl None"
+  Unset the |warning| parameter to disable this behavior. Set the |truncate|
-               .format( EscapeForVim( ToUnicode( message ) ) ) )
+  parameter to avoid hit-enter prompts (see :h hit-enter) when the message is
+  longer than the window width."""
+  echo_command = 'echom' if warning else 'echo'
 
+  # Displaying a new message while previous ones are still on the status line
+  # might lead to a hit-enter prompt or the message appearing without a
+  # newline so we do a redraw first.
+  vim.command( 'redraw' )
 
-# Unlike PostVimMesasge, this supports messages with newlines in them because it
+  if warning:
-# uses 'echo' instead of 'echomsg'. This also means that the message will NOT
+    vim.command( 'echohl WarningMsg' )
-# appear in Vim's message log.
+
-# Similarly to PostVimMesasge, we do a redraw first to clear any previous
+  message = ToUnicode( message )
-# messages, which might lead to this message appearing without a newline and/or
+
-# requring the "Press ENTER or type command to continue".
+  if truncate:
-def PostMultiLineNotice( message ):
+    vim_width = GetIntValue( '&columns' )
-  vim.command( "redraw | echohl WarningMsg | echo '{0}' | echohl None"
+
-               .format( EscapeForVim( ToUnicode( message ) ) ) )
+    message = message.replace( '\n', ' ' )
+    if len( message ) > vim_width:
+      message = message[ : vim_width - 4 ] + '...'
+
+    old_ruler = GetIntValue( '&ruler' )
+    old_showcmd = GetIntValue( '&showcmd' )
+    vim.command( 'set noruler noshowcmd' )
+
+    vim.command( "{0} '{1}'".format( echo_command,
+                                     EscapeForVim( message ) ) )
+
+    SetVariableValue( '&ruler', old_ruler )
+    SetVariableValue( '&showcmd', old_showcmd )
+  else:
+    for line in message.split( '\n' ):
+      vim.command( "{0} '{1}'".format( echo_command,
+                                       EscapeForVim( line ) ) )
+
+  if warning:
+    vim.command( 'echohl None' )
 
 
 def PresentDialog( message, choices, default_choice_index = 0 ):
@@ -542,33 +567,6 @@ def SelectFromList( prompt, items ):
   return selected
 
 
-def EchoText( text, log_as_message = True ):
-  def EchoLine( text ):
-    command = 'echom' if log_as_message else 'echo'
-    vim.command( "{0} '{1}'".format( command,
-                                     EscapeForVim( text ) ) )
-
-  for line in ToUnicode( text ).split( '\n' ):
-    EchoLine( line )
-
-
-# Echos text but truncates it so that it all fits on one line
-def EchoTextVimWidth( text ):
-  vim_width = GetIntValue( '&columns' )
-  truncated_text = ToUnicode( text )[ : int( vim_width * 0.9 ) ]
-  truncated_text.replace( '\n', ' ' )
-
-  old_ruler = GetIntValue( '&ruler' )
-  old_showcmd = GetIntValue( '&showcmd' )
-  vim.command( 'set noruler noshowcmd' )
-  vim.command( 'redraw' )
-
-  EchoText( truncated_text, False )
-
-  SetVariableValue( '&ruler', old_ruler )
-  SetVariableValue( '&showcmd', old_showcmd )
-
-
 def EscapeForVim( text ):
   return ToUnicode( text.replace( "'", "''" ) )
 
@@ -736,7 +734,8 @@ def ReplaceChunks( chunks ):
   if locations:
     SetQuickFixList( locations )
 
-  EchoTextVimWidth( "Applied " + str( len( chunks ) ) + " changes" )
+  PostVimMessage( 'Applied {0} changes'.format( len( chunks ) ),
+                  warning = False )
 
 
 def ReplaceChunksInBuffer( chunks, vim_buffer, locations ):
@@ -851,7 +850,7 @@ def InsertNamespace( namespace ):
   new_line = "{0}using {1};\n\n".format( existing_indent, namespace )
   replace_pos = { 'line_num': line + 1, 'column_num': 1 }
   ReplaceChunk( replace_pos, replace_pos, new_line, 0, 0 )
-  PostVimMessage( "Add namespace: {0}".format( namespace ) )
+  PostVimMessage( 'Add namespace: {0}'.format( namespace ), warning = False )
 
 
 def SearchInCurrentBuffer( pattern ):
@@ -926,7 +925,7 @@ def WriteToPreviewWindow( message ):
     # We couldn't get to the preview window, but we still want to give the user
     # the information we have. The only remaining option is to echo to the
     # status area.
-    EchoText( message )
+    PostVimMessage( message, warning = False )
 
 
 def CheckFilename( filename ):

python/ycm/youcompleteme.py

@@ -578,7 +578,8 @@ class YouCompleteMe( object ):
       debug_info = BaseRequest.PostDataToHandler( BuildRequestData(),
                                                   'detailed_diagnostic' )
       if 'message' in debug_info:
-        vimsupport.EchoText( debug_info[ 'message' ] )
+        vimsupport.PostVimMessage( debug_info[ 'message' ],
+                                   warning = False )
     except ServerError as e:
       vimsupport.PostVimMessage( str( e ) )