Skip to content

Make git.cmd.Git.CatFileContentStream iterable #1110

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

Merged
merged 1 commit into from
Jan 17, 2021

Conversation

Hex052
Copy link
Contributor

@Hex052 Hex052 commented Jan 16, 2021

Add next method to git.cmd.Git.CatFileContentStream, so it can actually be used as an iterable

Add __next__ method to git.cmd.Git.CatFileContentStream, so it can actually be used as an iterable
@Byron
Copy link
Member

Byron commented Jan 16, 2021

How does an iterator indicate 'end of iteration'? If I remember correctly, it needs to throw a special exception which would break the API of 'next()'.
If iteration should be implemented, that could of course be done but should not break existing APIs.
Maybe I am missing something though.

@Hex052
Copy link
Contributor Author

Hex052 commented Jan 16, 2021

To end iteration, an iterator needs to raise the exception StopIteration, which is currently done in the next() method. To avoid breaking anything that might possibly use the existing next() method, I just added a new function that called the existing one. The existing function will only raise StopIteration at the very end (not on blank lines) since readline() does not strip any trailing newlines.

I added __next__ because python requires both __iter__ and __next__ in the iterator.

Hopefully that clears it up?

@Byron Byron added this to the v3.1.13 - Bugfixes milestone Jan 17, 2021
@Byron
Copy link
Member

Byron commented Jan 17, 2021

Thanks a lot for filling in the blanks :) and for making this contribution!

@Byron Byron merged commit 4a1339a into gitpython-developers:master Jan 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants