Also, I notice you skip mypy because it produces errors. Why not pin the version and increment it periodically?

That's a great incentive, thanks so much! Using__fspath__() seems like it's using a private API, so it looks strange to my untrained eye. But if it's correct, I guess it's worth doing anyway?

Also, I notice you skip mypy because it produces errors. Why not pin the version and increment it periodically?

I don't see why this wouldn't work, great idea! Please feel free to set this up in your next PR :).

It seems that I won't get my copilot review here, so I wonder why you'd not be callingos.fspath(path) instead?

It seems that I won't get my copilot review here, so I wonder why you'd not be callingos.fspath(path) instead?
I agree that's much nicer and also gets rid of theif not isinstance(path, str) checks.

I don't see why this wouldn't work, great idea! Please feel free to set this up in your next PR :).

Sure thing

I've added a few tests, but lots of the calls work on internal APIs, so they won't make much difference. In these cases, callingos.fspath is still better as it makes the intention clearer. I also made a tool to check for redundant uses and the only one isgit/util.py:420.

I am putting the PR back to draft until there are tests that use the new "can handle paths encoding independently from the runtime encoding" to prove the changes are effective. Thanks again.

Sorry, I meant to add this comment with the changes I made yesterday:
I've gone through all the changes and made sure that they apply (and removed the ones that didn't). As a result, there are more tests for submodules and index, as well as the existing ones I added.

For example, theclone_from_pathlike test means that all thefs.path conversions inRepo.base are required and removing them will cause that test to fail. Is that what you mean by "can handle paths encoding independently from the runtime encoding"? If not, I can add tests that will do that.

No problem!

What I mean is that the point of this conversion is to prevent decoding issues related to paths. I.e. the user passes a filesystem path, but internally GitPython runsstr() on it which then tries to re-encode the path-bytes to the python runtime default encoding. This typically fails as soon as there is one non-ascii character.

You'd have to go back tomain and add such tests which should fail, to then show that this now works with the changes in this branch, for a particular scenario - like one of the ones I mentioned, but there might be more.

I've added some non-ASCII characters into the tests. However, the main point of the pull request is with objects that follow theos.Pathlike type hint, such as this:

importosfromdataclassesimportdataclass@dataclassclassCustomPathlike:path:strdef__fspath__(self)->str:returnself.pathcustom_pathlike=CustomPathlike("folder/file")str(custom_pathlike)# "CustomPathlike(path='folder/file')" - fails in "git add CustomPathlike(path='folder/file')"os.fspath(custom_pathlike)# "folder/file"  - works in "git add folder/file"

I realise that that was unclear in both the PR and the original issue (so I've added this example to the original issue).