Fix mypy warning "Missing return statement"

[EliahKagan]

In git.index.base.IndexFile.from_tree, control was never able to fall off the end, but mypy reported a "Missing return statement" because it could not infer that the context manager didn't suppress any exceptions.

This was for two reasons. An ExitStack context manager suppresses exceptions in some uses and not others, depending on the context manager objects its enter_context method is called with. In this case, that was sufficient to produce the mypy warning regardless of other changes, so I converted it to nested with-statements. The nesting depth is only 2, so this is not really any worse. (I also reworded the comments for clarity and adjusted their spacing.)

The more important cause, however, could also produce this warning in code that uses GitPython, as git.index.util.TemporaryFileSwap is public. Its __exit__ method was annotated to return bool. This was itself reported by mypy as an error, because a context manager __exit__ method that never suppresses exceptions should either always (implicitly) return None and be annotated as such, or it can return False as this implementation does but should then have its return type annotated as Literal[False].

So this also changes TemporaryFileSwap.__exit__'s return type annotation from bool to Literal[False]. If this were nonpublic or being newly designed, it may be better for it to instead return None implicitly, but I think that would technically be a breaking change (though it would be very unusual, and not a good idea, for any code to ever rely on the distinction).

With both these changes made, both those mypy warnings go away, and code using GitPython will not get a warning if it makes similar direct use of TemporaryFileSwap.

(An alternative might be to dedent the return statement out of the with-statement in IndexFile.from_tree, but this would make less clear to readers that it does not suppress exceptions. Furthermore, the change to TemporaryFileSwap.__exit__ would still have to be made for mypy to consider it correctly typed.)

[Byron]

Thanks again for all your great work! GitPython seems to have a limitless supply of little and big issues to fix 😅.

I just noticed that the commit message is the PR message, but with markdown annotations removed. Personally, I'd love to think that soon we will have tools that will render commit messages as markdown to make having such annotations valuable, and even if not I find markdown easy-enough on the eye to look at it verbatim.

Please do feel free to write markdown in commit messages (and even subject lines) to have less work in cases like these.

[EliahKagan]

I just noticed that the commit message is the PR message, but with markdown annotations removed

This is descriptively accurate, but the sequence was that I wrote it without Markdown, and then added the Markdown later. I'm accustomed to writing commit messages with little to no Markdown, and sometimes it would actually be easier for me to do so even if adding Markdown later. (This is not the case for text in general; I don't wait to add Markdown later in most documents that will ultimately be Markdown.)

Please do feel free to write markdown in commit messages (and even subject lines) to have less work in cases like these.

Thanks. There are definitely also some situations where writing it once with Markdown will be easier, and I'll strive to remember that option.

---

This kind of a side issue, but one narrow situation where I've found it is very useful to put Markdown in a commit message (when the commit is going on GitHub) is when talking about a specific decorator and writing its name with @ preceded. Without enclosing it in backticks, a user with the same name as the decorator may be pinged, and the text in the commit message formatted accordingly when viewed on GitHub.

Likewise, one situation where I've found it more important than usual to put Markdown in a pull request description is when otherwise a name with leading or trailing double-underscores would appear without them and in bold. I can see that a benefit of writing pull request titles in Markdown (usually I don't) is when something like this appears in them. The titles appear unrendered in most places, but in automatically generated release notes they are rendered, so one gets things like "del" when one means "__del__".