sageserpent-open / kineticMerge

Merge a heavily refactored codebase and stay sane.
MIT License
9 stars 1 forks source link

How should a deletion versus modification conflict be handled? #33

Closed sageserpent-open closed 1 month ago

sageserpent-open commented 3 months ago

As of Git commit SHA: 7873fcf9e93ba641cd9e9c8f5ac1325396d26e2c, coverage testing shows that the logic here and here is not covered.

These deal with the situation where the merge has not tweaked the modification, so either the modification should be left in the working directory if it was done in our branch, or restored from the blob id if done on their branch.

The reason this never happens is that the merge is presented with a synthetic empty input representing the side with the deletion; it then does a three-way merge between outright deletion of content versus the modification, picking up changes to be propagated from the modified side if sections have moved out of the deleted file into another file.

That's fine in itself, but it means that Kinetic Merge will produce a merged result that is just the modifications without the original content of the file prior to deletion. If that results in an empty merge result, then the conflict is trivially resolved as a straight deletion (this supports code motion in the presence of edits from one file to another), and if the merge result is non empty, it is presented as a tweaked conflict between the deletion and the merge result, not the modified file content.

Git on the other hand just shows a conflict between a deletion and the modified file content - it doesn't think of the deletion is implying a merge of loss of content at all.

Should Kinetic Merge try to emulate Git, or is it better that it tries to merge to some extent? If it did emulate Git, how would this work with code motion across files? What if two files swap their content on one branch, but are edited in place on the other?

sageserpent-open commented 1 month ago

Those code paths are now covered due to: 7ceed11cc605dda8518f76c5de530f2560dd1cae, df31b317ebb5c919ec0a9c103690251b9921e398 and 31725230153edb95af1597104ffb18d19e237db0.

There are also tests for swapping of two files on one branch with editing of one or both of the original files on another branch: fc7862855b7d876b70b94b9bec516ffdde7fb7c5, 2dab8f2a583fe0c70d1c43fc35a9f4c66e9b3ba2, d458fdebf7b1377943aede447e8dd99405b887e1 and 90cad461c4a431f3c81bf037204c07eae7ed2c27 - subsequent work has got them to pass.

Kinetic merge will result in a Git status of UD or DU for the merged file, depending on whether the modification was performed on our or their branch, respectively. This is in line with what Git does.

It will however merge the content deletion into the version of the file left in the working tree (as well as the staged entry for the side that performed the modification). This is in contrast to Git, which thinks that the deletion doesn't imply loss of content that should be merged - it simply leaves the other side as it is and asks the user to decide what to do.

As mentioned above, this fits in with the code motion cases where the 'deleted' file has simply moved elsewhere; I'm happy to leave it as it stands.