python-unidiff
python-unidiff copied to clipboard
matiasb
python-unidiff silently fails on string objects
Tried:
>>> diff = requests.get('https://github.com/matiasb/python-unidiff/pull/3.diff').content.decode('utf-8')
>>> p = PatchSet(diff)
>>> p
<PatchSet: []>
I would expect some kind of error message, since this is very misleading (internally python-unidiff iterates char-by-char over the string, and thus finds no patches)
Yeah, I agree. Thanks for reporting the issue.
I guess you already worked-around the issue, but FTR until this gets fixed, PatchSet expects a file-like object (so you could use StringIO to build a file-like object from the string).
In [27]: pkg_resources.get_distribution("unidiff").version
Out[27]: '0.5.4'
In [28]: diff = requests.get('https://github.com/matiasb/python-unidiff/pull/3.diff').content.decode('utf-8')
In [29]: p = PatchSet(diff)
In [30]: p
Out[30]: <PatchSet: []>
This still isn't working for me on the new version, but piping from curl into the CLI version does. Any advice?
(I see now this was the intended behavior, but it is confusing) Would you accept a patch that infers the type in the constructor and acts accordingly?
Right, PatchSet still expects a file-like object, but there is a new constructor, from_string:
>>> diff = requests.get('https://github.com/matiasb/python-unidiff/pull/3.diff').content.decode('utf-8')
>>> p = PatchSet.from_string(diff)
>>> p
<PatchSet: [<PatchedFile: .gitignore>, <PatchedFile: unidiff/patch.py>, <PatchedFile: unidiff/utils.py>]>
But it's true that it could fail if there is no file, instead of returning an empty PatchSet.
A patch would be welcome! Just in case, it should keep backwards compatibility.
i generally prefer explicit factories (from_string, from_bytes) over inferred behavior. things like multiple inheritance, builtins, isinstance can be tricky/break things when automagically deducing argument types - especially where strings are involved
