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

Branch diff against master shows incorrect files in two-dot mode #797

Closed
borekb opened this issue Jul 22, 2019 · 6 comments

Comments

@borekb
Copy link

commented Jul 22, 2019

Originally reported in #293 (comment).

v9.9 introduced new behavior for comparing with other branch:

Screen Recording 2019-07-22 at 15 06 05_05 (1)

The number of commits is correct but the files shown are incorrect.

@eamodio eamodio self-assigned this Jul 22, 2019

@eamodio eamodio added the type: bug label Jul 22, 2019

@eamodio eamodio added this to the Soon™ milestone Jul 22, 2019

@eamodio eamodio closed this in d25fee3 Jul 23, 2019

@eamodio eamodio modified the milestones: Soon™, Soonish™ Jul 23, 2019

@borekb

This comment has been minimized.

Copy link
Author

commented Jul 23, 2019

Thanks, works beautifully!

@borekb

This comment has been minimized.

Copy link
Author

commented Jul 23, 2019

This might be another related bug. I tried to open the same comparison in the Compare panel and it produces a different set of results:

Screenshot 2019-07-23 at 14 46 23

(In the upper part of the screenshot, there's a node from the Repositories pane which displays the correct numbers.)

@eamodio

This comment has been minimized.

Copy link
Owner

commented Jul 24, 2019

@borekb That behavior, while a bit confusing, is working correctly. Basically the issue is that when using the 2-dot (range) syntax the order of the compare matters, i.e. 211-component-for-complex-l18n..master vs master..211-component-for-complex-l18n.

So in the repository compare, I am always showing <branch/tag/ref-you-choose>..<current-branch>, because you want to see the changes in your current branch as compared the chosen "base". In your case that is: master..211-component-for-complex-l18n

While the 2nd compare (in the compare view), the order is determined by how you select what to compare (which you can easily swap). In your case that is: 211-component-for-complex-l18n..master

If you swap the compare, so it says Comparing master to 211-component-for-complex-l18n the 2 comparisons will match.

Given that, you can certainly argue that what I display in the repo compare is somewhat misleading, but to me it felt odd/wrong to switch the display order (e.g. to say master <--> 211-component-for-complex-l18n rather than 211-component-for-complex-l18n <--> master), because 211-component-for-complex-l18n is the current branch.

@borekb

This comment has been minimized.

Copy link
Author

commented Jul 24, 2019

Oh I see, it's indeed quite a UX challenge then. How about unifying the two this way:

  • Both use the icon (as in the Repositories panel) to save space.
  • Both use white for the current branch.
  • Both use gray for the other comparison target.
  • Both use the same format which looks like this:
    • master..211-component-for-complex-l18n
    • master...211-component-for-complex-l18n
    • 211-component-for-complex-l18n..develop

It doesn't look that good without colors but if the current branch stood out, maybe this could work? I'm generally happy when it's clear from the UI what I'm looking at.

One problem with the above is what to do when master in the examples above is some long ref. Maybe it could be substituted with the word "base" or something.

@eamodio

This comment has been minimized.

Copy link
Owner

commented Aug 6, 2019

@borekb FYI, in v9.9.3, I've reverse the order of the comparisons in the Compare view, so now they will match the repository compare -- so everything will be consistent and hopefully work as would be expected.

@borekb

This comment has been minimized.

Copy link
Author

commented Aug 6, 2019

Very nice, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.