Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork939
Add support for diffing against root commit#408
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| return s | ||
| elif isinstance(s, six.binary_type): | ||
| if PRE_PY27: | ||
| return s.decode(defenc) # we're screwed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
As I have never had too many contributors, there is no guide yet. However, would you consider removing this comment , possibly in favour of explaining why python prior to py2.6 is a problem ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
You're absolutely right here, this is inappropriate. Fixed inaae2a73.
| @@ -8,6 +8,7 @@ | |||
| # flake8: noqa | |||
| import sys | |||
| import six | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I think I looked very carefully, but couldn't find a new dependency. Is six available in Py2.7 and onwards by default ?
@nvie This is a nice one ! Thank so much for re-viving GitPython with your contributions. As you can see, I have left a few notes on the code itself, and am looking forward to what you think ! Something you could consider is to add new features to the the Last but definitely not least: Using the |
(I've reverted my mistake and force-pushed now to be a clean change again. Sorry for mixing this one up with the other feature.) |
Will do!
I can understand. Once everything is merged, I'll point our version to the latest master and make sure everything is stable. Then we can roll an update.
I'm using six here mostly to get a convenient import target for things like I'll address your feedback on this branch now. |
This enabled getting diff patches for root commits.
This alternative API does not prevent users from using the valid treeish"root".
I've rebased this work against the latest master and added commits to address your feedback. Let me know what you think! |
OK, I think this is done now, test cases fixed, and dependency on |
| return s.decode(defenc) # we're screwed | ||
| # Python 2.6 does not support the `errors` argument, so we cannot | ||
| # control the replacement of unsafe chars in it. | ||
| return s.decode(defenc) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Btw,@Byron, do you still want to keep supporting Python 2.6? This message is echoed when you run the Python 2.6 tests already:
py26 installed: DEPRECATION: Python 2.6 is no longer supported by the Python core team, please upgrade your Python. A future version of pip will drop support for Python 2.6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thanks for letting me know - under these circumstances it will be fine to drop support !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I just removed py2.6 support.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Cheers! 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
BTW: I was about to merge a few PRs, but work is ...catching up earlier than expected. Will try again tonight, and if these delays become unbearable to you, I'd love to make you a collaborator. Just let me know what you think.
Cheers :) !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
No worries, I've been there. Maintaining OSS libraries while also getting work done is a hard balance to strike. On the plus side, patching/maintaining this project is important for our team now, and we're committed to only invest in it more. So if you'd like to have a contributor on board, I'd be more than happy to help out there!
Since support was dropped.
This adds support for diff'ing against the initial commit (or any commit without parents, really). This is something that has classically been a bit hard to do since GitPython does not offer special support for this. (See#364, orhttp://stackoverflow.com/questions/33916648/get-the-diff-details-of-first-commit-in-gitpython)
I've opted to add this as another special case for the first argument to the
Commit.diff()call, right next to the currently allowedNone, theIndexclass, the commit SHA, etc. This plays nice since you'd never use this in combination with those existing options.It enables:
…which will produce a diff much like "git show <initial_commit>" would. Of course, it's possible to use a raw
repo.git.diff_tree()call for this, but that would return a raw string, not a nice DiffIndex object. The workaround suggesting to use the magic "empty tree sha" also does not work, as the diff output is then in the "reverse direction".Let me know what you think of this! Test cases included.