Auto merge of #2223 - puremourning:fixit-multiple-choice, r=micbou

[READY] Support selecting from the list of FixIts when multiple are supplied

# PR Prelude

Thank you for working on YCM! :)

**Please complete these steps and check these boxes (by putting an `x` inside
the brackets) _before_ filing your PR:**

- [X] I have read and understood YCM's [CONTRIBUTING][cont] document.
- [X] I have read and understood YCM's [CODE_OF_CONDUCT][code] document.
- [X] I have included tests for the changes in my PR. If not, I have included a
  rationale for why I haven't.
- [X] **I understand my PR may be closed if it becomes obvious I didn't
  actually perform all of these steps.**

# Why this change is necessary and useful

See PR https://github.com/Valloric/ycmd/pull/532

This PR presents the user with a list to choose from supplied FixIt suggestions when there are multiple suggestions on the same line. This not only helps with the child diagnostics mentioned in the ycmd PR, but also for cases where there are simply 2 diagnostics on the current line; the user can now select which of them to apply. Previously the one closest to the cursor was applied (which was always the first one supplied from the server).

I created a new `SelectFromList` method which uses `inputlist()` as the user interface from `confirm()` does not work well with long prompts. This interface is more like the suggestions when `set spell` is enabled, which should be familiar to many Vim users.

I considered updating the "auto namespace import" c sharp stuff to use the new `SelectFromList` method, but we know that the current implementation of that functionality is broken, so cannot be widely used.

[cont]: https://github.com/Valloric/YouCompleteMe/blob/master/CONTRIBUTING.md
[code]: https://github.com/Valloric/YouCompleteMe/blob/master/CODE_OF_CONDUCT.md

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/valloric/youcompleteme/2223)
<!-- Reviewable:end -->
This commit is contained in:
Homu 2016-07-12 07:28:49 +09:00
commit d5610e577c
8 changed files with 156 additions and 22 deletions

View File

@ -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

View File

@ -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

View File

@ -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 ) )

View File

@ -156,9 +156,11 @@ 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:
with patch( 'ycm.vimsupport.SelectFromList',
return_value = selection ):
request = CommandRequest( [ command ] )
request._response = response
request.RunPostCommandActionsIfNeeded()
@ -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 ):

View File

@ -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' )

View File

@ -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 ) )

View File

@ -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 <Esc>, 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'

2
third_party/ycmd vendored

@ -1 +1 @@
Subproject commit 1adbab446096ce1fccf950e2523e42fa424112a9
Subproject commit cc87a9c5fc659f488879c3cb5bce090a966cbb70