Bug 504474 - Come up with a set of consistent rules for what to show on the left and right sides of the compare viewer
Summary: Come up with a set of consistent rules for what to show on the left and right...
Status: RESOLVED DUPLICATE of bug 516450
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Compare (show other bugs)
Version: 4.6   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 4.7 M7   Edit
Assignee: Stefan Xenos CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-10-06 00:33 EDT by Stefan Xenos CLA
Modified: 2017-05-11 09:16 EDT (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Stefan Xenos CLA 2016-10-06 00:33:53 EDT
I'm not sure whether this is an EGit bug or a search bug.

Search > File > File Search
Enter some text
Replace...
Enter some text
Click "Preview"

Observe which side contains the state before the refactoring and which side contains the state after the refactoring.

Right-click on the project and select "Commit..."
Double-click on the unstaged change in order to view it in a compare editor.

Observed:

The EGit compare editor puts the before and after states on the opposite side as the refactoring preview compare editor.

Expected:

Both compare editors use the same side for the before state and after state.

It doesn't really matter which side both put it on since the user can reverse the compare editor if they don't like it... but if you're like me and always want to see the after state on the right and the before state on the left, there's no position you can position the toggle that works for both EGit and the compare preview window.

Please reverse either the EGit compare editor or the refactoring preview compare editor so that they match.
Comment 1 Stefan Xenos CLA 2016-10-06 00:35:48 EDT
Put another way: I want the patch to look the same in Eclipse as it will in gerrit... but one of the two compare editors is always going to show it in the reverse direction no matter how I position the toggle button.
Comment 2 Thomas Wolf CLA 2016-10-06 02:46:47 EDT

*** This bug has been marked as a duplicate of bug 457111 ***
Comment 3 Stefan Xenos CLA 2016-10-11 11:19:50 EDT
Reopening. This bug was filed as a follow-up for bug 213780... and through a chain of dupes, it has somehow gotten marked as a dupe of bug 213780.

The problem here is that EGit is inconsistent with the search view - no matter which way you flip that preference. Let's say you position the preference such that the Egit compare view shows the after state on the right and the before state on the left - then the search view will show the same diff reversed.

Some have made the claim that "before" and "after" are meaningless in some use-Eclipse cases and have used compare-this-with-workspace as an example. If "before" and "after" were indeed meaningless for a given use case then I agree that this bug wouldn't apply to that specific use-case (although we can still make things consistent in the cases where before and after do apply).

However, in the specific case of compare-this-with-workspace, there is a metaphor that gives meaning to "before" and "after". If you think of "compare-this-with-workspace" as a preview of the "replace-workspace-version-with-this" action then "before" and "after" are well-defined and the workspace version is the before state.
Comment 4 Stefan Xenos CLA 2016-10-11 11:20:48 EDT
If you believe that it is the search view rather than EGit that should change, please reassign this to the search component.
Comment 5 Thomas Wolf CLA 2016-10-11 14:27:32 EDT
If I understand you right, you do

Search > File > File Search
Enter some text
Replace...
Enter some text
Click "Preview"

and then Click "Ok", thus applying the changes.

Then open commit dialog, double click a changed file. Right?

I would say EGit's "Compare with HEAD" operation _is_ consistent with the search preview: both place the workspace content left and the other content right. That's what Eclipse usually does. It's also the way the synchronize view works. Left is workspace, right is remote content.

As I mentioned already on bug 457111, always placing the "newer" content right is going to cause even more confusion, because you'll have the workspace left in some cases, and right in some other cases. That's IMO even worse.
Comment 6 Stefan Xenos CLA 2016-10-11 15:27:33 EDT
> I would say EGit's "Compare with HEAD" operation _is_ consistent with the
> search preview: both place the workspace content left and the other content
> right.

Yes. It depends entirely on which rule you want applied universally. Should the left side always be the "before state" or should the left side always be the "workspace state"?

Normally they're the same thing since - in most cases - if you're comparing something with the workspace, you've viewing a preview of a future change that hasn't yet been applied to the workspace, so the before state *is* the workspace state and according to either rule, the workspace state goes on the left.

However, in the case of the git commit view, they're different. You're not viewing an anticipated future change. You're viewing a past change that led up to the current workspace state, so the after state is the workspace state. Applying the "before on left and after on right" rule puts the workspace on the right and applying the "workspace on left and other on right" puts the workspace on the left.

So the question is: is there any other tool with the notion of a working copy that reverses the display of its before and after states when necessary in order to always keep the working copy on the same side?

git gui + gitk: The closest notion command-line git has to the notion of a working copy is the state of the filesystem... and git uses before and after to identify the sides it uses for comparisons, not "matches the workspace" and "other". I tried looking at historical commits, staged commits, and unapplied diffs and all use the same appearance for their after state, regardless of which side corresponds to the current filesystem state.

gerrit: The closest notion gerrit has to a working copy is the state of master. It also allows edits within patches. Regardless of which one you consider analogous to the working copy within Eclipse, gerrit always puts the after state on the right and never reverses them.

In fact, Eclipse is the only tool I'm aware of which reverses the before and after states in some situations in order to keep the workspace state on the same side, so I think it's very likely that most users will expect the after state to always be in the same place.

...but let's say I'm wrong and there's a large number of users that like this behavior. If true, that would justify having a preference for such users. Behaving in a way consistent with other tools should at least be an option and should probably be the default.
Comment 7 Thomas Wolf CLA 2016-10-13 14:27:49 EDT
Note that you get the same behavior with Compare With->Local history...: workspace left, older version right.
Comment 8 Stefan Xenos CLA 2016-10-13 21:28:31 EDT
> Note that you get the same behavior with Compare With->Local history...:
> workspace left, older version right.

That wouldn't need to change regardless of whether your rule is "before state on the left" or "workspace verson on the left". Both would put the workspace version on the left in this use-case.

Your reference to "older version" makes me suspect I was unclear and that I gave the impression I was referring to timestamps when I used the words "before state" and "after state".

The Compare With->Local history action is a preview of the Replace With->Local history action, and as with any preview of a potential future change the "before state" is the version currently in the workspace and the "after state" would be the potential future replacement. Although the future replacement is a revert to an older state, I'm not suggesting that that should put it on the left.

Consider the following use-case:

File state 1: Initial file state
File state 2: (in local history) a line was removed
Workspace initially contains file state 2

1. User performs compare with -> local history (state 1)
2. User performs replace with -> local history (state 1)
3. User views the unstaged change in the git staging view
4. User commits the change and uses compare with-> previous version in the commit viewer.


If we use before on left/after on right, we get:

Step 1: left: state 2 (workspace version), right: state 1 (we're viewing a preview of the revert that will take the file from state 2 to state 1).

Step 3: left: state 2, right: state 1 (workspace version) (the revert has taken place and it looks exactly the same in the git staging view as it did when previewed using the compare with action)

Step 4: left: state 2, right: state 1 (when the historical change is viewed afterwards, it looks exactly the same as it did in the staging view and exactly the same as it did when previewed before the change occurred).

With this rule, the diff looks exactly the same when previewed as it does when it's in progress and when it's viewed after-the-fact in the history view.


If we use "workspace on the left", we get:

Step 1: left: state 2 (workspace version), right: state 1 (the fact that it's a preview of a potential future change is unimportant, but the fact that one of the states comes from the workspace puts that one on the left).

Step 3: left: state 1 (workspace version), right: state 2 (we're viewing a historic version that led up to the current workspace state, but keeping the workspace version on the left means that the line that was shown as an addition in step 1 now gets the appearance of a removal).

Step 4: left: ? right: ? (now that the change has been committed, both states are historical and neither is a live view of the workspace. "workspace on left" offers no guidance for which side each state should go on, but no matter which side we pick it will be inconsistent with either how the diff appeared when viewed as a preview or how it appeared when viewed as a change in progress).
Comment 9 Stefan Xenos CLA 2016-11-27 16:25:11 EST
Marking as a dupe of bug 508257 since that would also address this bug.

*** This bug has been marked as a duplicate of bug 508257 ***
Comment 10 Sergey Prigogin CLA 2016-12-06 16:39:55 EST
Reopened to continue the discussion on the desired set of consistent rules for what to show on the left and right sides of the compare viewer.
Comment 11 Dani Megert CLA 2017-05-10 12:36:41 EDT
Ping!
Comment 12 Stefan Xenos CLA 2017-05-10 12:38:05 EDT
I'll try to attach the proposal we came up with before I disappear from the Eclipse project.
Comment 13 Stefan Xenos CLA 2017-05-11 00:53:59 EDT
Dani, I've gone over the old discussion notes and emails between myself and sprigogin, and have filed bug 516450 describing our latest-and-best proposal.

I figured a new bug would be better than an additional comment here, so folks can see the most up-to-date version of the proposal without needing to piece it together from various comments.

I'll close this since it's replaced by bug 516450.
Comment 14 Stefan Xenos CLA 2017-05-11 00:54:24 EDT

*** This bug has been marked as a duplicate of bug 516450 ***