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.

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

Improved Moved #141

Merged
merged 1 commit into from
Sep 1, 2023
Merged

Improved Moved #141

merged 1 commit into from
Sep 1, 2023

Conversation

FintanH
Copy link
Collaborator

@FintanH FintanH commented Aug 30, 2023

In some cases a moved file can also be modified and in these cases there will be a diff as well as new Oid's for the two sides of the diff.

Add DiffFile to the Moved type, changing the insert_moved method to accept the DiffFile's and the DiffContent.

When serializing a Moved, if the old and the new are the same then the serialization of the diff files and content are skipped -- same as the old behaviour. Otherwise, the two sides are serialized along with the content.

Fixed #140

cc @sebastinez how will this affect radicle-interface/radicle-httpd for showing diffs for moved files?

In some cases a moved file can also be modified and in these cases
there will be a diff as well as new Oid's for the two sides of the
diff.

Add DiffFile to the Moved type, changing the insert_moved method to
accept the DiffFile's and the DiffContent.

When serializing a Moved, if the old and the new are the same then the
serialization of the diff files and content are skipped -- same as the
old behaviour. Otherwise, the two sides are serialized along with the
content.

Signed-off-by: Fintan Halpenny <fintan.halpenny@gmail.com>
X-Clacks-Overhead: GNU Terry Pratchett
@sebastinez
Copy link
Member

sebastinez commented Aug 31, 2023

Thanks @FintanH for the heads up, as far as I can see we would need to adapt the radicle-interface to display the diffs for moved files that have modifications.
But it wouldn't break radicle-httpd nor radicle-interface

I'll add a issue on radicle-interface to add this change to our types

Copy link
Member

@sebastinez sebastinez left a comment

Choose a reason for hiding this comment

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

Looks good to me, will add these changes to radicle-interface

@FintanH
Copy link
Collaborator Author

FintanH commented Aug 31, 2023

Thanks @FintanH for the heads up, as far as I can see we would need to adapt the radicle-interface to display the diffs for moved files that have modifications. But it wouldn't break radicle-httpd nor radicle-interface

I'll add a issue on radicle-interface to add this change to our types

That's great! Thanks for confirming that :)

sebastinez added a commit to radicle-dev/radicle-interface that referenced this pull request Aug 31, 2023
- Adds support for modified moved files see radicle-dev/radicle-git#141

Signed-off-by: Sebastian Martinez <me@sebastinez.dev>
@FintanH FintanH merged commit 31c6d7c into main Sep 1, 2023
24 checks passed
@FintanH FintanH deleted the surf/improve-moved-diff branch September 1, 2023 08:09
sebastinez added a commit to radicle-dev/radicle-interface that referenced this pull request Sep 18, 2023
- Adds support for modified moved files see radicle-dev/radicle-git#141

Signed-off-by: Sebastian Martinez <me@sebastinez.dev>
rudolfs pushed a commit to radicle-dev/radicle-interface that referenced this pull request Sep 19, 2023
- Adds support for modified moved files see radicle-dev/radicle-git#141

Signed-off-by: Sebastian Martinez <me@sebastinez.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

diff::Moved might require object IDs and file modes
2 participants