Merge pull request #917 from ZyX-I/fix-powerline-config-shell

Refactor powerline to create loggers and use run_cmd in powerline-config

powerline/__init__.py

@@ -13,7 +13,7 @@ from powerline.config import DEFAULT_SYSTEM_CONFIG_DIR
 from threading import Lock, Event
 
 
-def find_config_file(search_paths, config_file):
+def _find_config_file(search_paths, config_file):
 	config_file += '.json'
 	for path in search_paths:
 		config_file_path = os.path.join(path, config_file)
@@ -83,6 +83,143 @@ def get_fallback_logger():
 	return _fallback_logger
 
 
+def _generate_change_callback(lock, key, dictionary):
+	def on_file_change(path):
+		with lock:
+			dictionary[key] = True
+	return on_file_change
+
+
+def get_config_paths():
+	'''Get configuration paths from environment variables.
+
+	Uses $XDG_CONFIG_HOME and $XDG_CONFIG_DIRS according to the XDG specification.
+
+	:return: list of paths
+	'''
+	config_home = os.environ.get('XDG_CONFIG_HOME', os.path.join(os.path.expanduser('~'), '.config'))
+	config_path = os.path.join(config_home, 'powerline')
+	config_paths = [config_path]
+	config_dirs = os.environ.get('XDG_CONFIG_DIRS', DEFAULT_SYSTEM_CONFIG_DIR)
+	if config_dirs is not None:
+		config_paths.extend([os.path.join(d, 'powerline') for d in config_dirs.split(':')])
+	plugin_path = os.path.join(os.path.realpath(os.path.dirname(__file__)), 'config_files')
+	config_paths.append(plugin_path)
+	return config_paths
+
+
+def generate_config_finder(get_config_paths=get_config_paths):
+	'''Generate find_config_file function
+
+	This function will find .json file given its path.
+
+	:param function get_config_paths:
+		Function that being called with no arguments will return a list of paths 
+		that should be searched for configuration files.
+
+	:return:
+		Function that being given configuration file name will return full path 
+		to it or raise IOError if it failed to find the file.
+	'''
+	config_paths = get_config_paths()
+	return lambda cfg_path: _find_config_file(config_paths, cfg_path)
+
+
+def load_config(cfg_path, find_config_file, config_loader, loader_callback=None):
+	'''Load configuration file and setup watches
+
+	Watches are only set up if loader_callback is not None.
+
+	:param str cfg_path:
+		Path for configuration file that should be loaded.
+	:param function find_config_file:
+		Function that finds configuration file. Check out the description of 
+		the return value of ``generate_config_finder`` function.
+	:param ConfigLoader config_loader:
+		Configuration file loader class instance.
+	:param function loader_callback:
+		Function that will be called by config_loader when change to 
+		configuration file is detected.
+
+	:return: Configuration file contents.
+	'''
+	try:
+		path = find_config_file(cfg_path)
+	except IOError:
+		if loader_callback:
+			config_loader.register_missing(find_config_file, loader_callback, cfg_path)
+		raise
+	else:
+		if loader_callback:
+			config_loader.register(loader_callback, path)
+		return config_loader.load(path)
+
+
+def _get_log_handler(common_config):
+	'''Get log handler.
+
+	:param dict common_config:
+		Configuration dictionary used to create handler.
+
+	:return: logging.Handler subclass.
+	'''
+	log_file = common_config['log_file']
+	if log_file:
+		log_file = os.path.expanduser(log_file)
+		log_dir = os.path.dirname(log_file)
+		if not os.path.isdir(log_dir):
+			os.mkdir(log_dir)
+		return logging.FileHandler(log_file)
+	else:
+		return logging.StreamHandler()
+
+
+def create_logger(common_config):
+	'''Create logger according to provided configuration
+	'''
+	log_format = common_config['log_format']
+	formatter = logging.Formatter(log_format)
+
+	level = getattr(logging, common_config['log_level'])
+	handler = _get_log_handler(common_config)
+	handler.setLevel(level)
+	handler.setFormatter(formatter)
+
+	logger = logging.getLogger('powerline')
+	logger.setLevel(level)
+	logger.addHandler(handler)
+	return logger
+
+
+def finish_common_config(common_config):
+	'''Add default values to common config and expand ~ in paths
+
+	:param dict common_config:
+		Common configuration, as it was just loaded.
+
+	:return:
+		Copy of common configuration with all configuration keys and expanded 
+		paths.
+	'''
+	common_config = common_config.copy()
+	common_config.setdefault('paths', [])
+	common_config.setdefault('watcher', 'auto')
+	common_config.setdefault('log_level', 'WARNING')
+	common_config.setdefault('log_format', '%(asctime)s:%(levelname)s:%(message)s')
+	common_config.setdefault('term_truecolor', False)
+	common_config.setdefault('ambiwidth', 1)
+	common_config.setdefault('additional_escapes', None)
+	common_config.setdefault('reload_config', True)
+	common_config.setdefault('interval', None)
+	common_config.setdefault('log_file', None)
+
+	common_config['paths'] = [
+		os.path.expanduser(path) for path in common_config['paths']
+	]
+
+	return common_config
+
+
 class Powerline(object):
 	'''Main powerline class, entrance point for all powerline uses. Sets 
 	powerline up and loads the configuration.
@@ -132,16 +269,19 @@ class Powerline(object):
 		elif self.renderer_module[-1] == '.':
 			self.renderer_module = self.renderer_module[:-1]
 
-		config_paths = self.get_config_paths()
+		self.find_config_file = generate_config_finder(self.get_config_paths)
-		self.find_config_file = lambda cfg_path: find_config_file(config_paths, cfg_path)
 
 		self.cr_kwargs_lock = Lock()
-		self.create_renderer_kwargs = {
+		self.cr_kwargs = {}
-			'load_main': True,
+		self.cr_callbacks = {}
-			'load_colors': True,
+		for key in ('main', 'colors', 'colorscheme', 'theme'):
-			'load_colorscheme': True,
+			self.cr_kwargs['load_' + key] = True
-			'load_theme': True,
+			self.cr_callbacks[key] = _generate_change_callback(
-		}
+				self.cr_kwargs_lock,
+				'load_' + key,
+				self.cr_kwargs
+			)
+
 		self.shutdown_event = shutdown_event or Event()
 		self.config_loader = config_loader or ConfigLoader(shutdown_event=self.shutdown_event, run_once=run_once)
 		self.run_loader_update = False
@@ -182,35 +322,12 @@ class Powerline(object):
 
 				self.prev_common_config = self.common_config
 
-				self.common_config.setdefault('paths', [])
+				self.common_config = finish_common_config(self.common_config)
-				self.common_config.setdefault('watcher', 'auto')
-				self.common_config.setdefault('log_level', 'WARNING')
-				self.common_config.setdefault('log_format', '%(asctime)s:%(levelname)s:%(message)s')
-				self.common_config.setdefault('term_truecolor', False)
-				self.common_config.setdefault('ambiwidth', 1)
-				self.common_config.setdefault('additional_escapes', None)
-				self.common_config.setdefault('reload_config', True)
-				self.common_config.setdefault('interval', None)
-				self.common_config.setdefault('log_file', None)
-
-				self.common_config['paths'] = [
-					os.path.expanduser(path) for path in self.common_config['paths']
-				]
 
 				self.import_paths = self.common_config['paths']
 
 				if not self.logger:
-					log_format = self.common_config['log_format']
+					self.logger = create_logger(self.common_config)
-					formatter = logging.Formatter(log_format)
-
-					level = getattr(logging, self.common_config['log_level'])
-					handler = self.get_log_handler()
-					handler.setLevel(level)
-					handler.setFormatter(formatter)
-
-					self.logger = logging.getLogger('powerline')
-					self.logger.setLevel(level)
-					self.logger.addHandler(handler)
 
 				if not self.pl:
 					self.pl = PowerlineLogger(self.use_daemon_threads, self.logger, self.ext)
@@ -288,53 +405,28 @@ class Powerline(object):
 			else:
 				self.renderer = renderer
 
-	def get_log_handler(self):
-		'''Get log handler.
-
-		:param dict common_config:
-			Common configuration.
-
-		:return: logging.Handler subclass.
-		'''
-		log_file = self.common_config['log_file']
-		if log_file:
-			log_file = os.path.expanduser(log_file)
-			log_dir = os.path.dirname(log_file)
-			if not os.path.isdir(log_dir):
-				os.mkdir(log_dir)
-			return logging.FileHandler(log_file)
-		else:
-			return logging.StreamHandler()
-
 	@staticmethod
 	def get_config_paths():
 		'''Get configuration paths.
 
+		Should be overridden in subclasses in order to provide a way to override 
+		used paths.
+
 		:return: list of paths
 		'''
-		config_home = os.environ.get('XDG_CONFIG_HOME', os.path.join(os.path.expanduser('~'), '.config'))
+		return get_config_paths()
-		config_path = os.path.join(config_home, 'powerline')
-		config_paths = [config_path]
-		config_dirs = os.environ.get('XDG_CONFIG_DIRS', DEFAULT_SYSTEM_CONFIG_DIR)
-		if config_dirs is not None:
-			config_paths.extend([os.path.join(d, 'powerline') for d in config_dirs.split(':')])
-		plugin_path = os.path.join(os.path.realpath(os.path.dirname(__file__)), 'config_files')
-		config_paths.append(plugin_path)
-		return config_paths
 
 	def _load_config(self, cfg_path, type):
 		'''Load configuration and setup watches.'''
-		function = getattr(self, 'on_' + type + '_change')
+		return load_config(
-		try:
+			cfg_path,
-			path = self.find_config_file(cfg_path)
+			self.find_config_file,
-		except IOError:
+			self.config_loader,
-			self.config_loader.register_missing(self.find_config_file, function, cfg_path)
+			self.cr_callbacks[type]
-			raise
+		)
-		self.config_loader.register(function, path)
-		return self.config_loader.load(path)
 
 	def _purge_configs(self, type):
-		function = getattr(self, 'on_' + type + '_change')
+		function = self.cr_callbacks[type]
 		self.config_loader.unregister_functions(set((function,)))
 		self.config_loader.unregister_missing(set(((self.find_config_file, function),)))
 
@@ -392,23 +484,23 @@ class Powerline(object):
 		'''Updates/creates a renderer if needed.'''
 		if self.run_loader_update:
 			self.config_loader.update()
-		create_renderer_kwargs = None
+		cr_kwargs = None
 		with self.cr_kwargs_lock:
-			if self.create_renderer_kwargs:
+			if self.cr_kwargs:
-				create_renderer_kwargs = self.create_renderer_kwargs.copy()
+				cr_kwargs = self.cr_kwargs.copy()
-		if create_renderer_kwargs:
+		if cr_kwargs:
 			try:
-				self.create_renderer(**create_renderer_kwargs)
+				self.create_renderer(**cr_kwargs)
 			except Exception as e:
 				self.exception('Failed to create renderer: {0}', str(e))
 				if hasattr(self, 'renderer'):
 					with self.cr_kwargs_lock:
-						self.create_renderer_kwargs.clear()
+						self.cr_kwargs.clear()
 				else:
 					raise
 			else:
 				with self.cr_kwargs_lock:
-					self.create_renderer_kwargs.clear()
+					self.cr_kwargs.clear()
 
 	def render(self, *args, **kwargs):
 		'''Update/create renderer if needed and pass all arguments further to 
@@ -457,30 +549,9 @@ class Powerline(object):
 			self.renderer.shutdown()
 		except AttributeError:
 			pass
-		functions = (
+		functions = tuple(self.cr_callbacks.values())
-			self.on_main_change,
-			self.on_colors_change,
-			self.on_colorscheme_change,
-			self.on_theme_change,
-		)
 		self.config_loader.unregister_functions(set(functions))
-		self.config_loader.unregister_missing(set(((find_config_file, function) for function in functions)))
+		self.config_loader.unregister_missing(set(((self.find_config_file, function) for function in functions)))
-
-	def on_main_change(self, path):
-		with self.cr_kwargs_lock:
-			self.create_renderer_kwargs['load_main'] = True
-
-	def on_colors_change(self, path):
-		with self.cr_kwargs_lock:
-			self.create_renderer_kwargs['load_colors'] = True
-
-	def on_colorscheme_change(self, path):
-		with self.cr_kwargs_lock:
-			self.create_renderer_kwargs['load_colorscheme'] = True
-
-	def on_theme_change(self, path):
-		with self.cr_kwargs_lock:
-			self.create_renderer_kwargs['load_theme'] = True
 
 	def __enter__(self):
 		return self

powerline/bindings/config.py

@@ -8,6 +8,9 @@ import subprocess
 import re
 
 from powerline.config import TMUX_CONFIG_DIRECTORY
+from powerline.lib.config import ConfigLoader
+from powerline import generate_config_finder, load_config, create_logger, PowerlineLogger, finish_common_config
+from powerline.lib.shell import run_cmd
 
 
 TmuxVersionInfo = namedtuple('TmuxVersionInfo', ('major', 'minor', 'suffix'))
@@ -32,9 +35,9 @@ def run_tmux_command(*args):
 	_run_tmux(subprocess.check_call, args)
 
 
-def get_tmux_output(*args):
+def get_tmux_output(pl, *args):
 	'''Run tmux command and return its output'''
-	return _run_tmux(subprocess.check_output, args)
+	return _run_tmux(lambda cmd: run_cmd(pl, cmd), args)
 
 
 NON_DIGITS = re.compile('[^0-9]+')
@@ -42,8 +45,8 @@ DIGITS = re.compile('[0-9]+')
 NON_LETTERS = re.compile('[^a-z]+')
 
 
-def get_tmux_version():
+def get_tmux_version(pl):
-	version_string = get_tmux_output('-V')
+	version_string = get_tmux_output(pl, '-V')
 	_, version_string = version_string.split(' ')
 	version_string = version_string.strip()
 	major, minor = version_string.split('.')
@@ -96,7 +99,7 @@ def get_tmux_configs(version):
 			yield (fname, priority + file_version.minor * 10 + file_version.major * 10000)
 
 
-def source_tmux_files():
+def source_tmux_files(pl, args):
 	'''Source relevant version-specific tmux configuration files
 
 	Files are sourced in the following order:
@@ -104,6 +107,15 @@ def source_tmux_files():
 	* If files for same versions are to be sourced then first _minus files are 
 	  sourced, then _plus files and then files without _minus or _plus suffixes.
 	'''
-	version = get_tmux_version()
+	version = get_tmux_version(pl)
 	for fname, priority in sorted(get_tmux_configs(version), key=(lambda v: v[1])):
 		run_tmux_command('source', fname)
+
+
+def create_powerline_logger(args):
+	find_config_file = generate_config_finder()
+	config_loader = ConfigLoader(run_once=True)
+	config = load_config('config', find_config_file, config_loader)
+	common_config = finish_common_config(config['common'])
+	logger = create_logger(common_config)
+	return PowerlineLogger(use_daemon_threads=True, logger=logger, ext='config')

powerline/lint/__init__.py

@@ -1,7 +1,7 @@
 # vim:fileencoding=utf-8:noet
 
 from powerline.lint.markedjson import load
-from powerline import find_config_file, Powerline
+from powerline import generate_config_finder, get_config_paths
 from powerline.lib.config import load_json_config
 from powerline.lint.markedjson.error import echoerr, MarkedError
 from powerline.segments.vim import vim_modes
@@ -1035,7 +1035,8 @@ theme_spec = (Spec(
 
 
 def check(path=None, debug=False):
-	search_paths = [path] if path else Powerline.get_config_paths()
+	search_paths = [path] if path else get_config_paths()
+	find_config_file = generate_config_finder(lambda: search_paths)
 
 	logger = logging.getLogger('powerline-lint')
 	logger.setLevel(logging.DEBUG if debug else logging.ERROR)
@@ -1092,7 +1093,7 @@ def check(path=None, debug=False):
 
 	hadproblem = False
 	try:
-		main_config = load_json_config(find_config_file(search_paths, 'config'), load=load_config, open_file=open_file)
+		main_config = load_json_config(find_config_file('config'), load=load_config, open_file=open_file)
 	except IOError:
 		main_config = {}
 		sys.stderr.write('\nConfiguration file not found: config.json\n')
@@ -1108,7 +1109,7 @@ def check(path=None, debug=False):
 	import_paths = [os.path.expanduser(path) for path in main_config.get('common', {}).get('paths', [])]
 
 	try:
-		colors_config = load_json_config(find_config_file(search_paths, 'colors'), load=load_config, open_file=open_file)
+		colors_config = load_json_config(find_config_file('colors'), load=load_config, open_file=open_file)
 	except IOError:
 		colors_config = {}
 		sys.stderr.write('\nConfiguration file not found: colors.json\n')

scripts/powerline-config

@@ -12,7 +12,7 @@ except ImportError:
 
 
 TMUX_ACTIONS = {
-	'source': (lambda args: config.source_tmux_files()),
+	'source': config.source_tmux_files,
 }
 
 
@@ -30,4 +30,6 @@ if __name__ == '__main__':
 
 	args = parser.parse_args()
 
-	args.function(args)
+	pl = config.create_powerline_logger(args)
+
+	args.function(pl, args)

tests/lib/config_mock.py

@@ -22,7 +22,7 @@ def load_json_config(config_file_path, *args, **kwargs):
 		raise IOError(config_file_path)
 
 
-def find_config_file(config, search_paths, config_file):
+def _find_config_file(config, search_paths, config_file):
 	if config_file.endswith('raise') and config_file not in config:
 		raise IOError('fcf:' + config_file)
 	return config_file
@@ -114,7 +114,7 @@ class TestPowerline(Powerline):
 		return local_themes
 
 	def _will_create_renderer(self):
-		return self.create_renderer_kwargs
+		return self.cr_kwargs
 
 
 renderer = SimpleRenderer
@@ -158,7 +158,7 @@ def swap_attributes(cfg_container, powerline_module, replaces):
 	config_container = cfg_container
 	if not replaces:
 		replaces = {
-			'find_config_file': lambda *args: find_config_file(config_container['config'], *args),
+			'_find_config_file': lambda *args: _find_config_file(config_container['config'], *args),
 		}
 	for attr, val in replaces.items():
 		old_val = getattr(powerline_module, attr)