diff --git a/README.md b/README.md index fb8a9e7a..660edd7b 100644 --- a/README.md +++ b/README.md @@ -1359,8 +1359,11 @@ undone, and never saves or writes files to the disk. #### The `FixIt` subcommand -Where available, attempts to make changes to the buffer to correct the -diagnostic closest to the cursor position. +Where available, attempts to make changes to the buffer to correct diagnostics +on the current line. Where multiple suggestions are available (such as when +there are multiple ways to resolve a given warning, or where multiple +diagnostics are reported for the current line), the options are presented +and one can be selected. Completers which provide diagnostics may also provide trivial modifications to the source in order to correct the diagnostic. Examples include syntax errors diff --git a/doc/youcompleteme.txt b/doc/youcompleteme.txt index 4a7bfd7c..3cd1f4d5 100644 --- a/doc/youcompleteme.txt +++ b/doc/youcompleteme.txt @@ -1637,8 +1637,11 @@ undone, and never saves or writes files to the disk. ------------------------------------------------------------------------------- The *FixIt* subcommand -Where available, attempts to make changes to the buffer to correct the -diagnostic closest to the cursor position. +Where available, attempts to make changes to the buffer to correct diagnostics +on the current line. Where multiple suggestions are available (such as when +there are multiple ways to resolve a given warning, or where multiple +diagnostics are reported for the current line), the options are presented and +one can be selected. Completers which provide diagnostics may also provide trivial modifications to the source in order to correct the diagnostic. Examples include syntax errors diff --git a/python/ycm/client/command_request.py b/python/ycm/client/command_request.py index 6c5bc668..3e00c5a8 100644 --- a/python/ycm/client/command_request.py +++ b/python/ycm/client/command_request.py @@ -105,9 +105,19 @@ class CommandRequest( BaseRequest ): if not len( self._response[ 'fixits' ] ): vimsupport.EchoText( "No fixits found for current line" ) else: - chunks = self._response[ 'fixits' ][ 0 ][ 'chunks' ] try: - vimsupport.ReplaceChunks( chunks ) + fixit_index = 0 + + # When there are multiple fixit suggestions, present them as a list to + # the user hand have her choose which one to apply. + if len( self._response[ 'fixits' ] ) > 1: + fixit_index = vimsupport.SelectFromList( + "Multiple FixIt suggestions are available at this location. " + "Which one would you like to apply?", + [ fixit[ 'text' ] for fixit in self._response[ 'fixits' ] ] ) + + vimsupport.ReplaceChunks( + self._response[ 'fixits' ][ fixit_index ][ 'chunks' ] ) except RuntimeError as e: vimsupport.PostMultiLineNotice( str( e ) ) diff --git a/python/ycm/client/tests/command_request_test.py b/python/ycm/client/tests/command_request_test.py index dfc55e0f..c6ba97c7 100644 --- a/python/ycm/client/tests/command_request_test.py +++ b/python/ycm/client/tests/command_request_test.py @@ -156,15 +156,17 @@ class Response_Detection_test( object ): def FixIt_Response_test( self ): # Ensures we recognise and handle fixit responses with some dummy chunk data - def FixItTest( command, response, chunks ): + def FixItTest( command, response, chunks, selection ): with patch( 'ycm.vimsupport.ReplaceChunks' ) as replace_chunks: with patch( 'ycm.vimsupport.EchoText' ) as echo_text: - request = CommandRequest( [ command ] ) - request._response = response - request.RunPostCommandActionsIfNeeded() + with patch( 'ycm.vimsupport.SelectFromList', + return_value = selection ): + request = CommandRequest( [ command ] ) + request._response = response + request.RunPostCommandActionsIfNeeded() - replace_chunks.assert_called_with( chunks ) - echo_text.assert_not_called() + replace_chunks.assert_called_with( chunks ) + echo_text.assert_not_called() basic_fixit = { 'fixits': [ { @@ -177,25 +179,31 @@ class Response_Detection_test( object ): multi_fixit = { 'fixits': [ { + 'text': 'first', 'chunks': [ { 'dummy chunk contents': True } ] }, { - 'additional fixits are ignored currently': True + 'text': 'second', + 'chunks': [ { + 'dummy chunk contents': False + }] } ] } multi_fixit_first_chunks = multi_fixit[ 'fixits' ][ 0 ][ 'chunks' ] + multi_fixit_second_chunks = multi_fixit[ 'fixits' ][ 1 ][ 'chunks' ] tests = [ - [ 'AnythingYouLike', basic_fixit, basic_fixit_chunks ], - [ 'GoToEvenWorks', basic_fixit, basic_fixit_chunks ], - [ 'FixItWorks', basic_fixit, basic_fixit_chunks ], - [ 'and8434fd andy garbag!', basic_fixit, basic_fixit_chunks ], - [ 'additional fixits ignored', multi_fixit, multi_fixit_first_chunks ], + [ 'AnythingYouLike', basic_fixit, basic_fixit_chunks, 0 ], + [ 'GoToEvenWorks', basic_fixit, basic_fixit_chunks, 0 ], + [ 'FixItWorks', basic_fixit, basic_fixit_chunks, 0 ], + [ 'and8434fd andy garbag!', basic_fixit, basic_fixit_chunks, 0 ], + [ 'select from multiple 1', multi_fixit, multi_fixit_first_chunks, 0 ], + [ 'select from multiple 2', multi_fixit, multi_fixit_second_chunks, 1 ], ] for test in tests: - yield FixItTest, test[ 0 ], test[ 1 ], test[ 2 ] + yield FixItTest, test[ 0 ], test[ 1 ], test[ 2 ], test[ 3 ] def Message_Response_test( self ): diff --git a/python/ycm/tests/event_notification_test.py b/python/ycm/tests/event_notification_test.py index c065a1e9..9f2cdf3a 100644 --- a/python/ycm/tests/event_notification_test.py +++ b/python/ycm/tests/event_notification_test.py @@ -49,7 +49,7 @@ def PostVimMessage_Call( message ): def PostMultiLineNotice_Call( message ): """Return a mock.call object for a call to vimsupport.PostMultiLineNotice with the supplied message""" - return call( 'echohl WarningMsg | echo \'' + return call( 'redraw | echohl WarningMsg | echo \'' + message + '\' | echohl None' ) diff --git a/python/ycm/tests/vimsupport_test.py b/python/ycm/tests/vimsupport_test.py index 68402353..5b6e8769 100644 --- a/python/ycm/tests/vimsupport_test.py +++ b/python/ycm/tests/vimsupport_test.py @@ -1432,3 +1432,52 @@ def VimExpressionToPythonType_ObjectPassthrough_test( *args ): def VimExpressionToPythonType_GeneratorPassthrough_test( *args ): gen = ( x**2 for x in [ 1, 2, 3 ] ) eq_( vimsupport.VimExpressionToPythonType( gen ), gen ) + + +@patch( 'vim.eval', + new_callable = ExtendedMock, + side_effect = [ None, 2, None ] ) +def SelectFromList_LastItem_test( vim_eval ): + eq_( vimsupport.SelectFromList( 'test', [ 'a', 'b' ] ), + 1 ) + + vim_eval.assert_has_exact_calls( [ + call( 'inputsave()' ), + call( 'inputlist( ["test", "1: a", "2: b"] )' ), + call( 'inputrestore()' ) + ] ) + + +@patch( 'vim.eval', + new_callable = ExtendedMock, + side_effect = [ None, 1, None ] ) +def SelectFromList_FirstItem_test( vim_eval ): + eq_( vimsupport.SelectFromList( 'test', [ 'a', 'b' ] ), + 0 ) + + vim_eval.assert_has_exact_calls( [ + call( 'inputsave()' ), + call( 'inputlist( ["test", "1: a", "2: b"] )' ), + call( 'inputrestore()' ) + ] ) + + +@patch( 'vim.eval', side_effect = [ None, 3, None ] ) +def SelectFromList_OutOfRange_test( vim_eval ): + assert_that( calling( vimsupport.SelectFromList).with_args( 'test', + [ 'a', 'b' ] ), + raises( RuntimeError, vimsupport.NO_SELECTION_MADE_MSG ) ) + + +@patch( 'vim.eval', side_effect = [ None, 0, None ] ) +def SelectFromList_SelectPrompt_test( vim_eval ): + assert_that( calling( vimsupport.SelectFromList ).with_args( 'test', + [ 'a', 'b' ] ), + raises( RuntimeError, vimsupport.NO_SELECTION_MADE_MSG ) ) + + +@patch( 'vim.eval', side_effect = [ None, -199, None ] ) +def SelectFromList_Negative_test( vim_eval ): + assert_that( calling( vimsupport.SelectFromList ).with_args( 'test', + [ 'a', 'b' ] ), + raises( RuntimeError, vimsupport.NO_SELECTION_MADE_MSG ) ) diff --git a/python/ycm/vimsupport.py b/python/ycm/vimsupport.py index c4a600cf..c3a63915 100644 --- a/python/ycm/vimsupport.py +++ b/python/ycm/vimsupport.py @@ -44,6 +44,8 @@ FIXIT_OPENING_BUFFERS_MESSAGE_FORMAT = ( 'buffers. The quickfix list can then be used to review the changes. No ' 'files will be written to disk. Do you wish to continue?' ) +NO_SELECTION_MADE_MSG = "No valid selection was made; aborting." + def CurrentLineAndColumn(): """Returns the 0-based current line and 0-based current column.""" @@ -441,8 +443,11 @@ def PostVimMessage( message ): # Unlike PostVimMesasge, this supports messages with newlines in them because it # uses 'echo' instead of 'echomsg'. This also means that the message will NOT # appear in Vim's message log. +# Similarly to PostVimMesasge, we do a redraw first to clear any previous +# messages, which might lead to this message appearing without a newline and/or +# requring the "Press ENTER or type command to continue". def PostMultiLineNotice( message ): - vim.command( "echohl WarningMsg | echo '{0}' | echohl None" + vim.command( "redraw | echohl WarningMsg | echo '{0}' | echohl None" .format( EscapeForVim( ToUnicode( message ) ) ) ) @@ -458,6 +463,10 @@ def PresentDialog( message, choices, default_choice_index = 0 ): PresentDialog will return a 0-based index into the list or -1 if the dialog was dismissed by using , Ctrl-C, etc. + If you are presenting a list of options for the user to choose from, such as + a list of imports, or lines to insert (etc.), SelectFromList is a better + option. + See also: :help confirm() in vim (Note that vim uses 1-based indexes) @@ -478,6 +487,58 @@ def Confirm( message ): return bool( PresentDialog( message, [ "Ok", "Cancel" ] ) == 0 ) +def SelectFromList( prompt, items ): + """Ask the user to select an item from the list |items|. + + Presents the user with |prompt| followed by a numbered list of |items|, + from which they select one. The user is asked to enter the number of an + item or click it. + + |items| should not contain leading ordinals: they are added automatically. + + Returns the 0-based index in the list |items| that the user selected, or a + negative number if no valid item was selected. + + See also :help inputlist().""" + + vim_items = [ prompt ] + vim_items.extend( [ "{0}: {1}".format( i + 1, item ) + for i, item in enumerate( items ) ] ) + + # The vim documentation warns not to present lists larger than the number of + # lines of display. This is sound advice, but there really isn't any sensible + # thing we can do in that scenario. Testing shows that Vim just pages the + # message; that behaviour is as good as any, so we don't manipulate the list, + # or attempt to page it. + + # For an explanation of the purpose of inputsave() / inputrestore(), + # see :help input(). Briefly, it makes inputlist() work as part of a mapping. + vim.eval( 'inputsave()' ) + try: + # Vim returns the number the user entered, or the line number the user + # clicked. This may be wildly out of range for our list. It might even be + # negative. + # + # The first item is index 0, and this maps to our "prompt", so we subtract 1 + # from the result and return that, assuming it is within the range of the + # supplied list. If not, we return negative. + # + # See :help input() for explanation of the use of inputsave() and inpput + # restore(). It is done in try/finally in case vim.eval ever throws an + # exception (such as KeyboardInterrupt) + selected = int( vim.eval( "inputlist( " + + json.dumps( vim_items ) + + " )" ) ) - 1 + finally: + vim.eval( 'inputrestore()' ) + + if selected < 0 or selected >= len( items ): + # User selected something outside of the range + raise RuntimeError( NO_SELECTION_MADE_MSG ) + + return selected + + def EchoText( text, log_as_message = True ): def EchoLine( text ): command = 'echom' if log_as_message else 'echo'