Revise comments, docstrings, some messages, and a bit of code

[EliahKagan]

This revises comments and docstrings, to improve clarity, readability, stylistic consistency, in some cases accuracy, and (for docstrings) Sphinx formatting and cross-linking. It also contains a few other related changes.

Short summary of the most notable changes

In both comments and docstrings, my changes included rewording for clarity, as well as changes in capitalization, punctuation, and so forth.

For comments, which were in a variety of styles, I converted them to be sentences (or in some cases fragments formatted as sentences), except in a minority of cases when doing so seemed like it would worsen readability. I did this because, in addition to improving stylistic consistency and being encouraged by PEP-8, this tended to make their structure clearer (especially in cases of comments over multiple lines that were convertible to multiple separate sentences). This comment style was also one of the styles already represented throughout the codebase.

For docstrings, there were a number of places where the formatting looked one way when reading the raw docstring, but very different, and arguably less readable, as rendered by Sphinx. In cases where this was clearly unintended and the effect was significant, such as markup that was intended to represent an unordered list but instead represented a running paragraph with literal asterisks, I fixed the formatting. In cases where the intent was less clear, such as non-wrapping newlines conveying a break more major than that between sentences but less major than that between paragraphs, I preserved the style where possible but did not make changes to cause it to be newly reflected in rendered documentation.

Kinds of other changes

There are also some other changes, described below, including some code changes, mainly for clarity, that I believe to be sufficiently related or where there is otherwise a reason to include them here rather than proposing them separately.

Some specific changes (not to code) are described in commit messages but not this PR description, but this is noted below. Specifically, a handful of changes to docstrings are for accuracy, and to allow them to carry specific rationales and to be reviewed individually, they each have individual commits and are not currently relisted here.

Line length in docstrings

I found that many docstrings were already written, or almost entirely written, with a hard wrap of 88 columns. It makes sense that this would be done, because it is the black default, even though black does not enforce line length in docstrings, and furthermore the maximum line length this this project has been set at the much higher 120 columns. I think that a width of 120, or even 100, in docstrings, reduces readability when reading the docstrings in the code (of course it does not affect what Sphinx renders), so this is probably another reason a smaller width was used in most cases.

Where width was consistent, as well as where making it consistent didn't seem like it would improve readability, I left it alone. Otherwise, I prioritized consistency within each docstring and with nearby docstrings, but overall (when making changes that would justify it) preferred the prevailing 88 column width. To a much lesser extent, I preferred a width of 88 columns for comments, but only where that style was otherwise being used. I did not format code to make it narrower. We may want to reduce the black maximum line length from 120 to 100 or even the default of 88, but I definitely would consider that beyond the scope of this PR.

Documentation generated from tests

This includes changes to comments in portions of the two test modules from which documentation is generated. This is not special, but I wanted to note it here to properly characterize what parts of the generated documentation are and are not affected.

Other documentation - only one small change

Although most of the the docstring changes affect generated documentation, and the documentation in doc/ should also be revised and updated (and in some places stands to benefit more from such changes than anything here), the purpose of this pull request is not to revise or update the separate documentation in doc/.

This does include one change to the contents of doc/, to avoid introducing an inconsistency when rewording a docstring for clarity whose wording was also used in the tutorial. Other that that and a change in conf.py, it does not modify any tracked files in doc/.

How the changes are divided into commits

Formatting, rewording, and style changes such as to capitalization and punctuation, as well as the repair and introduction of Sphinx cross-linking changes and code-formatting changes (such as placing every multi-paragraph docstring's closing """ on its own line) are done in a small number of commits, most pertaining to organizational sections of the code.

In contrast, accuracy improvements, even though they account for a much smaller volume of change, are each done in their own commit, so they can have commit messages that explain their rationale and to make them easier to review.

Although I had considered describing each of those changes here, I have not thought of a way to do so that I think would clearly be more useful than the commit messages themselves. Furthermore, this description is already rather long. So I have not reproduced that information here. However, I would be pleased to expand this PR description on request, either about that or in other ways.

What is omitted

In areas where accuracy might be improved but it is unclear how, or where reviewing the changes seems like it would be easier done separately, or where the changes should be accompanied by code changes that themselves are not natural to include here, I have omitted such changes entirely from this pull request. (One such area I discovered while working on this but did not include in it was #1712, which I instead fixed in #1714.)

The changes in this PR are probably incomplete, in the sense that I most likely have missed some things.

In some areas, I have avoided making changes that I anticipate would create conflicts, or other confusion, with work I have on other feature branches that I plan to open PRs for soon. In practice, the only major omission that arises due to this is in test/test_util.py, which is mostly unrevised since I intend to propose changes there soon, for making xfail markings more precise, that will involve reorganizing that code.

Changes to code

Finally, please note that this PR does contain some code changes. They are as follows:

• A few messages, such as exception messages, are revised. I avoided doing this except when I thought it was a clear improvement and would not cause problems. When accuracy was the motivation, such a change has its own commit (as do other accuracy changes).
• Some string literals are changed, so that the use of raw string literals is consistent (for regular expressions and for Windows paths not ending in a backslash, and with r and R respectively). Commit messages contain further information about this. Conceptually, I felt this came along with fixing docstring bugs where '\' was intended to represent itself literally but instead represented '' because a non-raw docstring turned \' into '. There is also an argument to be made that strings are the core concept of this pull request (hence the branch name).
• Explicit inheritance from object is removed. This is a legacy of Python 2, where it was needed to make a new-style class. This project no longer supports (and is already considerably incompatible with) Python 2, and in Python 3 all classes are new-style classes. The reason this is a clarity improvement sufficiently related to documentation that it seemed appropriate to include here is that GitPython has an Object class that could be confused with object.
• Expressions of the form x.__class__ == t are changed to x.__class__ is t. In #1673, I fixed the corresponding problem for type(x) == t, but flake8 does not detect the .__class__ variant. The reason this change is convenient here is that most of these changes were on the same or an adjacent line to something I was already changing, so making them separately would in most cases have incurred a conflict. But the reason I think it's reasonable to include this here is that a number of GitPython's objects have type attributes that pertain to Git and GitPython's data model rather than to the Python object model, and that hold strings that are (correctly) compared with ==. Comparing x.__class__ with is, as the related type(x) is compared but as the unrelated x.type is not compared, makes the distinction easier to heed, I think.
• Python permits __slots__ = "slot" for a single slot slot, but it is discouraged in preference for __slots__ = ("slot",), which was already being done in most cases. I fixed the few occurrences of the former to bring them in line with that.

Although readability would be further improved by replacing two-argument super calls with zero-argument super()--which is also something Python 3 adds that is related to new-style classes and thus arguably beckoned by those last three bullet points--I have omitted this. The reason is that this change would require special attention in review to ensure that it is only done in cases where the meaning of the code is not changed. (That is likely every occurrence of super in the whole repository, but they would still have to be checked individually.)

[Byron]

Thanks! However, this was more straightforward than it might seem. Rather than operating from any kind of comprehensive knowledge of the documentation, I instead read "master repository" in the docstring and thought it might be clearer if written with the official term included, as "superproject (master repository)".

I am glad there is an alternative explanation, and one that makes you appear less like a brand-new AI that is being tested undercover ;).

I'm not sure there's any way to do that fully, because the description and comments on pull requests and issues can change at any time, including after they are merged/closed.

Depending on what the tool should be like, one can probably start with archived projects to be sure they (most probably) won't change. However, I also think that this should be more like a 'sync' that will pick up changes since the last sync. Maybe this can even allow to 'sync back' certain changes for a two-way connection. Of course, there is a lot of unknowns right now, I am merely dreaming something up 😁.

It would be useful for building public repositories representing "slices" of valuable content.

I really like that idea, and think that once frameworks exists that make certain 'sync' operations tenable it's possible to implement this for various sources without going crazy.

[..] from the email attachment history, with fairly uninformative but better-than-nothing auto-generated commit messages, with one commit per relevant email message.)

By that time it might even be straightforward enough to run it through a local transformer for actually useful summaries, and maybe more (like intelligence extractions based on a large context window).

As a more limited move, and something that might be worthwhile here, it should be feasible to make a bot, or even a browser extension or user script, that merges a pull request on GitHub and populates the merge commit message with the entire content of the pull request description.

That would be a super interesting application of a bot or browser plugin, and I'd not even be that surprised if it already existed somewhere. Whether it's worthwhile or 'good enough', I am also uncertain about.

As for solving the full problem of making all collaboration on public Git repositories accessible as close as possible to everywhere, and fully integrated, maybe something like ActivityPub would be needed? I'm interested in your thoughts about this, but I acknowledge it might be off-topic here or even for GitPython as a whole. I think this kind of integration is something you've mentioned is, or is in some way related to, an eventual aim of gitoxide, but I don't remember where or if you said that, and I may have misunderstood. If this is an eventual aim of gitoxide, then I could open a discussion thread about it there, but I don't want to assume that.

gitoxide would only serve as git implementation, and as such can be used to use git repositories as storage for such 'sync engines'. One could already do that today. Such a system, however, would definitely be a separate project and it's one in the far-future for me. If you would want to use gitoxide for something like it though, you are welcome to open a discussion there of course and we see where it leads.

Thanks! I'm not sure what I would do in gitoxide, which doesn't seem to need changes analogous to the ones I've been making here since it is a modern project. Also, although I will eventually be learning Rust, I don't know it currently.

I'd be happy to support you if you were to learn Rust by implementing a feature in gitoxide, no matter how small. Learning by doing, if you will.

Although people use git information from shell prompts to make assumptions that, if wrong, could lead to data loss, it seems to me that, at least after Byron/gitoxide#1049, gitoxide would be fully ready for this use.

Yes, having gix status will open up a lot of additional uses and will allow starship to finally stop using git and be much faster in doing what it does :D.
I wouldn't recommend using gix for it though as it's a forever-unstable developer tool, which really doesn't want to be git, but simply assures gix (the crate) would support something very close to that (i.e. a drop-in replacement). What one would want to do though is to write a prompt-generating program in Rust that uses gix (the crate) to emit the prompt as needed. Such a program already exists: [starship`](https://starship.rs) .

I bring this up because, if that functionality is not available but would be considered useful, then I might be interested in contributing it--though if someone else did so first then I would be only happy and could still potentially contribute improvements related to it. Other than that, I haven't thought of anything I would be able to contribute to gitoxide. I think it could be done all in the shell scripting languages whose interpreters' operation it would affect, or perhaps partly or mostly in Rust. I'm unsure if this should go in the gitoxide repository itself, or elsewhere, especially if it ends up being written entirely in the shell languages it targets. Also, if this feature does exist and I just didn't find it when searching, my apologies!

There is a tool that one day should be stable, and that is ein. ein is meant to be the opposite of git and completely rethink how one interacts with a git repository - the basic idea is to support a git workflow that is easy to understand, while still familiar to typical git users like me, but with the best possible user experience.

ein does have tools, which I already use regularly or have integrated into my zsh via the very simple

select-repo () {
        local root=~/dev
        echo -n $root/
        {
                cd $root
                ein --quiet t find
        } | sk -q "$1"
}

so I can jump into repositories by fuzzy-searching them.

And there is more. Maybe that's where one could add a tool that you always wanted and that coincidentally other may want too.

(In addition, I was working on this for small amounts of time accumulated over many days, so having these be separate commits was useful for enabling myself to know what was going on.)

It sounds like you are working with some sort of patch queue - if so, which one? I use Stacked Git.

However, since you seem to be okay with it, I'd be pleased to restate less information from commit messages into PRs, as that makes the PRs faster to write.

I think that will be a win-win-win! The third 'win' is for more of the rationale and reasoning going into git, possibly, which keeps it for posterity (as long the 'sync engine' isn't available that is ;)).

However, if this downside is considered acceptable, then I would be willing to do this at least somewhat more often.

Please do also optimize for whatever you think is best, contributing shouldn't be a chore and I'd never want to turn it into one. I admittedly also can't perform review the way I want, as GitHub doesn't make it easy to see which commit contributed to the final result that I am looking at. I also don't review commit by commit, which means I don't usually see or read all commit messages. This is solely a tooling issue though :/, and yes, one day I want to fix that too 😁.

Obviously this runs up against scale problems--in excess, it would hamper collaboration--but even then, each of the sub-ideas of a big idea can benefit from experimentation.

On top of what was already said, it does seem like a tooling issue maybe, or maybe I just misunderstand this entirely. But again, ideally making contributions is fun and I want to do my part to help with that.

Should this be done in gitdb and smmap as well?

I'd consider that a chore as both ideally go away at some point - if there is a breaking change I'd consider, this would be one of them. The other one would have to do with gitoxide integration, but that's me dreaming again.

With that said, I did notice some opportunities for further PRs, which I hope to open. :)

😁❤️🙏

[EliahKagan]

I'd be happy to support you if you were to learn Rust by implementing a feature in gitoxide, no matter how small. Learning by doing, if you will.

Thanks! There is a substantial chance I will want to do that.

What one would want to do though is to write a prompt-generating program in Rust that uses gix (the crate) to emit the prompt as needed. Such a program already exists: [starship`](https://starship.rs) .

Yes, I believe that is what I'm looking for!

It sounds like you are working with some sort of patch queue - if so, which one? I use Stacked Git.

Actually, I am unfamiliar with patch queues. I work on multiple features on multiple branches, and I use stashes, often amend my most recent commit on a feature branch, sometimes rebase, and occasionally cherry-pick. I use various tools to examine repositories, but for performing mutations I just use the available git commands, in nearly all cases.

On top of what was already said, it does seem like a tooling issue maybe, or maybe I just misunderstand this entirely.

Perhaps so. In particular, if interactive rebases were extremely fast and easy, I might use them to disseminate insights into commit messages more often. The main feature I might want for this, especially if most information were in commit messages rather than pull requests but really there are many reasons I want this, is automatic rewriting of commit hashes, at least for references to ancestors. If X is on a feature branch, and Y is a descendant of it on the same feature branch, and Y's message refers to X by hash, and I rebase that feature branch onto some ancestor of X (changing X's hash), it would be nice to have an easy way for the reference in Y's commit message to be updated to the new X. Of course there are various edge cases, since there is not always exactly one commit that corresponds to the old X after a rebase.

Should this be done in gitdb and smmap as well?

I'd consider that a chore as both ideally go away at some point - if there is a breaking change I'd consider, this would be one of them. The other one would have to do with gitoxide integration, but that's me dreaming again.

I don't follow. What I mean to ask here is merely if uses of two-argument super in gitdb and smmap should also be changed to use zero-argument super(). If so, it might make sense to also do things like remove explicit inheritance from the object builtin, but I'm not talking about any breaking changes, only changes that keep semantics the same.

[Byron]

Perhaps so. In particular, if interactive rebases were extremely fast and easy, I might use them to disseminate insights into commit messages more often. The main feature I might want for this, especially if most information were in commit messages rather than pull requests but really there are many reasons I want this, is automatic rewriting of commit hashes, at least for references to ancestors. If X is on a feature branch, and Y is a descendant of it on the same feature branch, and Y's message refers to X by hash, and I rebase that feature branch onto some ancestor of X (changing X's hash), it would be nice to have an easy way for the reference in Y's commit message to be updated to the new X. Of course there are various edge cases, since there is not always exactly one commit that corresponds to the old X after a rebase.

I truly think you should give Stacked Git a try then, even though it's not the only tool that can help. I am using it myself for nearly a year now and it transformed my workflow from 'one commit per change, record a history of all changes' to 'craft commits by topic or category, and keep updating them, going back and forth in the stack'. It's probably what you do with a rebase-amend workflow already, but Stacked Git is more convenient.

As for the commit-rewrite feature, I am sure you could propose it there or contribute it yourself at some point, it seems very doable.

I don't follow. What I mean to ask here is merely if uses of two-argument super in gitdb and smmap should also be changed to use zero-argument super(). If so, it might make sense to also do things like remove explicit inheritance from the object builtin, but I'm not talking about any breaking changes, only changes that keep semantics the same.

Apologies for causing confusion. I ttried to say that since I'd rather want to remove gitdb and smmap entirely it's probably not worth putting anymore time in it than is needed to keep it from falling apart. From my experience, these are very stable as well, so that should be fine.