Fix name collision

[NMertsch]

The bug

In git.config.MetaParserBuilder.__new__:

def __new__(name, bases, clsdict):  # <- `name` is the class name
    ...
    for name, method in methods:  # <- `name` is the current method name
        ...
    return super().__new__(cls, name, bases, clsdict)  # <- `name` is the name of the last method in clsdict

Downstream effects

• git.config.GitConfigParser.__name__ is "write", not "GitConfigParser"
• Sphinx autodoc misinterprets the nature of git.config.GitConfigParser as an alias of itself:
• 

Fixes #2023

[Byron]

Thanks, this PR would be great to land I am sure!

Do you have an intuition as to why CI is failing here?

[NMertsch]

Yes. There was an rst syntax error in CPython's configparser module: https://github.com/python/cpython/blob/v3.8.10/Lib/configparser.py#L768:

- `vars'
+ `vars`

configparser.RawConfigParser is the baseclass of GitConfigParser (the class which is now being documented due to the bugfix), so its get method (the one with the syntax error) is part of the public interface of GitConfigParser.

This only occurs in the pipelines for 3.8 and 3.9, so I guess it was fixed later but the fix was never backported.

I'm not sure what's the best way to handle this. I can think of multiple options, but I dislike all of them:

1. remove -W from SPHINXOPTS in doc/Makefile to not treat warnings as errors in the whole project.
2. modify the offending file in the Docs CI jobs before calling sphinx-build.
3. suppressing the warning type with suppress_warnings = ["..."] (docs). There is no way to suppress warnings only for specific files or lines, so this would disable the syntax check in the whole project. Also, I can't find the warning code in the error output, but I can find that out if you choose this option. This is a docutils warning, and sphinx has no way of suppressing it.

[NMertsch]

Here is how this would render without -W (treat warnings as errors):

It is not beautiful, but people will understand it.

If I understand correctly, this project is mostly in maintenance-only mode, so the risk of such issues being newly introduced to GitPython itself is fairly low. Maybe that's the best of all the bad options?

[NMertsch]

Sorry for causing this mess, I didn't expect any negative side effects from the bugfix.

If you want to close this PR, just go ahead, I don't have strong feeling about this and just wanted to contribute a quick win.

If you want me to investigate further in a specific direction, please let me know and I'll see what is feasible for me.

Thanks for this library, btw! :)

[Byron]

I see - the fix now causes the configuration to be pulled in correctly, and that results in a conflict in the way a docstring is written in the library or code we pull in.

The issue itself, now that I understand it, is also amazing as it's one of these inconsistencies that shouldn't even be possible. How can a variable in a for-loop survive the for-loop, and permanently overwrite a variable in the outer scope?

Let's go with the fix, and let only errors be errors during documentation generation.

Just one more request: could you add to the body of the most recent commit why the change was made? Just copy-paste a link to the discussion here, or better yet, copy-paste the respective comment text.

Thanks again.

Here is a copilot suggestion, but it might lack detail.

Remove -W from SPHINXOPTS due to warnings triggered by quoting of 'vars'

The Sphinx build was failing with -W enabled because warnings were generated related to the way the word 'vars' is quoted in docstrings. Treating warnings as errors (-W) caused otherwise harmless docstring quoting issues to block documentation builds. Removing -W allows the build to succeed despite these warnings.

[NMertsch]

Should be good now, please let me know if there is more to do.
Thank you for the quick and kind responses, I really appreciate it.

[Byron]

Thank you, I think the PR is ready to go now!