On 20 Apr 2016, at 20:30, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Sebastian Schuberth <sschuberth@xxxxxxxxx> writes: > >> Why do we need to remove the preamble at all, if present? Because I need the content of a valid Git LFS pointer file a few lines below: https://github.com/larsxschneider/git/blob/5ee7601c49b6eaa9da5eb47db5cf12b5d8d672c9/git-p4.py#L1085 This pointer file content is then written to the Git repository instead of the actual file content. >> If all we >> want is the oid, we should simply only look at the line that starts >> with that keyword, which would skip any preamble. Which is what you >> already do here. However, I'd probably use .splitlines() instead of >> .split('\n') and .startswith('oid ') (note the trailing space) instead >> of .startswith('oid') to ensure "oid" is a separate word. >> >> But then again, I wonder why there's so much split() logic involved in >> extracting the oid. Couldn't we replace all of that with a regexp like >> >> oid = re.search(r"^oid \w+:(\w+)", pointerFile, re.MULTILINE).group(1) > > Yup, all of that is a very useful suggestion. If we know how the > piece of information we want is identified in the output, > specifically looking for it would future-proof the code better, as > it will not be affected by future change that adds unexpected cruft > to the output we are reading from. I agree that this is a better solution. @Junio: Can you squash the patch below or do you prefer a v3? Thanks, Lars diff --git a/git-p4.py b/git-p4.py index ab52bc3..a0d529b 100755 --- a/git-p4.py +++ b/git-p4.py @@ -1073,8 +1073,7 @@ class GitLFS(LargeFileSystem): if pointerFile.startswith(preamble): pointerFile = pointerFile[len(preamble):] - oidEntry = [i for i in pointerFile.split('\n') if i.startswith('oid')] - oid = oidEntry[0].split(' ')[1].split(':')[1] + oid = re.search(r'^oid \w+:(\w+)', pointerFile, re.MULTILINE).group(1) localLargeFile = os.path.join( os.getcwd(), '.git', 'lfs', 'objects', oid[:2], oid[2:4],-- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html