René's URL Explorer Experiment

Title: Fix TemporaryFileSwap regression where file_path could not be Path by EliahKagan · Pull Request #1776 · gitpython-developers/GitPython · GitHub

Open Graph Title: Fix TemporaryFileSwap regression where file_path could not be Path by EliahKagan · Pull Request #1776 · gitpython-developers/GitPython

X Title: Fix TemporaryFileSwap regression where file_path could not be Path by EliahKagan · Pull Request #1776 · gitpython-developers/GitPython

Description: As noted in #1770 (comment), in 9e86053 (#1770) I accidentally introduced a regression in git.index.util.TemporaryFileSwap where the file_path parameter could no longer be a Path, even though it was (and remained) annotated in a way compatible with that. Passing a Path object instead of str failed with TypeError, with a message like this (with WindowsPath instead of PosixPath on Windows, but otherwise the same): TypeError: unsupported operand type(s) for +: 'PosixPath' and 'str' Although much code that results in TemporaryFileSwap being used is under test, which would reveal some kinds of regressions, TemporaryFileSwap itself did not have any tests. It is public, so even if a Path were never passed to it in GitPython's own use of it, applications that use GitPython may be making direct use of it and are justified in assuming it accepts both a str and a str-based Path, as annotated. This pull request: Adds a general test of TemporaryFileSwap. It does not test every code path that is supposed to be supported, such as code paths involving the file not existing in the first place, or the temporary file being deleted before it would be swapped back (those are deliberately covered in the code and may thus be valuable to test, and perhaps I, or someone else, could add tests for that later). However, I parameterized the test by the type of the file_path argument passed to TemporaryFileSwap, and the test shows both that it works when the type is str and that it fails, as described above, when the type is Path. Fixes the bug, and also changes how mkstemp is called to make it less unidiomatic. (However, it is still opening the file to reserve its name, and then immediately closing it, so this does not address the original misgivings noted in #1770 (review). See my comment in that review thread.) There are some further details, which I'd consider less essential but potentially of value, in the commit messages.

Open Graph Description: As noted in #1770 (comment), in 9e86053 (#1770) I accidentally introduced a regression in git.index.util.TemporaryFileSwap where the file_path parameter could no longer be a Path, even though it wa...

X Description: As noted in #1770 (comment), in 9e86053 (#1770) I accidentally introduced a regression in git.index.util.TemporaryFileSwap where the file_path parameter could no longer be a Path, even though it wa...

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

X: @github

direct link

Domain: github.com

Links:

Viewport: width=device-width