Skip to content
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

update porcelain.status to output string #890

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

update porcelain.status to output string #890

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8bfcb13
update porcelain.status to output string
DedSecer Jul 29, 2021
a7cf333
update API of porcelain.status
DedSecer Jul 30, 2021
e58448b
fix sytle checking
DedSecer Jul 30, 2021
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 dulwich/cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -466,13 +466,13 @@ def run(self, args):
for kind, names in status.staged.items():
for name in names:
sys.stdout.write(
"\t%s: %s\n" % (kind, name.decode(sys.getfilesystemencoding()))
"\t%s: %s\n" % (kind, name)
)
sys.stdout.write("\n")
if status.unstaged:
sys.stdout.write("Changes not staged for commit:\n\n")
for name in status.unstaged:
sys.stdout.write("\t%s\n" % name.decode(sys.getfilesystemencoding()))
sys.stdout.write("\t%s\n" % name)
sys.stdout.write("\n")
if status.untracked:
sys.stdout.write("Untracked files:\n\n")
Expand Down
3 changes: 3 additions & 0 deletions dulwich/porcelain.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1210,11 +1210,14 @@ def status(repo=".", ignored=False):
with open_repo_closing(repo) as r:
# 1. Get status of staged
tracked_changes = get_tree_changes(r)
for key in tracked_changes.keys():
tracked_changes[key] = [file.decode() for file in tracked_changes[key]]
# 2. Get status of unstaged
index = r.open_index()
normalizer = r.get_blob_normalizer()
filter_callback = normalizer.checkin_normalize
unstaged_changes = list(get_unstaged_changes(index, r.path, filter_callback))
unstaged_changes = [file.decode() for file in unstaged_changes]
Copy link
Owner

Choose a reason for hiding this comment

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

This is going to decode with the current encoding (e.g. utf-8) which may not work if the filename contains bytes that can't be decoded with the current encoding (e.g. utf-8) or even if they can it may not result in the string you were expecting (file system encoding and display encoding may be different).

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 we set the default encoding in the script which using dulwich to resolve it?

Copy link
Owner

Choose a reason for hiding this comment

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

That would be inconsistent with the way the rest of Dulwich works. And if you're making the caller worry about the encoding, why not let them do the decoding as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Generally, it can decode the bytes with defaultcoding, if it doesn't work, we just need to use # -*- coding: xxx -*- at the top of the file.(I think if the defaultcoding is incorrect, there will be other problems too)

Copy link
Owner

Choose a reason for hiding this comment

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

The encoding on top of the file is for the source file itself, not for decoding at run time.

If you're a user of Dulwich and you have a repo that happens to use latin1 as encoding while your current encoding is utf8, how would that work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But if current encoding doesn't match to the default encoding, Dulwich.status will still output bytes unreadable. Then what should we do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jelmer

Copy link
Owner

Choose a reason for hiding this comment

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

For some uses cases, bytes are sufficient. You also can choose what to do in that case if you want to display the output - you can ignore invalid characters, replace them with surrogates or e.g. use chardet to try to figure out the proper encoding if it isn't utf8.

Copy link
Owner

Choose a reason for hiding this comment

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

I'd be open to returning a surrogateescape-decoded string here, since it's possible for the caller to undo that and retrieve the original bytes but that would still be a breaking change for existing users of the API. We should also force utf8 in that case (since that's the convention for git repositories) rather than whatever the users' encoding is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the function in porcelain should work like the git subcommands(a higher function), or I should decode the bytes after using porcelain.status.


untracked_paths = get_untracked_paths(
r.path, r.path, index, exclude_ignored=not ignored
Expand Down
12 changes: 6 additions & 6 deletions dulwich/tests/test_porcelain.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1759,8 +1759,8 @@ def test_status_base(self):

results = porcelain.status(self.repo)

self.assertEqual(results.staged["add"][0], filename_add.encode("ascii"))
self.assertEqual(results.unstaged, [b"foo"])
self.assertEqual(results.staged["add"][0], filename_add)
self.assertEqual(results.unstaged, ["foo"])

def test_status_all(self):
del_path = os.path.join(self.repo.path, "foo")
Expand Down Expand Up @@ -1795,10 +1795,10 @@ def test_status_all(self):
f.write("origstuff")
results = porcelain.status(self.repo.path)
self.assertDictEqual(
{"add": [b"baz"], "delete": [b"foo"], "modify": [b"bar"]},
{"add": ["baz"], "delete": ["foo"], "modify": ["bar"]},
results.staged,
)
self.assertListEqual(results.unstaged, [b"blye"])
self.assertListEqual(results.unstaged, ["blye"])
self.assertListEqual(results.untracked, ["blyat"])

def test_status_crlf_mismatch(self):
Expand All @@ -1822,7 +1822,7 @@ def test_status_crlf_mismatch(self):

results = porcelain.status(self.repo)
self.assertDictEqual({"add": [], "delete": [], "modify": []}, results.staged)
self.assertListEqual(results.unstaged, [b"crlf"])
self.assertListEqual(results.unstaged, ["crlf"])
self.assertListEqual(results.untracked, [])

def test_status_autocrlf_true(self):
Expand Down Expand Up @@ -1878,7 +1878,7 @@ def test_status_autocrlf_input(self):
porcelain.add(repo=self.repo.path, paths=[file_path])

results = porcelain.status(self.repo)
self.assertDictEqual({"add": [b"crlf-new"], "delete": [], "modify": []}, results.staged)
self.assertDictEqual({"add": ["crlf-new"], "delete": [], "modify": []}, results.staged)
self.assertListEqual(results.unstaged, [])
self.assertListEqual(results.untracked, [])

Expand Down