Summary: | Proposal for a set of consistent rules for what to show on the left and right sides of the compare viewer | ||
---|---|---|---|
Product: | [Eclipse Project] Platform | Reporter: | Stefan Xenos <sxenos> |
Component: | Compare | Assignee: | Platform-Compare-Inbox <platform-compare-inbox> |
Status: | REOPENED --- | QA Contact: | |
Severity: | normal | ||
Priority: | P3 | CC: | daniel_megert, eclipse.sprigogin, kevintownsend4219, Lars.Vogel, markus.kell.r, matthias.sohn, p.bodnar, reckord, stepper |
Version: | 4.6 | Keywords: | helpwanted |
Target Milestone: | --- | ||
Hardware: | PC | ||
OS: | Linux | ||
See Also: | https://bugs.eclipse.org/bugs/show_bug.cgi?id=557993 | ||
Whiteboard: | stalebug | ||
Bug Depends on: | |||
Bug Blocks: | 508257 |
Description
Stefan Xenos
2017-05-11 00:50:51 EDT
*** Bug 504474 has been marked as a duplicate of this bug. *** I have not checked the proposal in depth yet, but I agree with the direction. Two points to keep in mind when implementing this: 1. APIs that talk about "left" and "right" must stay stable. No cheating / trying to be helpful for legacy code. Code that already follows the proposal must not need any change. 2. Start by defining an opt-out preference for users/products that prefer the current rules (team provider comparisons put the local workspace version on the left). The preference should initially be enabled by default. Once everything in the Eclipse SDK + EGit has adopted the preference, the default can be set to false and the feature can be announced. We should probably get someone who can speak for EGit on this cc list. (In reply to Stefan Xenos from comment #3) > We should probably get someone who can speak for EGit on this cc list. Adding Matthias. (In reply to Stefan Xenos from comment #0) > H) Compare With Branch, Tag, or Reference (EGit). The workspace version is > shown on the left (rule #1). > I) Compare With Commit (EGit). The workspace version is shown on the left > (rule #1). I can understand how you come to this by your set of rules, but isn't this highly inconsistent with any tooling provided by command-line git? When I do a "git diff <revision>" on my working tree, the "before"-state is always the <revision> and the "after"-state is my working tree. I would argue that this is the natural way to do it (I want to see what happened in my working tree compared to some other state, hence it is the "after"-state) Also, while it's "by the rules", it seems inconsistent to E), F) and G): > E) Compare With Previous Version (EGit) (on IResource). Action renamed to > "Show changes since previous commit". The workspace version is shown on the > right (rule #2). > F) Compare With HEAD Revision (EGit) (on IResource). Action renamed to "Show > uncommitted changes". The workspace version is shown on the right (rule #2). > G) Compare With Index (EGit). Action renamed to "Show unstaged changes". The > workspace version is shown on the right (rule #2). The only difference that I can see here between H) and I) on one hand, and E)-G) on the other is one of presumed intent, i.e. that for E)-G) the case would be stronger that it's about applying/reviewing an ongoing change, whereas H) and I) are "mere" comparisons. I would disagree with that interpretation. My use-case for comparing is pretty much the same in both cases: What's the change that my workspace version introduces? > L) Compare With Index With HEAD (EGit). Action renamed to "Show staged > changes". The index version is shown on the right (rule #2). > N) Compare with Working Tree in History view (EGit). The working tree > version is shown on the right (rule #1). > R) Compare with Workspace in History view (EGit). It will continue to keep > its current name. The workspace version is shown on the right (rule #1). I agree with the result here, having the workspace on the right, although for different reasons: not because the thing I clicked on should be on the left (rule #1), but because the after-state should be on the right (rule #2). > I would argue that this is the natural way to do it (I want to see > what happened in my working tree compared to some other state, hence it > is the "after"-state) That is one of the things the user might be trying to do. However, they might also be previewing a future revert to some other state, in which case the "after" state would be the historical state. Based on what the user intends to do with the information, either one might be the more intuitive presentation. This is why I'm proposing that we make both presentations available to the user and give them a predictable way to select between them. If they want their workspace version on the left, they right-click on the file and then select the other state in a dialog box. If they want the other state on the left, they right-click in the history view and select "compare with workspace". In both cases, the thing you right-clicked on goes on the left, which is a very simple rule for users to understand. Even git diff lets the user choose which version goes on which side. From "git help diff": > git diff [--options] <commit> <commit> [--] [<path>…] > This is to view the changes between two arbitrary <commit>. In this case, the one the user lists first goes on the "left". > Also, while it's "by the rules", it seems inconsistent to E), F) and G): The actions in E, F, and G will be named "Show", not "Compare". That informs the user that the action doesn't follow the rules for a compare action. If you're arguing that the before and after states are unambiguous for E, F, and G, I'd agree with you. That's why I proposed renaming the actions. However, with H and I, there is no one right answer since there are legitimate use cases for viewing the diff in both directions (understanding historical changes and preparing for a future revert). My hope is that the rules will be simple enough for end-users to understand, and as long as users understand the rules, anything that is "by the rules" will also feel consistent to the user. > My use-case for comparing is pretty much the same in both cases: What's > the change that my workspace version introduces? I'd completely agree in the case of E-G, where you're viewing your own changes in progress. However, when we're talking about comparisons with historical versions, I've encountered just as many cases where I wanted to revert to an old version as cases where I wanted to understand the changes introduced since an old revision... and if we were to pick only one of these use-cases as being legitimate, it's going to seem inconsistent to any user with the other use-case. That's why I'm suggesting we offer both and give the user a consistent way to understand and control what goes on each side. Just as on the command line "the thing you type first goes on the left", in the UI "the thing you right-clicked on goes on the left". If you want to view the opposite diff, click on the other thing. As you've probably seen in previous discussions about compare editors, what the user think of as the "after" state can be subjective and depend entirely on their intent. So let's get out of the business of trying to guess the user's most likely intent and just give them simple tools to view whatever diff they want to see. This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. As such, we're closing this bug. If you have further information on the current state of the bug, please add it and reopen this bug. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. -- The automated Eclipse Genie. Still relevant. Looking at this topic and I can see there was a good piece of work done, but I guess it is really hard for anybody to grasp it and take some concrete actions. IMHO it would help if there was a clear overview of what the current state is and what changes are proposed, together with linking all the related issues that already exist for this topic. Or maybe to break all the stuff into smaller, doable pieces? (Also, while Eclipse migrated from Bugzilla to GitHub, I wonder what is the best place for further discussion?) (Not that I would have time for some deep discussions, but I have also tried to somewhat summarize the current state of things here: https://bugs.eclipse.org/bugs/show_bug.cgi?id=550896#c6.) Petr, if you have a patch to improve the situation please propose it as PR or issue via GitHub. I think it is fine to change our defaults as long as we also fix the corresponding unit tests for which I never found the time |