From 586021c0450593172827d54b766c70f692ef253b Mon Sep 17 00:00:00 2001 From: nfnty Date: Mon, 1 Aug 2016 01:35:47 +0200 Subject: VCS: git: Fix log output containing control characters, Fixes #641 --- ranger/ext/vcs/git.py | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/ranger/ext/vcs/git.py b/ranger/ext/vcs/git.py index 8f4d9ff8..0fbd6496 100644 --- a/ranger/ext/vcs/git.py +++ b/ranger/ext/vcs/git.py @@ -4,13 +4,19 @@ """Git module""" from datetime import datetime -import json import os import re +import unicodedata from .vcs import Vcs, VcsError +def string_control_replace(string, replacement): + """Replace all unicode control characters with replacement""" + return ''.join( + (replacement if unicodedata.category(char)[0] == 'C' else char for char in string)) + + class Git(Vcs): """VCS implementation for Git""" _status_translations = ( @@ -40,16 +46,7 @@ class Git(Vcs): def _log(self, refspec=None, maxres=None, filelist=None): """Returns an array of dicts containing revision info for refspec""" - args = [ - '--no-pager', 'log', - '--pretty={' - '%x00short%x00:%x00%h%x00,' - '%x00revid%x00:%x00%H%x00,' - '%x00author%x00:%x00%an <%ae>%x00,' - '%x00date%x00:%ct,' - '%x00summary%x00:%x00%s%x00' - '}' - ] + args = ['--no-pager', 'log', '--pretty=%h%x00%H%x00%an <%ae>%x00%ct%x00%s%x00%x00'] if refspec: args += ['-1', refspec] elif maxres: @@ -58,18 +55,22 @@ class Git(Vcs): args += ['--'] + filelist try: - output = self._run(args).rstrip('\n') + output = self._run(args) except VcsError: return None if not output: return None log = [] - for line in output\ - .replace('\\', '\\\\').replace('"', '\\"').replace('\x00', '"').split('\n'): - line = json.loads(line) - line['date'] = datetime.fromtimestamp(line['date']) - log.append(line) + for line in output.rstrip('\x00\x00\n').split('\x00\x00\n'): + commit_hash_abbrev, commit_hash, author, timestamp, subject = line.split('\x00') + log.append({ + 'short': commit_hash_abbrev, + 'revid': commit_hash, + 'author': string_control_replace(author, ' '), + 'date': datetime.fromtimestamp(int(timestamp)), + 'summary': string_control_replace(subject, ' '), + }) return log def _status_translate(self, code): -- cgit 1.4.1-2-gfad0 From 1a2145bb3a605c52c25b3b54b15351b6bae922a9 Mon Sep 17 00:00:00 2001 From: nfnty Date: Mon, 8 Aug 2016 19:38:30 +0200 Subject: VCS: Add _run rstrip_newline Ensure that only one newline is rstripped --- ranger/ext/vcs/bzr.py | 22 ++++++++++---------- ranger/ext/vcs/git.py | 56 +++++++++++++++++++++++++-------------------------- ranger/ext/vcs/hg.py | 10 ++++----- ranger/ext/vcs/svn.py | 25 +++++++++++------------ ranger/ext/vcs/vcs.py | 11 ++++++++-- 5 files changed, 64 insertions(+), 60 deletions(-) diff --git a/ranger/ext/vcs/bzr.py b/ranger/ext/vcs/bzr.py index 5decc8b1..d0db1faf 100644 --- a/ranger/ext/vcs/bzr.py +++ b/ranger/ext/vcs/bzr.py @@ -25,7 +25,7 @@ class Bzr(Vcs): def _remote_url(self): """Remote url""" try: - return self._run(['config', 'parent_location']).rstrip('\n') or None + return self._run(['config', 'parent_location']) or None except VcsError: return None @@ -87,29 +87,29 @@ class Bzr(Vcs): statuses = set() # Paths with status - output = self._run(['status', '--short', '--no-classify']).rstrip('\n') - if not output: + lines = self._run(['status', '--short', '--no-classify']).split('\n') + if not lines: return 'sync' - for line in output.split('\n'): + for line in lines: statuses.add(self._status_translate(line[:2])) for status in self.DIRSTATUSES: if status in statuses: return status + return 'sync' def data_status_subpaths(self): statuses = {} # Ignored - output = self._run(['ls', '--null', '--ignored']).rstrip('\x00') - if output: - for path in output.split('\x00'): - statuses[path] = 'ignored' + paths = self._run(['ls', '--null', '--ignored']).split('\0')[:-1] + for path in paths: + statuses[path] = 'ignored' # Paths with status - output = self._run(['status', '--short', '--no-classify']).rstrip('\n') - for line in output.split('\n'): + lines = self._run(['status', '--short', '--no-classify']).split('\n') + for line in lines: statuses[os.path.normpath(line[4:])] = self._status_translate(line[:2]) return statuses @@ -121,7 +121,7 @@ class Bzr(Vcs): def data_branch(self): try: - return self._run(['nick']).rstrip('\n') or None + return self._run(['nick']) or None except VcsError: return None diff --git a/ranger/ext/vcs/git.py b/ranger/ext/vcs/git.py index 0fbd6496..06e066d2 100644 --- a/ranger/ext/vcs/git.py +++ b/ranger/ext/vcs/git.py @@ -36,13 +36,13 @@ class Git(Vcs): def _head_ref(self): """Returns HEAD reference""" - return self._run(['symbolic-ref', self.HEAD]).rstrip('\n') or None + return self._run(['symbolic-ref', self.HEAD]) or None def _remote_ref(self, ref): """Returns remote reference associated to given ref""" if ref is None: return None - return self._run(['for-each-ref', '--format=%(upstream)', ref]).rstrip('\n') or None + return self._run(['for-each-ref', '--format=%(upstream)', ref]) or None def _log(self, refspec=None, maxres=None, filelist=None): """Returns an array of dicts containing revision info for refspec""" @@ -62,8 +62,8 @@ class Git(Vcs): return None log = [] - for line in output.rstrip('\x00\x00\n').split('\x00\x00\n'): - commit_hash_abbrev, commit_hash, author, timestamp, subject = line.split('\x00') + for line in output[:-2].split('\0\0\n'): + commit_hash_abbrev, commit_hash, author, timestamp, subject = line.split('\0') log.append({ 'short': commit_hash_abbrev, 'revid': commit_hash, @@ -101,10 +101,10 @@ class Git(Vcs): # Paths with status skip = False - output = self._run(['status', '--porcelain', '-z']).rstrip('\x00') - if not output: + lines = self._run(['status', '--porcelain', '-z']).split('\0')[:-1] + if not lines: return 'sync' - for line in output.split('\x00'): + for line in lines: if skip: skip = False continue @@ -115,39 +115,37 @@ class Git(Vcs): for status in self.DIRSTATUSES: if status in statuses: return status + return 'sync' def data_status_subpaths(self): statuses = {} # Ignored directories - output = self._run([ + paths = self._run([ 'ls-files', '-z', '--others', '--directory', '--ignored', '--exclude-standard' - ]).rstrip('\x00') - if output: - for path in output.split('\x00'): - if path.endswith('/'): - statuses[os.path.normpath(path)] = 'ignored' + ]).split('\0')[:-1] + for path in paths: + if path.endswith('/'): + statuses[os.path.normpath(path)] = 'ignored' # Empty directories - output = self._run( - ['ls-files', '-z', '--others', '--directory', '--exclude-standard']).rstrip('\x00') - if output: - for path in output.split('\x00'): - if path.endswith('/'): - statuses[os.path.normpath(path)] = 'none' + paths = self._run( + ['ls-files', '-z', '--others', '--directory', '--exclude-standard']).split('\0')[:-1] + for path in paths: + if path.endswith('/'): + statuses[os.path.normpath(path)] = 'none' # Paths with status - output = self._run(['status', '--porcelain', '-z', '--ignored']).rstrip('\x00') - if output: - skip = False - for line in output.split('\x00'): - if skip: - skip = False - continue - statuses[os.path.normpath(line[3:])] = self._status_translate(line[:2]) - if line.startswith('R'): - skip = True + lines = self._run(['status', '--porcelain', '-z', '--ignored']).split('\0')[:-1] + skip = False + for line in lines: + if skip: + skip = False + continue + statuses[os.path.normpath(line[3:])] = self._status_translate(line[:2]) + if line.startswith('R'): + skip = True return statuses diff --git a/ranger/ext/vcs/hg.py b/ranger/ext/vcs/hg.py index cf15e35e..21460975 100644 --- a/ranger/ext/vcs/hg.py +++ b/ranger/ext/vcs/hg.py @@ -36,7 +36,7 @@ class Hg(Vcs): args += ['--'] + filelist try: - output = self._run(args).rstrip('\n') + output = self._run(args) except VcsError: return None if not output: @@ -56,13 +56,13 @@ class Hg(Vcs): def _remote_url(self): """Remote url""" try: - return self._run(['showconfig', 'paths.default']).rstrip('\n') or None + return self._run(['showconfig', 'paths.default']) or None except VcsError: return None def _status_translate(self, code): """Translate status code""" - for code_x, status in self._status_translations: # pylint: disable=invalid-name + for code_x, status in self._status_translations: if code in code_x: return status return 'unknown' @@ -80,7 +80,7 @@ class Hg(Vcs): if filelist: args += filelist else: - args += self.rootvcs.status_subpaths.keys() + args += self.rootvcs.status_subpaths.keys() # pylint: disable=no-member self._run(args, catchout=False) # Data interface @@ -117,7 +117,7 @@ class Hg(Vcs): return 'unknown' def data_branch(self): - return self._run(['branch']).rstrip('\n') or None + return self._run(['branch']) or None def data_info(self, rev=None): if rev is None: diff --git a/ranger/ext/vcs/svn.py b/ranger/ext/vcs/svn.py index 1813f857..1f09cd52 100644 --- a/ranger/ext/vcs/svn.py +++ b/ranger/ext/vcs/svn.py @@ -36,7 +36,7 @@ class SVN(Vcs): args += ['--'] + filelist try: - output = self._run(args).rstrip('\n') + output = self._run(args) except VcsError: return None if not output: @@ -66,7 +66,7 @@ class SVN(Vcs): def _remote_url(self): """Remote url""" try: - output = self._run(['info', '--xml']).rstrip('\n') + output = self._run(['info', '--xml']) except VcsError: return None if not output: @@ -86,7 +86,7 @@ class SVN(Vcs): if filelist: args += filelist else: - args += self.rootvcs.status_subpaths.keys() + args += self.rootvcs.status_subpaths.keys() # pylint: disable=no-member self._run(args, catchout=False) # Data Interface @@ -95,10 +95,10 @@ class SVN(Vcs): statuses = set() # Paths with status - output = self._run(['status']).rstrip('\n') - if not output: + lines = self._run(['status']).split('\n') + if not lines: return 'sync' - for line in output.split('\n'): + for line in lines: code = line[0] if code == ' ': continue @@ -113,13 +113,12 @@ class SVN(Vcs): statuses = {} # Paths with status - output = self._run(['status']).rstrip('\n') - if output: - for line in output.split('\n'): - code, path = line[0], line[8:] - if code == ' ': - continue - statuses[os.path.normpath(path)] = self._status_translate(code) + lines = self._run(['status']).split('\n') + for line in lines: + code, path = line[0], line[8:] + if code == ' ': + continue + statuses[os.path.normpath(path)] = self._status_translate(code) return statuses diff --git a/ranger/ext/vcs/vcs.py b/ranger/ext/vcs/vcs.py index 487c9405..9c7be653 100644 --- a/ranger/ext/vcs/vcs.py +++ b/ranger/ext/vcs/vcs.py @@ -109,7 +109,8 @@ class Vcs(object): # pylint: disable=too-many-instance-attributes # Generic - def _run(self, args, path=None, catchout=True, retbytes=False): + def _run(self, args, path=None, # pylint: disable=too-many-arguments + catchout=True, retbytes=False, rstrip_newline=True): """Run a command""" cmd = [self.repotype] + args if path is None: @@ -119,7 +120,13 @@ class Vcs(object): # pylint: disable=too-many-instance-attributes try: if catchout: output = subprocess.check_output(cmd, cwd=path, stderr=devnull) - return output if retbytes else output.decode('UTF-8') + if retbytes: + return output + else: + output = output.decode('UTF-8') + if rstrip_newline and output.endswith('\n'): + return output[:-1] + return output else: subprocess.check_call(cmd, cwd=path, stdout=devnull, stderr=devnull) except (subprocess.CalledProcessError, FileNotFoundError): -- cgit 1.4.1-2-gfad0