From 3d55748400f1267f08c53322791d43b74625a058 Mon Sep 17 00:00:00 2001 From: Strahinja Val Markovic Date: Tue, 8 Oct 2013 16:21:43 -0700 Subject: [PATCH] Correctly handling ycm_extra_conf files now The user is asked about loading unknown extra conf files, as they were before. --- python/ycm/client/base_request.py | 15 +++--- python/ycm/client/completion_request.py | 2 +- python/ycm/client/event_notification.py | 12 ++++- python/ycm/completers/cpp/clang_completer.py | 3 ++ python/ycm/completers/cpp/flags.py | 6 ++- python/ycm/extra_conf_store.py | 50 +++++++++++------- python/ycm/server/responses.py | 20 +++++++- python/ycm/server/tests/basic_test.py | 54 +++++++++++++++++++- python/ycm/server/tests/testdata/basic.cpp | 13 +++++ python/ycm/server/ycmd.py | 25 +++++++-- 10 files changed, 162 insertions(+), 38 deletions(-) create mode 100644 python/ycm/server/tests/testdata/basic.cpp diff --git a/python/ycm/client/base_request.py b/python/ycm/client/base_request.py index 7932bca4..ddd2b2c8 100644 --- a/python/ycm/client/base_request.py +++ b/python/ycm/client/base_request.py @@ -24,15 +24,11 @@ from retries import retries from requests_futures.sessions import FuturesSession from concurrent.futures import ThreadPoolExecutor from ycm import vimsupport +from ycm.server.responses import ServerError, UnknownExtraConf HEADERS = {'content-type': 'application/json'} EXECUTOR = ThreadPoolExecutor( max_workers = 4 ) -class ServerError( Exception ): - def __init__( self, message ): - super( ServerError, self ).__init__( message ) - - class BaseRequest( object ): def __init__( self ): pass @@ -103,7 +99,7 @@ def BuildRequestData( start_column = None, query = None ): def JsonFromFuture( future ): response = future.result() if response.status_code == requests.codes.server_error: - raise ServerError( response.json()[ 'message' ] ) + _RaiseExceptionForData( response.json() ) # We let Requests handle the other status types, we only handle the 500 # error code. @@ -137,3 +133,10 @@ def _CheckServerIsHealthyWithCache(): except: return False + +def _RaiseExceptionForData( data ): + if data[ 'exception' ][ 'TYPE' ] == UnknownExtraConf.__name__: + raise UnknownExtraConf( data[ 'exception' ][ 'extra_conf_file' ] ) + + raise ServerError( '{0}: {1}'.format( data[ 'exception' ][ 'TYPE' ], + data[ 'message' ] ) ) diff --git a/python/ycm/client/completion_request.py b/python/ycm/client/completion_request.py index 977a1122..e8af21a7 100644 --- a/python/ycm/client/completion_request.py +++ b/python/ycm/client/completion_request.py @@ -57,7 +57,7 @@ class CompletionRequest( BaseRequest ): for x in JsonFromFuture( self._response_future ) ] except Exception as e: vimsupport.PostVimMessage( str( e ) ) - return [] + return [] def _ConvertCompletionDataToVimData( completion_data ): diff --git a/python/ycm/client/event_notification.py b/python/ycm/client/event_notification.py index 5f79b4a5..9652413f 100644 --- a/python/ycm/client/event_notification.py +++ b/python/ycm/client/event_notification.py @@ -18,9 +18,11 @@ # along with YouCompleteMe. If not, see . from ycm import vimsupport +from ycm.server.responses import UnknownExtraConf from ycm.client.base_request import ( BaseRequest, BuildRequestData, JsonFromFuture ) + class EventNotification( BaseRequest ): def __init__( self, event_name, extra_data = None ): super( EventNotification, self ).__init__() @@ -51,10 +53,13 @@ class EventNotification( BaseRequest ): return [] try: - self._cached_response = JsonFromFuture( self._response_future ) + try: + self._cached_response = JsonFromFuture( self._response_future ) + except UnknownExtraConf as e: + if vimsupport.Confirm( str( e ) ): + _LoadExtraConfFile( e.extra_conf_file ) except Exception as e: vimsupport.PostVimMessage( str( e ) ) - return [] if not self._cached_response: return [] @@ -83,3 +88,6 @@ def SendEventNotificationAsync( event_name, extra_data = None ): event = EventNotification( event_name, extra_data ) event.Start() +def _LoadExtraConfFile( filepath ): + BaseRequest.PostDataToHandler( { 'filepath': filepath }, + 'load_extra_conf_file' ) diff --git a/python/ycm/completers/cpp/clang_completer.py b/python/ycm/completers/cpp/clang_completer.py index 7f50a7ba..154452fa 100644 --- a/python/ycm/completers/cpp/clang_completer.py +++ b/python/ycm/completers/cpp/clang_completer.py @@ -79,6 +79,9 @@ class ClangCompleter( Completer ): if self._completer.UpdatingTranslationUnit( ToUtf8IfNeeded( filename ) ): self._logger.info( PARSING_FILE_MESSAGE ) + # TODO: For this exception and the NO_COMPILE_FLAGS one, use a special + # exception class so that the client can be more silent about these + # messages. raise RuntimeError( PARSING_FILE_MESSAGE ) flags = self._FlagsForRequest( request_data ) diff --git a/python/ycm/completers/cpp/flags.py b/python/ycm/completers/cpp/flags.py index a896986f..385bdc03 100644 --- a/python/ycm/completers/cpp/flags.py +++ b/python/ycm/completers/cpp/flags.py @@ -37,6 +37,7 @@ class Flags( object ): # It's caches all the way down... self.flags_for_file = {} self.special_clang_flags = _SpecialClangIncludes() + self.no_extra_conf_file_warning_posted = False def FlagsForFile( self, filename, add_special_clang_flags = True ): @@ -45,7 +46,10 @@ class Flags( object ): except KeyError: module = extra_conf_store.ModuleForSourceFile( filename ) if not module: - raise RuntimeError( NO_EXTRA_CONF_FILENAME_MESSAGE ) + if not self.no_extra_conf_file_warning_posted: + self.no_extra_conf_file_warning_posted = True + raise RuntimeError( NO_EXTRA_CONF_FILENAME_MESSAGE ) + return None results = module.FlagsForFile( filename ) diff --git a/python/ycm/extra_conf_store.py b/python/ycm/extra_conf_store.py index 277b9d61..5253e021 100644 --- a/python/ycm/extra_conf_store.py +++ b/python/ycm/extra_conf_store.py @@ -24,27 +24,29 @@ import imp import random import string import sys +from threading import Lock from ycm import user_options_store +from ycm.server.responses import UnknownExtraConf from fnmatch import fnmatch # Constants YCM_EXTRA_CONF_FILENAME = '.ycm_extra_conf.py' -CONFIRM_CONF_FILE_MESSAGE = ('Found {0}. Load? \n\n(Question can be turned ' - 'off with options, see YCM docs)') # Singleton variables _module_for_module_file = {} +_module_for_module_file_lock = Lock() _module_file_for_source_file = {} +_module_file_for_source_file_lock = Lock() -class UnknownExtraConf( Exception ): - def __init__( self, extra_conf_file ): - message = CONFIRM_CONF_FILE_MESSAGE.format( extra_conf_file ) - super( UnknownExtraConf, self ).__init__( message ) - self.extra_conf_file = extra_conf_file + +def Reset(): + global _module_for_module_file, _module_file_for_source_file + _module_for_module_file = {} + _module_file_for_source_file = {} def ModuleForSourceFile( filename ): - return _Load( ModuleFileForSourceFile( filename ) ) + return Load( ModuleFileForSourceFile( filename ) ) def ModuleFileForSourceFile( filename ): @@ -52,11 +54,12 @@ def ModuleFileForSourceFile( filename ): order and return the filename of the first module that was allowed to load. If no module was found or allowed to load, None is returned.""" - if not filename in _module_file_for_source_file: - for module_file in _ExtraConfModuleSourceFilesForFile( filename ): - if _Load( module_file ): - _module_file_for_source_file[ filename ] = module_file - break + with _module_file_for_source_file_lock: + if not filename in _module_file_for_source_file: + for module_file in _ExtraConfModuleSourceFilesForFile( filename ): + if Load( module_file ): + _module_file_for_source_file[ filename ] = module_file + break return _module_file_for_source_file.setdefault( filename ) @@ -84,7 +87,8 @@ def _CallExtraConfMethod( function_name ): def _Disable( module_file ): """Disables the loading of a module for the current session.""" - _module_for_module_file[ module_file ] = None + with _module_for_module_file_lock: + _module_for_module_file[ module_file ] = None def _ShouldLoad( module_file ): @@ -102,10 +106,15 @@ def _ShouldLoad( module_file ): if _MatchesGlobPattern( module_file, glob.lstrip('!') ): return not is_blacklisted + # We disable the file if it's unknown so that we don't ask the user about it + # repeatedly. Raising UnknownExtraConf should result in the client sending + # another request to load the module file if the user explicitly chooses to do + # that. + _Disable( module_file ) raise UnknownExtraConf( module_file ) -def _Load( module_file, force = False ): +def Load( module_file, force = False ): """Load and return the module contained in a file. Using force = True the module will be loaded regardless of the criteria in _ShouldLoad. @@ -115,11 +124,13 @@ def _Load( module_file, force = False ): return None if not force: - if module_file in _module_for_module_file: - return _module_for_module_file[ module_file ] + with _module_for_module_file_lock: + if module_file in _module_for_module_file: + return _module_for_module_file[ module_file ] if not _ShouldLoad( module_file ): - return _Disable( module_file ) + _Disable( module_file ) + return None # This has to be here because a long time ago, the ycm_extra_conf.py files # used to import clang_helpers.py from the cpp folder. This is not needed @@ -129,7 +140,8 @@ def _Load( module_file, force = False ): module = imp.load_source( _RandomName(), module_file ) del sys.path[ 0 ] - _module_for_module_file[ module_file ] = module + with _module_for_module_file_lock: + _module_for_module_file[ module_file ] = module return module diff --git a/python/ycm/server/responses.py b/python/ycm/server/responses.py index 450106d9..f0ca6364 100644 --- a/python/ycm/server/responses.py +++ b/python/ycm/server/responses.py @@ -19,6 +19,21 @@ import os +CONFIRM_CONF_FILE_MESSAGE = ('Found {0}. Load? \n\n(Question can be turned ' + 'off with options, see YCM docs)') + +class ServerError( Exception ): + def __init__( self, message ): + super( ServerError, self ).__init__( message ) + + +class UnknownExtraConf( ServerError ): + def __init__( self, extra_conf_file ): + message = CONFIRM_CONF_FILE_MESSAGE.format( extra_conf_file ) + super( UnknownExtraConf, self ).__init__( message ) + self.extra_conf_file = extra_conf_file + + def BuildGoToResponse( filepath, line_num, column_num, description = None ): response = { @@ -80,9 +95,10 @@ def BuildDiagnosticData( filepath, } -def BuildExceptionResponse( error_message, traceback ): +def BuildExceptionResponse( exception, traceback ): return { - 'message': error_message, + 'exception': exception, + 'message': str( exception ), 'traceback': traceback } diff --git a/python/ycm/server/tests/basic_test.py b/python/ycm/server/tests/basic_test.py index 0ba9e1b1..8c2cdc5b 100644 --- a/python/ycm/server/tests/basic_test.py +++ b/python/ycm/server/tests/basic_test.py @@ -17,9 +17,11 @@ # You should have received a copy of the GNU General Public License # along with YouCompleteMe. If not, see . +import os +import httplib from webtest import TestApp from .. import ycmd -from ..responses import BuildCompletionData +from ..responses import BuildCompletionData, UnknownExtraConf from nose.tools import ok_, eq_, with_setup from hamcrest import ( assert_that, has_items, has_entry, contains, contains_string, has_entries ) @@ -73,6 +75,15 @@ def Setup(): ycmd.SetServerStateToDefaults() +def PathToTestDataDir(): + dir_of_current_script = os.path.dirname( os.path.abspath( __file__ ) ) + return os.path.join( dir_of_current_script, 'testdata' ) + + +def PathToTestFile( test_basename ): + return os.path.join( PathToTestDataDir(), test_basename ) + + @with_setup( Setup ) def GetCompletions_IdentifierCompleter_Works_test(): app = TestApp( ycmd.app ) @@ -91,7 +102,7 @@ def GetCompletions_IdentifierCompleter_Works_test(): @with_setup( Setup ) -def GetCompletions_ClangCompleter_Works_test(): +def GetCompletions_ClangCompleter_WorksWithExplicitFlags_test(): app = TestApp( ycmd.app ) contents = """ struct Foo { @@ -122,6 +133,45 @@ int main() CompletionEntryMatcher( 'y' ) ) ) +@with_setup( Setup ) +def GetCompletions_ClangCompleter_UnknownExtraConfException_test(): + app = TestApp( ycmd.app ) + filepath = PathToTestFile( 'basic.cpp' ) + completion_data = BuildRequest( filepath = filepath, + filetype = 'cpp', + contents = open( filepath ).read(), + force_semantic = True ) + + response = app.post_json( '/completions', + completion_data, + expect_errors = True ) + + eq_( response.status_code, httplib.INTERNAL_SERVER_ERROR ) + assert_that( response.json, + has_entry( 'exception', + has_entry( 'TYPE', UnknownExtraConf.__name__ ) ) ) + + +@with_setup( Setup ) +def GetCompletions_ClangCompleter_WorksWhenExtraConfExplicitlyAllowed_test(): + app = TestApp( ycmd.app ) + app.post_json( '/load_extra_conf_file', + { 'filepath': PathToTestFile( '.ycm_extra_conf.py' ) } ) + + filepath = PathToTestFile( 'basic.cpp' ) + completion_data = BuildRequest( filepath = filepath, + filetype = 'cpp', + contents = open( filepath ).read(), + line_num = 10, + column_num = 6, + start_column = 6 ) + + results = app.post_json( '/completions', completion_data ).json + assert_that( results, has_items( CompletionEntryMatcher( 'c' ), + CompletionEntryMatcher( 'x' ), + CompletionEntryMatcher( 'y' ) ) ) + + @with_setup( Setup ) def GetCompletions_ForceSemantic_Works_test(): app = TestApp( ycmd.app ) diff --git a/python/ycm/server/tests/testdata/basic.cpp b/python/ycm/server/tests/testdata/basic.cpp new file mode 100644 index 00000000..a20e2dd0 --- /dev/null +++ b/python/ycm/server/tests/testdata/basic.cpp @@ -0,0 +1,13 @@ +struct Foo { + int x; + int y; + char c; +}; + +int main() +{ + Foo foo; + // The location after the dot is line 11, col 7 + foo. +} + diff --git a/python/ycm/server/ycmd.py b/python/ycm/server/ycmd.py index 889b2b7d..0619c193 100755 --- a/python/ycm/server/ycmd.py +++ b/python/ycm/server/ycmd.py @@ -36,17 +36,19 @@ utils.AddThirdPartyFoldersToSysPath() import logging import json import bottle +import argparse +import httplib from bottle import run, request, response import server_state from ycm import user_options_store from ycm.server.responses import BuildExceptionResponse -import argparse -import httplib +from ycm import extra_conf_store # num bytes for the request body buffer; request.json only works if the request # size is less than this bottle.Request.MEMFILE_MAX = 300 * 1024 +# TODO: rename these to _lower_case SERVER_STATE = None LOGGER = None app = bottle.Bottle() @@ -72,7 +74,6 @@ def EventNotification(): return _JsonResponse( response_data ) - @app.post( '/run_completer_command' ) def RunCompleterCommand(): LOGGER.info( 'Received command request' ) @@ -141,6 +142,13 @@ def GetDetailedDiagnostic(): return _JsonResponse( completer.GetDetailedDiagnostic( request_data ) ) +@app.post( '/load_extra_conf_file' ) +def LoadExtraConfFile(): + LOGGER.info( 'Received extra conf load request' ) + request_data = request.json + extra_conf_store.Load( request_data[ 'filepath' ], force = True ) + + @app.post( '/debug_info' ) def DebugInfo(): # This can't be at the top level because of possible extra conf preload @@ -167,13 +175,19 @@ def DebugInfo(): # The type of the param is Bottle.HTTPError @app.error( httplib.INTERNAL_SERVER_ERROR ) def ErrorHandler( httperror ): - return _JsonResponse( BuildExceptionResponse( str( httperror.exception ), + return _JsonResponse( BuildExceptionResponse( httperror.exception, httperror.traceback ) ) def _JsonResponse( data ): response.set_header( 'Content-Type', 'application/json' ) - return json.dumps( data ) + return json.dumps( data, default = _UniversalSerialize ) + + +def _UniversalSerialize( obj ): + serialized = obj.__dict__.copy() + serialized[ 'TYPE' ] = type( obj ).__name__ + return serialized def _GetCompleterForRequestData( request_data ): @@ -205,6 +219,7 @@ def SetServerStateToDefaults(): LOGGER = logging.getLogger( __name__ ) user_options_store.LoadDefaults() SERVER_STATE = server_state.ServerState( user_options_store.GetAll() ) + extra_conf_store.Reset() def Main():