feat(git): Add binary field to FileChanges objects

Author: Ishirui

src/dda/utils/git/changeset.py

@@ -31,6 +31,9 @@ class ChangedFile(Struct, frozen=True):
     type: ChangeType
     """The type of change that was made to the file: added, modified, or deleted."""
 
+    binary: bool
+    """Whether the changed file was a binary file."""
+
     patch: str
     """
     The patch representing the changes to the file, in unified diff format.
@@ -135,6 +138,7 @@ def generate_from_diff_output(cls, diff_output: str | list[str]) -> Generator[Se
                 yield cls(
                     file=current_file,
                     type=current_type,
+                    binary=False,  # TODO: Support binaries
                     patch="\n".join(current_patch_lines),
                 )
                 current_file = None

src/dda/utils/git/remote.py

@@ -66,7 +66,9 @@ def get_details_and_changes_for_commit(self, commit: Commit) -> tuple[CommitDeta
             ChangedFile(
                 file=Path(file_obj["filename"]),
                 type=get_change_type_from_github_status(file_obj["status"]),
-                patch=file_obj["patch"],
+                # GitHub does not have anything else to indicate binary files
+                binary="patch" not in file_obj,
+                patch=file_obj.get("patch", ""),
             )
             for file_obj in data["files"]
         )

tests/tools/git/fixtures/repo_states/complex_changes/expected_changeset.json

@@ -1 +1 @@
-{"_changes":{"Path('file2.txt')":{"file":"Path('file2.txt')","type":"D","patch":"@@ -1,4 +0,0 @@\n-I am file2 !\n-I feel like I take up space for nothing...\n-I have a feeling like I won't exist pretty soon :/\n-"},"Path('file3.txt')":{"file":"Path('file3.txt')","type":"A","patch":"@@ -0,0 +1,2 @@\n+I am file3 !\n+I'm new around here, hopefully everyone treats me nice :)"},"Path('file4.txt')":{"file":"Path('file4.txt')","type":"M","patch":"@@ -2 +2 @@ I am file4.\n-People often tell me I am unreliable.\n+People often tell me I am THE BEST.\n@@ -4,3 +4,2 @@ Things like:\n-- You always change !\n-- I can never count on you...\n-- I didn't recognize you !\n+- You rock !\n+- I wish I were you !\n@@ -8 +7,3 @@ Do you think they have a point ?\n-I'd need to look at my own history to know...\n+Pah ! Who am I kidding, they're OBVIOUSLY RIGHT.\n+Arrogance ? What is that, an italian ice cream flavor ?\n+Get outta here !"},"Path('file5.txt')":{"file":"Path('file5.txt')","type":"D","patch":"@@ -1,5 +0,0 @@\n-I am a humble file.\n-Soon I will change name.\n-I think I'll also take this as an opportunity to change myself.\n-New name, new me !\n-Or is that not how the saying goes ?"},"Path('file5_new.txt')":{"file":"Path('file5_new.txt')","type":"A","patch":"@@ -0,0 +1,5 @@\n+I am a humble file.\n+Hey I have a new name !\n+Wow, I feel much better now.\n+New name, new me !\n+Or is that not how the saying goes ?"}}}
+{"_changes":{"Path('file2.txt')":{"file":"Path('file2.txt')","type":"D","binary":false,"patch":"@@ -1,4 +0,0 @@\n-I am file2 !\n-I feel like I take up space for nothing...\n-I have a feeling like I won't exist pretty soon :/\n-"},"Path('file3.txt')":{"file":"Path('file3.txt')","type":"A","binary":false,"patch":"@@ -0,0 +1,2 @@\n+I am file3 !\n+I'm new around here, hopefully everyone treats me nice :)"},"Path('file4.txt')":{"file":"Path('file4.txt')","type":"M","binary":false,"patch":"@@ -2 +2 @@ I am file4.\n-People often tell me I am unreliable.\n+People often tell me I am THE BEST.\n@@ -4,3 +4,2 @@ Things like:\n-- You always change !\n-- I can never count on you...\n-- I didn't recognize you !\n+- You rock !\n+- I wish I were you !\n@@ -8 +7,3 @@ Do you think they have a point ?\n-I'd need to look at my own history to know...\n+Pah ! Who am I kidding, they're OBVIOUSLY RIGHT.\n+Arrogance ? What is that, an italian ice cream flavor ?\n+Get outta here !"},"Path('file5.txt')":{"file":"Path('file5.txt')","type":"D","binary":false,"patch":"@@ -1,5 +0,0 @@\n-I am a humble file.\n-Soon I will change name.\n-I think I'll also take this as an opportunity to change myself.\n-New name, new me !\n-Or is that not how the saying goes ?"},"Path('file5_new.txt')":{"file":"Path('file5_new.txt')","type":"A","binary":false,"patch":"@@ -0,0 +1,5 @@\n+I am a humble file.\n+Hey I have a new name !\n+Wow, I feel much better now.\n+New name, new me !\n+Or is that not how the saying goes ?"}}}

tests/tools/git/fixtures/repo_states/file_add/expected_changeset.json

@@ -1 +1 @@
-{"_changes":{"Path('file2.txt')":{"file":"Path('file2.txt')","type":"A","patch":"@@ -0,0 +1,3 @@\n+file2\n+I am a new file in the repo !\n+That's incredible."}}}
+{"_changes":{"Path('file2.txt')":{"file":"Path('file2.txt')","type":"A","binary":false,"patch":"@@ -0,0 +1,3 @@\n+file2\n+I am a new file in the repo !\n+That's incredible."}}}

tests/tools/git/fixtures/repo_states/file_delete/expected_changeset.json

@@ -1 +1 @@
-{"_changes":{"Path('file2.txt')":{"file":"Path('file2.txt')","type":"D","patch":"@@ -1,3 +0,0 @@\n-file2\n-I will be deleted, unfortunately.\n-That's quite sad."}}}
+{"_changes":{"Path('file2.txt')":{"file":"Path('file2.txt')","type":"D","binary":false,"patch":"@@ -1,3 +0,0 @@\n-file2\n-I will be deleted, unfortunately.\n-That's quite sad."}}}

tests/tools/git/fixtures/repo_states/file_edits/expected_changeset.json

@@ -1 +1 @@
-{"_changes":{"Path('added_lines.txt')":{"file":"Path('added_lines.txt')","type":"M","patch":"@@ -2,0 +3,4 @@ I wonder what that could mean ?\n+\n+But of course !\n+I get some added lines.\n+That makes sense."},"Path('added_lines_middle.txt')":{"file":"Path('added_lines_middle.txt')","type":"M","patch":"@@ -2,0 +3,2 @@ I have a bit more text than added_lines.\n+Nobody expects the Spanish Inquisition !\n+My developer really wonders if cracking jokes in test data is against company policy."},"Path('deleted_lines.txt')":{"file":"Path('deleted_lines.txt')","type":"M","patch":"@@ -2,2 +2 @@ Hmm, my name is deleted_lines.\n-I wonder what that could mean ?\n-I don't want to be shortened, that does not seem fun.\n+"},"Path('deleted_lines_middle.txt')":{"file":"Path('deleted_lines_middle.txt')","type":"M","patch":"@@ -2,3 +1,0 @@ Hmm, my name is deleted_lines_middle.\n-I wonder what that could mean ?\n-I don't want to be shortened, that does not seem fun.\n-Thinking about it, being short is also kind of nice though."},"Path('edited_lines.txt')":{"file":"Path('edited_lines.txt')","type":"M","patch":"@@ -4,8 +4,5 @@ I have a big block of text here:\n-Lorem ipsum dolor sit amet, consectetur adipiscing elit.\n-Vestibulum ornare cursus diam, quis hendrerit nibh.\n-Donec sollicitudin neque in tempus ornare.\n-Integer sit amet pretium quam.\n-Maecenas lacinia augue id est malesuada, vitae fermentum justo faucibus.\n-Aenean posuere nisi tincidunt nisi pharetra blandit.\n-Integer sed nulla sed eros aliquet eleifend quis ac quam.\n-Orci varius natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus.\n+What kind of dev uses lorem ipsum ?\n+Keyboard mashing is where it's at !\n+Check this out:\n+phq w4q3t'p£tfnjklifewqkpjnoq23bjikkijq4ovikobjqv4kioblj;\n+vqpewkvnkqknbpjlo[iqervkb[jplofqvwer[kpjlqfp[okqolp[f;vn]]]]]"}}}
+{"_changes":{"Path('added_lines.txt')":{"file":"Path('added_lines.txt')","type":"M","binary":false,"patch":"@@ -2,0 +3,4 @@ I wonder what that could mean ?\n+\n+But of course !\n+I get some added lines.\n+That makes sense."},"Path('added_lines_middle.txt')":{"file":"Path('added_lines_middle.txt')","type":"M","binary":false,"patch":"@@ -2,0 +3,2 @@ I have a bit more text than added_lines.\n+Nobody expects the Spanish Inquisition !\n+My developer really wonders if cracking jokes in test data is against company policy."},"Path('deleted_lines.txt')":{"file":"Path('deleted_lines.txt')","type":"M","binary":false,"patch":"@@ -2,2 +2 @@ Hmm, my name is deleted_lines.\n-I wonder what that could mean ?\n-I don't want to be shortened, that does not seem fun.\n+"},"Path('deleted_lines_middle.txt')":{"file":"Path('deleted_lines_middle.txt')","type":"M","binary":false,"patch":"@@ -2,3 +1,0 @@ Hmm, my name is deleted_lines_middle.\n-I wonder what that could mean ?\n-I don't want to be shortened, that does not seem fun.\n-Thinking about it, being short is also kind of nice though."},"Path('edited_lines.txt')":{"file":"Path('edited_lines.txt')","type":"M","binary":false,"patch":"@@ -4,8 +4,5 @@ I have a big block of text here:\n-Lorem ipsum dolor sit amet, consectetur adipiscing elit.\n-Vestibulum ornare cursus diam, quis hendrerit nibh.\n-Donec sollicitudin neque in tempus ornare.\n-Integer sit amet pretium quam.\n-Maecenas lacinia augue id est malesuada, vitae fermentum justo faucibus.\n-Aenean posuere nisi tincidunt nisi pharetra blandit.\n-Integer sed nulla sed eros aliquet eleifend quis ac quam.\n-Orci varius natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus.\n+What kind of dev uses lorem ipsum ?\n+Keyboard mashing is where it's at !\n+Check this out:\n+phq w4q3t'p£tfnjklifewqkpjnoq23bjikkijq4ovikobjqv4kioblj;\n+vqpewkvnkqknbpjlo[iqervkb[jplofqvwer[kpjlqfp[okqolp[f;vn]]]]]"}}}

tests/tools/git/fixtures/repo_states/file_rename/expected_changeset.json

@@ -1 +1 @@
-{"_changes":{"Path('file1.txt')":{"file":"Path('file1.txt')","type":"D","patch":"@@ -1,3 +0,0 @@\n-I am a humble file.\n-Soon I will change name.\n-But that won't change who I am, my contents !"},"Path('file1_new.txt')":{"file":"Path('file1_new.txt')","type":"A","patch":"@@ -0,0 +1,3 @@\n+I am a humble file.\n+Soon I will change name.\n+But that won't change who I am, my contents !"},"Path('file2.txt')":{"file":"Path('file2.txt')","type":"D","patch":"@@ -1,5 +0,0 @@\n-I am a humble file.\n-Soon I will change name.\n-I think I'll also take this as an opportunity to change myself.\n-New name, new me !\n-Or is that not how the saying goes ?"},"Path('file2_new.txt')":{"file":"Path('file2_new.txt')","type":"A","patch":"@@ -0,0 +1,5 @@\n+I am a humble file.\n+Hey I have a new name !\n+Wow, I feel much better now.\n+New name, new me !\n+Or is that not how the saying goes ?"}}}
+{"_changes":{"Path('file1.txt')":{"file":"Path('file1.txt')","type":"D","binary":false,"patch":"@@ -1,3 +0,0 @@\n-I am a humble file.\n-Soon I will change name.\n-But that won't change who I am, my contents !"},"Path('file1_new.txt')":{"file":"Path('file1_new.txt')","type":"A","binary":false,"patch":"@@ -0,0 +1,3 @@\n+I am a humble file.\n+Soon I will change name.\n+But that won't change who I am, my contents !"},"Path('file2.txt')":{"file":"Path('file2.txt')","type":"D","binary":false,"patch":"@@ -1,5 +0,0 @@\n-I am a humble file.\n-Soon I will change name.\n-I think I'll also take this as an opportunity to change myself.\n-New name, new me !\n-Or is that not how the saying goes ?"},"Path('file2_new.txt')":{"file":"Path('file2_new.txt')","type":"A","binary":false,"patch":"@@ -0,0 +1,5 @@\n+I am a humble file.\n+Hey I have a new name !\n+Wow, I feel much better now.\n+New name, new me !\n+Or is that not how the saying goes ?"}}}

tests/tools/git/test_git.py

@@ -197,12 +197,14 @@ def test_get_changes_with_base(app: Application, mocker: Any, repo_testcase: str
 
     # Test with working tree changes
     working_tree_changes = ChangeSet({
-        Path("test.txt"): ChangedFile(file=Path("test.txt"), type=ChangeType.ADDED, patch="@@ -0,0 +1 @@\n+test")
+        Path("test.txt"): ChangedFile(
+            file=Path("test.txt"), type=ChangeType.ADDED, binary=False, patch="@@ -0,0 +1 @@\n+test"
+        )
     })
     mocker.patch("dda.tools.git.Git.get_working_tree_changes", return_value=working_tree_changes)
 
     changeset_with_working_tree = git.get_changes_with_base(base_commit.sha1, include_working_tree=True)
     expected_changeset_with_working_tree = expected_changeset | ChangeSet.from_iter([
-        ChangedFile(file=Path("test.txt"), type=ChangeType.ADDED, patch="@@ -0,0 +1 @@\n+test")
+        ChangedFile(file=Path("test.txt"), type=ChangeType.ADDED, binary=False, patch="@@ -0,0 +1 @@\n+test")
     ])
     assert_changesets_equal(changeset_with_working_tree, expected_changeset_with_working_tree)