Skip to content

Show context for diff view of merge conflict and introduce compareAll.#74231

Merged
rebornix merged 6 commits intomasterfrom
rebornix/bettermerge
May 24, 2019
Merged

Show context for diff view of merge conflict and introduce compareAll.#74231
rebornix merged 6 commits intomasterfrom
rebornix/bettermerge

Conversation

@rebornix
Copy link
Copy Markdown
Member

@rebornix rebornix commented May 23, 2019

To improve the merge conflict resolve experience, a new setting merge-conflict.diffViewContext was added to show the context around the conflict

Before: there is no context around the diff
compare-withoutcontext

After: with "merge-conflict.diffViewContext": 3
compare-with-context

A new command Compare All is introduced to display the full compare between current workspace and incoming changes, with which you can compare the old content, the incoming one, and the latest local content on disk

compare-all

@rebornix rebornix changed the title Merge pull request #74089 from microsoft/rr/comments Show context for diff view of merge conflict and introduce compareAll. May 23, 2019
@rebornix rebornix requested a review from chrmarti May 23, 2019 22:28

let text = '';
let lastPosition = new vscode.Position(0, 0);
ranges.forEach(rangeObj => {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We concatenate the before/after ranges to two new documents and then diff the two - if I understand correctly. This could result in a different diff than when comparing the two original before/after documents. Is that going to be a problem?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This could result in a different diff than when comparing the two original before/after documents

Yes and no, let me elaborate more. No, because the after document is the file content on disk, which already resolves some conflicts successfully, so compareAll shows the difference of conflicts which are not resolved. From this perspective, the diff view of compareAll is correct.

Yes if we are thinking in a three-diff way. Say the users is trying to merge two branch, whose heads are A and B. A is upstream, and B is a branch you created a few weeks ago. After merging, there is a merge conflict. The diff view of compareAll is different from A --- B then. However in this case, a simple comparison between A and B doesn't really work as most changes in A --- B are probably not related to the work you are doing. To avoid confusion, you may want to see a three-way-diff editor, which represents the diff view between A, B and their common nearest ancestor. We can do this later on as it requires introducing a new diff algorithm and editor pane.

@rebornix
Copy link
Copy Markdown
Member Author

rebornix commented May 24, 2019

After discussion during today's sit-down, the code change is being stripped to

  • Compare command now always show the full diff view and scroll to the active conflict position
  • merge-conflict.diffViewPosition controls where the diff view should be opened

compare-with-full-context

@rebornix rebornix merged commit 146115b into master May 24, 2019
@rebornix rebornix deleted the rebornix/bettermerge branch August 8, 2019 00:16
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants