Auto merge of #1694 - puremourning:detailed-info-subcommands, r=Valloric

Allow subcommands to display information in the preview window

References
====

https://github.com/Valloric/YouCompleteMe/issues/1653

There will be some PRs coming to ycmd that will supply documentation in this format

Background
====

This adds support to YCM for displaying "detailed info" (e.g. quick-info, documentation, etc.) from a completer subcommand response. We display the info in the preview window.

Technical notes
====

Use of the preview window
----

We display the detailed info text in the preview window. Vim's preview window is designed to display actual files, not scratch data. Our approach is to open a temporary file, even though that file is never  written. This way, all of Vim's existing settings for the preview window (and people's configured mappings) just work. This is also consistent with showing the documentation in the preview
window during completion (although, Vim fixes the preview window height to 3 in that case).

Other plugins have more complicated functions for this (such as eclim), or Scratch.vim, but this approach is simple and doesn't require external dependencies or additional settings. 


 `wincmd P` etc.
---

The approach in `vimsupport.py` of jumping around with `wincmd P` and `wincmd p` was taken pretty much directly from vim's `:help preview-window` (specifically `:help CursorHold-example`), so while ugly it is apparently the 'recommended' way.

`vim` module mocking
----

I had to change the way we were mocking the `vim` module in the tests. From the commit message:

    This required fixing a sort-of-bug in which the mock'd Vim module was always
    only set once, and could not be changed outside of the module which created it.
    This meant that it wasn't easy to have arbitrary tests, because it was dependent
    on the order in which the tests execute as to whether the return from
    MockVimModule() was actually the one in use.

    The solution was to make the mock'd vim module a singleton, imported by any
    tests which require fiddling with it. This should hopefully help testing going
    forwards.

I committed this separately to help review (hopefully).

python/ycm/client/command_request.py

@@ -69,6 +69,8 @@ class CommandRequest( BaseRequest ):
       self._HandleFixitResponse()
     elif 'message' in self._response:
       self._HandleMessageResponse()
+    elif 'detailed_info' in self._response:
+      self._HandleDetailedInfoResponse()
 
 
   def _HandleGotoResponse( self ):
@@ -99,6 +101,10 @@ class CommandRequest( BaseRequest ):
     vimsupport.EchoText( self._response[ 'message' ] )
 
 
+  def _HandleDetailedInfoResponse( self ):
+    vimsupport.WriteToPreviewWindow( self._response[ 'detailed_info' ] )
+
+
 def SendCommandRequest( arguments, completer ):
   request = CommandRequest( arguments, completer )
   # This is a blocking call.

python/ycm/client/completion_request.py

@@ -21,10 +21,10 @@ from ycmd.utils import ToUtf8IfNeeded
 from ycm.client.base_request import ( BaseRequest, JsonFromFuture,
                                       HandleServerException,
                                       MakeServerException )
-import os
 
 TIMEOUT_SECONDS = 0.5
 
+
 class CompletionRequest( BaseRequest ):
   def __init__( self, request_data ):
     super( CompletionRequest, self ).__init__()
@@ -85,7 +85,7 @@ def ConvertCompletionDataToVimData( completion_data ):
   if 'detailed_info' in completion_data:
     vim_data[ 'info' ] = ToUtf8IfNeeded( completion_data[ 'detailed_info' ] )
     if doc_string:
-      vim_data[ 'info' ] += os.linesep + doc_string
+      vim_data[ 'info' ] += '\n' + doc_string
   elif doc_string:
     vim_data[ 'info' ] = doc_string
 

python/ycm/client/tests/completion_request_test.py

@@ -20,7 +20,6 @@
 from nose.tools import eq_
 from ycm.test_utils import MockVimModule
 vim_mock = MockVimModule()
-import os
 
 from .. import completion_request
 
@@ -57,7 +56,7 @@ class ConvertCompletionResponseToVimDatas_test:
       'abbr': 'MENU TEXT',
       'menu': 'EXTRA MENU INFO',
       'kind': 'k',
-      'info': 'DETAILED INFO' + os.linesep + 'DOC STRING',
+      'info': 'DETAILED INFO\nDOC STRING',
       'dup' : 1,
     } )
 

python/ycm/test_utils.py

@@ -20,17 +20,44 @@
 from mock import MagicMock
 import sys
 
+# 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,
+# and subsquent assignments to sys.modules[ 'vim' ] don't retrospectively update
+# them. The result is that while running the tests, we must assign only one
+# instance of MagicMock to sys.modules[ 'vim' ] and always return it.
+#
+# More explanation is available:
+# https://github.com/Valloric/YouCompleteMe/pull/1694
+VIM_MOCK = MagicMock()
+
 def MockVimModule():
   """The 'vim' module is something that is only present when running inside the
-  Vim Python interpreter, so we replace it with a MagicMock for tests. """
+  Vim Python interpreter, so we replace it with a MagicMock for tests. If you
+  need to add additional mocks to vim module functions, then use 'patch' from
+  mock module, to ensure that the state of the vim mock is returned before the
+  next test. That is:
+
+    from ycm.test_utils import MockVimModule
+    from mock import patch
+
+    # Do this once
+    MockVimModule()
+
+    @patch( 'vim.eval', return_value='test' )
+    @patch( 'vim.command', side_effect=ValueError )
+    def test( vim_command, vim_eval ):
+      # use vim.command via vim_command, e.g.:
+      vim_command.assert_has_calls( ... )
+
+  Failure to use this approach may lead to unexpected failures in other
+  tests."""
 
   def VimEval( value ):
     if value == "g:ycm_min_num_of_chars_for_completion":
       return 0
     return ''
 
-  vim_mock = MagicMock()
+  VIM_MOCK.eval = MagicMock( side_effect = VimEval )
-  vim_mock.eval = MagicMock( side_effect = VimEval )
+  sys.modules[ 'vim' ] = VIM_MOCK
-  sys.modules[ 'vim' ] = vim_mock
-  return vim_mock
 
+  return VIM_MOCK

python/ycm/tests/__init__.py

@@ -0,0 +1 @@
+

python/ycm/tests/syntax_parse_test.py

@@ -17,11 +17,12 @@
 # You should have received a copy of the GNU General Public License
 # along with YouCompleteMe.  If not, see <http://www.gnu.org/licenses/>.
 
+from ycm.test_utils import MockVimModule
+MockVimModule()
+
 import os
 from nose.tools import eq_
 from hamcrest import assert_that, has_items
-from ycm.test_utils import MockVimModule
-vim_mock = MockVimModule()
 from ycm import syntax_parse
 
 

python/ycm/tests/vimsupport_test.py

@@ -17,8 +17,12 @@
 # You should have received a copy of the GNU General Public License
 # along with YouCompleteMe.  If not, see <http://www.gnu.org/licenses/>.
 
+from ycm.test_utils import MockVimModule
+MockVimModule()
+
 from ycm import vimsupport
 from nose.tools import eq_
+from mock import MagicMock, call, patch
 
 
 def ReplaceChunk_SingleLine_Repl_1_test():
@@ -576,3 +580,88 @@ def _BuildChunk( start_line, start_column, end_line, end_column,
     },
     'replacement_text': replacement_text
   }
+
+
+def _Mock_tempname( arg ):
+  if arg == 'tempname()':
+    return '_TEMP_FILE_'
+
+  raise ValueError( 'Unexpected evaluation: ' + arg )
+
+
+@patch( 'vim.eval', side_effect=_Mock_tempname )
+@patch( 'vim.command' )
+@patch( 'vim.current' )
+def WriteToPreviewWindow_test( vim_current, vim_command, vim_eval ):
+  vim_current.window.options.__getitem__ = MagicMock( return_value = True )
+
+  vimsupport.WriteToPreviewWindow( "test" )
+
+  vim_command.assert_has_calls( [
+    call( 'silent! pclose!' ),
+    call( 'silent! pedit! _TEMP_FILE_' ),
+    call( 'silent! wincmd P' ),
+    call( 'silent! wincmd p' ) ] )
+
+  vim_current.buffer.__setitem__.assert_called_with(
+      slice( None, None, None ), [ 'test' ] )
+
+  vim_current.buffer.options.__setitem__.assert_has_calls( [
+    call( 'buftype', 'nofile' ),
+    call( 'swapfile', False ),
+    call( 'modifiable', False ),
+    call( 'modified', False ),
+    call( 'readonly', True ),
+  ], any_order = True )
+
+
+@patch( 'vim.eval', side_effect=_Mock_tempname )
+@patch( 'vim.current' )
+def WriteToPreviewWindow_MultiLine_test( vim_current, vim_eval ):
+  vim_current.window.options.__getitem__ = MagicMock( return_value = True )
+  vimsupport.WriteToPreviewWindow( "test\ntest2" )
+
+  vim_current.buffer.__setitem__.assert_called_with(
+      slice( None, None, None ), [ 'test', 'test2' ] )
+
+
+@patch( 'vim.eval', side_effect=_Mock_tempname )
+@patch( 'vim.command' )
+@patch( 'vim.current' )
+def WriteToPreviewWindow_JumpFail_test( vim_current, vim_command, vim_eval ):
+  vim_current.window.options.__getitem__ = MagicMock( return_value = False )
+
+  vimsupport.WriteToPreviewWindow( "test" )
+
+  vim_command.assert_has_calls( [
+    call( 'silent! pclose!' ),
+    call( 'silent! pedit! _TEMP_FILE_' ),
+    call( 'silent! wincmd P' ),
+    call( "echom 'test'" ),
+  ] )
+
+  vim_current.buffer.__setitem__.assert_not_called()
+  vim_current.buffer.options.__setitem__.assert_not_called()
+
+
+@patch( 'vim.eval', side_effect=_Mock_tempname )
+@patch( 'vim.command' )
+@patch( 'vim.current' )
+def WriteToPreviewWindow_JumpFail_MultiLine_test( vim_current,
+                                                  vim_command,
+                                                  vim_eval ):
+
+  vim_current.window.options.__getitem__ = MagicMock( return_value = False )
+
+  vimsupport.WriteToPreviewWindow( "test\ntest2" )
+
+  vim_command.assert_has_calls( [
+    call( 'silent! pclose!' ),
+    call( 'silent! pedit! _TEMP_FILE_' ),
+    call( 'silent! wincmd P' ),
+    call( "echom 'test'" ),
+    call( "echom 'test2'" ),
+  ] )
+
+  vim_current.buffer.__setitem__.assert_not_called()
+  vim_current.buffer.options.__setitem__.assert_not_called()

python/ycm/vimsupport.py

@@ -575,3 +575,65 @@ def SearchInCurrentBuffer( pattern ):
 
 def LineTextInCurrentBuffer( line ):
   return vim.current.buffer[ line ]
+
+
+def ClosePreviewWindow():
+  """ Close the preview window if it is present, otherwise do nothing """
+  vim.command( 'silent! pclose!' )
+
+
+def JumpToPreviewWindow():
+  """ Jump the vim cursor to the preview window, which must be active. Returns
+  boolean indicating if the cursor ended up in the preview window """
+  vim.command( 'silent! wincmd P' )
+  return vim.current.window.options[ 'previewwindow' ]
+
+
+def JumpToPreviousWindow():
+  """ Jump the vim cursor to its previous window position """
+  vim.command( 'silent! wincmd p' )
+
+
+def OpenFileInPreviewWindow( filename ):
+  """ Open the supplied filename in the preview window """
+  vim.command( 'silent! pedit! ' + filename )
+
+
+def WriteToPreviewWindow( message ):
+  """ Display the supplied message in the preview window """
+
+  # This isn't something that comes naturally to Vim. Vim only wants to show
+  # tags and/or actual files in the preview window, so we have to hack it a
+  # little bit. We generate a temporary file name and "open" that, then write
+  # the data to it. We make sure the buffer can't be edited or saved. Other
+  # approaches include simply opening a split, but we want to take advantage of
+  # the existing Vim options for preview window height, etc.
+
+  ClosePreviewWindow()
+
+  OpenFileInPreviewWindow( vim.eval( 'tempname()' ) )
+
+  if JumpToPreviewWindow():
+    # We actually got to the preview window. By default the preview window can't
+    # be changed, so we make it writable, write to it, then make it read only
+    # again.
+    vim.current.buffer.options[ 'modifiable' ] = True
+    vim.current.buffer.options[ 'readonly' ]   = False
+
+    vim.current.buffer[:] = message.splitlines()
+
+    vim.current.buffer.options[ 'buftype' ]    = 'nofile'
+    vim.current.buffer.options[ 'swapfile' ]   = False
+    vim.current.buffer.options[ 'modifiable' ] = False
+    vim.current.buffer.options[ 'readonly' ]   = True
+
+    # We need to prevent closing the window causing a warning about unsaved
+    # file, so we pretend to Vim that the buffer has not been changed.
+    vim.current.buffer.options[ 'modified' ]   = False
+
+    JumpToPreviousWindow()
+  else:
+    # 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 )