René's URL Explorer Experiment

Title: In handle_process_output don't forward finalizer result by EliahKagan · Pull Request #1788 · gitpython-developers/GitPython · GitHub

Open Graph Title: In handle_process_output don't forward finalizer result by EliahKagan · Pull Request #1788 · gitpython-developers/GitPython

X Title: In handle_process_output don't forward finalizer result by EliahKagan · Pull Request #1788 · gitpython-developers/GitPython

Description: The git.cmd.handle_process_output function is non-public (git.cmd.__all__ only lists Git) but used throughout GitPython and referenced in the git.util.RemoteProgress.new_message_handler docstring. Its finalizer argument is annotated to accept an optional callable that always returns None. It is always used this way in GitPython and RemoteProcess.new_message_handler is annotated accordingly. However, the handle_process_output docstring and implementation had documented the return value as the result of the finalizer, and the implementation had forwarded that result if passed. This modifies the docstring and implementation to disregard any result, in accordance with the everywhere-annotated assumption that the finalizer is conceptually void and the absence of any code in GitPython that uses the result of calling handle_process_output. None is now implicitly returned, simplifying the implementation and bringing it and the docstring in line with annotations and usage. This would be a breaking change if done on a public function, but because handle_process_output is nonpublic (and documentation for it is omitted by Sphinx, so users probably don't wrongly think it is public), I believe this is safe and non-breaking. This builds on #1755, which mentioned this issue but did not fix it. However, there is another way to fix it, which is what would have to be done instead of this if the function were public, and which I think is what should be done if, for any reason, it is intended that it continue to forward the result of a finalizer in the event that a finalizer ever returns one. The implementation could be put back to what it was before or something similar to it, and the annotations could be updated to allow the function to return a value. It occurs to me that this may have been what was intended all along, and that the annotations might not have reflected it due to the absence of @overload support in some type checker that was in use at the time, or perhaps unawareness of it. (GitPython is using @overload elsewhere at present.) I was not sure which approach should be taken to fix the inconsistency in handle_process_output, so I chose the simpler approach of not forwarding the result. However, if you would prefer to retain the old behavior (even aspects of it that are not in use in GitPython) and instead fix the annotations, I'd be pleased to do so. In that case, either this pull request could be closed and a new one created, or I could add another commit to it for the change.

Open Graph Description: The git.cmd.handle_process_output function is non-public (git.cmd.__all__ only lists Git) but used throughout GitPython and referenced in the git.util.RemoteProgress.new_message_handler docstring. ...

X Description: The git.cmd.handle_process_output function is non-public (git.cmd.__all__ only lists Git) but used throughout GitPython and referenced in the git.util.RemoteProgress.new_message_handler docstring. ...

Opengraph URL: https://github.com/gitpython-developers/GitPython/pull/1788

X: @github

direct link

Domain: github.com

Links:

Viewport: width=device-width