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

repo.blame should allow passing None as the rev parameter. #1835

Closed
Gaubbe opened this issue Feb 21, 2024 · 3 comments · Fixed by #1846
Closed

repo.blame should allow passing None as the rev parameter. #1835

Gaubbe opened this issue Feb 21, 2024 · 3 comments · Fixed by #1846

Comments

@Gaubbe
Copy link
Contributor

Gaubbe commented Feb 21, 2024

This issue only affects the type hint given to the rev parameter, as passing None to the function does work and gives the expected behavior, which is running the git blame -p -- <file> command without a revision, which gives back the blame including changes to the local working directory that haven't been committed yet.

While the code works at runtime, language servers such as pylance give back an error, as None is not a valid type according to the type hints.

@Byron
Copy link
Member

Byron commented Feb 21, 2024

Thanks for sharing your finding!

It does seems to make sense to adjust the annotations to explicitly allow this use-case.

@Gaubbe
Copy link
Contributor Author

Gaubbe commented Feb 22, 2024

I'll gladly make a pull request with the fix if you or other contributors don't have anything more to say about this issue.

Are there any other places in the code that comes to mind for which such a fix could also make sense? The only other git command that I use without specifiying a revision is git diff, but the Diffable interface seems to already support passing None as the other parameter.

@Byron
Copy link
Member

Byron commented Feb 22, 2024

Thanks for offering! It's OK to keep the PR focussed on the problem at hand.

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

Successfully merging a pull request may close this issue.

2 participants