Use friendlier names than "ours" and "theirs" for conflict sides

[smashwilson]

Presenting conflict sides as "ours" and "theirs" is unclear, especially when:

• You're merging two branches that are both "yours"
• You're applying a stash
• You're rebasing (git actually flips the sides in this case!)

The ref name of each side of the conflict does appear in the conflict banners, but it's often something like "HEAD" which is likely to be confusing for git newcomers.

I'd like to present a clearer name in addition to the "ours" and "theirs" nomenclature, possibly a logical name of the ref based on output from git describe or git rev-parse. I'm open to suggestions about what command that should be :grin:

Follow-on work from #385.

/cc @maxbrunsfeld

[simurai]

To make it more clear, how about adding the commit message? It already does it for "ours":

Maybe add the committer avatar, name and time?

Currently it could link to .com to see more infos. Then it's pretty clear and you probably don't even have to pay attention to the "ours/theirs". Easy to always pick your change and ignore the rest. :stuck_out_tongue_winking_eye:

[simurai]

Then the <<<< |||| >>>>> might not even be needed? They're just there for text only when no other UI is available. And you don't want to edit these lines anyways.

[simurai]

Hey, isn't it that in most cases, you don't pick either of the 2 changes, but a combination of both? In my (limited) merge conflict career I rarely came across the situation that two people changed exactly the same word/name/number. Most conflicts happened because stuff gets moved around or if in the same line something got changed in two different places.. like

• base: div class="xxx" title="xxx"
• theirs: div class="changed" title="xxx"
• ours: div class="xxx" title="changed"

So I usually pick one change and then copy & paste from the other change.

• modified: div class="changed" title="changed"

Is it possible to make that automagic? :sparkles: Like take only the difference from both and apply them together? Like have an additional Use both button:

There would still be the "Dismiss" and "Resolve as Theirs Then Ours" options if you want to hand edit it.

Anyways, not that high on the priority list, just maybe an idea for "one day ™".

[anaisbetts]

How about instead of "ours" show "on this computer" and instead of "theirs" show "on origin/BRANCH"

[simurai]

How about instead of "ours" show "on this computer" and instead of "theirs" show "on origin/BRANCH"

Currently would be fine, but at some point we probably want to support merging two local branches and also stashing?

Maybe just indicate which are the changes from the current branch (that you're on).

[smashwilson]

Maybe add the committer avatar, name and time?

Ohh I really like this idea and mockup :sparkles: :smile:

Then the <<<< |||| >>>>> might not even be needed? They're just there for text only when no other UI is available. And you don't want to edit these lines anyways.

They are not needed! Fun fact: the original package hacks this in by using overlay decorations for the control bars that are positioned exactly over the <<<< |||| >>>> ("banner") lines. That's a pretty terrible way to do it because (a) it constrains the control bars to specific dimensions, which are somewhat too small to gracefully render the buttons and (b) you can still technically move the cursor beneath the decoration and edit the banner lines even though you can't see them.

Another fairly simple alternative would be to delete them during parse. I don't like that because it modifies the buffer before you do anything and because it messes up the line numbers.

I think the Right Way to do this :tm: requires some upstream core work: adding a way to decorate the banner lines to cause the TextEditor to omit them from rendering. @nathansobo and I talked about this, I remember.

Is it possible to make that automagic? ✨ Like take only the difference from both and apply them together? Like have an additional Use both button: [..]

Hmm! That's a neat idea. Or maybe even just an "I Feel Lucky 🍀 " button or menu option that falls back to a word/character-level merge?

How about instead of "ours" show "on this computer" and instead of "theirs" show "on origin/BRANCH"

Currently would be fine, but at some point we probably want to support merging two local branches and also stashing?

It may be interesting to try to detect common scenarios like "local branch, merging in remote tracking branch from GitHub remote" to show specific phrasing like that.

Maybe just indicate which are the changes from the current branch (that you're on).

That's actually exactly what "ours" means now :wink: The "ours" side of the conflict will always be the changes from the branch that was HEAD immediately before you ran the command the caused the conflicts (merge, rebase, cherry-pick, stash, ...). In git-land that's true for everything but rebase, but I handle that case explicitly.

[simurai]

Another fairly simple alternative would be to delete them during parse. I don't like that because it modifies the buffer before you do anything...

Yeah, that could be too intrusive. We're still an editor, after all. Ok, here another exploration. Where the code mostly is untouched and all the controls are moved into the gutter for clutter free editing:

☝️ This is what you would see when clicking on an item in the conflicts list. Some notes:

• After the theirs/base/ours is an additional ******* Modified section.
  • It's the same idea as above. It tries to automatically merge the theirs and ours changes and shows it as a suggestion. If it's not possible it would just stay empty as a placeholder.
  • The main idea here is that you wouldn't edit the above (theirs/base/ours) and only keep it as a reference. It's nice to always be able to check how it originally used to be while doing crazy edits. It should take away the fear of accidentally having missed something.
  • Of course if a change is very small, you can always change theirs/ours directly and not use the modified section.
• Clicking the avatars shows the commit that introduced the change in a popup/tooltip. Can be used to further investigate if needed.
• 🍔 button opens a menu with additional options.

Then once you made a choice or modified to your liking you can click on any of the 4 buttons. It would keep that part of the code and clean up the rest:

• The status is now shown as "resolved"
• You can still undo (and mark as un-resolved) in the menu. It would show the state before clicking a button, so any modified code wouldn't change. Just that it shows the other changes again.
• There is also a current/total (1/3) indicator. The down/up buttons can be used to navigate to the next/prev conflict.

Like always, there are probably edge cases that need to be considered.. like:

• If a change only removes code, there might not be enough space to fit the button/avatar. Is the color coded border enough?
• What about conflicts that happen on the first or last line?
• What about if a conflict spans across many lines? Should the buttons stay grouped together or spread out and vertically centered or at the top of each change?

Buuuut.. all that said. Let's leave this simmer for a while and maybe come back to it later. https://github.com/atom/github/pull/385 is already a great help and using it for a while maybe gives more ideas. I just wanted to post this here while my mind is still in the conflict zone.

/cc 👀 @atom/design

[calebmeyer]

• 🍔 button opens a menu with additional options

What other options?

For the side panel buttons, in the mockup they are aligned with their text. What happens when the text is too large to fit on one screen? Do they try to align themselves with their respective sections, or stay in place?

Something I'd like to see is a "I'll handle this one myself" button (maybe styled as a close button?) that removes all the UI. For the side panel idea, it works out that way pretty nicely (the UI is out of the way of editing if I want to edit)

[50Wliu]

You can already remove the UI per-conflict through the additional hamburger menu options. https://github.com/atom/github/issues/1078#issuecomment-320984164