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

Diff and Patch interface refactored #346

Closed
wants to merge 2 commits into from

Conversation

petrhosek
Copy link
Contributor

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
Copy link
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
Copy link

👍

@jdavid
Copy link
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
Copy link
Contributor Author

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

@alexband
Copy link

just 🆒

@jdavid
Copy link
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
Copy link
Contributor Author

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
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
Commes from PR #346
@v6
Copy link

v6 commented Sep 16, 2015

// , This pull request is stale, no?

@jdavid
Copy link
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.

@jdavid
Copy link
Member

jdavid commented Nov 9, 2019

Closing, sorry it didn't go through at the time.

@jdavid jdavid closed this Nov 9, 2019
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.

None yet

4 participants