From ba9885bb317e4b462fe8b44a2b6df3115ea3b3f5 Mon Sep 17 00:00:00 2001 From: Ben Jackson Date: Sat, 29 Dec 2018 21:27:15 +0000 Subject: [PATCH 1/2] Support for multi-line completions This replaces a non-snippet completion containing newlines with a FixIt which replaces the entered 'trigger' text with the multi-line text. We use the 'fixit completion' mechanism for this, but include a special indicator that tells the client to put the cursor position _after_ the inserted text, as we would for a normal completion. --- cpp/ycm/ClangCompleter/FixIt.h | 2 + cpp/ycm/ycm_core.cpp | 3 +- .../language_server_completer.py | 114 ++++++++++++------ ycmd/responses.py | 9 +- 4 files changed, 87 insertions(+), 41 deletions(-) diff --git a/cpp/ycm/ClangCompleter/FixIt.h b/cpp/ycm/ClangCompleter/FixIt.h index 4bf2f59fd3..4ceae17052 100644 --- a/cpp/ycm/ClangCompleter/FixIt.h +++ b/cpp/ycm/ClangCompleter/FixIt.h @@ -52,6 +52,8 @@ struct FixIt { /// multiple diagnostics offering different fixit options. The text is /// displayed to the user, allowing them choose which diagnostic to apply. std::string text; + + bool is_completion{ false }; }; } // namespace YouCompleteMe diff --git a/cpp/ycm/ycm_core.cpp b/cpp/ycm/ycm_core.cpp index 454f2cfe8d..dd118b920d 100644 --- a/cpp/ycm/ycm_core.cpp +++ b/cpp/ycm/ycm_core.cpp @@ -197,7 +197,8 @@ PYBIND11_MODULE( ycm_core, mod ) .def( py::init<>() ) .def_readonly( "chunks", &FixIt::chunks ) .def_readonly( "location", &FixIt::location ) - .def_readonly( "text", &FixIt::text ); + .def_readonly( "text", &FixIt::text ) + .def_readonly( "is_completion", &FixIt::is_completion ); py::bind_vector< std::vector< FixIt > >( mod, "FixItVector" ); diff --git a/ycmd/completers/language_server/language_server_completer.py b/ycmd/completers/language_server/language_server_completer.py index f029da0bf3..afb98c229f 100644 --- a/ycmd/completers/language_server/language_server_completer.py +++ b/ycmd/completers/language_server/language_server_completer.py @@ -2263,14 +2263,6 @@ def _InsertionTextForItem( request_data, item ): selecting this completion - start_codepoint = the start column at which the text should be inserted )""" - # We do not support completion types of "Snippet". This is implicit in that we - # don't say it is a "capability" in the initialize request. - # Abort this request if the server is buggy and ignores us. - assert lsp.INSERT_TEXT_FORMAT[ - item.get( 'insertTextFormat' ) or 1 ] == 'PlainText' - - fixits = None - start_codepoint = request_data[ 'start_codepoint' ] # We will always have one of insertText or label if 'insertText' in item and item[ 'insertText' ]: @@ -2278,13 +2270,13 @@ def _InsertionTextForItem( request_data, item ): else: insertion_text = item[ 'label' ] - additional_text_edits = [] + fixits = [] + filepath = request_data[ 'filepath' ] + contents = None - # Per the protocol, textEdit takes precedence over insertText, and must be - # on the same line (and containing) the originally requested position. These - # are a pain, and require fixing up later in some cases, as most of our - # clients won't be able to apply arbitrary edits (only 'completion', as - # opposed to 'content assist'). + # Per the protocol, textEdit takes precedence over insertText, and the initial + # range of the edit must be on the same line (and containing) the originally + # requested position. if 'textEdit' in item and item[ 'textEdit' ]: text_edit = item[ 'textEdit' ] start_codepoint = _GetCompletionItemStartCodepointOrReject( text_edit, @@ -2293,25 +2285,71 @@ def _InsertionTextForItem( request_data, item ): insertion_text = text_edit[ 'newText' ] if '\n' in insertion_text: - # jdt.ls can return completions which generate code, such as - # getters/setters and entire anonymous classes. + # FIXME: If this logic actually worked, then we should do it for + # everything and not just hte multi-line completions ? Would that allow us + # to avoid the _GetCompletionItemStartCodepointOrReject logic ? Possibly, + # but it would look strange in the UI cycling through items. In any case, + # doing both should be complete. + + # servers can return completions which generate code, such as + # getters/setters and entire anonymous classes. These contain newlines in + # the generated textEdit. This is irksome because ycmd's clients don't + # necessarily support that. Certainly, Vim doesn't. # - # In order to support this we would need to do something like: - # - invent some insertion_text based on label/insertText (or perhaps - # '' - # - insert a textEdit in additionalTextEdits which deletes this - # insertion - # - or perhaps just modify this textEdit to undo that change? - # - or perhaps somehow support insertion_text of '' (this doesn't work - # because of filtering/sorting, etc.). - # - insert this textEdit in additionalTextEdits + # However, we do have 'fixits' in completion responses which we can lean + # on. # - # These textEdits would need a lot of fixing up and is currently out of - # scope. + # In order to support this we: + # - use the item's label as the intial insertion text with the start + # codepoint set to the query codepoint + # - insert a textEdit in additionalTextEdits which deletes this + # insertion + # - insert another textEdit in additionalTextEdits which applies this + # textedit + insertion_text = item[ 'label' ] + start_codepoint = request_data[ 'start_codepoint' ] + + # Add a fixit which removes the inserted label # - # These sorts of completions aren't really in the spirit of ycmd at the - # moment anyway. So for now, we just ignore this candidate. - raise IncompatibleCompletionException( insertion_text ) + # TODO: Perhaps we should actually supply a completion with an empty + # insertion_text, than have the _client_ deal with this. One key advantage + # to that is that the client can then decide where to put the cursor. + # Currently, the Vim client puts the cursor in the wrong place (i.e. + # before the text, rather than after it). + completion_fixit_chunks = [ + responses.FixItChunk( + '', + responses.Range( + responses.Location( request_data[ 'line_num' ], + start_codepoint, + filepath ), + responses.Location( request_data[ 'line_num' ], + start_codepoint + len( insertion_text ), + filepath ), + ) + ) + ] + # FIXME: The problem with this is that it _might_ break the offsets in any + # additionalTextEdits + fixits.append( + responses.FixIt( completion_fixit_chunks[ 0 ].range.start_, + completion_fixit_chunks, + item[ 'label' ] ) + ) + # Add a fixit which applies this textEdit + contents = GetFileLines( request_data, filepath ) + completion_fixit_chunks = [ + responses.FixItChunk( + text_edit[ 'newText' ], + _BuildRange( contents, filepath, text_edit[ 'range' ] ) + ) + ] + fixits.append( + responses.FixIt( completion_fixit_chunks[ 0 ].range.start_, + completion_fixit_chunks, + item[ 'label' ], + is_completion = True ) + ) else: # Calculate the start codepoint based on the overlapping text in the # insertion text and the existing text. This is the behavior of Visual @@ -2320,21 +2358,23 @@ def _InsertionTextForItem( request_data, item ): start_codepoint -= FindOverlapLength( request_data[ 'prefix' ], insertion_text ) - additional_text_edits.extend( item.get( 'additionalTextEdits' ) or [] ) - + additional_text_edits = item.get( 'additionalTextEdits' ) or [] if additional_text_edits: - filepath = request_data[ 'filepath' ] - contents = GetFileLines( request_data, filepath ) + + # We might have already extracted the contents + if contents is None: + contents = GetFileLines( request_data, filepath ) + chunks = [ responses.FixItChunk( e[ 'newText' ], _BuildRange( contents, filepath, e[ 'range' ] ) ) for e in additional_text_edits ] - fixits = responses.BuildFixItResponse( - [ responses.FixIt( chunks[ 0 ].range.start_, chunks ) ] ) + fixits.append( responses.FixIt( chunks[ 0 ].range.start_, chunks ) ) - return insertion_text, fixits, start_codepoint + extra_data = responses.BuildFixItResponse( fixits ) if fixits else None + return insertion_text, extra_data, start_codepoint def FindOverlapLength( line_value, insertion_text ): diff --git a/ycmd/responses.py b/ycmd/responses.py index 1780294dbf..9b453ce793 100644 --- a/ycmd/responses.py +++ b/ycmd/responses.py @@ -194,11 +194,12 @@ class FixIt: must be byte offsets into the UTF-8 encoded version of the appropriate buffer. """ - def __init__( self, location, chunks, text = '' ): + def __init__( self, location, chunks, text = '', is_completion = False ): """location of type Location, chunks of type list""" self.location = location self.chunks = chunks self.text = text + self.is_completion = is_completion class FixItChunk: @@ -290,14 +291,16 @@ def BuildFixItData( fixit ): return { 'command': fixit.command, 'text': fixit.text, - 'resolve': fixit.resolve + 'resolve': fixit.resolve, + 'is_completion': fixit.is_completion } else: return { 'location': BuildLocationData( fixit.location ), 'chunks' : [ BuildFixitChunkData( x ) for x in fixit.chunks ], 'text': fixit.text, - 'resolve': False + 'resolve': False, + 'is_completion': fixit.is_completion, } return { From 124dbc57ea398d6217413b2a91fa1a97303a2742 Mon Sep 17 00:00:00 2001 From: Ben Jackson Date: Sun, 27 Jan 2019 09:07:21 +0000 Subject: [PATCH 2/2] Support snippet completions In general this is terrible, compared to working normal completion, but some servers (json, yaml) will only ever return snippet completions, and they will not be convinved to change. The approach is to put the snippet text in the extra_data and have the client expand the snippet in-place on selecting the completion item. As many servers will agressively over-use snippets when the client claims they support it, we _dont'_ claim we support it, but provide a mechanism for extra_conf files to override the capabilities that we declare. This allows users to enable snippets where they make sense, i.e. for the json LS. --- .../language_server_completer.py | 52 +++++++++++++++---- .../language_server_protocol.py | 17 ++++-- ycmd/utils.py | 44 ++++++++++++++++ 3 files changed, 99 insertions(+), 14 deletions(-) diff --git a/ycmd/completers/language_server/language_server_completer.py b/ycmd/completers/language_server/language_server_completer.py index afb98c229f..df9eacda23 100644 --- a/ycmd/completers/language_server/language_server_completer.py +++ b/ycmd/completers/language_server/language_server_completer.py @@ -1046,7 +1046,7 @@ def _CandidatesFromCompletionItems( self, items, resolve, request_data ): item[ '_resolved' ] = True try: - insertion_text, extra_data, start_codepoint = ( + insertion_text, snippet, extra_data, start_codepoint = ( _InsertionTextForItem( request_data, item ) ) except IncompatibleCompletionException: LOGGER.exception( 'Ignoring incompatible completion suggestion %s', @@ -1059,6 +1059,13 @@ def _CandidatesFromCompletionItems( self, items, resolve, request_data ): extra_data = {} if extra_data is None else extra_data extra_data[ 'item' ] = item + if snippet: + extra_data = {} if extra_data is None else extra_data + extra_data[ 'snippet' ] = { + 'snippet': snippet, + 'trigger_string': insertion_text + } + min_start_codepoint = min( min_start_codepoint, start_codepoint ) # Build a ycmd-compatible completion for the text as we received it. Later @@ -1267,12 +1274,18 @@ def GetSettings( self, module, request_data ): def _GetSettingsFromExtraConf( self, request_data ): - self._settings = self.DefaultSettings( request_data ) + ls = self.DefaultSettings( request_data ) + # In case no module is found + self._settings = { + 'ls': ls + } module = extra_conf_store.ModuleForSourceFile( request_data[ 'filepath' ] ) if module: - settings = self.GetSettings( module, request_data ) - self._settings.update( settings.get( 'ls', {} ) ) + self._settings = self.GetSettings( module, request_data ) + ls.update( self._settings.get( 'ls', {} ) ) + self._settings[ 'ls' ] = ls + # Only return the dir if it was found in the paths; we don't want to use # the path of the global extra conf as a project root dir. if not extra_conf_store.IsGlobalExtraConfModule( module ): @@ -1690,7 +1703,8 @@ def _SendInitialize( self, request_data, extra_conf_dir ): # clear how/where that is specified. msg = lsp.Initialize( request_id, self._project_directory, - self._settings ) + self._settings.get( 'ls', {} ), + self._settings.get( 'capabilities', {} ) ) def response_handler( response, message ): if message is None: @@ -1803,7 +1817,7 @@ def _HandleInitializeInPollThread( self, response ): # configuration should be send in response to a workspace/configuration # request? self.GetConnection().SendNotification( - lsp.DidChangeConfiguration( self._settings ) ) + lsp.DidChangeConfiguration( self._settings.get( 'ls', {} ) ) ) # Notify the other threads that we have completed the initialize exchange. self._initialize_response = None @@ -2259,13 +2273,21 @@ def _InsertionTextForItem( request_data, item ): Returns a tuple ( - insertion_text = the text to insert + - snippet = optional snippet text - fixits = ycmd fixit which needs to be applied additionally when selecting this completion - start_codepoint = the start column at which the text should be inserted )""" start_codepoint = request_data[ 'start_codepoint' ] + label = item[ 'label' ] + insertion_text_is_snippet = False # We will always have one of insertText or label if 'insertText' in item and item[ 'insertText' ]: + # 1 = PlainText + # 2 = Snippet + if lsp.INSERT_TEXT_FORMAT[ item.get( 'insertTextFormat', 1 ) ] == 'Snippet': + insertion_text_is_snippet = True + insertion_text = item[ 'insertText' ] else: insertion_text = item[ 'label' ] @@ -2284,7 +2306,7 @@ def _InsertionTextForItem( request_data, item ): insertion_text = text_edit[ 'newText' ] - if '\n' in insertion_text: + if '\n' in insertion_text and not insertion_text_is_snippet: # FIXME: If this logic actually worked, then we should do it for # everything and not just hte multi-line completions ? Would that allow us # to avoid the _GetCompletionItemStartCodepointOrReject logic ? Possibly, @@ -2306,9 +2328,17 @@ def _InsertionTextForItem( request_data, item ): # insertion # - insert another textEdit in additionalTextEdits which applies this # textedit + # + # On the other hand, if the insertion text is a snippet, then the snippet + # system will handle the expansion. insertion_text = item[ 'label' ] start_codepoint = request_data[ 'start_codepoint' ] + # FIXME: + # So forced-completion breaks here, as the textEdit is formulated to + # remove the existing "prefix". E.g. typing getT, the edit + # attempts to replace getT with the new code. + # Add a fixit which removes the inserted label # # TODO: Perhaps we should actually supply a completion with an empty @@ -2340,7 +2370,7 @@ def _InsertionTextForItem( request_data, item ): contents = GetFileLines( request_data, filepath ) completion_fixit_chunks = [ responses.FixItChunk( - text_edit[ 'newText' ], + text_edit[ 'newText' ], # FIXME: This could also be a Snippet _BuildRange( contents, filepath, text_edit[ 'range' ] ) ) ] @@ -2374,7 +2404,11 @@ def _InsertionTextForItem( request_data, item ): fixits.append( responses.FixIt( chunks[ 0 ].range.start_, chunks ) ) extra_data = responses.BuildFixItResponse( fixits ) if fixits else None - return insertion_text, extra_data, start_codepoint + + if insertion_text_is_snippet: + return label, insertion_text, extra_data, start_codepoint + + return insertion_text, None, extra_data, start_codepoint def FindOverlapLength( line_value, insertion_text ): diff --git a/ycmd/completers/language_server/language_server_protocol.py b/ycmd/completers/language_server/language_server_protocol.py index 65bdd789c7..5a3268c0ea 100644 --- a/ycmd/completers/language_server/language_server_protocol.py +++ b/ycmd/completers/language_server/language_server_protocol.py @@ -24,7 +24,8 @@ from ycmd.utils import ( ByteOffsetToCodepointOffset, ToBytes, - ToUnicode ) + ToUnicode, + UpdateDict ) Error = collections.namedtuple( 'RequestError', [ 'code', 'reason' ] ) @@ -223,7 +224,10 @@ def BuildResponse( request, parameters ): return _BuildMessageData( message ) -def Initialize( request_id, project_directory, settings ): +def Initialize( request_id, + project_directory, + settings, + capabilities ): """Build the Language Server initialize request""" return BuildRequest( request_id, 'initialize', { @@ -231,8 +235,11 @@ def Initialize( request_id, project_directory, settings ): 'rootPath': project_directory, 'rootUri': FilePathToUri( project_directory ), 'initializationOptions': settings, - 'capabilities': { - 'workspace': { 'applyEdit': True, 'documentChanges': True }, + 'capabilities': UpdateDict( { + 'workspace': { + 'applyEdit': True, + 'documentChanges': True, + }, 'textDocument': { 'codeAction': { 'codeActionLiteralSupport': { @@ -278,7 +285,7 @@ def Initialize( request_id, project_directory, settings ): }, }, }, - }, + }, capabilities ), } ) diff --git a/ycmd/utils.py b/ycmd/utils.py index 1404d43e0e..770d44c968 100644 --- a/ycmd/utils.py +++ b/ycmd/utils.py @@ -25,6 +25,7 @@ import tempfile import time import threading +import collections.abc as collections_abc LOGGER = logging.getLogger( 'ycmd' ) ROOT_DIR = os.path.normpath( os.path.join( os.path.dirname( __file__ ), '..' ) ) @@ -508,3 +509,46 @@ def GetClangResourceDir(): CLANG_RESOURCE_DIR = GetClangResourceDir() + + +def UpdateDict( target, override ): + """Apply the updates in |override| to the dict |target|. This is like + dict.update, but recursive. i.e. if the existing element is a dict, then + override elements of the sub-dict rather than wholesale replacing. + + e.g. + + UpdateDict( + { + 'outer': { 'inner': { 'key': 'oldValue', 'existingKey': True } } + }, + { + 'outer': { 'inner': { 'key': 'newValue' } }, + 'newKey': { 'newDict': True }, + } + ) + + yields: + + { + outer: { + inner: { + key: 'newValue', + exitingKey: True + } + }, + newKey: { newDict: True } + } + + """ + + for key, value in override.items(): + current_value = target.get( key ) + if not isinstance( current_value, collections_abc.Mapping ): + target[ key ] = value + elif isinstance( value, collections_abc.Mapping ): + target[ key ] = UpdateDict( current_value, value ) + else: + target[ key ] = value + + return target