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

Diff and Patch interface refactored #346

Open
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@petrhosek
Contributor

petrhosek commented Feb 15, 2014

This is a complete refactoring of the diff and patch interface. The changes include:

  • Splitting Diff and Patch into separate classes and files, including respective tests. This largely follwos the recent development in libgit2.
  • Introducing DiffDelta, DiffFile and DiffLine classes and their respective iterators.
  • Moving the recently introduced Blob diff methods into patch again to be in line with the respective libgit2 interface.

The biggest difference, apart from the code cleanup, is the lazy evaluation due to heavy use of iterators rather than evaluating everything ahead of time as in case of the existing code, which tends to be slow on larger projects.

I have tried to make the interface as "Pythonic" as possible, while closely following the underlying libgit2 C interface. However, some of you might think of a better abstraction and I'd happy to incorporate further changes into the patch.

@jdavid

This comment has been minimized.

Member

jdavid commented Feb 16, 2014

This is a pretty large commit, if you could split it the review process would be easier and faster.

@alexband

This comment has been minimized.

alexband commented Feb 18, 2014

👍

@jdavid

This comment has been minimized.

Member

jdavid commented Jul 18, 2014

If you are still interested, it would be best to rewrite the whole patch/diff stuff with cffi.

@petrhosek

This comment has been minimized.

Contributor

petrhosek commented Aug 21, 2014

I have reimplemented both patch and diff with CFFI as suggested.

@alexband

This comment has been minimized.

alexband commented Aug 22, 2014

just 🆒

@jdavid

This comment has been minimized.

Member

jdavid commented Aug 22, 2014

Hello Petr,

Ok, this is still a huge PR, but I have started to look at it, it will take time though.

Thanks!

@petrhosek

This comment has been minimized.

Contributor

petrhosek commented Aug 22, 2014

Thanks, please let me know if there is anything you want me to change.

jdavid added a commit that referenced this pull request Jan 23, 2015

jdavid added a commit that referenced this pull request Jan 26, 2015

jdavid added a commit that referenced this pull request Jan 29, 2015

jdavid added a commit that referenced this pull request Jan 30, 2015

jdavid added a commit that referenced this pull request Feb 12, 2015

New DiffLine
Comes from PR #346

One difference, DiffLine.origin is a T_CHAR instead of T_OBJECT

jdavid added a commit that referenced this pull request Mar 30, 2015

@v6

This comment has been minimized.

v6 commented Sep 16, 2015

// , This pull request is stale, no?

@jdavid

This comment has been minimized.

Member

jdavid commented Sep 17, 2015

Yes, too big of a PR to review. I tried to redo bit by bit and made some commits, but don't have time to finish.

Feel free to propose changes (PRs) concerning Diff if you wish, but avoid huge PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment