FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Make NULL_TREE and Index precisely annotatable · gitpython-developers/GitPython@65863a2 · GitHub

/ GitPython Public

Commit 65863a2

Browse files
EliahKagan
committed
Make NULL_TREE and Index precisely annotatable
This creates a git.diff.DiffConstants enumeration and makes the git.diff.NULL_TREE and git.diff.Diffable.Index objects constants in it. This allows them (including as an alternative in a union) to be annotated as literals: - Literal[DiffConstants.NULL_TREE] - Literal[DIffConstants.INDEX] Although the enumeration type must unfortunately be included in the annotations as shown above (at least mypy requires this), using the objects/values themselves does not require any code to change. So this shouldn't break anything at runtime for code using GitPython, unless it has relied on NULL_TREE being a direct instance of object, or relied on Diffable.Index (all useful uses of which were also as an opaque constant) being defined as a class. More specifically, all the ways that NULL_TREE and Index could be *accessed* before are still working, because: - NULL_TREE is aliased at module level, where it was before. It is also aliased at class level in Diffable, for consistency. - INDEX is aliased at class level in Diffable, as Index, where it was before. This way, Diffable.Index can still be used. It is also aliased, in the same scope, as INDEX. Because it is, and in effect has always been, a constant, the new INDEX spelling is preferable. But there is no major disadvantage of the old Index spelling, so in docstrings I have made no effort at this time to discourage its use. (If GitPython ever uses all-caps identifiers strictly as constants, then the clarity benefit of ensuring only the INDEX version is used will be greater, and then it might make sense to deprecate Index. However, this seems unlikely to happen as it would be a breaking change, due to the way functions like git.reset rebind Git.GIT_PYTHON_GIT_EXECUTABLE.) INDEX is also aliased at module level, for consistency. - NULL_TREE is still included in git.diff.__all__, causing it to be recognized as public in git.diff and also to be accessible as an attribute of the top-level git module (which currently uses wildcard imports). For consistency, I have also included INDEX in __all__. - Because there is a benefit to being able to freely referene the new DiffConstants enumeration in annotations (though in practice this may mostly be to implementers of new Diffable subclasses), I have also included DiffConstants in __all__. In addition, the name DiffConstants (rather than, e.g., Constants) should avoid confusion even if it ends up in another scope unexpectedly. To avoid a situation where users/developers may erroneously think these aliases are different from each other, I have documented the situation in the docstrings for each, referring to the others. (Sphinx does not automatically use the original docstring for an aliased name introduced in this way, and there is also arguably a clarity benefit to their differing wording, such as how each refers *only* to the others.) Other docstings are also updated. This commit completes the change begun in 2f5e258 before this, resolving the one mypy error it added. But this does not complete the larger change begun in 0e1df29: - One of the effects of this change is to make it possible to annotate precisely for NULL_TREE, either by using Literal[DiffConstants.NULL_TREE] or consolidating it with the Literal[DiffConstants.INDEX] alternative by including DiffConstants in the union instead of either/both of them. - But that is not yet done here, and when it is done for Diffable.diff, the LSP issue in IndexFile.diff, which does not currently accommodate NULL_TREE, will resurface (or, rather, be rightly revealed by mypy, in a way that is specifically clear).
1 parent 2f5e258 commit 65863a2

3 files changed

Lines changed: 81 additions & 28 deletions

File tree

‎git/diff.py‎

Lines changed: 79 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
# This module is part of GitPython and is released under the
44
# 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/
55

6+
import enum
67
import re
78

89
from git.cmd import handle_process_output
@@ -22,13 +23,12 @@
2223
Match,
2324
Optional,
2425
Tuple,
25-
Type,
2626
TypeVar,
2727
Union,
2828
TYPE_CHECKING,
2929
cast,
3030
)
31-
from git.types import Literal, PathLike, final
31+
from git.types import Literal, PathLike
3232

3333
if TYPE_CHECKING:
3434
from .objects.tree import Tree
@@ -48,10 +48,55 @@
4848
# ------------------------------------------------------------------------
4949

5050

51-
__all__ = ("Diffable", "DiffIndex", "Diff", "NULL_TREE")
51+
__all__ = ("DiffConstants", "NULL_TREE", "INDEX", "Diffable", "DiffIndex", "Diff")
5252

53-
NULL_TREE = object()
54-
"""Special object to compare against the empty tree in diffs."""
53+
54+
@enum.unique
55+
class DiffConstants(enum.Enum):
56+
"""Special objects for :meth:`Diffable.diff`.
57+
58+
See the :meth:`Diffable.diff` method's ``other`` parameter, which accepts various
59+
values including these.
60+
61+
:note:
62+
These constants are also available as attributes of the :mod:`git.diff` module,
63+
the :class:`Diffable` class and its subclasses and instances, and the top-level
64+
:mod:`git` module.
65+
"""
66+
67+
NULL_TREE = enum.auto()
68+
"""Stand-in indicating you want to compare against the empty tree in diffs.
69+
70+
Also accessible as :const:`git.NULL_TREE`, :const:`git.diff.NULL_TREE`, and
71+
:const:`Diffable.NULL_TREE`.
72+
"""
73+
74+
INDEX = enum.auto()
75+
"""Stand-in indicating you want to diff against the index.
76+
77+
Also accessible as :const:`git.INDEX`, :const:`git.diff.INDEX`, and
78+
:const:`Diffable.INDEX`, as well as :const:`Diffable.Index`. The latter has been
79+
kept for backward compatibility and made an alias of this, so it may still be used.
80+
"""
81+
82+
83+
NULL_TREE: Literal[DiffConstants.NULL_TREE] = DiffConstants.NULL_TREE
84+
"""Stand-in indicating you want to compare against the empty tree in diffs.
85+
86+
See :meth:`Diffable.diff`, which accepts this as a value of its ``other`` parameter.
87+
88+
This is an alias of :const:`DiffConstants.NULL_TREE`, which may also be accessed as
89+
:const:`git.NULL_TREE` and :const:`Diffable.NULL_TREE`.
90+
"""
91+
92+
INDEX: Literal[DiffConstants.INDEX] = DiffConstants.INDEX
93+
"""Stand-in indicating you want to diff against the index.
94+
95+
See :meth:`Diffable.diff`, which accepts this as a value of its ``other`` parameter.
96+
97+
This is an alias of :const:`DiffConstants.INDEX`, which may also be accessed as
98+
:const:`git.INDEX` and :const:`Diffable.INDEX`, as well as :const:`Diffable.Index`.
99+
"""
55100

56101
_octal_byte_re = re.compile(rb"\\([0-9]{3})")
57102

@@ -84,7 +129,7 @@ class Diffable:
84129
compatible type.
85130
86131
:note:
87-
Subclasses require a repo member, as it is the case for
132+
Subclasses require a :attr:`repo` member, as it is the case for
88133
:class:`~git.objects.base.Object` instances. For practical reasons we do not
89134
derive from :class:`~git.objects.base.Object`.
90135
"""
@@ -94,9 +139,25 @@ class Diffable:
94139
repo: "Repo"
95140
"""Repository to operate on. Must be provided by subclass or sibling class."""
96141

97-
@final
98-
class Index:
99-
"""Stand-in indicating you want to diff against the index."""
142+
NULL_TREE = NULL_TREE
143+
"""Stand-in indicating you want to compare against the empty tree in diffs.
144+
145+
See the :meth:`diff` method, which accepts this as a value of its ``other``
146+
parameter.
147+
148+
This is the same as :const:`DiffConstants.NULL_TREE`, and may also be accessed as
149+
:const:`git.NULL_TREE` and :const:`git.diff.NULL_TREE`.
150+
"""
151+
152+
INDEX = Index = INDEX
153+
"""Stand-in indicating you want to diff against the index.
154+
155+
See the :meth:`diff` method, which accepts this as a value of its ``other``
156+
parameter.
157+
158+
This is the same as :const:`DiffConstants.INDEX`, and may also be accessed as
159+
:const:`git.INDEX` and :const:`git.diff.INDEX`.
160+
"""
100161

101162
def _process_diff_args(
102163
self,
@@ -112,7 +173,7 @@ def _process_diff_args(
112173

113174
def diff(
114175
self,
115-
other: Union[Type["Index"], "Tree", "Commit", str, None] = Index,
176+
other: Union[Literal[DiffConstants.INDEX], "Tree", "Commit", str, None] = INDEX,
116177
paths: Union[PathLike, List[PathLike], Tuple[PathLike, ...], None] = None,
117178
create_patch: bool = False,
118179
**kwargs: Any,
@@ -125,20 +186,15 @@ def diff(
125186
126187
* If ``None``, we will be compared to the working tree.
127188
128-
* If :class:`~git.types.Tree_ish`, it will be compared against the
129-
respective tree. (See https://git-scm.com/docs/gitglossary#def_tree-ish.)
130-
This can also be passed as a string.
189+
* If a :class:`~git.types.Tree_ish` or string, it will be compared against
190+
the respective tree.
131191
132-
* If :class:`Diffable.Index`, it will be compared against the index. Use the
133-
type object :class:`Index` itself, without attempting to instantiate it.
134-
(That is, you should treat :class:`Index` as an opqaue constant. Don't
135-
rely on it being a class or even callable.)
192+
* If :const:`INDEX`, it will be compared against the index.
136193
137-
* If :attr:`git.NULL_TREE <NULL_TREE>`, it will compare against the empty
138-
tree.
194+
* If :const:`NULL_TREE`, it will compare against the empty tree.
139195
140-
This parameter defaults to :class:`Diffable.Index` (rather than ``None``) so
141-
that the method will not by default fail on bare repositories.
196+
This parameter defaults to :const:`INDEX` (rather than ``None``) so that the
197+
method will not by default fail on bare repositories.
142198
143199
:param paths:
144200
This a list of paths or a single path to limit the diff to. It will only
@@ -185,7 +241,7 @@ def diff(
185241
paths = [paths]
186242

187243
diff_cmd = self.repo.git.diff
188-
if other is Diffable.Index:
244+
if other is INDEX:
189245
args.insert(0, "--cached")
190246
elif other is NULL_TREE:
191247
args.insert(0, "-r") # Recursive diff-tree.
@@ -218,7 +274,7 @@ def diff(
218274

219275

220276
class DiffIndex(List[T_Diff]):
221-
R"""An Index for diffs, allowing a list of :class:`Diff`\s to be queried by the diff
277+
R"""An index for diffs, allowing a list of :class:`Diff`\s to be queried by the diff
222278
properties.
223279
224280
The class improves the diff handling convenience.

‎git/index/base.py‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,11 +76,10 @@
7676
Sequence,
7777
TYPE_CHECKING,
7878
Tuple,
79-
Type,
8079
Union,
8180
)
8281

83-
from git.types import Commit_ish, PathLike
82+
from git.types import Commit_ish, Literal, PathLike
8483

8584
if TYPE_CHECKING:
8685
from subprocess import Popen
@@ -1479,7 +1478,7 @@ def reset(
14791478
# @ default_index, breaks typing for some reason, copied into function
14801479
def diff(
14811480
self,
1482-
other: Union[Type["git_diff.Diffable.Index"], "Tree", "Commit", str, None] = git_diff.Diffable.Index,
1481+
other: Union[Literal[git_diff.DiffConstants.INDEX], "Tree", "Commit", str, None] = git_diff.INDEX,
14831482
paths: Union[PathLike, List[PathLike], Tuple[PathLike, ...], None] = None,
14841483
create_patch: bool = False,
14851484
**kwargs: Any,

‎git/types.py‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
TypedDict,
2323
Protocol,
2424
SupportsIndex as SupportsIndex,
25-
final,
2625
runtime_checkable,
2726
)
2827
else:
@@ -31,7 +30,6 @@
3130
SupportsIndex as SupportsIndex,
3231
TypedDict,
3332
Protocol,
34-
final,
3533
runtime_checkable,
3634
)
3735

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL