Need better handling of conflicts involving a Delete/Rename

[goran-w]

Handling of merge conflicts was recently improved, by handling submodule conflicts differently (#1356) and by removing redundant conflicting entries from the Staged area (#1359). However, cases of "unmerged paths" involving a Delete or Rename operation are still not handled well.

The IsConflictResolved check works fine for the following conflict-related <XY> values (of the git-status "Short Format" and --porcelain formats) where a Diff can be performed:

X, Y
----
A, A = both added
U, U = both modified

However, for the following conflict-related values (involving a Delete or Rename) a Diff can NOT be performed, but the IsConflictResolved check incorrectly returns true:

X, Y
----
D, U = deleted by us, (modified by them)
U, D = (modified by us), deleted by them

D, D = both deleted                    = both renamed from (see notes below)
A, U = added by us, (modified by them) = renamed to by us (see notes below)
U, A = (modified by us), added by them = renamed to by them (see notes below)

For these, the IsConflictResolved check should instead return false (or simply be skipped, as we now do for Submodule items).

Then we can choose to resolve the conflict by Theirs or Mine (either keep the modification and discard the delete, or vice versa).

However, to see immediately which side of Theirs and Mine has the Deletion, more info would need to be presented in the Conflict view for these cases (based on the detailed <XY> status)! I.e, we should be presented with the two merge-contributors along with info on which one of them was Deleted/Renamed and which one was Modified.

Additionally, special care needs to be taken when resolving Deleted/Renamed conflicts using Theirs or Mine, since the Git commands involved may need to be adjusted. For example, resolving a file with DD (both deleted) status using either choice (in the current implementation) will result in an error: path <...> does not have [their|our] version (Commands used here should probably instead be git add or git rm.)

Notes:

• Remember that the 'X / Y' values for unmerged paths do NOT stand for 'Index / Working-area' but INSTEAD stand for 'Our / Their' branch. These values should still be stored in the Change class (as a separate conflict-type enum perhaps) and passed into the Conflict class, in order to make more educated assumptions about the conflict and present better information about the two merge-contributors when needed.
• The [DD, AU, UA] cases actually work in combination, to represent a "rename/rename" conflict - when a file is renamed into two different target paths on the two branches, the old path becomes a DD conflict and the two new paths become a AU and a UA conflict, respectively.
• The rename/rename (both renamed) conflicts are complicated since they involve not 1 but 3 files, and if there are also other conflicting files then it's not obvious which 3 files are related (info on original-path seems to be missing in git-status for these conflicts). Ideally we could (perhaps) find out which 3 files belong together (old file, our renamed file, their renamed file) and present a special view where we could choose which ones to keep/discard. But that would make for a separate feature request - for now let's just fix the basics so that all types of conflicts can be resolved using the Conflict view...

Besides the above fix, it would be very helpful to expand the ChangeStatusIcon for Conflict, from a single '!' icon/badge into the following variants:

• ±! = Modified conflict (UU)
• +! = Added conflict (AA)
• -! = Deleted conflict (DU, UD)
• ➜! = Renamed conflict (DD, AU, UA)
• S! = Submodule conflict (UU) - (See Use separate icon/badge for Submodule in Changes lists #1369)

(This could be a implemented as a single wider badge/icon or two separate ones - the ordinary Change icon and the additional Conflict one.)

That way, we could see at a glance which conflicts involve Addition / Deletion / Rename / Submodule, this is important information since these need special care. For example, conflicts involving Deletions / Renames will make "Open all conflicts in external mergetool" fail, so we'd want to resolve them early before handling the more common conflicts involving only Modifications. Also, Renamed conflicts will be easier to understand if they are indicated as such.

Finally, the Conflict view currently allows 'Open external mergetool' on Submodule conflicts. However, that will not work, so that button should be disabled (or hidden) in that case (but we still need the Use Theirs and Use Mine buttons available). The same is true for any of the conflict types involving a Delete or Rename!

Further background:
The errors/warnings we get from git-merge when a merges causes conflicts are listing more details on the type of conflict.
Here are most of the possible variants (and their corresponding git-status <XY> values) :

• CONFLICT (content) = UU
• CONFLICT (binary) = UU
• CONFLICT (submodule) = UU
• CONFLICT (add/add) = AA
• CONFLICT (modify/delete) = DU or UD
• CONFLICT (rename/delete) = DU or UD
• CONFLICT (rename/rename) = DD + AU + UA
• CONFLICT (rename involved in collision) = ❓
• CONFLICT (file/directory) = ❓
• CONFLICT (file location) = ❓
• CONFLICT (implicit dir rename) = ❓
• CONFLICT (directory rename split) = ❓
• CONFLICT (distinct types) = ❓

(These messages, and some more, can be found in the Git source-code: https://github.com/git/git/blob/master/merge-ort.c)
Also, some more information can be found in the docs for git-merge-tree.

[love-linger]

I would like to show the detail conflict information in the conflict info view instead of introducing more icons.

[goran-w]

I would like to show the detail conflict information in the conflict info view instead of introducing more icons.

We are losing valuable information in the list during conflicts, since we can't see what kind of change the conflict involves as it's currently a single catch-all icon for all types of conflicts.

Git command-line itself gives more useful info than a universal "conflict" marker when listing these, both during the merge and in git-status.

[love-linger]

The key idea of this app is makes it simple to use git.

I just want to let users know there's a conflict in given path. If they want to know the detail information of the conflict or to resolve it, they can go to the conflict view to see and use options provided in conflict view.

[goran-w]

The key idea of this app is makes it simple to use git.

Not only simple, I hope, but also productive!

When there are hundreds of conflicts, any additional info we can get at-a-glance to mentally sort and make sense of the list, without having to click through each item, is extremely valuable.

The "conflict" info should be an additional qualifier on the Change, not the single categorization.

The default git status output gives more information for "unmerged paths" - "both added", "deleted by them" etc.

My suggestion was to condense that info a bit (since it's essentially a dual status of AA, UD etc) into a single status (icon) of Modified, Added, Deleted or (possibly, see table above) Renamed - based on the presence of an A or D status that becomes the "dominating" one, making it into a "structural" conflict that can't be resolved by external mergetool. (Also, Submodule conflict should have its own status, as debated elsewhere.)

[love-linger]

more icons means you must remember all these conflict icons. as you said, if there're lots of conflicts with different conflict types, it may not make it easier to find out which is conflict!

[goran-w]

No, these are the same icons we already use for normal changes, only with an additional ! conflict qualifier (or a separate ! icon next to it).

[goran-w]

I'm thinking something like this, where we have a Change-type icon followed by an additional Conflict-qualifier icon...

Here I used the Renamed icon for DD/AU/UA, but maybe that's jumping to conclusions so we could instead simply use Deleted for DD and Added for AU/UA...

More details, like the more specific dual status of DD (both deleted), AU (added by us) etc should be given inside the Conflict view.

[goran-w]

Ooh, I see in fbc8edc that you added icons/badges for the AA, UD, DU etc "dual" statuses in the Conflict view. Great!

These icons/badges (or similar ones) could now be reused in the Unstaged list (replacing the ! icon), and that would be "good enough" for what I'm requesting above regarding the icons. 👍

To recap, what I want/need is to see not only that a file is Conflicted but also that the conflict involves an Add / Delete. (Rename-handling is something to consider for the future, but this will do fine for now.) Showing the two-letter conflict status in the list would make for an immense improvement!

NOTE: The tooltip for this two-letter conflict badge in the Unstaged list could then be extended to say "Conflict (deleted by us)" etc.

[goran-w]

Besides the icons discussed above, there are a couple more things to fix before this issue can be closed:

• The USE THEIRS and USE MINE buttons could perhaps be swapped, to match the (us, them) order of the two-letter (UD etc) conflict status.
• More importantly, the USE THEIRS and USE MINE buttons do not currently work for all variants:
  • For DU/UD, the Mine/Theirs side(s) matching the Delete side(s) will fail - instead, git rm <file> should be used (since the non-delete modification is the one "left in tree" by Git). This also means that for the opposite side we could simply do git add <file>.
  • For DD, both the Mine/Theirs sides will fail - instead, git rm <file> (or git add <file>) could be used for either (both) sides - it doesn't seem to matter (and this conflict actually seems redundant/unnecessary).
  • For AU/UA, the Mine/Theirs side(s) matching the Unmerged side will fail - instead, git rm <file> should be used (and git add <file> could be used for the opposite side).

[goran-w]

Related commits: 75015d5, 60cd210, 1bd2044, (a9734ea)

With these, my issues are addressed. However, there are a couple more things to fix regarding tooltips:

• The tooltip for USE THEIRS / USE MINE buttons still shows the old Git command (git checkout --theirs/--ours) even for the cases where a different command had to be used.
• The conflict-icon tooltip (for a conflicted file in the Unstaged list) should appear also over the AA/DU/.. badge next to the conflict-icon (currently the file-path tooltip appears there instead, which is not as helpful).

[goran-w]

More related commits: 26307e2, 6620bd1 - these addressed the remaining tooltip issues. 👍

HOWEVER, I've now realized the earlier related commit 60cd210 addressed most of the problematic Use Theirs/Use Mine cases, except for the following two: 😯

• For AU (added by us), Use Theirs will still fail. It should use git rm <file> (instead of git checkout --theirs <file>).
• For UA (added by them), Use Mine will still fail. It should use git rm <file> (instead of git checkout --ours <file>).

This issue can be quickly fixed like this:

• Add ConflictReason.AddedByUs to the special if-condition applied in UseTheirs()
• Add ConflictReason.AddedByThem to the special if-condition applied in UseMine().

NOTE: Apparently we currently use rm <file> followed by git add <file>, instead of simply git rm <file>, but I suppose these are (more or less) equivalent...

For future reference, a DD + AU + UA triple (i.e a "rename/rename" conflict) can be consistently handled by applying either Use Theirs or Use Mine to all 3 files alike. (This will result in either [applying their renaming to our already renamed file] or [keeping our renaming which means discarding their renamed file].) It should also be noted that the "rename/rename" conflict type seems to be applicable only to unmodified files, since a content change in the renamed file on either side seems to cause other conflict types to appear instead.

[love-linger]

Could you please provide a mini repo can contains conflicts marked as AU or UA?