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

Opening Ahead/Behind files isn't showing the desire diff (e.g. diff with the merge base) #1218

Closed
VaasRamsay opened this issue Nov 29, 2020 · 20 comments
Assignees
Labels
bug Something isn't working needs-verification Indicates a request for community verification
Milestone

Comments

@VaasRamsay
Copy link

Earlier there used to be an option to switch between three/two dot comparisons. I can't seem to find that option now (after moving to Source Control tab). It's by default comparing in full-mode.

@eamodio
Copy link
Member

eamodio commented Nov 29, 2020

That option was removed with the change in how the comparison works now -- with the Behind and After groupings.

Is there a particular use case that you are trying to accomplish?

@jankap
Copy link

jankap commented Dec 1, 2020

I'm missing this one, too. It's more close to the way Git works, i.e. I'm using the shell:gitlens:bitbucket ui in approximately 1:1:1 fractions, that means I still need to know what gitlens is doing since I need to reproduce it in the terminal, too. By looking at the log I could see that there are still two and three dot comparisons, but it's not that clear anymore from the UI.

Use case is to switch between a "full diff" and a "merge diff", e.g. to check pull requests.

@obambrough
Copy link

obambrough commented Dec 7, 2020

@eamodio I used the three dot diff to perform code reviews. With Gitlens now, if I compare two branches it has sections for Behind and Ahead, and within those sections it shows the changed files within those commits (e.g. 48 files in the Behind section and 15 files in the Ahead section), except if you open the files to see how they were changed it shows a diff with changes from both branches - so you can't tell if the changes were added in the current branch or changes made to the comparison branch (which I'm not code reviewing.)

With winmerge configured as my diff viewer If I run git difftool branchA...branchB then it shows me 15 files that have been changed in branchB. Within winmerge if I open one of those files it only shows changes that have been introduced from branchB's commits - its not showing any changes from the same file that may have occurred in branchA's commits since branchB branched (but if I open that same file from GitLens, as it appears under both the Behind and Ahead file changes it isn't showing the changes from either branch but both branches.)

For my use-case I don't think I need the two/three dot switch anymore but when opening a file under the 'files changed' section of Behind or Ahead I think it should only show the diff from changes made in that branch's commits.

@fevb
Copy link

fevb commented Dec 8, 2020

I used frequently the two/three dot comparison when cherry-picking. A very common use case is when one is developing in a devel branch and wants to cherry-pick a subset of relevant commits that will go in a feature branch which will then be submitted as a PR.

The new comparison mode with behind and after groupings is convenient when merging branches, but is not well suited for cherry picking: it can happen that the diff of a file between the two branches is empty (as per e.g. git diff devel-branch feature-branch my_file.txt), but the new comparison mode still shows the file because the commits are different.

Being able to choose the comparison mode would be a great feature, it is the only thing keeping me from upgrading at the moment to version >= 11.

@jods4
Copy link

jods4 commented Dec 8, 2020

except if you open the files to see how they were changed it shows a diff with changes from both branches - so you can't tell if the changes were added in the current branch or changes made to the comparison branch (which I'm not code reviewing.)

Same here.
I used the three-dots comparison to code-review changes from a branch that might now be a bit behind.
For the code-review use case Ahead > x files changed would be a good replacement for ..., if the diff was coherent.
Ahead shows the files like diff A...B --name-only would 👍
When you diff a file, showing diff A..B -- somefile.js is incoherent and makes the feature unusable for code review.

Hopefully it's just a temporary bug!

@jopotts
Copy link

jopotts commented Dec 15, 2020

Agree with all comments above. Three-dot comparison is essential for code reviews, so from that perspective the feature is broken. Please please raise the priority of this issue. Thank you for all the hard work Eric!

@eamodio
Copy link
Member

eamodio commented Dec 16, 2020

@jods4 Can you expand upon the diff not being coherent?

When you diff a file, showing diff A..B -- somefile.js is incoherent and makes the feature unusable for code review.

What do you mean by this? Are you saying that when you click on a file under Ahead > x files changed the actual diff is not correct?

I want to understand this so I can get it fixed up ASAP.

@eamodio
Copy link
Member

eamodio commented Dec 16, 2020

Also can anyone give me an example from a repo that shows the desired behavior in GitLens 10 vs the undesired in GitLens 11?

@eamodio eamodio added bug Something isn't working needs-more-info Indicates that further information is required and removed question labels Dec 16, 2020
@eamodio eamodio self-assigned this Dec 16, 2020
@eamodio eamodio added this to the Soon™ milestone Dec 16, 2020
@jods4
Copy link

jods4 commented Dec 16, 2020

@eamodio
It's just one dot, so it is subtle: the diff of files always uses 2 dots.
The incoherence is that under Ahead you have the file list that is the output of diff A...B --name-only (3 dots) but when you click on a file it's diff A..B (2 dots) that is displayed. What would be coherent is that the same diff command is used in this single context, i.e. that the file diff was three-dots as well, just like the list.

That makes sense because Behind is what you missed from the other branch, i.e. B...A, Ahead is what this branch did, i.e. A...B and the unnamed root category is simply the direct comparison A..B.

Looks like currently the files lists match this behaviour, but when I open the actual file diff it is always 2 dots A..B that is used, no matter where I open it from.

If it worked properly, for me it would be a good enough substitute for the v10 three-dots feature (which was arguably rather obscure to people who aren't very familiar with git).

@eamodio eamodio changed the title Where is the three/two dot comparison option in the new version? Opening Ahead/Behind files isn't showing the desire diff (e.g. diff with the merge base) Dec 20, 2020
@eamodio eamodio added needs-verification Indicates a request for community verification pending-release Indicates that the issue been resolved, but has not yet been released to the stable edition and removed needs-more-info Indicates that further information is required labels Dec 20, 2020
@eamodio
Copy link
Member

eamodio commented Dec 20, 2020

I've made a change to the x files changed nodes under both Ahead & Behind, now when opening a diff it will be a diff between the merge base and the tip. So x files changed under Behind will now only show changes that were made on the comparee branch, while under Ahead it will only show changes that were made on the comparer branch. While the root x files changes node will continue to be a comparison between the comparee and comparer as it is today.

Can you please verify this fix in tomorrow's insiders edition?

You can install the insiders edition from here. Be sure to disable/uninstall the stable version of GitLens first.

@eamodio
Copy link
Member

eamodio commented Dec 23, 2020

@jods4 @jopotts @fevb @obambrough @jankap @SamBreaksThings Thoughts? 😄

@obambrough
Copy link

@eamodio I disabled gitlens, reloaded VS Code, installed the insider edition (2020.12.2304) and reloaded VS Code again. I then compared a branch against the master branch but there was no difference in behaviour for the ahead/behind files. If I expand the ahead node and then expand the files changed node and look at one of the files that I know was changed in both branches I am seeing a diff that contains the changes from both branches, not just the changes in the current branch (which is what the triple dot diff would give me.)

e.g.

master/task.py:

   +     if start >= stop:
   +        start = stop - 1

pr-343/task.py:
+ last_time = get_last_time(account_id)

then do the compare of pr-343 with master I only want to see the one line that was changed in that PR but what I see under the ahead/files changed node is:

   -     if start >= stop:
   -        start = stop - 1
   ...
   + last_time = get_last_time(account_id)

This was the same behaviour I was seeing with the regular gitlens (not insiders edition.)

@eamodio
Copy link
Member

eamodio commented Dec 23, 2020

@obambrough Hrm -- what do you get if you run git merge-base --fork-point <ref1> <ref2>, where <ref1> is your branch and <ref2> is master (or whatever 2 branches you are comparing)?

@obambrough
Copy link

Nothing is returned when I run the git merge-base --fork-point command with the branches in either order. If it helps when I remove the --fork-point mode modifier then it returns the commit that I believe would be appropriate for performing the diff against.

@eamodio
Copy link
Member

eamodio commented Dec 24, 2020

Awesome. Thanks for the info. Should make an easy fix!

@eamodio eamodio removed the pending-release Indicates that the issue been resolved, but has not yet been released to the stable edition label Dec 24, 2020
@eamodio eamodio modified the milestones: Soon™, Shipped Dec 24, 2020
@eamodio
Copy link
Member

eamodio commented Dec 24, 2020

@obambrough I pushed the fix to both the latest insiders and the 11.1 stable release.

@obambrough
Copy link

I tested against the GitLens (Insiders) 2020.12.2400 and it worked great! The ahead/behind files changed nodes provide a diff that shows what changed in the respective branches and the files changed node (that isn't underneath the ahead/behind node) shows a diff between the current HEAD of each branch.

I can't speak for the others but I think this achieves allowing me to perform a code review which is what I used the three dot option for in the older Gitlens version. Thank you!

@jods4
Copy link

jods4 commented Jan 1, 2021

@eamodio sorry I was on a holiday break.
The new behavior is a perfect substitute for the old three-dots buttons for me.
Thank you and have a great 2021!

@fevb
Copy link

fevb commented Jan 2, 2021

Sorry for the late response, I am also satisfied with the new behavior for the cherry-picking use case I mentioned before. I can now use the 'files changed' group instead of the before and after groups in the compare section. Thank you!

@github-actions
Copy link

github-actions bot commented Feb 2, 2021

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 2, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working needs-verification Indicates a request for community verification
Projects
None yet
Development

No branches or pull requests

7 participants