Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

python-unidiff silently fails on string objects #26

Open
vmalloc opened this issue Jul 26, 2016 · 7 comments
Open

python-unidiff silently fails on string objects #26

vmalloc opened this issue Jul 26, 2016 · 7 comments

Comments

@vmalloc
Copy link

vmalloc commented Jul 26, 2016

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)

@vmalloc vmalloc changed the title python-unidiff does not work on Python 3.x python-unidiff silently fails on string objects Jul 26, 2016
@matiasb
Copy link
Owner

matiasb commented Jul 30, 2016

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).

@MaxBittker
Copy link
Contributor

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?

@MaxBittker
Copy link
Contributor

update: PatchSet.from_string(diff_data) does work!

@MaxBittker
Copy link
Contributor

(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?

@matiasb
Copy link
Owner

matiasb commented Jun 2, 2017

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.

@MaxBittker
Copy link
Contributor

can probably close this, may want to document the new usage

@earonesty
Copy link

earonesty commented Jan 4, 2021

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants