Content-Length: 7388 | pFad | http://github.com/gitpython-developers/GitPython/pull/1755.patch

thub.com From 39e2dff35798a08a18a14c03311924def092860c Mon Sep 17 00:00:00 2001 From: Travis Runner Date: Sun, 3 Dec 2023 00:24:28 -0500 Subject: [PATCH] Don't return with operand when conceptually void This changes `return None` to `return` where it appears in functions that are "conceptually void," i.e. cases where a function always returns None, its purpose is never to provide a return value to the caller, and it would be a bug (or at least confusing style) to use the return value or have the call as a subexpression. This is already the prevailing style, so this is a consistency improvement, as well as improving clarity by avoiding giving the impression that "None" is significant in those cases. Of course, functions that sometimes return values other than None do not have "return None" replaced with a bare "return" statement. Those are left alone (when they return None, it means something). For the most part this is straightforward, but: - The handle_stderr local function in IndexFile.checkout is also slightly further refactored to better express what the early return is doing. - The "None" operand is removed as usual in GitConfigParser.read, even though a superclass has a same-named method with a return type that is not None. Usually when this happens, the superclass method returns something like Optional[Something], and it is a case of return type covariance, in which case the None operands should still be written. Here, however, the overriding method does not intend to be an override: it has an incompatible signature and cannot be called as if it were the superclass method, nor is its return type compatible. - The "None" operand is *retained* in git.cmd.handle_process_output even though the return type is annotated as None, because the function also returns the expression `finalizer(process)`. The argument `finalizer` is itself annotated to return None, but it looks like the intent may be to play along with the strange case that `finalizer` returns a non-None value, and forward it. --- git/cmd.py | 4 ++-- git/config.py | 9 +++++---- git/index/base.py | 8 ++++---- git/index/fun.py | 2 +- git/objects/tree.py | 2 +- git/objects/util.py | 4 ++-- git/util.py | 2 +- 7 files changed, 16 insertions(+), 15 deletions(-) diff --git a/git/cmd.py b/git/cmd.py index 4dc76556e..5f96e33c9 100644 --- a/git/cmd.py +++ b/git/cmd.py @@ -528,13 +528,13 @@ def _terminate(self) -> None: try: if proc.poll() is not None: self.status = self._status_code_if_terminate or proc.poll() - return None + return except OSError as ex: log.info("Ignored error after process had died: %r", ex) # It can be that nothing really exists anymore... if os is None or getattr(os, "kill", None) is None: - return None + return # Try to kill it. try: diff --git a/git/config.py b/git/config.py index 983d71e86..5708a7385 100644 --- a/git/config.py +++ b/git/config.py @@ -203,7 +203,8 @@ def __setitem__(self, key: str, value: _T) -> None: def add(self, key: str, value: Any) -> None: if key not in self: super().__setitem__(key, [value]) - return None + return + super().__getitem__(key).append(value) def setall(self, key: str, values: List[_T]) -> None: @@ -579,7 +580,7 @@ def read(self) -> None: # type: ignore[override] :raise IOError: If a file cannot be handled """ if self._is_initialized: - return None + return self._is_initialized = True files_to_read: List[Union[PathLike, IO]] = [""] @@ -697,7 +698,7 @@ def write(self) -> None: a file lock""" self._assure_writable("write") if not self._dirty: - return None + return if isinstance(self._file_or_files, (list, tuple)): raise AssertionError( @@ -711,7 +712,7 @@ def write(self) -> None: "Skipping write-back of configuration file as include files were merged in." + "Set merge_includes=False to prevent this." ) - return None + return # END stop if we have include files fp = self._file_or_files diff --git a/git/index/base.py b/git/index/base.py index 6c6462039..112ad3feb 100644 --- a/git/index/base.py +++ b/git/index/base.py @@ -166,7 +166,7 @@ def _set_cache_(self, attr: str) -> None: except OSError: # In new repositories, there may be no index, which means we are empty. self.entries: Dict[Tuple[PathLike, StageType], IndexEntry] = {} - return None + return # END exception handling try: @@ -1210,9 +1210,9 @@ def checkout( def handle_stderr(proc: "Popen[bytes]", iter_checked_out_files: Iterable[PathLike]) -> None: stderr_IO = proc.stderr if not stderr_IO: - return None # Return early if stderr empty. - else: - stderr_bytes = stderr_IO.read() + return # Return early if stderr empty. + + stderr_bytes = stderr_IO.read() # line contents: stderr = stderr_bytes.decode(defenc) # git-checkout-index: this already exists diff --git a/git/index/fun.py b/git/index/fun.py index eaf5f51ff..97f2c88e6 100644 --- a/git/index/fun.py +++ b/git/index/fun.py @@ -83,7 +83,7 @@ def run_commit_hook(name: str, index: "IndexFile", *args: str) -> None: """ hp = hook_path(name, index.repo.git_dir) if not os.access(hp, os.X_OK): - return None + return env = os.environ.copy() env["GIT_INDEX_FILE"] = safe_decode(str(index.path)) diff --git a/git/objects/tree.py b/git/objects/tree.py index 4d94a5d24..a08adf48b 100644 --- a/git/objects/tree.py +++ b/git/objects/tree.py @@ -68,7 +68,7 @@ def git_cmp(t1: TreeCacheTup, t2: TreeCacheTup) -> int: def merge_sort(a: List[TreeCacheTup], cmp: Callable[[TreeCacheTup, TreeCacheTup], int]) -> None: if len(a) < 2: - return None + return mid = len(a) // 2 lefthalf = a[:mid] diff --git a/git/objects/util.py b/git/objects/util.py index 3021fec39..3e8f8dcc9 100644 --- a/git/objects/util.py +++ b/git/objects/util.py @@ -500,8 +500,8 @@ def addToStack( depth: int, ) -> None: lst = self._get_intermediate_items(item) - if not lst: # empty list - return None + if not lst: # Empty list + return if branch_first: stack.extendleft(TraverseNT(depth, i, src_item) for i in lst) else: diff --git a/git/util.py b/git/util.py index 01cdcf773..aaf662060 100644 --- a/git/util.py +++ b/git/util.py @@ -644,7 +644,7 @@ def _parse_progress_line(self, line: AnyStr) -> None: self.line_dropped(line_str) # Note: Don't add this line to the other lines, as we have to silently # drop it. - return None + return # END handle op code # Figure out stage.

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier! Saves Data!