Community
Participate
Working Groups
The Compare editor is cool, but in order to show good Diffs, it would be nice to use the usual colors as other diff tools (Gerrit, GitHub) are using, so that: * Modified lines would be shown with yellow background on both sides * Removed lines are shown with Red background on "reference" document * Added lines are shown with Green background on "Local" document
Found myself straining my eyes again with the compare editor yesterday. We've already got horizontal line delimiters, it would just be a matter of setting a background colour in them, green if on the new pane, red if on the old one, yellow if in both. Does this really belong to EGit though? I think this functionality should be part of the text compare tool, it's useful when comparing two local files or local history as well. Should the bug be moved?
By the way, this was implemented at the time in a plugin that no longer seems to exist. See https://stackoverflow.com/questions/16942962/change-colours-in-eclipse-diff for a nice screenshot with colours on the diff view.
Yes, the bug should be moved. Done so.
(In reply to Thomas Wolf from comment #3) > Yes, the bug should be moved. Done so. I'm not sure it's so easy and only on Platform side. Indeed, IIRC, the general compare editor cannot assume that there is one of both sides that is a baseline and forcing red/green color upstream could lead to confusion for some other cases. I have the impression that if something has to be done, it's more in EGit configuring the compare editor with colors, and I -unless I'm mistaken- there is nothing missing in Platform to enable this.
But shouldn't the left pane always be considered as baseline in the context of the text compare editor? The user has a button "Swap Left and Right View" as well as a preference in General -> Compare/Patch -> Text Compare to decide which document he wants to use as the baseline for the current comparison view. For instance in EGit, the user could use these settings to choose whether he wants to view the differences of the index compared to local or the differences of local compared to the index, and always treating left pane as baseline in the compare view would enable those two mindsets nicely.
(In reply to Pierre-Yves B. from comment #5) > But shouldn't the left pane always be considered as baseline in the context > of the text compare editor? I'm not sure it would help in the general use-case of compare editor. When a user is comparing 2 current files (such as log files or similar), is there really one that can be assumed as being a baseline? And would color really help?
(In reply to Mickael Istria from comment #6) > I'm not sure it would help in the general use-case of compare editor. When a > user is comparing 2 current files (such as log files or similar), is there > really one that can be assumed as being a baseline? And would color really > help? I do generally find it helpful. Most tools do use different colours for left and right panes even outside the context of version control: vimdiff, IntelliJ, VSCode, etc. They aren't necessarily red and green though. When you've got a lot of complex changes one under the other, it's often hard to see whether a line belongs to a change block or is between two change blocks. I think the background of the change block has a different colour, but the transparency is so high and by default it's grey, so it's barely noticeable. Tweaking the default colour and increasing the background transparency of the change block would go a long way in improving the readability when using the text compare tool, however, having completely different colours would help even more in my opinion.
I completely agree with Pierre-Yves about needing better colors. The Eclipse compare editor is my least favorite compare tool in big part because because I can barely see the changes.
So the current compare editor has 4 states/colors (that can be configured in the Colors and Fonts page): incoming *change*, outgoing *change*, conflict and resolved conflict. The grain of the logic is not about a line being removed/added/changed but about a change being applied (and when doing conflict resolution, it's actually better). To enable more colors, we'd probably need to drill down in the concept of "Change" and have those changes allow different color according to their payload. But it seems to me that would require some new preferences.
I think we can try something like: if there are only "outgoing" changes (typical case when comparing versions), we can enable some "one-way" comparison and make use of the red and green to emphasize adding/removal like more compare tools are doing.
(In reply to Mickael Istria from comment #10) > I think we can try something like: if there are only "outgoing" changes > (typical case when comparing versions), we can enable some "one-way" > comparison and make use of the red and green to emphasize adding/removal > like more compare tools are doing. Makes sense, it would be a good thing to try!
New Gerrit change created: https://git.eclipse.org/r/146651
(In reply to Pierre-Yves B. from comment #11) > Makes sense, it would be a good thing to try! Just spent some time on it and authored this (WIP) patch: https://git.eclipse.org/r/146651 It seems to me that the idea just works and is relatively safe to include. On the patch, what still needs to be done is to declare proper color definitions that user can change.
Great, will have a look in the evening.
I'm quite happy with the current patch and would like to merge it soon. @Pierre-Yves: I let you give it a try. If functionality and code is good enough to you, I'll merge it.
Gerrit change https://git.eclipse.org/r/146651 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.team.git/commit/?id=0e6b3ac15a797e6cc5fbeb49f4e9b5db48d89cb9
Mickael, could you attach a screen shot please? I guess you want to add this to N&N anyway?
New Gerrit change created: https://git.eclipse.org/r/146724
Gerrit change https://git.eclipse.org/r/146724 was merged to [master]. Commit: http://git.eclipse.org/c/www.eclipse.org/eclipse/news.git/commit/?id=16f5f5d8a7fcfd9212ca6a145298da5c0c53a6b4
(In reply to Andrey Loskutov from comment #17) > Mickael, could you attach a screen shot please? I guess you want to add this > to N&N anyway? Yes, the screenshot will be in N&N within a few minutes (I just merged it in).
(In reply to Mickael Istria from comment #20) > Yes, the screenshot will be in N&N within a few minutes (I just merged it > in). Actually, it's already visible on cgit: https://git.eclipse.org/c/www.eclipse.org/eclipse/news.git/plain/4.13/images/compare-editor-colors.png?id=16f5f5d8a7fcfd9212ca6a145298da5c0c53a6b4
If "General -> Compare / Patch -> Swap left and right" option is set, colors are wrong and the opposite of what should be. This is extremely confusing now. This must be fixed.
One point more: not sure if this is a bug or not, but the compare preview on "General -> Compare / Patch" does not show "green" color for additions. Probably this example could be extended - or there is a bug.
(In reply to Andrey Loskutov from comment #22) > If "General -> Compare / Patch -> Swap left and right" option is set, colors > are wrong and the opposite of what should be. This is extremely confusing > now. > > This must be fixed. If swap left and right is set, I would expect red for deletions on the left and green for additions on the right.
(In reply to Andrey Loskutov from comment #23) > One point more: not sure if this is a bug or not, but the compare preview on > "General -> Compare / Patch" does not show "green" color for additions. > Probably this example could be extended - or there is a bug. The Red/Green for addition and removal only show up in case of 2 way compare. The preview is a 3 way compare. We should probably show a separate preview on the same for the 2 way compare.
(In reply to Pierre-Yves B. from comment #24) > (In reply to Andrey Loskutov from comment #22) > > If "General -> Compare / Patch -> Swap left and right" option is set, colors > > are wrong and the opposite of what should be. This is extremely confusing > > now. > > > > This must be fixed. > > If swap left and right is set, I would expect red for deletions on the left > and green for additions on the right. Sorry but all this "swap left and right" is confusing and what we can expect from it is too. Can you please give concrete steps to reproduce with filenames, where are those files in the viewer, what do you expect and what do you get?
Just try to see differences on *any* commit in any git repo you have access. Please note, that "swap" option is only confusing for Eclipse users, but is the default for the rest of the world (including gerrit). New changes are always on the right side.
(In reply to Andrey Loskutov from comment #27) > Just try to see differences on *any* commit in any git repo you have access. > Please note, that "swap" option is only confusing for Eclipse users, but is > the default for the rest of the world (including gerrit). New changes are > always on the right side. Ok, thank you for this clarification. It was unclear to me whether it was only about the "where to show baseline" (actual behavior) or "which side is baseline" (my erroneous understanding).
New Gerrit change created: https://git.eclipse.org/r/146813
Gerrit change https://git.eclipse.org/r/146813 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.team.git/commit/?id=417b6047c1ee012024e9fe086d073afcbc9c3611
(In reply to Andrey Loskutov from comment #27) > Just try to see differences on *any* commit in any git repo you have access. > Please note, that "swap" option is only confusing for Eclipse users, but is > the default for the rest of the world (including gerrit). New changes are > always on the right side. I'm glad to hear that someone else is annoyed by this. I've opened Bug 549668.
New Gerrit change created: https://git.eclipse.org/r/146830
Created attachment 279443 [details] Screenshot of preference page (In reply to Eclipse Genie from comment #32) > New Gerrit change created: https://git.eclipse.org/r/146830 Here is what preference page looks like with a 2-way compare preview. Feedback more than welcome.
(In reply to Mickael Istria from comment #33) > Created attachment 279443 [details] > Screenshot of preference page > > (In reply to Eclipse Genie from comment #32) > > New Gerrit change created: https://git.eclipse.org/r/146830 > > Here is what preference page looks like with a 2-way compare preview. > Feedback more than welcome. Now, with names, 3 way compare part looks really strange. 3 way always means there are 3 participants - but we see only two? Also with 3 way compare one can have additions and deletions, and I would expect to see green/red lines. For this bug I would check if 3 way pane could show all possible change types, so that we see rhe new feqture too. Why 2 way compare does not have big blue direction arrow (3 way has)? Let us move this part (2 way vs 3 way) to another bug, and sorry for hijacking this one (I assumed a bug or so).
(In reply to Andrey Loskutov from comment 34) > Now, with names, 3 way compare part looks really strange. I always found 3-way compare strange and not usable, so this makes the preview more realistic ;) > 3 way always means > there are 3 participants - but we see only two? Yes 1 ancestor and 2 changes (incoming and outgoing). > Also with 3 way compare one > can have additions and deletions, and I would expect to see green/red lines. What's highlighted with 3 way compare isn't the content of the change,but the origin (ingoing or outgoing). > For this bug I would check if 3 way pane could show all possible change > types, so that we see rhe new feqture too. I think it unfortunately make things too complex to show both origin and change «load» (addition/removal) with the same color. Can you please suggest how things would look like on the preview example? > Why 2 way compare does not have big blue direction arrow (3 way has)? I don't know, but it seems like it never had one.
(In reply to Mickael Istria from comment #35) > (In reply to Andrey Loskutov from comment 34) > > Now, with names, 3 way compare part looks really strange. > > I always found 3-way compare strange and not usable, so this makes the > preview more realistic ;) Should we switch default pane to show 2 way compare only? > > 3 way always means > > there are 3 participants - but we see only two? > > Yes 1 ancestor and 2 changes (incoming and outgoing). I wanted to say that I miss the ancestor pane. > > Also with 3 way compare one > > can have additions and deletions, and I would expect to see green/red lines. > > What's highlighted with 3 way compare isn't the content of the change,but > the origin (ingoing or outgoing). Even more strange, or let say it differently - I didn't get it. > > For this bug I would check if 3 way pane could show all possible change > > types, so that we see rhe new feqture too. > > I think it unfortunately make things too complex to show both origin and > change «load» (addition/removal) with the same color. > Can you please suggest how things would look like on the preview example? I personally believe that the "classic" two-way compare "before/after" with all the "classic" red/green lines is what we should show by default. I honestly have no idea what the current preview is supposed to tell me - where "incoming" addition is blue and "outgoing" is grey. WTF? It might be coming from CVS days, but in Git era it doesn't match anything I understand. > > Why 2 way compare does not have big blue direction arrow (3 way has)? > > I don't know, but it seems like it never had one. Not that important.
(In reply to Andrey Loskutov from comment #36) > Should we switch default pane to show 2 way compare only? You mean default pane in the preview on the Preference Page? I'd be fine hiding the 3-way one if you think it's better. FWIW, the 3-way compare is not really a diff viewer but more a "merge" viewer.
(In reply to Mickael Istria from comment #37) > (In reply to Andrey Loskutov from comment #36) > > Should we switch default pane to show 2 way compare only? > > You mean default pane in the preview on the Preference Page? > I'd be fine hiding the 3-way one if you think it's better. Yep. But I think this is worth a dedicated bug, WDYT? From my POV this one looks good now with the latest patch.
(In reply to Andrey Loskutov from comment #38) > Yep. But I think this is worth a dedicated bug, WDYT? Ok. I let you open it ;) > From my POV this one looks good now with the latest patch. Should I merge the patch with both 2-way and 3-way previews?
(In reply to Mickael Istria from comment #39) > > From my POV this one looks good now with the latest patch. > > Should I merge the patch with both 2-way and 3-way previews? Once we are done with review, yes :-) (from my POV)
Gerrit change https://git.eclipse.org/r/146830 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.team.git/commit/?id=a36a62d0c3fb05a9f9eed61f1d127566b83b8479
Awesome. One suggestion Gerrit (see https://git.eclipse.org/r/#/c/147311/1/org.eclipse.debug.core/core/org/eclipse/debug/core/model/LaunchConfigurationDelegate.java) uses green also for changed lines. Can we also use green for changed lines?
Colord are in the theme and can be changed trivially by the project or users. Let's discuss them in a.separate ticket as it's likely to be ong discussion thatdoesn't question this current "iteration"
