Mark "old" and "new" in compare editors

[stephan-herrmann]

Description

When comparing files from the History view, we are presented with two versions that are distinguished only by their hash. I suggest to add labels like "old" and "new" to this display.

As a bonus feature hovering over the file name on either side could show details of the commit shown.

Motivation

In the context of eclipse-platform/eclipse.platform#2566 people will be caught off guard with flipped sides, so we should try hard to help people recognize which side is which. IMHO hashes are not perfectly suited for this.

Alternatives considered

No response

Additional context

No response

[msohn]

I agree to your observation and we should improve this.

If one of the sides is the working tree version this side can be named "new" or more specific "working tree" and the other one "old".

Though when comparing commits I think it's not as easy as "old" and "new".

EGit's history view uses topological sorting giving precedence to the parent relationship between commits over their timestamps. git log does the same and offers multiple options how to sort by timestamps. In most cases this should be sufficient to determine which side is "predecessor" or "successor" with respect to the version graph. So maybe we should name the version "new" which is shown above the other version in the history view.

Though there are some less common cases were naming sides isn't obvious:

• timestamps might be off since git is a distributed versioning system and commits can be created on different machines concurrently and their clocks might be out of synch.
• How to name sides if versions on different branches are compared which diverged from a common predecessor.
• A repository can contain multiple disconnected version graphs. You can create such disconnected version graphs e.g. by creating multiple orphan branches or by pushing an unrelated repository into another one. How should sides be named if two versions are compared which reside in different disconnected version graphs ?

I think in these corner cases we should not try to name the sides.

[stephan-herrmann]

I think in these corner cases we should not try to name the sides.

I agree.

Implementing a "best effort" would already go a long way, i.e., adding labels only when a clear ancestor-descendant relationship can be determined.

[tomaswolf]

I'd suggest to avoid "new" and "old" altogether.

Comparing a working tree file against a commit or the index: labels should already be clear enough.

For comparing arbitrary commits, one cannot always determine a clear relationship. So just show, in addition to the commit hash, the commit time formatted via the PreferenceBasedDateFormatter. (And if that is off, so be it.)

Really determining the parent-child relation between two commits might also add performance overhead that could be unacceptable. IIRC, older EGit even tried to find the commit that last touched a file when showing the comparison, but that turned out to be too expensive.

[stephan-herrmann]

So just show, in addition to the commit hash, the commit time formatted via the PreferenceBasedDateFormatter. (And if that is off, so be it.)

Is that a suggestion for an alternative improvement or something that should already work?
(I never heard of PreferenceBasedDateFormatter and would have no idea where this could be turned on or off).

[tomaswolf]

The suggestion is not to show "old" or "new" but the commit time stamps. The PreferenceBasedDateFormatter is the EGit UI class that handles user date format preferences for formatting git timestamps as set at Preferences→Version Control (Team)→Git→Date Format.

[stephan-herrmann]

The suggestion is not to show "old" or "new" but the commit time stamps.

OK, as a modification of the request this looks fine to me.

The PreferenceBasedDateFormatter is the EGit UI class that handles user date format preferences for formatting git timestamps as set at Preferences→Version Control (Team)→Git→Date Format.

So we can change the format, but it looks like it cannot be "turned off" as in "(And if that is off, so be it.)", ergo nothing to worry about, right?

[tomaswolf]

So we can change the format, but it looks like it cannot be "turned off" as in "(And if that is off, so be it.)", ergo nothing to worry about, right?

I meant "if the commit time is off", "off" meaning here "incorrect" or "misleading". The formatter cannot be turned off, EGit always formats timestamps in some way for presentation. And I did not want to propose another preference whether or not to show that timestamp in the headers of the merge viewers in the compare editor.

[stephan-herrmann]

So we can change the format, but it looks like it cannot be "turned off" as in "(And if that is off, so be it.)", ergo nothing to worry about, right?

I meant "if the commit time is off", "off" meaning here "incorrect" or "misleading". The formatter cannot be turned off, EGit always formats timestamps in some way for presentation.

Thanks for clarifying. Seeing "preference based" and "off" in the same context sent me down the wrong alley, but now we're on the same page :)

[tomaswolf]

See Gerrit change 1237206.