Skip to content

Conversation

@jnrbsn
Copy link
Contributor

@jnrbsn jnrbsn commented Mar 25, 2019

This is an attempt to finish up the work in PR #790 originally done by @erikvanzijst. Thanks to him for the initial work.

Patch.patch assumes all content to be encoded in UTF-8 and forcefully replaces any non-decodeable sequences. This can lead to corruption for content that either does not conform to any specific encoding altogether, or uses an encoding that is incompatible with, or ambiguous to UTF-8.

As discussed in #790, this change deprecates Patch.patch in favor of Patch.text and adds Patch.data, which returns the unmodified, raw bytes.

This is an attempt to finish up the work in PR #790 originally done by
@erikvanzijst. Thanks to him for the initial work.

Patch.patch assumes all content to be encoded in UTF-8 and forcefully
replaces any non-decodeable sequences. This can lead to corruption for
content that either does not conform to any specific encoding altogether,
or uses an encoding that is incompatible with, or ambiguous to UTF-8.

As discussed in #790, this change deprecates Patch.patch in favor of
Patch.text and adds Patch.data, which returns the unmodified, raw bytes.
@jnrbsn
Copy link
Contributor Author

jnrbsn commented Mar 25, 2019

@jdavid Can you take a look at this? I think I implemented what you described in #790.

@pypingou
Copy link

I'd argue this is not deprecating, it's purely replacing. Deprecating implies there is a warning and no API breakage (yet).

@pypingou
Copy link

You could keep .patch present have it emit a warning and make its output be the one of .text. This would keep backward compatibility and warn users about upcoming changes to the API.

@jnrbsn
Copy link
Contributor Author

jnrbsn commented Mar 26, 2019

@pypingou That's exactly what I already did. Patch.patch raises a DeprecationWarning and literally just calls Patch.text.

@pypingou
Copy link

Oh, I see it now, sorry about that I had misread the diff initially :(

@jdavid jdavid merged commit 8b07235 into libgit2:master Mar 30, 2019
netbsd-srcmastr referenced this pull request in NetBSD/pkgsrc May 1, 2019
0.28.1 (2019-04-19)
-------------------------

- Now works with pycparser 2.18 and above
  `#846 <https://github.com/libgit2/pygit2/issues/846>`_

- Now ``Repository.write_archive(..)`` keeps the file mode
  `#616 <https://github.com/libgit2/pygit2/issues/616>`_
  `#898 <https://github.com/libgit2/pygit2/pull/898>`_

- New ``Patch.data`` returns the raw contents of the patch as a byte string
  `#790 <https://github.com/libgit2/pygit2/pull/790>`_
  `#893 <https://github.com/libgit2/pygit2/pull/893>`_

- New ``Patch.text`` returns the contents of the patch as a text string,
  deprecates `Patch.patch`
  `#790 <https://github.com/libgit2/pygit2/pull/790>`_
  `#893 <https://github.com/libgit2/pygit2/pull/893>`_

Deprecations:

- ``Patch.patch`` is deprecated, use ``Patch.text`` instead
netbsd-srcmastr referenced this pull request in NetBSD/pkgsrc May 19, 2019
0.28.1 (2019-04-19)
-------------------------

- Now works with pycparser 2.18 and above
  `#846 <https://github.com/libgit2/pygit2/issues/846>`_

- Now ``Repository.write_archive(..)`` keeps the file mode
  `#616 <https://github.com/libgit2/pygit2/issues/616>`_
  `#898 <https://github.com/libgit2/pygit2/pull/898>`_

- New ``Patch.data`` returns the raw contents of the patch as a byte string
  `#790 <https://github.com/libgit2/pygit2/pull/790>`_
  `#893 <https://github.com/libgit2/pygit2/pull/893>`_

- New ``Patch.text`` returns the contents of the patch as a text string,
  deprecates `Patch.patch`
  `#790 <https://github.com/libgit2/pygit2/pull/790>`_
  `#893 <https://github.com/libgit2/pygit2/pull/893>`_

Deprecations:

- ``Patch.patch`` is deprecated, use ``Patch.text`` instead
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

Successfully merging this pull request may close these issues.

3 participants