Skip to content

In handle_process_output don't forward finalizer result #1788

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
Dec 26, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
In handle_process_output don't forward finalizer result
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 annotationd 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.
  • Loading branch information
EliahKagan committed Dec 25, 2023
commit 7057b9b81d69a734217b2ee3d6f817c12e3d880a
5 changes: 1 addition & 4 deletions git/cmd.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,6 @@ def handle_process_output(

This function returns once the finalizer returns.

:return: Result of finalizer
:param process: :class:`subprocess.Popen` instance
:param stdout_handler: f(stdout_line_string), or None
:param stderr_handler: f(stderr_line_string), or None
Expand Down Expand Up @@ -205,9 +204,7 @@ def pump_stream(
stderr_handler(error_str) # type: ignore

if finalizer:
return finalizer(process)
else:
return None
finalizer(process)


def dashify(string: str) -> str:
Expand Down