Skip to content

Commit

Permalink
Auto merge of ycm-core#1555 - puremourning:fixit, r=Valloric
Browse files Browse the repository at this point in the history
[RFC] YCM Client support for FixIt commands

This change provides the client-side support in YouCompleteMe for vim providing a new :YcmFixIt command which applies changes to the vim buffer to implement trivial fixes to common coding errors where the completer provides instructions on how to fix a diagnostic. A note is added "(FixIt)" to the diagnostic text echo'd in vim when a FixIt is available.

While this may seem out of the scope of ycmd/YouCompleteMe (as a completion engine), I think it is a really useful feature, and provided straight out of libclang. so I'm sending this slightly incomplete PR (along with the associated ycmd change) to see what the appetite is within the user and maintainer community, and to get feedback on the approach.

I know that I need to write some tests, and perhaps tidy up some TODOs, but I would really appreciate any feedback on the approach. Also, I should confess that I haven't yet run the existing tests as I'm having trouble getting them to run on my mac. Hoping that travis will help me out :)

I know I also need to update the readme.

Testing has been performed manually by going through the list of .c, .cpp and .m files in clang's tests/FixIt folder and ensuring that by hitting :YcmFixIt (actually "@:" a lot!) on each diagnostic, the diagnostic went away (assuming a FixIt was available). I also mangled a number of the clang test such that the fix was spread across multiple lines, etc.

I also haven't tested this with completers other than clang completer (due to having trouble one with diagnostics getting them to work) , but of course I will prior to this being ready to merge.

I also understand that adding the vim modelines might be divisive, but the standard in this project doesn't match my usual setting :) Happy to remove them if people react angrily.

Once again thanks for the time and a great (and improving!) product.

I have signed the CLA

Demo:
![ycmfixit](https://cloud.githubusercontent.com/assets/10584846/8396802/8fce516c-1dad-11e5-91e7-a26253e489fd.gif)
  • Loading branch information
homu committed Aug 5, 2015
2 parents baaa8be + 98c4d71 commit c946aa5
Show file tree
Hide file tree
Showing 5 changed files with 686 additions and 16 deletions.
47 changes: 45 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -655,7 +655,15 @@ You may want to map this command to a key; try putting `nnoremap <F5>
### The `:YcmDiags` command

Calling this command will fill Vim's `locationlist` with errors or warnings if
any were detected in your file and then open it.
any were detected in your file and then open it. If a given error or warning can
be fixed by a call to `:YcmCompleter FixIt`, then ` (FixIt available)` is
appended to the error or warning text. See the `FixIt` completer subcommand for
more information.

NOTE: The absense of ` (FixIt available)` does not strictly imply a fix-it is
not available as not all completers are able to provide this indication. For
example, the c-sharp completer provides many fix-its but does not add this
additional indication.

The `g:ycm_open_loclist_on_ycm_diags` option can be used to prevent the location
list from opening, but still have it filled with new diagnostic data. See the
Expand Down Expand Up @@ -817,6 +825,40 @@ NOTE: Causes reparsing of the current translation unit.
Supported in filetypes: `c, cpp, objc, objcpp`
### The `FixIt` subcommand
Where available, attempts to make changes to the buffer to correct the
diagnostic closest to the cursor position.
Completers which provide diagnostics may also provide trivial modifications to
the source in order to correct the diagnostic. Examples include syntax errors
such as missing trailing semi-colons, spurious characters, or other errors which
the semantic engine can deterministically suggest corrections.
If no fix-it is available for the current line, or there is no diagnostic on the
current line, this command has no effect on the current buffer. If any
modifications are made, the number of changes made to the buffer is echo'd and
the user may use the editor's undo command to revert.
When a diagnostic is available, and `g:ycm_echo_current_diagnostic` is set to 1,
then the text ` (FixIt)` is appended to the echo'd diagnostic when the
completer is able to add this indication. The text ` (FixIt available)` is
also appended to the diagnostic text in the output of the `:YcmDiags` command
for any diagnostics with available fix-its (where the completer can provide this
indication).
NOTE: Causes re-parsing of the current translation unit.
NOTE: After applying a fix-it, the diagnostics UI is not immediately updated.
This is due to a technical restriction in vim, and moving the cursor, or issuing
the the `:YcmForceCompileAndDiagnostics` command will refresh the diagnostics.
Repeated invocations of the `FixIt` command on a given line, however, _do_ apply
all diagnostics as expected without requiring refreshing of the diagnostics UI.
This is particularly useful where there are multiple diagnostics on one line, or
where after fixing one diagnostic, another fix-it is available.
Supported in filetypes: `c, cpp, objc, objcpp, cs`
### The `StartServer` subcommand
Starts the semantic-engine-as-localhost-server for those semantic engines that
Expand Down Expand Up @@ -1075,7 +1117,8 @@ Default: `1`
### The `g:ycm_echo_current_diagnostic` option
When this option is set, YCM will echo the text of the diagnostic present on the
current line when you move your cursor to that line.
current line when you move your cursor to that line. If a `FixIt` is available
for the current diagnostic, then ` (FixIt)` is appended.
This option is part of the Syntastic compatibility layer; if the option is not
set, YCM will fall back to the value of the `g:syntastic_echo_current_error`
Expand Down
72 changes: 62 additions & 10 deletions python/ycm/client/command_request.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ def __init__( self, arguments, completer_target = None ):
else 'filetype_default' )
self._is_goto_command = (
self._arguments and self._arguments[ 0 ].startswith( 'GoTo' ) )
self._is_fixit_command = (
self._arguments and self._arguments[ 0 ].startswith( 'FixIt' ) )
self._response = None


Expand All @@ -55,23 +57,73 @@ def Start( self ):
def Response( self ):
return self._response


def RunPostCommandActionsIfNeeded( self ):
if not self.Done() or not self._response:
return

if self._is_goto_command:
if isinstance( self._response, list ):
defs = [ _BuildQfListItem( x ) for x in self._response ]
vim.eval( 'setqflist( %s )' % repr( defs ) )
vim.eval( 'youcompleteme#OpenGoToList()' )
else:
vimsupport.JumpToLocation( self._response[ 'filepath' ],
self._response[ 'line_num' ],
self._response[ 'column_num' ] )
self._HandleGotoResponse()
elif self._is_fixit_command:
self._HandleFixitResponse()
elif 'message' in self._response:
vimsupport.EchoText( self._response['message'] )
self._HandleMessageResponse()

def _HandleGotoResponse( self ):
if isinstance( self._response, list ):
defs = [ _BuildQfListItem( x ) for x in self._response ]
vim.eval( 'setqflist( %s )' % repr( defs ) )
vim.eval( 'youcompleteme#OpenGoToList()' )
else:
vimsupport.JumpToLocation( self._response[ 'filepath' ],
self._response[ 'line_num' ],
self._response[ 'column_num' ] )

def _HandleFixitResponse( self ):
if not len( self._response[ 'fixits' ] ):
vimsupport.EchoText( "No fixits found for current line" )
else:
fixit = self._response[ 'fixits' ][ 0 ]

# We need to track the difference in length, but ensuring we apply fixes
# in ascending order of insertion point.
fixit[ 'chunks' ].sort( key = lambda chunk: (
str(chunk[ 'range' ][ 'start' ][ 'line_num' ])
+ ','
+ str(chunk[ 'range' ][ 'start' ][ 'column_num' ])
))

# Remember the line number we're processing. Negative line number means we
# haven't processed any lines yet (by nature of being not equal to any
# real line number).
last_line = -1

# Counter of changes applied, so the user has a mental picture of the
# undo history this change is creating.
num_fixed = 0
line_delta = 0
for chunk in fixit[ 'chunks' ]:
if chunk[ 'range' ][ 'start' ][ 'line_num' ] != last_line:
# If this chunk is on a different line than the previous chunk,
# then ignore previous deltas (as offsets won't have changed).
last_line = chunk[ 'range' ][ 'end' ][ 'line_num' ]
char_delta = 0

(new_line_delta, new_char_delta) = vimsupport.ReplaceChunk(
chunk[ 'range' ][ 'start' ],
chunk[ 'range' ][ 'end' ],
chunk[ 'replacement_text' ],
line_delta, char_delta )
line_delta += new_line_delta
char_delta += new_char_delta

num_fixed = num_fixed + 1

vimsupport.EchoTextVimWidth("FixIt applied "
+ str(num_fixed)
+ " changes")

def _HandleMessageResponse( self ):
vimsupport.EchoText( self._response[ 'message' ] )

def SendCommandRequest( arguments, completer ):
request = CommandRequest( arguments, completer )
Expand Down
9 changes: 6 additions & 3 deletions python/ycm/diagnostic_interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ def OnCursorMoved( self ):
if self._user_options[ 'echo_current_diagnostic' ]:
self._EchoDiagnosticForLine( line )


def UpdateWithNewDiagnostics( self, diags ):
normalized_diags = [ _NormalizeDiagnostic( x ) for x in diags ]
self._buffer_number_to_line_to_diags = _ConvertDiagListToDict(
Expand All @@ -62,7 +61,6 @@ def UpdateWithNewDiagnostics( self, diags ):
vimsupport.SetLocationList(
vimsupport.ConvertDiagnosticsToQfList( normalized_diags ) )


def _EchoDiagnosticForLine( self, line_num ):
buffer_num = vim.current.buffer.number
diags = self._buffer_number_to_line_to_diags[ buffer_num ][ line_num ]
Expand All @@ -72,7 +70,12 @@ def _EchoDiagnosticForLine( self, line_num ):
vimsupport.EchoText( '', False )
self._diag_message_needs_clearing = False
return
vimsupport.EchoTextVimWidth( diags[ 0 ][ 'text' ] )

text = diags[ 0 ][ 'text' ]
if diags[ 0 ].get( 'fixit_available', False ):
text += ' (FixIt)'

vimsupport.EchoTextVimWidth( text )
self._diag_message_needs_clearing = True


Expand Down
Loading

0 comments on commit c946aa5

Please sign in to comment.