Skip to content
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

MergetoolDiffExchange has undesired behavior #9

Open
MoezGholami opened this issue Oct 10, 2019 · 1 comment
Open

MergetoolDiffExchange has undesired behavior #9

MoezGholami opened this issue Oct 10, 2019 · 1 comment

Comments

@MoezGholami
Copy link

MoezGholami commented Oct 10, 2019

The current implementation of MergetoolDiffExchange for example, does not work for a situation where both sides of the working window are other valid windows.
For example, when one has BMR layout and is in the BASE window can normally put the diff in the MERGE window by MergetoolDiffExchangeRight. However, in the MERGE window, the diff from the BASE windo (left side) is not got with MergetoolDiffExchangeLeft, instead MergetoolDiffExchangeRight does the job (which is the opposite of what we intend).
3 way diff vertical split layout

Tracking the issue, I realized the DiffExchange function here is problematic. On MergetoolDiffExchangeLeft in MERGE window (BMR layout), it reverses the direction (which was a good thing in putting the diff from B to M with MergetoolDiffExchangeRight) and gets the diff from the REMOTE.
I believe we should not determine the direction by reversing it as it may consider another window which does not have to do anything with what we want (R in the BMR for example). We already know both exchange parties.
I have solved this issue locally and will send a pull request.

@MoezGholami MoezGholami mentioned this issue Oct 10, 2019
@MoezGholami
Copy link
Author

#10 pull request resolves this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant