Skip to content

Fix blob filter types #1459

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Jul 2, 2022
Merged

Fix blob filter types #1459

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
da88a6d
Fix blob filter types
AustinScola Jun 21, 2022
a6ce118
Add stage type to all
AustinScola Jun 21, 2022
d171365
Move stage type def
AustinScola Jun 21, 2022
1314d63
Change to not stringify paths
AustinScola Jun 26, 2022
c6a018b
Fix pathlike type annotation typo
AustinScola Jun 26, 2022
cc80e6b
Remove usage of `PosixPath.is_relative_to`
AustinScola Jun 26, 2022
420d2af
Use generator instead of map
AustinScola Jun 26, 2022
ed45d5b
Fix blob filter path shorter than filter path
AustinScola Jun 28, 2022
da59d74
Remove stage type as parameter from blob filter test
AustinScola Jul 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions git/index/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
from .typ import (
BaseIndexEntry,
IndexEntry,
StageType,
)
from .util import TemporaryFileSwap, post_clear_cache, default_index, git_working_dir

Expand Down Expand Up @@ -83,13 +84,12 @@
from git.util import Actor


StageType = int
Treeish = Union[Tree, Commit, str, bytes]

# ------------------------------------------------------------------------------------


__all__ = ("IndexFile", "CheckoutError")
__all__ = ("IndexFile", "CheckoutError", "StageType")


class IndexFile(LazyMixin, git_diff.Diffable, Serializable):
Expand Down
10 changes: 6 additions & 4 deletions git/index/typ.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,11 @@
if TYPE_CHECKING:
from git.repo import Repo

StageType = int

# ---------------------------------------------------------------------------------

__all__ = ("BlobFilter", "BaseIndexEntry", "IndexEntry")
__all__ = ("BlobFilter", "BaseIndexEntry", "IndexEntry", "StageType")

# { Invariants
CE_NAMEMASK = 0x0FFF
Expand Down Expand Up @@ -48,10 +50,10 @@ def __init__(self, paths: Sequence[PathLike]) -> None:
"""
self.paths = paths

def __call__(self, stage_blob: Blob) -> bool:
path = stage_blob[1].path
def __call__(self, stage_blob: Tuple[StageType, Blob]) -> bool:
path: str = str(stage_blob[1].path)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Paths can't be converted to str as this implies an interpreter defined encoding.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain what you mean by this or give an example?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before the change it looked like this:

path = stage_blob[1].path

And after the change it should still be looking like this but possibly with type annotations added to please the type checker. Creating new strings with str(…) changes the way the program functions, and does so in a breaking way due to encodings being unspecified for paths on linux.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to use paths instead of strs. Still not really sure what you mean by encodings being unspecified for paths on linux.

for p in self.paths:
if path.startswith(p):
if path.startswith(str(p)):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There has to be a way to do this without str(…), please see above for the reason.

return True
# END for each path in filter paths
return False
Expand Down