-
Notifications
You must be signed in to change notification settings - Fork 2
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
Question: how should code motion of a section of code on one branch versus deletion on another be handled? #5
Comments
Experimented with this by doing a simple Git merge of a file that is unchanged on our branch, but has a line added in the middle on their branch, then doing Git thinks the file is conflicted, as hoped - in fact even if the file in the working directory is deleted, it is still happy and reports a conflict. IntelliJ is bit more wily and demands that at very least there is a file at the correct path in the working directory - however, like Git it uses the stages as the source of truth about the merge, so if the conflict marker is completely malformed (or even if the file is empty), IntelliJ will show its usual three-way diff conflict resolution window. So far, so good. The wrinkle is that IntelliJ is too intelligent - it spots the absence of any genuine conflict and reports the added line is an ordinary diff contributed from their branch only - which is actually the case here. So yes, Kinetic Merge can represent the code motion versus deletion as a conflict, but IntelliJ will simply treat it is something that can be trivially merged. Most people will probably execute the auto-resolve action prior to dealing with leftover conflicts, so the net result will be to favour code motion over deletion. Given that the synthetic not-quite-a-real-conflict will often be mixed in with genuine changes from one branch or another, it is obvious that if IntelliJ is used, the user probably won't be made aware of the conflict in this case! |
One approach is to accept this as a necessary evil - at least the merge conflict is reported, even though the markers are ignored and the conflict optimised away by IntelliJ. Perhaps a synthetic section could be added to represent the deletion, thus forcing the user to have to think about resolving the conflict manually? Conflict markers already mess up the merged code, so one could have something like:
No, this won't work as IntelliJ is reading from the blobs, and we don't want these to have anything other than 'real' code in them. |
For now, accept this as wrinkle - so if a code section is both deleted and moved, then any good merging tool will likely cleanly resolve this in favour of the move. |
After doing the groundwork for #11 , now that it obvious that Git will allow synthetic blobs to be used for staging entries, then we can fudge the marker workaround to represent the deletion - so we add |
On reflection after looking at #6 again, why can't this be treated as a straight divergence, analogous to #4? This time the divergence syntax would be something like This way, the user is forced to think about the deletion prior to the handover to conflict resolution, just as they would have to for a move-versus-move divergence. |
My mood today is to favour upfront resolution as a divergence. Not only is it more obvious what is happening, there is also the danger that in going down the marker route, the user may be misled into thinking that accepting, say, our side of
will automagically propagate the deletion - whereas they will be surprised to see Of course, such a divergence can only be resolved by |
Given the entry in the command table for Let's do that... (Updated #6 accordingly). |
Re-reading this, if a section is moved on one side and deleted on the other, isn't this just an extreme example of propagating an edit of a moved section to the destination? So the outcome is simply to propagate the deletion of the section. There is a twist - if a section moves on one side, but is deleted on the other and it's original containing file still exists, then this naturally can be regarded as extreme editing, thus the deletion is propagated. If on the other hand the section moves on one side and it's original containing file is deleted, this feels like a divergent move if the section moves out of the file on the first side, and would probably be a simple file deletion if it moves around in the original file on the first side. |
We have tests for a section moving on one side and being deleted on another - the deletion is migrated (propagated in the older terminology). I don't think there is any test for a section moving out of a file on one side and the entire file being deleted on the other - this is actually a twist on #4... |
This is a generalisation of #4 to when it is just a section of code that is moved or deleted as opposed to an entire file.
An obvious and desirable outcome would be to treat this as an ordinary conflict, where the code either exists in in its new location (could be in the same file, or buried in another file, or maybe in its own new file) - or does not.
If the code is moved around in the same file or into an existing file, then because it either does this or doesn't exist at all, then we can mark this as a kind of conflict where the code is on one side of the conflict and nothing on the other side. Where I'm a little hesitant here is whether Git and your typical post-merge clean up tool (such as IntelliJ's merge resolution) can handle a conflict where 'our' side has code, 'their' side is empty and the common ancestor is empty too. Need to check that this is feasible, I know that a straight deletion of common ancestor code versus an edit is easily represented as a conflict and is handled by the likes of IntelliJ.
The third case where the code is moved to its own new file as reminiscent of #4 and should probably be handled as such.
The text was updated successfully, but these errors were encountered: