Skip to content

Commit

Permalink
Correct and clarify Diffable.diff docstring
Browse files Browse the repository at this point in the history
In cd16a35 (gitpython-developers#1725), I had taken "Treeish" to mean the type of that
exact name, git.index.base.Treeish. But that type is only used
within the git.index package (actually only in git.index.base
itself). It is also nonpublic: git.index.base.__all__ exists and
does not list it.

So most likely this was not intended in the git.diff.Diffable.diff
docstring. Even if intended, it does not appear accurate, since the
git.index.base.Treeish union includes bytes, and the logic in
Diffable.diff and its helpers does not appear to accommodate bytes.

A closer type is the public git.types.Tree_ish union, which is
narrower than git.index.base.Treeish, including neither str nor
bytes. However, it does not include str, and Diffable.diff does
accept str to specify a tree-ish for diff-ing. It may be that
"Treeish" in the pre-gitpython-developers#1725 docstring was capitalized for some
reason other than to identify a type defined in GitPython's code.

For now, I've changed it to refer to git.types.Tree_ish, but also
explicitly documented that a string can be used to specify a
tree-ish -- which is independently valuable, since previously the
effect of passing a str instance to the diff method was not stated
anywhere in the method docstring. To clarify further, I included a
link to tree-ish in gitglossary(7) as well.

In addition, the original wording before cd16a35 had included
"(type)", which I had erroneously assumed was just meant to state
that it is a type (i.e. a class), so I had wrongly removed it
without replacing it with anything when making it into a reference
to a type. But it was really an attempt to clarify that
Diffable.Index should be used directly, rather than an instance of
it. That is in effect the opposite of merely pointing out that it
is a class; it is to express that it should be used in a way that
does not depend in any way on it being a class. This commit has the
docstring explicitly state that.
  • Loading branch information
EliahKagan committed Mar 8, 2024
1 parent 1cdec7a commit ed6ead9
Showing 1 changed file with 16 additions and 7 deletions.
23 changes: 16 additions & 7 deletions git/diff.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,12 +122,21 @@ def diff(
This the item to compare us with.
* If ``None``, we will be compared to the working tree.
* If :class:`~git.index.base.Treeish`, it will be compared against the
respective tree.
* If :class:`Diffable.Index`, it will be compared against the index.
* If :attr:`git.NULL_TREE`, it will compare against the empty tree.
* It defaults to :class:`Diffable.Index` so that the method will not by
default fail on bare repositories.
* If :class:`~git.types.Tree_ish`, it will be compared against the
respective tree. (See https://git-scm.com/docs/gitglossary#def_tree-ish.)
This can also be passed as a string.
* If :class:`Diffable.Index`, it will be compared against the index. Use the
type object :class:`Index` itself, without attempting to instantiate it.
(That is, you should treat :class:`Index` as an opqaue constant. Don't
rely on it being a class or even callable.)
* If :attr:`git.NULL_TREE <NULL_TREE>`, it will compare against the empty
tree.
This parameter defaults to :class:`Diffable.Index` (rather than ``None``) so
that the method will not by default fail on bare repositories.
:param paths:
This a list of paths or a single path to limit the diff to. It will only
Expand All @@ -143,7 +152,7 @@ def diff(
sides of the diff.
:return:
:class:`DiffIndex`
A :class:`DiffIndex` representing the computed diff.
:note:
On a bare repository, `other` needs to be provided as
Expand Down

0 comments on commit ed6ead9

Please sign in to comment.